| Message ID | 20240423195732.3353522-1-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: phy: micrel: Add support for PTP_PF_EXTTS for lan8814 | expand |
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;
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; > >
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; >> >> >
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; > > > > > > > > >
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.
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
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;
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(-)