| Message ID | 20230807110048.2611456-1-danishanwar@ti.com |
|---|---|
| Headers | show |
| Series | Introduce IEP driver and packet timestamping support | expand |
> @@ -210,6 +210,9 @@ void icssg_config_ipg(struct prueth_emac *emac) > case SPEED_100: > icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); > break; > + case SPEED_10: > + icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); > + break; Since that looks like a typO, you might want to add a comment. Adnrew
On 07/08/23 7:55 pm, Andrew Lunn wrote: >> @@ -210,6 +210,9 @@ void icssg_config_ipg(struct prueth_emac *emac) >> case SPEED_100: >> icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); >> break; >> + case SPEED_10: >> + icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); >> + break; > > Since that looks like a typO, you might want to add a comment. > > Adnrew Sure, Andrew. I'll add the below comment in 'case SPEED_10' so that it doesn't seem like a typo. case SPEED_10: /* IPG for 10M is same as 100M */ icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); break;
On 07/08/2023 14:00, MD Danish Anwar wrote: > From: Grygorii Strashko <grygorii.strashko@ti.com> > > For AM65x SR2.0 it's required to enable IEP1 in raw 64bit mode which is > used by PRU FW to monitor the link and apply w/a for 10M link issue. > Note. No public errata available yet. > > Without this w/a the PRU FW will stuck if link state changes under TX > traffic pressure. > > Hence, add support for 10M full duplex for AM65x SR2.0: > - add new IEP API to enable IEP, but without PTP support > - add pdata quirk_10m_link_issue to enable 10M link issue w/a. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > Signed-off-by: MD Danish Anwar <danishanwar@ti.com> > --- > drivers/net/ethernet/ti/icssg/icss_iep.c | 26 ++++++++++++++++++++ > drivers/net/ethernet/ti/icssg/icss_iep.h | 2 ++ > drivers/net/ethernet/ti/icssg/icssg_config.c | 6 +++++ > drivers/net/ethernet/ti/icssg/icssg_prueth.c | 17 +++++++++++-- > 4 files changed, 49 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c > index 455c803dea36..527f17430f05 100644 > --- a/drivers/net/ethernet/ti/icssg/icss_iep.c > +++ b/drivers/net/ethernet/ti/icssg/icss_iep.c > @@ -721,6 +721,32 @@ void icss_iep_put(struct icss_iep *iep) > } > EXPORT_SYMBOL_GPL(icss_iep_put); > > +void icss_iep_init_fw(struct icss_iep *iep) > +{ > + /* start IEP for FW use in raw 64bit mode, no PTP support */ > + iep->clk_tick_time = iep->def_inc; > + iep->cycle_time_ns = 0; > + iep->ops = NULL; > + iep->clockops_data = NULL; > + icss_iep_set_default_inc(iep, iep->def_inc); > + icss_iep_set_compensation_inc(iep, iep->def_inc); > + icss_iep_set_compensation_count(iep, 0); > + regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG, iep->refclk_freq / 10); /* 100 ms pulse */ > + regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0); > + if (iep->plat_data->flags & ICSS_IEP_SLOW_COMPEN_REG_SUPPORT) > + icss_iep_set_slow_compensation_count(iep, 0); > + > + icss_iep_enable(iep); > + icss_iep_settime(iep, 0); > +} > +EXPORT_SYMBOL_GPL(icss_iep_init_fw); > + > +void icss_iep_exit_fw(struct icss_iep *iep) > +{ > + icss_iep_disable(iep); > +} > +EXPORT_SYMBOL_GPL(icss_iep_exit_fw); > + > int icss_iep_init(struct icss_iep *iep, const struct icss_iep_clockops *clkops, > void *clockops_data, u32 cycle_time_ns) > { > diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.h b/drivers/net/ethernet/ti/icssg/icss_iep.h > index 9c7f4d0a0916..803a4b714893 100644 > --- a/drivers/net/ethernet/ti/icssg/icss_iep.h > +++ b/drivers/net/ethernet/ti/icssg/icss_iep.h > @@ -35,5 +35,7 @@ int icss_iep_exit(struct icss_iep *iep); > int icss_iep_get_count_low(struct icss_iep *iep); > int icss_iep_get_count_hi(struct icss_iep *iep); > int icss_iep_get_ptp_clock_idx(struct icss_iep *iep); > +void icss_iep_init_fw(struct icss_iep *iep); > +void icss_iep_exit_fw(struct icss_iep *iep); > > #endif /* __NET_TI_ICSS_IEP_H */ > diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c > index ab648d3efe85..4c2b5d496670 100644 > --- a/drivers/net/ethernet/ti/icssg/icssg_config.c > +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c > @@ -210,6 +210,9 @@ void icssg_config_ipg(struct prueth_emac *emac) > case SPEED_100: > icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); > break; > + case SPEED_10: > + icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); > + break; > default: > /* Other links speeds not supported */ > netdev_err(emac->ndev, "Unsupported link speed\n"); > @@ -440,6 +443,9 @@ void icssg_config_set_speed(struct prueth_emac *emac) > case SPEED_100: > fw_speed = FW_LINK_SPEED_100M; > break; > + case SPEED_10: > + fw_speed = FW_LINK_SPEED_10M; > + break; > default: > /* Other links speeds not supported */ > netdev_err(emac->ndev, "Unsupported link speed\n"); > diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c > index b82a718fd602..216918162960 100644 > --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c > +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c > @@ -1131,7 +1131,6 @@ static int emac_phy_connect(struct prueth_emac *emac) > > /* remove unsupported modes */ > phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT); > - phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT); > phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); > phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); > phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_Pause_BIT); > @@ -2081,13 +2080,20 @@ static int prueth_probe(struct platform_device *pdev) > goto free_pool; > } > > + if (prueth->pdata.quirk_10m_link_issue) { > + /* Enable IEP1 for FW in 64bit mode as W/A for 10M FD link detect issue under TX > + * traffic. > + */ > + icss_iep_init_fw(prueth->iep1); This is the only place where IEP1 is used. You should add all IEP1 related code in this patch. > + } > + > /* setup netdev interfaces */ > if (eth0_node) { > ret = prueth_netdev_init(prueth, eth0_node); > if (ret) { > dev_err_probe(dev, ret, "netdev init %s failed\n", > eth0_node->name); > - goto netdev_exit; > + goto exit_iep; > } > prueth->emac[PRUETH_MAC0]->iep = prueth->iep0; > } > @@ -2158,6 +2164,10 @@ static int prueth_probe(struct platform_device *pdev) > prueth_netdev_exit(prueth, eth_node); > } > > +exit_iep: > + if (prueth->pdata.quirk_10m_link_issue) > + icss_iep_exit_fw(prueth->iep1); > + > free_pool: > gen_pool_free(prueth->sram_pool, > (unsigned long)prueth->msmcram.va, msmc_ram_size); > @@ -2203,6 +2213,9 @@ static void prueth_remove(struct platform_device *pdev) > prueth_netdev_exit(prueth, eth_node); > } > > + if (prueth->pdata.quirk_10m_link_issue) > + icss_iep_exit_fw(prueth->iep1); > + > icss_iep_put(prueth->iep1); > icss_iep_put(prueth->iep0); >
On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: > This series introduces Industrial Ethernet Peripheral (IEP) driver to > support timestamping of ethernet packets and thus support PTP and PPS > for PRU ICSSG ethernet ports. > > This series also adds 10M full duplex support for ICSSG ethernet driver. > > There are two IEP instances. IEP0 is used for packet timestamping while IEP1 > is used for 10M full duplex support. > > This is v2 of the series [v1]. It addresses comments made on [v1]. > This series is based on linux-next(#next-20230807). > > Changes from v1 to v2: > *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs > in patch 3 and 4 were not following reverse xmas tree variable declaration. > Fixed it in this version. > *) Addressed Conor's comments and removed unsupported SoCs from compatible > comment in patch 1. I'm sorry I missed responding there before you sent v2, it was a bank holiday yesterday. I'm curious why you removed them, rather than just added them with a fallback to the ti,am654-icss-iep compatible, given your comment that "the same compatible currently works for all these 3 SoCs". Thanks, Conor. > *) Addded patch 2 which was not part of v1. Patch 2, adds IEP node to dt > bindings for ICSSG. > > [v1] https://lore.kernel.org/all/20230803110153.3309577-1-danishanwar@ti.com/ > > Thanks and Regards, > Md Danish Anwar > > Grygorii Strashko (1): > net: ti: icssg-prueth: am65x SR2.0 add 10M full duplex support > > MD Danish Anwar (1): > dt-bindings: net: Add iep node in ICSSG driver dt binding > > Md Danish Anwar (1): > dt-bindings: net: Add ICSS IEP > > Roger Quadros (2): > net: ti: icss-iep: Add IEP driver > net: ti: icssg-prueth: add packet timestamping and ptp support > > .../devicetree/bindings/net/ti,icss-iep.yaml | 37 + > .../bindings/net/ti,icssg-prueth.yaml | 7 + > drivers/net/ethernet/ti/Kconfig | 12 + > drivers/net/ethernet/ti/Makefile | 1 + > drivers/net/ethernet/ti/icssg/icss_iep.c | 961 ++++++++++++++++++ > drivers/net/ethernet/ti/icssg/icss_iep.h | 41 + > drivers/net/ethernet/ti/icssg/icssg_config.c | 6 + > drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 21 + > drivers/net/ethernet/ti/icssg/icssg_prueth.c | 433 +++++++- > drivers/net/ethernet/ti/icssg/icssg_prueth.h | 28 +- > 10 files changed, 1540 insertions(+), 7 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/ti,icss-iep.yaml > create mode 100644 drivers/net/ethernet/ti/icssg/icss_iep.c > create mode 100644 drivers/net/ethernet/ti/icssg/icss_iep.h > > -- > 2.34.1 >
On 08/08/23 5:38 pm, Conor Dooley wrote: > On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: >> This series introduces Industrial Ethernet Peripheral (IEP) driver to >> support timestamping of ethernet packets and thus support PTP and PPS >> for PRU ICSSG ethernet ports. >> >> This series also adds 10M full duplex support for ICSSG ethernet driver. >> >> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 >> is used for 10M full duplex support. >> >> This is v2 of the series [v1]. It addresses comments made on [v1]. >> This series is based on linux-next(#next-20230807). >> >> Changes from v1 to v2: >> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs >> in patch 3 and 4 were not following reverse xmas tree variable declaration. >> Fixed it in this version. >> *) Addressed Conor's comments and removed unsupported SoCs from compatible >> comment in patch 1. > > I'm sorry I missed responding there before you sent v2, it was a bank > holiday yesterday. I'm curious why you removed them, rather than just > added them with a fallback to the ti,am654-icss-iep compatible, given > your comment that "the same compatible currently works for all these > 3 SoCs". I removed them as currently the driver is being upstreamed only for AM654x, once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will add them here. If at that time we are still using same compatible, then I will modify the comment otherwise add new compatible. As of now, I don't see the need of adding other SoCs in iep binding as IEP driver up-streaming is only planned for AM654x as of now. > > Thanks, > Conor. > >> *) Addded patch 2 which was not part of v1. Patch 2, adds IEP node to dt >> bindings for ICSSG. >> >> [v1] https://lore.kernel.org/all/20230803110153.3309577-1-danishanwar@ti.com/ >> >> Thanks and Regards, >> Md Danish Anwar >> >> Grygorii Strashko (1): >> net: ti: icssg-prueth: am65x SR2.0 add 10M full duplex support >> >> MD Danish Anwar (1): >> dt-bindings: net: Add iep node in ICSSG driver dt binding >> >> Md Danish Anwar (1): >> dt-bindings: net: Add ICSS IEP >> >> Roger Quadros (2): >> net: ti: icss-iep: Add IEP driver >> net: ti: icssg-prueth: add packet timestamping and ptp support >> >> .../devicetree/bindings/net/ti,icss-iep.yaml | 37 + >> .../bindings/net/ti,icssg-prueth.yaml | 7 + >> drivers/net/ethernet/ti/Kconfig | 12 + >> drivers/net/ethernet/ti/Makefile | 1 + >> drivers/net/ethernet/ti/icssg/icss_iep.c | 961 ++++++++++++++++++ >> drivers/net/ethernet/ti/icssg/icss_iep.h | 41 + >> drivers/net/ethernet/ti/icssg/icssg_config.c | 6 + >> drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 21 + >> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 433 +++++++- >> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 28 +- >> 10 files changed, 1540 insertions(+), 7 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> create mode 100644 drivers/net/ethernet/ti/icssg/icss_iep.c >> create mode 100644 drivers/net/ethernet/ti/icssg/icss_iep.h >> >> -- >> 2.34.1 >>
On 08/08/2023 15:18, Md Danish Anwar wrote: > On 08/08/23 5:38 pm, Conor Dooley wrote: >> On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: >>> This series introduces Industrial Ethernet Peripheral (IEP) driver to >>> support timestamping of ethernet packets and thus support PTP and PPS >>> for PRU ICSSG ethernet ports. >>> >>> This series also adds 10M full duplex support for ICSSG ethernet driver. >>> >>> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 >>> is used for 10M full duplex support. >>> >>> This is v2 of the series [v1]. It addresses comments made on [v1]. >>> This series is based on linux-next(#next-20230807). >>> >>> Changes from v1 to v2: >>> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs >>> in patch 3 and 4 were not following reverse xmas tree variable declaration. >>> Fixed it in this version. >>> *) Addressed Conor's comments and removed unsupported SoCs from compatible >>> comment in patch 1. >> >> I'm sorry I missed responding there before you sent v2, it was a bank >> holiday yesterday. I'm curious why you removed them, rather than just >> added them with a fallback to the ti,am654-icss-iep compatible, given >> your comment that "the same compatible currently works for all these >> 3 SoCs". > > I removed them as currently the driver is being upstreamed only for AM654x, > once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will > add them here. If at that time we are still using same compatible, then I will > modify the comment otherwise add new compatible. > > As of now, I don't see the need of adding other SoCs in iep binding as IEP > driver up-streaming is only planned for AM654x as of now. But, is there any difference in IEP hardware/driver for the other SoCs? AFAIK the same IP is used on all SoCs. If there is no hardware/code change then we don't need to introduce a new compatible. The comment for all SoCs can already be there right from the start.
On 08/08/23 5:52 pm, Roger Quadros wrote: > > > On 08/08/2023 15:18, Md Danish Anwar wrote: >> On 08/08/23 5:38 pm, Conor Dooley wrote: >>> On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: >>>> This series introduces Industrial Ethernet Peripheral (IEP) driver to >>>> support timestamping of ethernet packets and thus support PTP and PPS >>>> for PRU ICSSG ethernet ports. >>>> >>>> This series also adds 10M full duplex support for ICSSG ethernet driver. >>>> >>>> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 >>>> is used for 10M full duplex support. >>>> >>>> This is v2 of the series [v1]. It addresses comments made on [v1]. >>>> This series is based on linux-next(#next-20230807). >>>> >>>> Changes from v1 to v2: >>>> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs >>>> in patch 3 and 4 were not following reverse xmas tree variable declaration. >>>> Fixed it in this version. >>>> *) Addressed Conor's comments and removed unsupported SoCs from compatible >>>> comment in patch 1. >>> >>> I'm sorry I missed responding there before you sent v2, it was a bank >>> holiday yesterday. I'm curious why you removed them, rather than just >>> added them with a fallback to the ti,am654-icss-iep compatible, given >>> your comment that "the same compatible currently works for all these >>> 3 SoCs". >> >> I removed them as currently the driver is being upstreamed only for AM654x, >> once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will >> add them here. If at that time we are still using same compatible, then I will >> modify the comment otherwise add new compatible. >> >> As of now, I don't see the need of adding other SoCs in iep binding as IEP >> driver up-streaming is only planned for AM654x as of now. > > But, is there any difference in IEP hardware/driver for the other SoCs? > AFAIK the same IP is used on all SoCs. > > If there is no hardware/code change then we don't need to introduce a new compatible. > The comment for all SoCs can already be there right from the start. > There is no code change. The same compatible is used for other SoCs. Even if the code is same I was thinking to keep the compatible as below now - ti,am654-icss-iep # for K3 AM65x SoCs and once other SoCs are introduced, I will just modify the comment, - ti,am654-icss-iep # for K3 AM65x, AM64x SoCs But we can also keep the all SoCs in comment right from start as well. I am fine with both. Conor / Roger, Please let me know which approach should I go with in next revision?
On Tue, Aug 08, 2023 at 06:06:11PM +0530, Md Danish Anwar wrote: > On 08/08/23 5:52 pm, Roger Quadros wrote: > > > > > > On 08/08/2023 15:18, Md Danish Anwar wrote: > >> On 08/08/23 5:38 pm, Conor Dooley wrote: > >>> On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: > >>>> This series introduces Industrial Ethernet Peripheral (IEP) driver to > >>>> support timestamping of ethernet packets and thus support PTP and PPS > >>>> for PRU ICSSG ethernet ports. > >>>> > >>>> This series also adds 10M full duplex support for ICSSG ethernet driver. > >>>> > >>>> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 > >>>> is used for 10M full duplex support. > >>>> > >>>> This is v2 of the series [v1]. It addresses comments made on [v1]. > >>>> This series is based on linux-next(#next-20230807). > >>>> > >>>> Changes from v1 to v2: > >>>> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs > >>>> in patch 3 and 4 were not following reverse xmas tree variable declaration. > >>>> Fixed it in this version. > >>>> *) Addressed Conor's comments and removed unsupported SoCs from compatible > >>>> comment in patch 1. > >>> > >>> I'm sorry I missed responding there before you sent v2, it was a bank > >>> holiday yesterday. I'm curious why you removed them, rather than just > >>> added them with a fallback to the ti,am654-icss-iep compatible, given > >>> your comment that "the same compatible currently works for all these > >>> 3 SoCs". > >> > >> I removed them as currently the driver is being upstreamed only for AM654x, > >> once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will > >> add them here. If at that time we are still using same compatible, then I will > >> modify the comment otherwise add new compatible. > >> > >> As of now, I don't see the need of adding other SoCs in iep binding as IEP > >> driver up-streaming is only planned for AM654x as of now. > > > > But, is there any difference in IEP hardware/driver for the other SoCs? > > AFAIK the same IP is used on all SoCs. > > > > If there is no hardware/code change then we don't need to introduce a new compatible. > > The comment for all SoCs can already be there right from the start. > > > > There is no code change. The same compatible is used for other SoCs. Even if > the code is same I was thinking to keep the compatible as below now > > - ti,am654-icss-iep # for K3 AM65x SoCs > > and once other SoCs are introduced, I will just modify the comment, > > - ti,am654-icss-iep # for K3 AM65x, AM64x SoCs > > But we can also keep the all SoCs in comment right from start as well. I am > fine with both. > Conor / Roger, Please let me know which approach should I go with in next revision? IMO, "ti,am564-icss-iep" goes in the driver and the other SoCs get specific compatibles in the binding with "ti,am564-icss-iep" as a fallback.
On 08/08/23 4:44 pm, Roger Quadros wrote: > > > On 07/08/2023 14:00, MD Danish Anwar wrote: >> From: Grygorii Strashko <grygorii.strashko@ti.com> >> >> For AM65x SR2.0 it's required to enable IEP1 in raw 64bit mode which is >> used by PRU FW to monitor the link and apply w/a for 10M link issue. >> Note. No public errata available yet. >> >> Without this w/a the PRU FW will stuck if link state changes under TX >> traffic pressure. >> >> Hence, add support for 10M full duplex for AM65x SR2.0: >> - add new IEP API to enable IEP, but without PTP support >> - add pdata quirk_10m_link_issue to enable 10M link issue w/a. >> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> Signed-off-by: MD Danish Anwar <danishanwar@ti.com> >> --- >> drivers/net/ethernet/ti/icssg/icss_iep.c | 26 ++++++++++++++++++++ >> drivers/net/ethernet/ti/icssg/icss_iep.h | 2 ++ >> drivers/net/ethernet/ti/icssg/icssg_config.c | 6 +++++ >> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 17 +++++++++++-- >> 4 files changed, 49 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c >> index 455c803dea36..527f17430f05 100644 >> --- a/drivers/net/ethernet/ti/icssg/icss_iep.c >> +++ b/drivers/net/ethernet/ti/icssg/icss_iep.c >> @@ -721,6 +721,32 @@ void icss_iep_put(struct icss_iep *iep) >> } >> EXPORT_SYMBOL_GPL(icss_iep_put); >> >> +void icss_iep_init_fw(struct icss_iep *iep) >> +{ >> + /* start IEP for FW use in raw 64bit mode, no PTP support */ >> + iep->clk_tick_time = iep->def_inc; >> + iep->cycle_time_ns = 0; >> + iep->ops = NULL; >> + iep->clockops_data = NULL; >> + icss_iep_set_default_inc(iep, iep->def_inc); >> + icss_iep_set_compensation_inc(iep, iep->def_inc); >> + icss_iep_set_compensation_count(iep, 0); >> + regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG, iep->refclk_freq / 10); /* 100 ms pulse */ >> + regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0); >> + if (iep->plat_data->flags & ICSS_IEP_SLOW_COMPEN_REG_SUPPORT) >> + icss_iep_set_slow_compensation_count(iep, 0); >> + >> + icss_iep_enable(iep); >> + icss_iep_settime(iep, 0); >> +} >> +EXPORT_SYMBOL_GPL(icss_iep_init_fw); >> + >> +void icss_iep_exit_fw(struct icss_iep *iep) >> +{ >> + icss_iep_disable(iep); >> +} >> +EXPORT_SYMBOL_GPL(icss_iep_exit_fw); >> + >> int icss_iep_init(struct icss_iep *iep, const struct icss_iep_clockops *clkops, >> void *clockops_data, u32 cycle_time_ns) >> { >> diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.h b/drivers/net/ethernet/ti/icssg/icss_iep.h >> index 9c7f4d0a0916..803a4b714893 100644 >> --- a/drivers/net/ethernet/ti/icssg/icss_iep.h >> +++ b/drivers/net/ethernet/ti/icssg/icss_iep.h >> @@ -35,5 +35,7 @@ int icss_iep_exit(struct icss_iep *iep); >> int icss_iep_get_count_low(struct icss_iep *iep); >> int icss_iep_get_count_hi(struct icss_iep *iep); >> int icss_iep_get_ptp_clock_idx(struct icss_iep *iep); >> +void icss_iep_init_fw(struct icss_iep *iep); >> +void icss_iep_exit_fw(struct icss_iep *iep); >> >> #endif /* __NET_TI_ICSS_IEP_H */ >> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c >> index ab648d3efe85..4c2b5d496670 100644 >> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c >> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c >> @@ -210,6 +210,9 @@ void icssg_config_ipg(struct prueth_emac *emac) >> case SPEED_100: >> icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); >> break; >> + case SPEED_10: >> + icssg_mii_update_ipg(prueth->mii_rt, slice, MII_RT_TX_IPG_100M); >> + break; >> default: >> /* Other links speeds not supported */ >> netdev_err(emac->ndev, "Unsupported link speed\n"); >> @@ -440,6 +443,9 @@ void icssg_config_set_speed(struct prueth_emac *emac) >> case SPEED_100: >> fw_speed = FW_LINK_SPEED_100M; >> break; >> + case SPEED_10: >> + fw_speed = FW_LINK_SPEED_10M; >> + break; >> default: >> /* Other links speeds not supported */ >> netdev_err(emac->ndev, "Unsupported link speed\n"); >> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c >> index b82a718fd602..216918162960 100644 >> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c >> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c >> @@ -1131,7 +1131,6 @@ static int emac_phy_connect(struct prueth_emac *emac) >> >> /* remove unsupported modes */ >> phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT); >> - phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT); >> phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); >> phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); >> phy_remove_link_mode(ndev->phydev, ETHTOOL_LINK_MODE_Pause_BIT); >> @@ -2081,13 +2080,20 @@ static int prueth_probe(struct platform_device *pdev) >> goto free_pool; >> } >> >> + if (prueth->pdata.quirk_10m_link_issue) { >> + /* Enable IEP1 for FW in 64bit mode as W/A for 10M FD link detect issue under TX >> + * traffic. >> + */ >> + icss_iep_init_fw(prueth->iep1); > > This is the only place where IEP1 is used. > You should add all IEP1 related code in this patch. Sure Roger, I'll do that. > >> + } >> + >> /* setup netdev interfaces */ >> if (eth0_node) { >> ret = prueth_netdev_init(prueth, eth0_node); >> if (ret) { >> dev_err_probe(dev, ret, "netdev init %s failed\n", >> eth0_node->name); >> - goto netdev_exit; >> + goto exit_iep; >> } >> prueth->emac[PRUETH_MAC0]->iep = prueth->iep0; >> } >> @@ -2158,6 +2164,10 @@ static int prueth_probe(struct platform_device *pdev) >> prueth_netdev_exit(prueth, eth_node); >> } >> >> +exit_iep: >> + if (prueth->pdata.quirk_10m_link_issue) >> + icss_iep_exit_fw(prueth->iep1); >> + >> free_pool: >> gen_pool_free(prueth->sram_pool, >> (unsigned long)prueth->msmcram.va, msmc_ram_size); >> @@ -2203,6 +2213,9 @@ static void prueth_remove(struct platform_device *pdev) >> prueth_netdev_exit(prueth, eth_node); >> } >> >> + if (prueth->pdata.quirk_10m_link_issue) >> + icss_iep_exit_fw(prueth->iep1); >> + >> icss_iep_put(prueth->iep1); >> icss_iep_put(prueth->iep0); >> >
On 08/08/23 6:15 pm, Conor Dooley wrote: > On Tue, Aug 08, 2023 at 06:06:11PM +0530, Md Danish Anwar wrote: >> On 08/08/23 5:52 pm, Roger Quadros wrote: >>> >>> >>> On 08/08/2023 15:18, Md Danish Anwar wrote: >>>> On 08/08/23 5:38 pm, Conor Dooley wrote: >>>>> On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: >>>>>> This series introduces Industrial Ethernet Peripheral (IEP) driver to >>>>>> support timestamping of ethernet packets and thus support PTP and PPS >>>>>> for PRU ICSSG ethernet ports. >>>>>> >>>>>> This series also adds 10M full duplex support for ICSSG ethernet driver. >>>>>> >>>>>> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 >>>>>> is used for 10M full duplex support. >>>>>> >>>>>> This is v2 of the series [v1]. It addresses comments made on [v1]. >>>>>> This series is based on linux-next(#next-20230807). >>>>>> >>>>>> Changes from v1 to v2: >>>>>> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs >>>>>> in patch 3 and 4 were not following reverse xmas tree variable declaration. >>>>>> Fixed it in this version. >>>>>> *) Addressed Conor's comments and removed unsupported SoCs from compatible >>>>>> comment in patch 1. >>>>> >>>>> I'm sorry I missed responding there before you sent v2, it was a bank >>>>> holiday yesterday. I'm curious why you removed them, rather than just >>>>> added them with a fallback to the ti,am654-icss-iep compatible, given >>>>> your comment that "the same compatible currently works for all these >>>>> 3 SoCs". >>>> >>>> I removed them as currently the driver is being upstreamed only for AM654x, >>>> once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will >>>> add them here. If at that time we are still using same compatible, then I will >>>> modify the comment otherwise add new compatible. >>>> >>>> As of now, I don't see the need of adding other SoCs in iep binding as IEP >>>> driver up-streaming is only planned for AM654x as of now. >>> >>> But, is there any difference in IEP hardware/driver for the other SoCs? >>> AFAIK the same IP is used on all SoCs. >>> >>> If there is no hardware/code change then we don't need to introduce a new compatible. >>> The comment for all SoCs can already be there right from the start. >>> >> >> There is no code change. The same compatible is used for other SoCs. Even if >> the code is same I was thinking to keep the compatible as below now >> >> - ti,am654-icss-iep # for K3 AM65x SoCs >> >> and once other SoCs are introduced, I will just modify the comment, >> >> - ti,am654-icss-iep # for K3 AM65x, AM64x SoCs >> >> But we can also keep the all SoCs in comment right from start as well. I am >> fine with both. > >> Conor / Roger, Please let me know which approach should I go with in next revision? > > IMO, "ti,am564-icss-iep" goes in the driver and the other SoCs get > specific compatibles in the binding with "ti,am564-icss-iep" as a > fallback. Sure. Then as for now, "ti,am654-icss-iep" goes in the driver, I will keep the dt binding compatible as below (as it was earlier in v1.) - ti,am654-icss-iep # for K3 AM65x, J721E and AM64x SoCs When new SoCs are introduced I can add specific bindings for them with "ti,am654-icss-iep" being the fallback.
Hi Conor, On 09/08/23 10:31 am, Md Danish Anwar wrote: > On 08/08/23 6:15 pm, Conor Dooley wrote: >> On Tue, Aug 08, 2023 at 06:06:11PM +0530, Md Danish Anwar wrote: >>> On 08/08/23 5:52 pm, Roger Quadros wrote: >>>> >>>> >>>> On 08/08/2023 15:18, Md Danish Anwar wrote: >>>>> On 08/08/23 5:38 pm, Conor Dooley wrote: >>>>>> On Mon, Aug 07, 2023 at 04:30:43PM +0530, MD Danish Anwar wrote: >>>>>>> This series introduces Industrial Ethernet Peripheral (IEP) driver to >>>>>>> support timestamping of ethernet packets and thus support PTP and PPS >>>>>>> for PRU ICSSG ethernet ports. >>>>>>> >>>>>>> This series also adds 10M full duplex support for ICSSG ethernet driver. >>>>>>> >>>>>>> There are two IEP instances. IEP0 is used for packet timestamping while IEP1 >>>>>>> is used for 10M full duplex support. >>>>>>> >>>>>>> This is v2 of the series [v1]. It addresses comments made on [v1]. >>>>>>> This series is based on linux-next(#next-20230807). >>>>>>> >>>>>>> Changes from v1 to v2: >>>>>>> *) Addressed Simon's comment to fix reverse xmas tree declaration. Some APIs >>>>>>> in patch 3 and 4 were not following reverse xmas tree variable declaration. >>>>>>> Fixed it in this version. >>>>>>> *) Addressed Conor's comments and removed unsupported SoCs from compatible >>>>>>> comment in patch 1. >>>>>> >>>>>> I'm sorry I missed responding there before you sent v2, it was a bank >>>>>> holiday yesterday. I'm curious why you removed them, rather than just >>>>>> added them with a fallback to the ti,am654-icss-iep compatible, given >>>>>> your comment that "the same compatible currently works for all these >>>>>> 3 SoCs". >>>>> >>>>> I removed them as currently the driver is being upstreamed only for AM654x, >>>>> once I start up-streaming the ICSSG driver for AM64 and any other SoC. I will >>>>> add them here. If at that time we are still using same compatible, then I will >>>>> modify the comment otherwise add new compatible. >>>>> >>>>> As of now, I don't see the need of adding other SoCs in iep binding as IEP >>>>> driver up-streaming is only planned for AM654x as of now. >>>> >>>> But, is there any difference in IEP hardware/driver for the other SoCs? >>>> AFAIK the same IP is used on all SoCs. >>>> >>>> If there is no hardware/code change then we don't need to introduce a new compatible. >>>> The comment for all SoCs can already be there right from the start. >>>> >>> >>> There is no code change. The same compatible is used for other SoCs. Even if >>> the code is same I was thinking to keep the compatible as below now >>> >>> - ti,am654-icss-iep # for K3 AM65x SoCs >>> >>> and once other SoCs are introduced, I will just modify the comment, >>> >>> - ti,am654-icss-iep # for K3 AM65x, AM64x SoCs >>> >>> But we can also keep the all SoCs in comment right from start as well. I am >>> fine with both. >> >>> Conor / Roger, Please let me know which approach should I go with in next revision? >> >> IMO, "ti,am564-icss-iep" goes in the driver and the other SoCs get >> specific compatibles in the binding with "ti,am564-icss-iep" as a >> fallback. > > Sure. Then as for now, "ti,am654-icss-iep" goes in the driver, I will keep the > dt binding compatible as below (as it was earlier in v1.) > > - ti,am654-icss-iep # for K3 AM65x, J721E and AM64x SoCs > > When new SoCs are introduced I can add specific bindings for them with > "ti,am654-icss-iep" being the fallback. > I checked internally and IEP hardware / driver is same across all TI K3 SoCs. Compatible "ti,am654-icss-iep" will be same for all SoCs. I don't think we need to introduce different compatibles for different SoCs in future as they will be using same hardware / driver. For now I will have below as compatible in dt bindings. This will not change in future. When new SoCs are added, they can just use this compatible itself. The driver will always use "ti,am654-icss-iep" as compatible. - ti,am654-icss-iep # for all TI K3 SoCs