* [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
@ 2024-04-23 19:57 Horatiu Vultur
2024-04-24 10:57 ` Vadim Fedorenko
2024-04-26 1:44 ` Jakub Kicinski
0 siblings, 2 replies; 7+ messages in thread
From: Horatiu Vultur @ 2024-04-23 19:57 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
richardcochran
Cc: netdev, linux-kernel, Horatiu Vultur
Extend the PTP programmable gpios to implement also PTP_PF_EXTTS
function. The pins can be configured to capture both of rising
and falling edge. Once the event is seen, then an interrupt is
generated and the LTC is saved in the registers.
On lan8814 only GPIO 3 can be configured for this.
This was tested using:
ts2phc -m -l 7 -s generic -f ts2phc.cfg
Where the configuration was the following:
---
[global]
ts2phc.pin_index 3
[eth0]
---
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
drivers/net/phy/micrel.c | 182 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 181 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 0e310a5e2bff0..2d11f38cbc243 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -167,6 +167,9 @@
#define PTP_CMD_CTL_PTP_LTC_STEP_SEC_ BIT(5)
#define PTP_CMD_CTL_PTP_LTC_STEP_NSEC_ BIT(6)
+#define PTP_COMMON_INT_ENA 0x0204
+#define PTP_COMMON_INT_ENA_GPIO_CAP_EN BIT(2)
+
#define PTP_CLOCK_SET_SEC_HI 0x0205
#define PTP_CLOCK_SET_SEC_MID 0x0206
#define PTP_CLOCK_SET_SEC_LO 0x0207
@@ -179,6 +182,27 @@
#define PTP_CLOCK_READ_NS_HI 0x022C
#define PTP_CLOCK_READ_NS_LO 0x022D
+#define PTP_GPIO_SEL 0x0230
+#define PTP_GPIO_SEL_GPIO_SEL(pin) ((pin) << 8)
+#define PTP_GPIO_CAP_MAP_LO 0x0232
+
+#define PTP_GPIO_CAP_EN 0x0233
+#define PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(gpio) BIT(gpio)
+#define PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(gpio) (BIT(gpio) << 8)
+
+#define PTP_GPIO_RE_LTC_SEC_HI_CAP 0x0235
+#define PTP_GPIO_RE_LTC_SEC_LO_CAP 0x0236
+#define PTP_GPIO_RE_LTC_NS_HI_CAP 0x0237
+#define PTP_GPIO_RE_LTC_NS_LO_CAP 0x0238
+#define PTP_GPIO_FE_LTC_SEC_HI_CAP 0x0239
+#define PTP_GPIO_FE_LTC_SEC_LO_CAP 0x023A
+#define PTP_GPIO_FE_LTC_NS_HI_CAP 0x023B
+#define PTP_GPIO_FE_LTC_NS_LO_CAP 0x023C
+
+#define PTP_GPIO_CAP_STS 0x023D
+#define PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(gpio) BIT(gpio)
+#define PTP_GPIO_CAP_STS_PTP_GPIO_FE_STS(gpio) (BIT(gpio) << 8)
+
#define PTP_OPERATING_MODE 0x0241
#define PTP_OPERATING_MODE_STANDALONE_ BIT(0)
@@ -274,6 +298,7 @@
#define LAN8814_PTP_GPIO_NUM 24
#define LAN8814_PTP_PEROUT_NUM 2
+#define LAN8814_PTP_EXTTS_NUM 3
#define LAN8814_BUFFER_TIME 2
@@ -3124,12 +3149,102 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
return 0;
}
+static void lan8814_ptp_extts_on(struct phy_device *phydev, int pin, u32 flags)
+{
+ u16 tmp;
+
+ /* Set as gpio input */
+ tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
+ tmp &= ~LAN8814_GPIO_DIR_BIT(pin);
+ lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
+
+ /* Map the pin to ltc pin 0 of the capture map registers */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
+ tmp |= pin;
+ lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
+
+ /* Enable capture on the edges of the ltc pin */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
+ if (flags & PTP_RISING_EDGE)
+ tmp |= PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0);
+ if (flags & PTP_FALLING_EDGE)
+ tmp |= PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0);
+ lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
+
+ /* Enable interrupt top interrupt */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
+ tmp |= PTP_COMMON_INT_ENA_GPIO_CAP_EN;
+ lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
+}
+
+static void lan8814_ptp_extts_off(struct phy_device *phydev, int pin)
+{
+ u16 tmp;
+
+ /* Set as gpio out */
+ tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
+ tmp |= LAN8814_GPIO_DIR_BIT(pin);
+ lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
+
+ /* Enable alternate, 0:for alternate function, 1:gpio */
+ tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin));
+ tmp &= ~LAN8814_GPIO_EN_BIT(pin);
+ lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin), tmp);
+
+ /* Clear the mapping of pin to registers 0 of the capture registers */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
+ tmp &= ~GENMASK(3, 0);
+ lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
+
+ /* Disable capture on both of the edges */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
+ tmp &= ~PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(pin);
+ tmp &= ~PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(pin);
+ lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
+
+ /* Disable interrupt top interrupt */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
+ tmp &= ~PTP_COMMON_INT_ENA_GPIO_CAP_EN;
+ lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
+}
+
+static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
+ struct ptp_clock_request *rq, int on)
+{
+ struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
+ ptp_clock_info);
+ struct phy_device *phydev = shared->phydev;
+ int pin;
+
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_EXTTS_EDGES |
+ PTP_STRICT_FLAGS))
+ return -EOPNOTSUPP;
+
+ pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
+ rq->extts.index);
+ if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
+ return -EINVAL;
+
+ mutex_lock(&shared->shared_lock);
+ if (on)
+ lan8814_ptp_extts_on(phydev, pin, rq->extts.flags);
+ else
+ lan8814_ptp_extts_off(phydev, pin);
+
+ mutex_unlock(&shared->shared_lock);
+
+ return 0;
+}
+
static int lan8814_ptpci_enable(struct ptp_clock_info *ptpci,
struct ptp_clock_request *rq, int on)
{
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
return lan8814_ptp_perout(ptpci, rq, on);
+ case PTP_CLK_REQ_EXTTS:
+ return lan8814_ptp_extts(ptpci, rq, on);
default:
return -EINVAL;
}
@@ -3148,6 +3263,10 @@ static int lan8814_ptpci_verify(struct ptp_clock_info *ptp, unsigned int pin,
if (pin >= LAN8814_PTP_PEROUT_NUM || pin != chan)
return -1;
break;
+ case PTP_PF_EXTTS:
+ if (pin != LAN8814_PTP_EXTTS_NUM)
+ return -1;
+ break;
default:
return -1;
}
@@ -3320,6 +3439,64 @@ static void lan8814_handle_ptp_interrupt(struct phy_device *phydev, u16 status)
}
}
+static int lan8814_gpio_process_cap(struct lan8814_shared_priv *shared)
+{
+ struct phy_device *phydev = shared->phydev;
+ struct ptp_clock_event ptp_event = {0};
+ unsigned long nsec;
+ s64 sec;
+ u16 tmp;
+
+ /* This is 0 because whatever was the input pin it was mapped it to
+ * ltc gpio pin 0
+ */
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_SEL);
+ tmp |= PTP_GPIO_SEL_GPIO_SEL(0);
+ lanphy_write_page_reg(phydev, 4, PTP_GPIO_SEL, tmp);
+
+ tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_STS);
+ if (!(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(0)) &&
+ !(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_FE_STS(0)))
+ return -1;
+
+ if (tmp & BIT(0)) {
+ sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_HI_CAP);
+ sec <<= 16;
+ sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_LO_CAP);
+
+ nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_HI_CAP) & 0x3fff;
+ nsec <<= 16;
+ nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
+ } else {
+ sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_HI_CAP);
+ sec <<= 16;
+ sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_LO_CAP);
+
+ nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_NS_HI_CAP) & 0x3fff;
+ nsec <<= 16;
+ nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
+ }
+
+ ptp_event.index = 0;
+ ptp_event.timestamp = ktime_set(sec, nsec);
+ ptp_event.type = PTP_CLOCK_EXTTS;
+ ptp_clock_event(shared->ptp_clock, &ptp_event);
+
+ return 0;
+}
+
+static int lan8814_handle_gpio_interrupt(struct phy_device *phydev, u16 status)
+{
+ struct lan8814_shared_priv *shared = phydev->shared->priv;
+ int ret;
+
+ mutex_lock(&shared->shared_lock);
+ ret = lan8814_gpio_process_cap(shared);
+ mutex_unlock(&shared->shared_lock);
+
+ return ret;
+}
+
static int lan8804_config_init(struct phy_device *phydev)
{
int val;
@@ -3424,6 +3601,9 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
ret = IRQ_HANDLED;
}
+ if (!lan8814_handle_gpio_interrupt(phydev, irq_status))
+ ret = IRQ_HANDLED;
+
return ret;
}
@@ -3541,7 +3721,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
snprintf(shared->ptp_clock_info.name, 30, "%s", phydev->drv->name);
shared->ptp_clock_info.max_adj = 31249999;
shared->ptp_clock_info.n_alarm = 0;
- shared->ptp_clock_info.n_ext_ts = 0;
+ shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
shared->ptp_clock_info.pps = 0;
shared->ptp_clock_info.pin_config = shared->pin_config;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-23 19:57 [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814 Horatiu Vultur
@ 2024-04-24 10:57 ` Vadim Fedorenko
2024-04-24 19:12 ` Horatiu Vultur
2024-04-26 1:44 ` Jakub Kicinski
1 sibling, 1 reply; 7+ messages in thread
From: Vadim Fedorenko @ 2024-04-24 10:57 UTC (permalink / raw)
To: Horatiu Vultur, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, richardcochran
Cc: netdev, linux-kernel
On 23/04/2024 20:57, Horatiu Vultur wrote:
> Extend the PTP programmable gpios to implement also PTP_PF_EXTTS
> function. The pins can be configured to capture both of rising
> and falling edge. Once the event is seen, then an interrupt is
> generated and the LTC is saved in the registers.
> On lan8814 only GPIO 3 can be configured for this.
>
> This was tested using:
> ts2phc -m -l 7 -s generic -f ts2phc.cfg
>
> Where the configuration was the following:
> ---
> [global]
> ts2phc.pin_index 3
>
> [eth0]
> ---
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
I'm not sure what happened to (fac63186f116 net: phy: micrel: Add
support for PTP_PF_EXTTS for lan8841), looks like this patch is the
rework previous with the limit to GPIO 3 only. In this case comments
below are applicable.
> ---
> drivers/net/phy/micrel.c | 182 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 181 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 0e310a5e2bff0..2d11f38cbc243 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -167,6 +167,9 @@
> #define PTP_CMD_CTL_PTP_LTC_STEP_SEC_ BIT(5)
> #define PTP_CMD_CTL_PTP_LTC_STEP_NSEC_ BIT(6)
>
> +#define PTP_COMMON_INT_ENA 0x0204
> +#define PTP_COMMON_INT_ENA_GPIO_CAP_EN BIT(2)
> +
> #define PTP_CLOCK_SET_SEC_HI 0x0205
> #define PTP_CLOCK_SET_SEC_MID 0x0206
> #define PTP_CLOCK_SET_SEC_LO 0x0207
> @@ -179,6 +182,27 @@
> #define PTP_CLOCK_READ_NS_HI 0x022C
> #define PTP_CLOCK_READ_NS_LO 0x022D
>
> +#define PTP_GPIO_SEL 0x0230
> +#define PTP_GPIO_SEL_GPIO_SEL(pin) ((pin) << 8)
> +#define PTP_GPIO_CAP_MAP_LO 0x0232
> +
> +#define PTP_GPIO_CAP_EN 0x0233
> +#define PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(gpio) BIT(gpio)
> +#define PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(gpio) (BIT(gpio) << 8)
> +
> +#define PTP_GPIO_RE_LTC_SEC_HI_CAP 0x0235
> +#define PTP_GPIO_RE_LTC_SEC_LO_CAP 0x0236
> +#define PTP_GPIO_RE_LTC_NS_HI_CAP 0x0237
> +#define PTP_GPIO_RE_LTC_NS_LO_CAP 0x0238
> +#define PTP_GPIO_FE_LTC_SEC_HI_CAP 0x0239
> +#define PTP_GPIO_FE_LTC_SEC_LO_CAP 0x023A
> +#define PTP_GPIO_FE_LTC_NS_HI_CAP 0x023B
> +#define PTP_GPIO_FE_LTC_NS_LO_CAP 0x023C
> +
> +#define PTP_GPIO_CAP_STS 0x023D
> +#define PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(gpio) BIT(gpio)
> +#define PTP_GPIO_CAP_STS_PTP_GPIO_FE_STS(gpio) (BIT(gpio) << 8)
> +
> #define PTP_OPERATING_MODE 0x0241
> #define PTP_OPERATING_MODE_STANDALONE_ BIT(0)
>
> @@ -274,6 +298,7 @@
>
> #define LAN8814_PTP_GPIO_NUM 24
> #define LAN8814_PTP_PEROUT_NUM 2
> +#define LAN8814_PTP_EXTTS_NUM 3
>
> #define LAN8814_BUFFER_TIME 2
>
> @@ -3124,12 +3149,102 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
> return 0;
> }
>
> +static void lan8814_ptp_extts_on(struct phy_device *phydev, int pin, u32 flags)
> +{
> + u16 tmp;
> +
> + /* Set as gpio input */
> + tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
> + tmp &= ~LAN8814_GPIO_DIR_BIT(pin);
> + lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
> +
> + /* Map the pin to ltc pin 0 of the capture map registers */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
> + tmp |= pin;
> + lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
> +
> + /* Enable capture on the edges of the ltc pin */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
> + if (flags & PTP_RISING_EDGE)
> + tmp |= PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0);
> + if (flags & PTP_FALLING_EDGE)
> + tmp |= PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0);
> + lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
> +
> + /* Enable interrupt top interrupt */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
> + tmp |= PTP_COMMON_INT_ENA_GPIO_CAP_EN;
> + lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
> +}
> +
> +static void lan8814_ptp_extts_off(struct phy_device *phydev, int pin)
> +{
> + u16 tmp;
> +
> + /* Set as gpio out */
> + tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
> + tmp |= LAN8814_GPIO_DIR_BIT(pin);
> + lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
> +
> + /* Enable alternate, 0:for alternate function, 1:gpio */
> + tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin));
> + tmp &= ~LAN8814_GPIO_EN_BIT(pin);
> + lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin), tmp);
> +
> + /* Clear the mapping of pin to registers 0 of the capture registers */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
> + tmp &= ~GENMASK(3, 0);
> + lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
> +
> + /* Disable capture on both of the edges */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
> + tmp &= ~PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(pin);
> + tmp &= ~PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(pin);
> + lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
> +
> + /* Disable interrupt top interrupt */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
> + tmp &= ~PTP_COMMON_INT_ENA_GPIO_CAP_EN;
> + lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
> +}
> +
> +static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
> + struct ptp_clock_request *rq, int on)
> +{
> + struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
> + ptp_clock_info);
> + struct phy_device *phydev = shared->phydev;
> + int pin;
> +
> + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> + PTP_EXTTS_EDGES |
> + PTP_STRICT_FLAGS))
> + return -EOPNOTSUPP;
> +
> + pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
> + rq->extts.index);
> + if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
> + return -EINVAL;
I'm not sure how will enable request pass this check?
In lan8814_ptp_probe_once pins are initialized with PTP_PF_NONE,
and ptp_find_pin will always return -1, which will end up with
-EINVAL here and never hit lan8814_ptp_extts_on/lan8814_ptp_extts_off
> +
> + mutex_lock(&shared->shared_lock);
> + if (on)
> + lan8814_ptp_extts_on(phydev, pin, rq->extts.flags);
> + else
> + lan8814_ptp_extts_off(phydev, pin);
> +
> + mutex_unlock(&shared->shared_lock);
> +
> + return 0;
> +}
> +
> static int lan8814_ptpci_enable(struct ptp_clock_info *ptpci,
> struct ptp_clock_request *rq, int on)
> {
> switch (rq->type) {
> case PTP_CLK_REQ_PEROUT:
> return lan8814_ptp_perout(ptpci, rq, on);
> + case PTP_CLK_REQ_EXTTS:
> + return lan8814_ptp_extts(ptpci, rq, on);
> default:
> return -EINVAL;
> }
> @@ -3148,6 +3263,10 @@ static int lan8814_ptpci_verify(struct ptp_clock_info *ptp, unsigned int pin,
> if (pin >= LAN8814_PTP_PEROUT_NUM || pin != chan)
> return -1;
> break;
> + case PTP_PF_EXTTS:
> + if (pin != LAN8814_PTP_EXTTS_NUM)
Here the check states that exactly GPIO 3 can have EXTTS function, but
later in the config...
> + return -1;
> + break;
> default:
> return -1;
> }
> @@ -3320,6 +3439,64 @@ static void lan8814_handle_ptp_interrupt(struct phy_device *phydev, u16 status)
> }
> }
>
> +static int lan8814_gpio_process_cap(struct lan8814_shared_priv *shared)
> +{
> + struct phy_device *phydev = shared->phydev;
> + struct ptp_clock_event ptp_event = {0};
> + unsigned long nsec;
> + s64 sec;
> + u16 tmp;
> +
> + /* This is 0 because whatever was the input pin it was mapped it to
> + * ltc gpio pin 0
> + */
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_SEL);
> + tmp |= PTP_GPIO_SEL_GPIO_SEL(0);
> + lanphy_write_page_reg(phydev, 4, PTP_GPIO_SEL, tmp);
> +
> + tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_STS);
> + if (!(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(0)) &&
> + !(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_FE_STS(0)))
> + return -1;
> +
> + if (tmp & BIT(0)) {
> + sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_HI_CAP);
> + sec <<= 16;
> + sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_LO_CAP);
> +
> + nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_HI_CAP) & 0x3fff;
> + nsec <<= 16;
> + nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
> + } else {
> + sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_HI_CAP);
> + sec <<= 16;
> + sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_LO_CAP);
> +
> + nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_NS_HI_CAP) & 0x3fff;
> + nsec <<= 16;
> + nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
> + }
> +
> + ptp_event.index = 0;
> + ptp_event.timestamp = ktime_set(sec, nsec);
> + ptp_event.type = PTP_CLOCK_EXTTS;
> + ptp_clock_event(shared->ptp_clock, &ptp_event);
> +
> + return 0;
> +}
> +
> +static int lan8814_handle_gpio_interrupt(struct phy_device *phydev, u16 status)
> +{
> + struct lan8814_shared_priv *shared = phydev->shared->priv;
> + int ret;
> +
> + mutex_lock(&shared->shared_lock);
> + ret = lan8814_gpio_process_cap(shared);
> + mutex_unlock(&shared->shared_lock);
> +
> + return ret;
> +}
> +
> static int lan8804_config_init(struct phy_device *phydev)
> {
> int val;
> @@ -3424,6 +3601,9 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
> ret = IRQ_HANDLED;
> }
>
> + if (!lan8814_handle_gpio_interrupt(phydev, irq_status))
> + ret = IRQ_HANDLED;
> +
> return ret;
> }
>
> @@ -3541,7 +3721,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
> snprintf(shared->ptp_clock_info.name, 30, "%s", phydev->drv->name);
> shared->ptp_clock_info.max_adj = 31249999;
> shared->ptp_clock_info.n_alarm = 0;
> - shared->ptp_clock_info.n_ext_ts = 0;
> + shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
Here ptp_clock is configured to have 3 pins supporting EXTTS.
Looks like it should be n_ext_ts = 1;
> shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
> shared->ptp_clock_info.pps = 0;
> shared->ptp_clock_info.pin_config = shared->pin_config;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-24 10:57 ` Vadim Fedorenko
@ 2024-04-24 19:12 ` Horatiu Vultur
2024-04-25 0:10 ` Vadim Fedorenko
0 siblings, 1 reply; 7+ messages in thread
From: Horatiu Vultur @ 2024-04-24 19:12 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
richardcochran, netdev, linux-kernel
The 04/24/2024 11:57, Vadim Fedorenko wrote:
Hi Vadim,
>
> On 23/04/2024 20:57, Horatiu Vultur wrote:
> > Extend the PTP programmable gpios to implement also PTP_PF_EXTTS
> > function. The pins can be configured to capture both of rising
> > and falling edge. Once the event is seen, then an interrupt is
> > generated and the LTC is saved in the registers.
> > On lan8814 only GPIO 3 can be configured for this.
> >
> > This was tested using:
> > ts2phc -m -l 7 -s generic -f ts2phc.cfg
> >
> > Where the configuration was the following:
> > ---
> > [global]
> > ts2phc.pin_index 3
> >
> > [eth0]
> > ---
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>
> I'm not sure what happened to (fac63186f116 net: phy: micrel: Add
> support for PTP_PF_EXTTS for lan8841), looks like this patch is the
> rework previous with the limit to GPIO 3 only. In this case comments
> below are applicable.
These are two different PHYs:
1. lan8814 which is a quad PHY and the patch is this PHY
2. lan8841 which is a single PHY. And the commit that you mention it was
for that PHY.
So this commit is not rework of the commit that you mention.
...
>
> > +static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
> > + struct ptp_clock_request *rq, int on)
> > +{
> > + struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
> > + ptp_clock_info);
> > + struct phy_device *phydev = shared->phydev;
> > + int pin;
> > +
> > + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > + PTP_EXTTS_EDGES |
> > + PTP_STRICT_FLAGS))
> > + return -EOPNOTSUPP;
> > +
> > + pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
> > + rq->extts.index);
> > + if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
> > + return -EINVAL;
>
> I'm not sure how will enable request pass this check?
> In lan8814_ptp_probe_once pins are initialized with PTP_PF_NONE,
> and ptp_find_pin will always return -1, which will end up with
> -EINVAL here and never hit lan8814_ptp_extts_on/lan8814_ptp_extts_off
>
Why ptp_find_pin will always return -1? Because we can set the function
of the pin.
...
> }
> > @@ -3148,6 +3263,10 @@ static int lan8814_ptpci_verify(struct ptp_clock_info *ptp, unsigned int pin,
> > if (pin >= LAN8814_PTP_PEROUT_NUM || pin != chan)
> > return -1;
> > break;
> > + case PTP_PF_EXTTS:
> > + if (pin != LAN8814_PTP_EXTTS_NUM)
>
> Here the check states that exactly GPIO 3 can have EXTTS function, but
> later in the config...
...
>
> > + return -1;
> > + break;
> > default:
> > return -1;
> > }
> >
> > @@ -3541,7 +3721,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
> > snprintf(shared->ptp_clock_info.name, 30, "%s", phydev->drv->name);
> > shared->ptp_clock_info.max_adj = 31249999;
> > shared->ptp_clock_info.n_alarm = 0;
> > - shared->ptp_clock_info.n_ext_ts = 0;
> > + shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
>
> Here ptp_clock is configured to have 3 pins supporting EXTTS.
> Looks like it should be n_ext_ts = 1;
Good point, let me have a look at this.
>
> > shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
> > shared->ptp_clock_info.pps = 0;
> > shared->ptp_clock_info.pin_config = shared->pin_config;
>
>
--
/Horatiu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-24 19:12 ` Horatiu Vultur
@ 2024-04-25 0:10 ` Vadim Fedorenko
2024-04-25 18:29 ` Horatiu Vultur
0 siblings, 1 reply; 7+ messages in thread
From: Vadim Fedorenko @ 2024-04-25 0:10 UTC (permalink / raw)
To: Horatiu Vultur
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
richardcochran, netdev, linux-kernel
On 24/04/2024 20:12, Horatiu Vultur wrote:
> The 04/24/2024 11:57, Vadim Fedorenko wrote:
>
> Hi Vadim,
>
>>
>> On 23/04/2024 20:57, Horatiu Vultur wrote:
>>> Extend the PTP programmable gpios to implement also PTP_PF_EXTTS
>>> function. The pins can be configured to capture both of rising
>>> and falling edge. Once the event is seen, then an interrupt is
>>> generated and the LTC is saved in the registers.
>>> On lan8814 only GPIO 3 can be configured for this.
>>>
>>> This was tested using:
>>> ts2phc -m -l 7 -s generic -f ts2phc.cfg
>>>
>>> Where the configuration was the following:
>>> ---
>>> [global]
>>> ts2phc.pin_index 3
>>>
>>> [eth0]
>>> ---
>>>
>>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>>
>> I'm not sure what happened to (fac63186f116 net: phy: micrel: Add
>> support for PTP_PF_EXTTS for lan8841), looks like this patch is the
>> rework previous with the limit to GPIO 3 only. In this case comments
>> below are applicable.
>
> These are two different PHYs:
> 1. lan8814 which is a quad PHY and the patch is this PHY
> 2. lan8841 which is a single PHY. And the commit that you mention it was
> for that PHY.
> So this commit is not rework of the commit that you mention.
Ah, I see, sorry for the mess..
>
> ...
>
>>
>>> +static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
>>> + struct ptp_clock_request *rq, int on)
>>> +{
>>> + struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
>>> + ptp_clock_info);
>>> + struct phy_device *phydev = shared->phydev;
>>> + int pin;
>>> +
>>> + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
>>> + PTP_EXTTS_EDGES |
>>> + PTP_STRICT_FLAGS))
>>> + return -EOPNOTSUPP;
>>> +
>>> + pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
>>> + rq->extts.index);
>>> + if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
>>> + return -EINVAL;
>>
>> I'm not sure how will enable request pass this check?
>> In lan8814_ptp_probe_once pins are initialized with PTP_PF_NONE,
>> and ptp_find_pin will always return -1, which will end up with
>> -EINVAL here and never hit lan8814_ptp_extts_on/lan8814_ptp_extts_off
>>
>
> Why ptp_find_pin will always return -1? Because we can set the function
> of the pin.
ah, I see, PTP_PIN_SETFUNC + PTP_EXTTS_REQUEST ioctls will do the
configuration. Maybe make GPIO 3 as PTP_PF_EXTTS function by default?
> ...
>
> > }
>>> @@ -3148,6 +3263,10 @@ static int lan8814_ptpci_verify(struct ptp_clock_info *ptp, unsigned int pin,
>>> if (pin >= LAN8814_PTP_PEROUT_NUM || pin != chan)
>>> return -1;
>>> break;
>>> + case PTP_PF_EXTTS:
>>> + if (pin != LAN8814_PTP_EXTTS_NUM)
>>
>> Here the check states that exactly GPIO 3 can have EXTTS function, but
>> later in the config...
>
> ...
>>
>>> + return -1;
>>> + break;
>>> default:
>>> return -1;
>>> }
>>>
>>> @@ -3541,7 +3721,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
>>> snprintf(shared->ptp_clock_info.name, 30, "%s", phydev->drv->name);
>>> shared->ptp_clock_info.max_adj = 31249999;
>>> shared->ptp_clock_info.n_alarm = 0;
>>> - shared->ptp_clock_info.n_ext_ts = 0;
>>> + shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
>>
>> Here ptp_clock is configured to have 3 pins supporting EXTTS.
>> Looks like it should be n_ext_ts = 1;
>
> Good point, let me have a look at this.
I have checked it while checking enable part. Conditions in ptp_ioctl
give no options to limit lowest number of pin which supports EXTTS.
I think that the ptp_clock_info documentation is misleading here:
* @n_ext_ts: The number of external time stamp channels.
should be replaced to something like "max index of external time
stamp channel".
With all above the patch LGTM!
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>
>>
>>> shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
>>> shared->ptp_clock_info.pps = 0;
>>> shared->ptp_clock_info.pin_config = shared->pin_config;
>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-25 0:10 ` Vadim Fedorenko
@ 2024-04-25 18:29 ` Horatiu Vultur
0 siblings, 0 replies; 7+ messages in thread
From: Horatiu Vultur @ 2024-04-25 18:29 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
richardcochran, netdev, linux-kernel
The 04/25/2024 01:10, Vadim Fedorenko wrote:
> > > > +static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
> > > > + struct ptp_clock_request *rq, int on)
> > > > +{
> > > > + struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
> > > > + ptp_clock_info);
> > > > + struct phy_device *phydev = shared->phydev;
> > > > + int pin;
> > > > +
> > > > + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > > > + PTP_EXTTS_EDGES |
> > > > + PTP_STRICT_FLAGS))
> > > > + return -EOPNOTSUPP;
> > > > +
> > > > + pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
> > > > + rq->extts.index);
> > > > + if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
> > > > + return -EINVAL;
> > >
> > > I'm not sure how will enable request pass this check?
> > > In lan8814_ptp_probe_once pins are initialized with PTP_PF_NONE,
> > > and ptp_find_pin will always return -1, which will end up with
> > > -EINVAL here and never hit lan8814_ptp_extts_on/lan8814_ptp_extts_off
> > >
> >
> > Why ptp_find_pin will always return -1? Because we can set the function
> > of the pin.
>
> ah, I see, PTP_PIN_SETFUNC + PTP_EXTTS_REQUEST ioctls will do the
> configuration. Maybe make GPIO 3 as PTP_PF_EXTTS function by default?
I would like not to make GPIO 3 to have PTP_PF_EXTTS function by default
just to keep similar with the other driver in this file.
>
> > ...
> >
> > > }
> > > > @@ -3148,6 +3263,10 @@ static int lan8814_ptpci_verify(struct ptp_clock_info *ptp, unsigned int pin,
> > > > if (pin >= LAN8814_PTP_PEROUT_NUM || pin != chan)
> > > > return -1;
> > > > break;
> > > > + case PTP_PF_EXTTS:
> > > > + if (pin != LAN8814_PTP_EXTTS_NUM)
> > >
> > > Here the check states that exactly GPIO 3 can have EXTTS function, but
> > > later in the config...
> >
> > ...
> > >
> > > > + return -1;
> > > > + break;
> > > > default:
> > > > return -1;
> > > > }
> > > >
> > > > @@ -3541,7 +3721,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
> > > > snprintf(shared->ptp_clock_info.name, 30, "%s", phydev->drv->name);
> > > > shared->ptp_clock_info.max_adj = 31249999;
> > > > shared->ptp_clock_info.n_alarm = 0;
> > > > - shared->ptp_clock_info.n_ext_ts = 0;
> > > > + shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
> > >
> > > Here ptp_clock is configured to have 3 pins supporting EXTTS.
> > > Looks like it should be n_ext_ts = 1;
> >
> > Good point, let me have a look at this.
>
> I have checked it while checking enable part. Conditions in ptp_ioctl
> give no options to limit lowest number of pin which supports EXTTS.
>
> I think that the ptp_clock_info documentation is misleading here:
>
> * @n_ext_ts: The number of external time stamp channels.
>
> should be replaced to something like "max index of external time
> stamp channel".
>
> With all above the patch LGTM!
Thanks for looking at this.
>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>
> >
> > >
> > > > shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
> > > > shared->ptp_clock_info.pps = 0;
> > > > shared->ptp_clock_info.pin_config = shared->pin_config;
> > >
> > >
> >
>
--
/Horatiu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-23 19:57 [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814 Horatiu Vultur
2024-04-24 10:57 ` Vadim Fedorenko
@ 2024-04-26 1:44 ` Jakub Kicinski
2024-04-26 7:07 ` Horatiu Vultur
1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2024-04-26 1:44 UTC (permalink / raw)
To: Horatiu Vultur
Cc: andrew, hkallweit1, linux, davem, edumazet, pabeni,
richardcochran, netdev, linux-kernel
On Tue, 23 Apr 2024 21:57:32 +0200 Horatiu Vultur wrote:
> Where the configuration was the following:
> ---
> [global]
> ts2phc.pin_index 3
>
> [eth0]
> ---
You'll have to resend, sorry, the --- lines are "cut-off markers" for
git-am. So your commit message will get mangled. Either replace them or
indent the config sample by a couple of spaces, that does the trick.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814
2024-04-26 1:44 ` Jakub Kicinski
@ 2024-04-26 7:07 ` Horatiu Vultur
0 siblings, 0 replies; 7+ messages in thread
From: Horatiu Vultur @ 2024-04-26 7:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew, hkallweit1, linux, davem, edumazet, pabeni,
richardcochran, netdev, linux-kernel
The 04/25/2024 18:44, Jakub Kicinski wrote:
>
> On Tue, 23 Apr 2024 21:57:32 +0200 Horatiu Vultur wrote:
> > Where the configuration was the following:
> > ---
> > [global]
> > ts2phc.pin_index 3
> >
> > [eth0]
> > ---
>
> You'll have to resend, sorry, the --- lines are "cut-off markers" for
> git-am. So your commit message will get mangled. Either replace them or
> indent the config sample by a couple of spaces, that does the trick.
That is true. I will indent the config sample with some spaces.
Thanks.
> --
> pw-bot: cr
--
/Horatiu
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-26 7:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-23 19:57 [PATCH net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814 Horatiu Vultur
2024-04-24 10:57 ` Vadim Fedorenko
2024-04-24 19:12 ` Horatiu Vultur
2024-04-25 0:10 ` Vadim Fedorenko
2024-04-25 18:29 ` Horatiu Vultur
2024-04-26 1:44 ` Jakub Kicinski
2024-04-26 7:07 ` Horatiu Vultur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).