* [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down @ 2021-12-06 10:13 Philippe Schenker 2021-12-06 10:13 ` [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once Philippe Schenker ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Philippe Schenker @ 2021-12-06 10:13 UTC (permalink / raw) To: netdev, Joakim Zhang, Fabio Estevam, Fugang Duan, David S . Miller, Russell King, Andrew Lunn, Jakub Kicinski Cc: Philippe Schenker, linux-kernel If a hardware-design is able to control power to the Ethernet PHY and relying on software to do a reset, the PHY does no longer work after resuming from suspend, given the PHY does need a hardware-reset. The Freescale fec driver does currently control the reset-signal of a phy but does not issue a reset on resume. On Toradex Apalis iMX8 board we do have such a design where we also don't place the RC circuit to delay the reset-line by hardware. Hence we fully rely on software to do so. Since I didn't manage to get the needed parts of Apalis iMX8 working with mainline this patchset was only tested on the downstream kernel toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release imx_5.4.70_2.3.0. [2] The affected code is still the same on mainline kernel, which would actually make me comfortable merging this patch, but due to this fact I'm sending this as RFC maybe someone else is able to test this code. This patchset aims to change the behavior by resetting the ethernet PHY in fec_resume. A short description of the patches can be found below, please find a detailed description in the commit-messages of the respective patches. [PATCH 2/2] net: fec: reset phy in resume if it was powered down This patch calls fec_reset_phy just after regulator enable in fec_resume, when the phy is resumed [PATCH 1/2] net: fec: make fec_reset_phy not only usable once This patch prepares the function fec_reset_phy to be called multiple times. It stores the data around the reset-gpio in fec_enet_private. This patch aims to do no functional changes. [1] http://git.toradex.com/cgit/linux-toradex.git/log/?h=toradex_5.4-2.3.x-imx [2] https://source.codeaurora.org/external/imx/linux-imx/log/?h=imx_5.4.70_2.3.0 Philippe Schenker (2): net: fec: make fec_reset_phy not only usable once net: fec: reset phy in resume if it was powered down drivers/net/ethernet/freescale/fec.h | 6 ++ drivers/net/ethernet/freescale/fec_main.c | 98 ++++++++++++++++------- 2 files changed, 73 insertions(+), 31 deletions(-) -- 2.34.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once 2021-12-06 10:13 [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Philippe Schenker @ 2021-12-06 10:13 ` Philippe Schenker 2021-12-06 13:13 ` Andrew Lunn 2021-12-06 10:13 ` [RFC PATCH 2/2] net: fec: reset phy in resume if it was powered down Philippe Schenker 2021-12-07 1:58 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got " Joakim Zhang 2 siblings, 1 reply; 13+ messages in thread From: Philippe Schenker @ 2021-12-06 10:13 UTC (permalink / raw) To: netdev, Joakim Zhang, Fabio Estevam, Fugang Duan, David S . Miller, Russell King, Andrew Lunn, Jakub Kicinski Cc: Philippe Schenker, linux-kernel inside fec_reset_phy devm_gpio_request_once is called hence making the function only callable once as the gpio is not stored somewhere nor freed at the end. Create a new function to collect the data around phy-reset-gpio from devicetree and store it in fec_enet_private. Make fec_reset_phy use the data stored in fec_enet_private, so this function can be called multiple times. Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com> --- drivers/net/ethernet/freescale/fec.h | 6 ++ drivers/net/ethernet/freescale/fec_main.c | 94 +++++++++++++++-------- 2 files changed, 69 insertions(+), 31 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 7b4961daa254..466607bbf9cf 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -631,6 +631,12 @@ struct fec_enet_private { int pps_enable; unsigned int next_counter; + /* PHY reset signal */ + bool phy_reset_active_high; + int phy_reset_duration; + int phy_reset_gpio; + int phy_reset_post_delay; + u64 ethtool_stats[]; }; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index bc418b910999..92840f18c48f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3588,62 +3588,90 @@ static int fec_enet_init(struct net_device *ndev) } #ifdef CONFIG_OF -static int fec_reset_phy(struct platform_device *pdev) +static int fec_reset_phy_probe(struct platform_device *pdev, + struct net_device *ndev) { - int err, phy_reset; - bool active_high = false; - int msec = 1, phy_post_delay = 0; struct device_node *np = pdev->dev.of_node; + struct fec_enet_private *fep = netdev_priv(ndev); + int tmp, ret; if (!np) return 0; - err = of_property_read_u32(np, "phy-reset-duration", &msec); + tmp = 1; + ret = of_property_read_u32(np, "phy-reset-duration", &tmp); /* A sane reset duration should not be longer than 1s */ - if (!err && msec > 1000) - msec = 1; + if (!ret && tmp > 1000) + tmp = 1; + + fep->phy_reset_duration = tmp; - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); - if (phy_reset == -EPROBE_DEFER) - return phy_reset; - else if (!gpio_is_valid(phy_reset)) + tmp = of_get_named_gpio(np, "phy-reset-gpios", 0); + if (tmp == -EPROBE_DEFER) + return tmp; + else if (!gpio_is_valid(tmp)) return 0; - err = of_property_read_u32(np, "phy-reset-post-delay", &phy_post_delay); + fep->phy_reset_gpio = tmp; + + tmp = 0; + ret = of_property_read_u32(np, "phy-reset-post-delay", &tmp); /* valid reset duration should be less than 1s */ - if (!err && phy_post_delay > 1000) + if (!ret && tmp > 1000) return -EINVAL; - active_high = of_property_read_bool(np, "phy-reset-active-high"); + fep->phy_reset_post_delay = tmp; - err = devm_gpio_request_one(&pdev->dev, phy_reset, - active_high ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, - "phy-reset"); - if (err) { - dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err); - return err; + fep->phy_reset_active_high = + of_property_read_bool(np, "phy-reset-active-high"); + + ret = devm_gpio_request_one(&pdev->dev, fep->phy_reset_gpio, + fep->phy_reset_active_high ? + GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, + "phy-reset"); + if (ret) { + dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", ret); + return ret; } - if (msec > 20) - msleep(msec); + return 0; +} + +static int fec_reset_phy(struct net_device *ndev) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + + gpio_set_value_cansleep(fep->phy_reset_gpio, + fep->phy_reset_active_high); + + if (fep->phy_reset_duration > 20) + msleep(fep->phy_reset_duration); else - usleep_range(msec * 1000, msec * 1000 + 1000); + usleep_range(fep->phy_reset_duration * 1000, + fep->phy_reset_duration * 1000 + 1000); - gpio_set_value_cansleep(phy_reset, !active_high); + gpio_set_value_cansleep(fep->phy_reset_gpio, + !fep->phy_reset_active_high); - if (!phy_post_delay) + if (!fep->phy_reset_post_delay) return 0; - if (phy_post_delay > 20) - msleep(phy_post_delay); + if (fep->phy_reset_post_delay > 20) + msleep(fep->phy_reset_post_delay); else - usleep_range(phy_post_delay * 1000, - phy_post_delay * 1000 + 1000); + usleep_range(fep->phy_reset_post_delay * 1000, + fep->phy_reset_post_delay * 1000 + 1000); return 0; } #else /* CONFIG_OF */ -static int fec_reset_phy(struct platform_device *pdev) +static int fec_reset_phy_probe(struct platform_device *pdev, + struct net_device *ndev) +{ + return 0; +} + +static int fec_reset_phy(struct net_device *ndev) { /* * In case of platform probe, the reset has been done @@ -3918,7 +3946,11 @@ fec_probe(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); - ret = fec_reset_phy(pdev); + ret = fec_reset_phy_probe(pdev, ndev); + if (ret) + goto failed_reset; + + ret = fec_reset_phy(ndev); if (ret) goto failed_reset; -- 2.34.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once 2021-12-06 10:13 ` [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once Philippe Schenker @ 2021-12-06 13:13 ` Andrew Lunn 0 siblings, 0 replies; 13+ messages in thread From: Andrew Lunn @ 2021-12-06 13:13 UTC (permalink / raw) To: Philippe Schenker Cc: netdev, Joakim Zhang, Fabio Estevam, Fugang Duan, David S . Miller, Russell King, Jakub Kicinski, linux-kernel > #ifdef CONFIG_OF > -static int fec_reset_phy(struct platform_device *pdev) > +static int fec_reset_phy_probe(struct platform_device *pdev, > + struct net_device *ndev) > { > - int err, phy_reset; > - bool active_high = false; > - int msec = 1, phy_post_delay = 0; > struct device_node *np = pdev->dev.of_node; > + struct fec_enet_private *fep = netdev_priv(ndev); > + int tmp, ret; > > if (!np) > return 0; > > - err = of_property_read_u32(np, "phy-reset-duration", &msec); > + tmp = 1; > + ret = of_property_read_u32(np, "phy-reset-duration", &tmp); > /* A sane reset duration should not be longer than 1s */ > - if (!err && msec > 1000) > - msec = 1; > + if (!ret && tmp > 1000) > + tmp = 1; > + > + fep->phy_reset_duration = tmp; If you don't change the names msec and ret, this code would be unchanged. It then becomes a lot easier to see you have not changed, the code, only moved it around. Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 2/2] net: fec: reset phy in resume if it was powered down 2021-12-06 10:13 [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Philippe Schenker 2021-12-06 10:13 ` [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once Philippe Schenker @ 2021-12-06 10:13 ` Philippe Schenker 2021-12-07 1:58 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got " Joakim Zhang 2 siblings, 0 replies; 13+ messages in thread From: Philippe Schenker @ 2021-12-06 10:13 UTC (permalink / raw) To: netdev, Joakim Zhang, Fabio Estevam, Fugang Duan, David S . Miller, Russell King, Andrew Lunn, Jakub Kicinski Cc: Philippe Schenker, linux-kernel If a board solely rely on a GPIO to reset the PHY after power-up, the PHY won't work after a power-down where the power was cut. Reset the PHY after power-enable in fec_resume. Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com> --- drivers/net/ethernet/freescale/fec_main.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 92840f18c48f..41c3825cd768 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -4118,6 +4118,10 @@ static int __maybe_unused fec_resume(struct device *dev) ret = regulator_enable(fep->reg_phy); if (ret) return ret; + + ret = fec_reset_phy(ndev); + if (ret) + return ret; } rtnl_lock(); -- 2.34.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down 2021-12-06 10:13 [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Philippe Schenker 2021-12-06 10:13 ` [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once Philippe Schenker 2021-12-06 10:13 ` [RFC PATCH 2/2] net: fec: reset phy in resume if it was powered down Philippe Schenker @ 2021-12-07 1:58 ` Joakim Zhang 2021-12-10 13:51 ` Philippe Schenker 2 siblings, 1 reply; 13+ messages in thread From: Joakim Zhang @ 2021-12-07 1:58 UTC (permalink / raw) To: Philippe Schenker, netdev, Fabio Estevam, David S . Miller, Russell King, Andrew Lunn, Jakub Kicinski Cc: linux-kernel Hi Philippe, > -----Original Message----- > From: Philippe Schenker <philippe.schenker@toradex.com> > Sent: 2021年12月6日 18:13 > To: netdev@vger.kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>; > Fabio Estevam <festevam@gmail.com>; Fugang Duan > <fugang.duan@nxp.com>; David S . Miller <davem@davemloft.net>; Russell > King <linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; Jakub > Kicinski <kuba@kernel.org> > Cc: Philippe Schenker <philippe.schenker@toradex.com>; > linux-kernel@vger.kernel.org > Subject: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down > > > If a hardware-design is able to control power to the Ethernet PHY and relying > on software to do a reset, the PHY does no longer work after resuming from > suspend, given the PHY does need a hardware-reset. > The Freescale fec driver does currently control the reset-signal of a phy but > does not issue a reset on resume. > > On Toradex Apalis iMX8 board we do have such a design where we also don't > place the RC circuit to delay the reset-line by hardware. Hence we fully rely > on software to do so. > Since I didn't manage to get the needed parts of Apalis iMX8 working with > mainline this patchset was only tested on the downstream kernel > toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release > imx_5.4.70_2.3.0. [2] The affected code is still the same on mainline kernel, > which would actually make me comfortable merging this patch, but due to > this fact I'm sending this as RFC maybe someone else is able to test this code. > > This patchset aims to change the behavior by resetting the ethernet PHY in > fec_resume. A short description of the patches can be found below, please > find a detailed description in the commit-messages of the respective > patches. > > [PATCH 2/2] net: fec: reset phy in resume if it was powered down > > This patch calls fec_reset_phy just after regulator enable in fec_resume, > when the phy is resumed > > [PATCH 1/2] net: fec: make fec_reset_phy not only usable once > > This patch prepares the function fec_reset_phy to be called multiple times. It > stores the data around the reset-gpio in fec_enet_private. > This patch aims to do no functional changes. > > [1] > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.tor > adex.com%2Fcgit%2Flinux-toradex.git%2Flog%2F%3Fh%3Dtoradex_5.4-2.3.x > -imx&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c138ed9232 > 4a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda > ta=Bw%2BZdqhAjPXqKJFZCXp0mtId1x9mkX6f6MW2ky6U1ww%3D&res > erved=0 > [2] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsourc > e.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Flog%2F%3Fh%3Dimx_ > 5.4.70_2.3.0&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c13 > 8ed92324a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > 0%7C0%7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > &sdata=of9z9hfVhHakVScLxCdEo%2BXmd2B9Ad9X8Rry6GjEZE4%3D&a > mp;reserved=0 > In fec driver, it has supported hardware reset for PHY when MAC resume back, fec_resume() -> phy_init_hw() -> phy_device_reset() de-assert the reset signal, you only need implement the properties which PHY core provided. I think you should not use deprecated reset properties provided by fec driver, instead the common reset properties provided by PHY core. Please check the dt-bindings for more details: Documentation/devicetree/bindings/net/fsl,fec.yaml Documentation/devicetree/bindings/net/ethernet-phy.yaml Best Regards, Joakim Zhang > Philippe Schenker (2): > net: fec: make fec_reset_phy not only usable once > net: fec: reset phy in resume if it was powered down > > drivers/net/ethernet/freescale/fec.h | 6 ++ > drivers/net/ethernet/freescale/fec_main.c | 98 ++++++++++++++++------- > 2 files changed, 73 insertions(+), 31 deletions(-) > > -- > 2.34.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down 2021-12-07 1:58 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got " Joakim Zhang @ 2021-12-10 13:51 ` Philippe Schenker 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini 2021-12-13 4:39 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Joakim Zhang 0 siblings, 2 replies; 13+ messages in thread From: Philippe Schenker @ 2021-12-10 13:51 UTC (permalink / raw) To: festevam, linux, kuba, netdev, davem, qiangqing.zhang, andrew Cc: linux-kernel On Tue, 2021-12-07 at 01:58 +0000, Joakim Zhang wrote: > > Hi Philippe, > > > -----Original Message----- > > From: Philippe Schenker <philippe.schenker@toradex.com> > > Sent: 2021年12月6日 18:13 > > To: netdev@vger.kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>; > > Fabio Estevam <festevam@gmail.com>; Fugang Duan > > <fugang.duan@nxp.com>; David S . Miller <davem@davemloft.net>; > > Russell > > King <linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; Jakub > > Kicinski <kuba@kernel.org> > > Cc: Philippe Schenker <philippe.schenker@toradex.com>; > > linux-kernel@vger.kernel.org > > Subject: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered > > down > > > > > > If a hardware-design is able to control power to the Ethernet PHY > > and relying > > on software to do a reset, the PHY does no longer work after > > resuming from > > suspend, given the PHY does need a hardware-reset. > > The Freescale fec driver does currently control the reset-signal of > > a phy but > > does not issue a reset on resume. > > > > On Toradex Apalis iMX8 board we do have such a design where we also > > don't > > place the RC circuit to delay the reset-line by hardware. Hence we > > fully rely > > on software to do so. > > Since I didn't manage to get the needed parts of Apalis iMX8 working > > with > > mainline this patchset was only tested on the downstream kernel > > toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release > > imx_5.4.70_2.3.0. [2] The affected code is still the same on > > mainline kernel, > > which would actually make me comfortable merging this patch, but due > > to > > this fact I'm sending this as RFC maybe someone else is able to test > > this code. > > > > This patchset aims to change the behavior by resetting the ethernet > > PHY in > > fec_resume. A short description of the patches can be found below, > > please > > find a detailed description in the commit-messages of the respective > > patches. > > > > [PATCH 2/2] net: fec: reset phy in resume if it was powered down > > > > This patch calls fec_reset_phy just after regulator enable in > > fec_resume, > > when the phy is resumed > > > > [PATCH 1/2] net: fec: make fec_reset_phy not only usable once > > > > This patch prepares the function fec_reset_phy to be called multiple > > times. It > > stores the data around the reset-gpio in fec_enet_private. > > This patch aims to do no functional changes. > > > > [1] > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.tor > > adex.com%2Fcgit%2Flinux-toradex.git%2Flog%2F%3Fh%3Dtoradex_5.4-2.3.x > > -imx&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c138ed9232 > > 4a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > > 7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda > > ta=Bw%2BZdqhAjPXqKJFZCXp0mtId1x9mkX6f6MW2ky6U1ww%3D&res > > erved=0 > > [2] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsourc > > e.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Flog%2F%3Fh%3Dimx_ > > 5.4.70_2.3.0&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c13 > > 8ed92324a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > > 0%7C0%7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > > &sdata=of9z9hfVhHakVScLxCdEo%2BXmd2B9Ad9X8Rry6GjEZE4%3D&a > > mp;reserved=0 > > > > In fec driver, it has supported hardware reset for PHY when MAC resume > back, > > fec_resume() -> phy_init_hw() -> phy_device_reset() de-assert the > reset signal, you only need implement > the properties which PHY core provided. > > I think you should not use deprecated reset properties provided by fec > driver, instead the common > reset properties provided by PHY core. > > Please check the dt-bindings for more details: > Documentation/devicetree/bindings/net/fsl,fec.yaml > Documentation/devicetree/bindings/net/ethernet-phy.yaml > > Best Regards, > Joakim Zhang Hi Joakim and many thanks for that hint! I tried that out but unfortunately it still does not work due to phy_init_hw() only deasserting the reset. For that to work, for example a call of phy_device_reset(ndev->phydev, 1); in fec_suspend or in fec_resume before enabling the supply would work, in order to assert that reset. I see now two ways to go to fix our issue: 1. Assert the phy-reset gpio in fec_suspend() or fec_resume() 2. Add support for regulators in drivers/net/phy/phy-core.c and handle the phy-reset properly in there with assert-us and deassert-us delays. As you probably have a much better overview: Do you see another possibility to handle phy-reset after resuming? Or which way shall I choose to go forward? Thanks in advance for any advice Philippe > > Philippe Schenker (2): > > net: fec: make fec_reset_phy not only usable once > > net: fec: reset phy in resume if it was powered down > > > > drivers/net/ethernet/freescale/fec.h | 6 ++ > > drivers/net/ethernet/freescale/fec_main.c | 98 ++++++++++++++++---- > > --- > > 2 files changed, 73 insertions(+), 31 deletions(-) > > > > -- > > 2.34.0 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-10 13:51 ` Philippe Schenker @ 2021-12-11 13:01 ` Francesco Dolcini 2021-12-11 14:15 ` Russell King (Oracle) ` (2 more replies) 2021-12-13 4:39 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Joakim Zhang 1 sibling, 3 replies; 13+ messages in thread From: Francesco Dolcini @ 2021-12-11 13:01 UTC (permalink / raw) To: philippe.schenker, andrew, qiangqing.zhang Cc: davem, festevam, kuba, linux-kernel, linux, netdev, Francesco Dolcini Perform a PHY reset in phy_init_hw() to ensure that the PHY is working after resume. This is required if the PHY was powered down in suspend like it is done by the freescale FEC driver in fec_suspend(). Link: https://lore.kernel.org/netdev/20211206101326.1022527-1-philippe.schenker@toradex.com/ Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> --- Philippe: what about something like that? Only compile tested, but I see no reason for this not solving the issue. Any delay required on the reset can be specified using reset-assert-us/reset-deassert-us. --- drivers/net/phy/phy_device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 74d8e1dc125f..7eab0c054adf 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1158,7 +1158,8 @@ int phy_init_hw(struct phy_device *phydev) { int ret = 0; - /* Deassert the reset signal */ + /* phy reset required if the phy was powered down during suspend */ + phy_device_reset(phydev, 1); phy_device_reset(phydev, 0); if (!phydev->drv) -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini @ 2021-12-11 14:15 ` Russell King (Oracle) 2021-12-11 14:57 ` Francesco Dolcini 2021-12-14 11:58 ` Francesco Dolcini 2021-12-13 4:40 ` Joakim Zhang 2021-12-13 10:57 ` Philippe Schenker 2 siblings, 2 replies; 13+ messages in thread From: Russell King (Oracle) @ 2021-12-11 14:15 UTC (permalink / raw) To: Francesco Dolcini Cc: philippe.schenker, andrew, qiangqing.zhang, davem, festevam, kuba, linux-kernel, netdev On Sat, Dec 11, 2021 at 02:01:46PM +0100, Francesco Dolcini wrote: > Perform a PHY reset in phy_init_hw() to ensure that the PHY is working > after resume. This is required if the PHY was powered down in suspend > like it is done by the freescale FEC driver in fec_suspend(). > > Link: https://lore.kernel.org/netdev/20211206101326.1022527-1-philippe.schenker@toradex.com/ > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > --- > > Philippe: what about something like that? Only compile tested, but I see no reason for this not solving the issue. > > Any delay required on the reset can be specified using reset-assert-us/reset-deassert-us. > > --- > drivers/net/phy/phy_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 74d8e1dc125f..7eab0c054adf 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1158,7 +1158,8 @@ int phy_init_hw(struct phy_device *phydev) > { > int ret = 0; > > - /* Deassert the reset signal */ > + /* phy reset required if the phy was powered down during suspend */ > + phy_device_reset(phydev, 1); > phy_device_reset(phydev, 0); > > if (!phydev->drv) I don't particularly like this - this impacts everyone who is using phylib at this point, whereas no reset was happening if the reset was already deasserted here. In the opening remarks to this series, it is stated: If a hardware-design is able to control power to the Ethernet PHY and relying on software to do a reset, the PHY does no longer work after resuming from suspend, given the PHY does need a hardware-reset. This requirement is conditional on the hardware design, it isn't a universal requirement and won't apply everywhere. I think it needs to be described in firmware that this action is required. That said... Please check the datasheet on the PHY regarding application of power and reset. You may find that the PHY datasheet requires the reset to be held active from power up until the clock input is stable - this could mean you need some other arrangement to assert the PHY reset signal after re-applying power sooner than would happen by the proposed point in the kernel. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-11 14:15 ` Russell King (Oracle) @ 2021-12-11 14:57 ` Francesco Dolcini 2021-12-14 11:58 ` Francesco Dolcini 1 sibling, 0 replies; 13+ messages in thread From: Francesco Dolcini @ 2021-12-11 14:57 UTC (permalink / raw) To: Russell King (Oracle) Cc: Francesco Dolcini, philippe.schenker, andrew, qiangqing.zhang, davem, festevam, kuba, linux-kernel, netdev Hello, On Sat, Dec 11, 2021 at 02:15:54PM +0000, Russell King (Oracle) wrote: > I don't particularly like this - this impacts everyone who is using > phylib at this point, whereas no reset was happening if the reset was > already deasserted here. I understand your concern, but I do not believe that this can create any issue. The code should be able to handle the situation in which the PHY is getting out of reset at that time. > In the opening remarks to this series, it is stated: > > If a hardware-design is able to control power to the Ethernet PHY and > relying on software to do a reset, the PHY does no longer work after > resuming from suspend, given the PHY does need a hardware-reset. > > This requirement is conditional on the hardware design, it isn't a > universal requirement and won't apply everywhere. I think it needs to > be described in firmware that this action is required. That said... > > Please check the datasheet on the PHY regarding application of power and > reset. You may find that the PHY datasheet requires the reset to be held > active from power up until the clock input is stable - this could mean > you need some other arrangement to assert the PHY reset signal after > re-applying power sooner than would happen by the proposed point in the > kernel. I checked before sending this patch, the phy is a KSZ9131 [1] and according to the power sequence timing the reset should be toggled after the power-up. No requirement on the clock or other signals. The HW design is quite simple, we have a regulator controlling the PHY power, and a GPIO controlling the reset. On suspend we remove the power (FEC driver), on resume after enabling the power the PHY require a reset, but nobody is doing it. The issue here is that the phy regulator is handled by the FEC driver, while the RESET_N GPIO can be controlled by both the FEC driver or the phylib. The initial proposal was to handle this into the FEC driver, but it was not considered a good idea, therefore this new proposal. One more comment about that, I do not believe that asserting the reset in the suspend path is a good idea, in the general situation in which the PHY is powered in suspend the power-consumption is likely to be higher if the device is in reset compared to software power-down using the BMCR register. > universal requirement and won't apply everywhere. I think it needs to > be described in firmware that this action is required. That said... Are you thinking at a DTS property? Not sure to understand how do you envision this. At the moment the regulator is not handled by the phylib, and the property should be something like reset-on-resume, I guess ... Francesco [1] https://ww1.microchip.com/downloads/en/DeviceDoc/00002841C.pdf ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-11 14:15 ` Russell King (Oracle) 2021-12-11 14:57 ` Francesco Dolcini @ 2021-12-14 11:58 ` Francesco Dolcini 1 sibling, 0 replies; 13+ messages in thread From: Francesco Dolcini @ 2021-12-14 11:58 UTC (permalink / raw) To: Russell King (Oracle) Cc: Francesco Dolcini, philippe.schenker, andrew, qiangqing.zhang, davem, festevam, kuba, linux-kernel, netdev On Sat, Dec 11, 2021 at 02:15:54PM +0000, Russell King (Oracle) wrote: > I don't particularly like this - this impacts everyone who is using > phylib at this point, whereas no reset was happening if the reset was > already deasserted here. Let's drop this patch, Philippe will send a new patch adding a phy_reset_after_power_on() function similar to phy_reset_after_clk_enable(). Francesco ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini 2021-12-11 14:15 ` Russell King (Oracle) @ 2021-12-13 4:40 ` Joakim Zhang 2021-12-13 10:57 ` Philippe Schenker 2 siblings, 0 replies; 13+ messages in thread From: Joakim Zhang @ 2021-12-13 4:40 UTC (permalink / raw) To: Francesco Dolcini, philippe.schenker, andrew Cc: davem, festevam, kuba, linux-kernel, linux, netdev Hi Francesco, > -----Original Message----- > From: Francesco Dolcini <francesco.dolcini@toradex.com> > Sent: 2021年12月11日 21:02 > To: philippe.schenker@toradex.com; andrew@lunn.ch; Joakim Zhang > <qiangqing.zhang@nxp.com> > Cc: davem@davemloft.net; festevam@gmail.com; kuba@kernel.org; > linux-kernel@vger.kernel.org; linux@armlinux.org.uk; > netdev@vger.kernel.org; Francesco Dolcini <francesco.dolcini@toradex.com> > Subject: [PATCH net-next] net: phy: perform a PHY reset on resume > > Perform a PHY reset in phy_init_hw() to ensure that the PHY is working after > resume. This is required if the PHY was powered down in suspend like it is > done by the freescale FEC driver in fec_suspend(). > > Link: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k > ernel.org%2Fnetdev%2F20211206101326.1022527-1-philippe.schenker%40tor > adex.com%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C408 > 258b86fec4c39a1b708d9bca6755f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 > C0%7C0%7C637748245394278104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi > MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300 > 0&sdata=m17Q5b3CZVI89xmplVVwVvCHEXZrMkY6dYIAmz2v3CE%3D&a > mp;reserved=0 > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > --- > > Philippe: what about something like that? Only compile tested, but I see no > reason for this not solving the issue. > > Any delay required on the reset can be specified using > reset-assert-us/reset-deassert-us. Looks fine for me. We can trigger a hardware reset first, then following a soft reset and phy configurations, the logic seems reasonable. Also need PHY maintainers give more comments. Best Regards, Joakim Zhang > --- > drivers/net/phy/phy_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 74d8e1dc125f..7eab0c054adf 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1158,7 +1158,8 @@ int phy_init_hw(struct phy_device *phydev) { > int ret = 0; > > - /* Deassert the reset signal */ > + /* phy reset required if the phy was powered down during suspend */ > + phy_device_reset(phydev, 1); > phy_device_reset(phydev, 0); > > if (!phydev->drv) > -- > 2.25.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next] net: phy: perform a PHY reset on resume 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini 2021-12-11 14:15 ` Russell King (Oracle) 2021-12-13 4:40 ` Joakim Zhang @ 2021-12-13 10:57 ` Philippe Schenker 2 siblings, 0 replies; 13+ messages in thread From: Philippe Schenker @ 2021-12-13 10:57 UTC (permalink / raw) To: Francesco Dolcini, andrew, qiangqing.zhang Cc: festevam, linux, netdev, davem, kuba, linux-kernel On Sat, 2021-12-11 at 14:01 +0100, Francesco Dolcini wrote: > Perform a PHY reset in phy_init_hw() to ensure that the PHY is working > after resume. This is required if the PHY was powered down in suspend > like it is done by the freescale FEC driver in fec_suspend(). > > Link: > https://lore.kernel.org/netdev/20211206101326.1022527-1-philippe.schenker@toradex.com/ > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > > --- > > Philippe: what about something like that? Only compile tested, but I > see no reason for this not solving the issue. > > Any delay required on the reset can be specified using reset-assert- > us/reset-deassert-us. That would of course be the easiest way. However I understand Russel's concerns here, as every PHY is again different and this is basically a hardware-specific design. I like Joakin's idea to add a phy_reset_after_power_on() function in phylib similar to phy_reset_after_clk_enable(). I will prepare a patchset for that so we can discuss further there. Philippe > > --- > drivers/net/phy/phy_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy_device.c > b/drivers/net/phy/phy_device.c > index 74d8e1dc125f..7eab0c054adf 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1158,7 +1158,8 @@ int phy_init_hw(struct phy_device *phydev) > { > int ret = 0; > > - /* Deassert the reset signal */ > + /* phy reset required if the phy was powered down during > suspend */ > + phy_device_reset(phydev, 1); > phy_device_reset(phydev, 0); > > if (!phydev->drv) ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down 2021-12-10 13:51 ` Philippe Schenker 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini @ 2021-12-13 4:39 ` Joakim Zhang 1 sibling, 0 replies; 13+ messages in thread From: Joakim Zhang @ 2021-12-13 4:39 UTC (permalink / raw) To: Philippe Schenker, festevam, linux, kuba, netdev, davem, andrew Cc: linux-kernel Hi Philippe, > -----Original Message----- > From: Philippe Schenker <philippe.schenker@toradex.com> > Sent: 2021年12月10日 21:51 > To: festevam@gmail.com; linux@armlinux.org.uk; kuba@kernel.org; > netdev@vger.kernel.org; davem@davemloft.net; Joakim Zhang > <qiangqing.zhang@nxp.com>; andrew@lunn.ch > Cc: linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered > down > > On Tue, 2021-12-07 at 01:58 +0000, Joakim Zhang wrote: > > > > Hi Philippe, > > > > > -----Original Message----- > > > From: Philippe Schenker <philippe.schenker@toradex.com> > > > Sent: 2021年12月6日 18:13 > > > To: netdev@vger.kernel.org; Joakim Zhang > <qiangqing.zhang@nxp.com>; > > > Fabio Estevam <festevam@gmail.com>; Fugang Duan > > > <fugang.duan@nxp.com>; David S . Miller <davem@davemloft.net>; > > > Russell King <linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; > > > Jakub Kicinski <kuba@kernel.org> > > > Cc: Philippe Schenker <philippe.schenker@toradex.com>; > > > linux-kernel@vger.kernel.org > > > Subject: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered > > > down > > > > > > > > > If a hardware-design is able to control power to the Ethernet PHY > > > and relying on software to do a reset, the PHY does no longer work > > > after resuming from suspend, given the PHY does need a > > > hardware-reset. > > > The Freescale fec driver does currently control the reset-signal of > > > a phy but does not issue a reset on resume. > > > > > > On Toradex Apalis iMX8 board we do have such a design where we also > > > don't place the RC circuit to delay the reset-line by hardware. > > > Hence we fully rely on software to do so. > > > Since I didn't manage to get the needed parts of Apalis iMX8 working > > > with mainline this patchset was only tested on the downstream kernel > > > toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release > > > imx_5.4.70_2.3.0. [2] The affected code is still the same on > > > mainline kernel, which would actually make me comfortable merging > > > this patch, but due to this fact I'm sending this as RFC maybe > > > someone else is able to test this code. > > > > > > This patchset aims to change the behavior by resetting the ethernet > > > PHY in fec_resume. A short description of the patches can be found > > > below, please find a detailed description in the commit-messages of > > > the respective patches. > > > > > > [PATCH 2/2] net: fec: reset phy in resume if it was powered down > > > > > > This patch calls fec_reset_phy just after regulator enable in > > > fec_resume, when the phy is resumed > > > > > > [PATCH 1/2] net: fec: make fec_reset_phy not only usable once > > > > > > This patch prepares the function fec_reset_phy to be called multiple > > > times. It stores the data around the reset-gpio in fec_enet_private. > > > This patch aims to do no functional changes. > > > > > > [1] > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit > > > .tor%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Ca43416 > 6e62464 > > > > c4b69c408d9bbe420ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C637 > > > > 747410747358279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi > LCJQIjoi > > > > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8u1EudB0 > aDkC > > > K9tzrqVznAWY6mDgBIHqIQCyDWrsH4g%3D&reserved=0 > > > > adex.com%2Fcgit%2Flinux-toradex.git%2Flog%2F%3Fh%3Dtoradex_5.4-2.3.x > > > > -imx&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c138ed9232 > > > > 4a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > > > > 7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > > > > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda > > > > ta=Bw%2BZdqhAjPXqKJFZCXp0mtId1x9mkX6f6MW2ky6U1ww%3D&res > > > erved=0 > > > [2] > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fso > > > urc > > > > e.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Flog%2F%3Fh%3Dimx_ > > > > 5.4.70_2.3.0&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c13 > > > > 8ed92324a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > > > > 0%7C0%7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > > > > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > > > > &sdata=of9z9hfVhHakVScLxCdEo%2BXmd2B9Ad9X8Rry6GjEZE4%3D&a > > > mp;reserved=0 > > > > > > > In fec driver, it has supported hardware reset for PHY when MAC resume > > back, > > > > fec_resume() -> phy_init_hw() -> phy_device_reset() de-assert the > > reset signal, you only need implement the properties which PHY core > > provided. > > > > I think you should not use deprecated reset properties provided by fec > > driver, instead the common reset properties provided by PHY core. > > > > Please check the dt-bindings for more details: > > Documentation/devicetree/bindings/net/fsl,fec.yaml > > Documentation/devicetree/bindings/net/ethernet-phy.yaml > > > > Best Regards, > > Joakim Zhang > > Hi Joakim and many thanks for that hint! I tried that out but unfortunately it > still does not work due to phy_init_hw() only deasserting the reset. For that > to work, for example a call of phy_device_reset(ndev->phydev, 1); in > fec_suspend or in fec_resume before enabling the supply would work, in > order to assert that reset. Yes, you are right. It only de-assert the reset in phy_init_hw(), should not enough for you. It seems that phy reset would not be performed during suspend/resume in the phylib. AFAIK, a hardware reset automatically generated when power on for most PHYs, the PHY you used may be special, and not sure why phylib has not taken reset into suspend/resume scenario, only perform PHY suspend/resume. > I see now two ways to go to fix our issue: > > 1. Assert the phy-reset gpio in fec_suspend() or fec_resume() > > 2. Add support for regulators in drivers/net/phy/phy-core.c and handle the > phy-reset properly in there with assert-us and deassert-us delays. > > As you probably have a much better overview: Do you see another possibility > to handle phy-reset after resuming? Or which way shall I choose to go > forward? This should be a common issue for specific PHY, and I prefer to performing reset in phylib. Could you please take a look at phy_reset_after_clk_enable()? Is it possible to add a function like phy_reset_after_power_on() (adding extra macro, e.g. PHY_RST_AFTER_POWER_ON) for these PHYs? Best Regards, Joakim Zhang > Thanks in advance for any advice > Philippe > > > > Philippe Schenker (2): > > > net: fec: make fec_reset_phy not only usable once > > > net: fec: reset phy in resume if it was powered down > > > > > > drivers/net/ethernet/freescale/fec.h | 6 ++ > > > drivers/net/ethernet/freescale/fec_main.c | 98 > ++++++++++++++++---- > > > --- > > > 2 files changed, 73 insertions(+), 31 deletions(-) > > > > > > -- > > > 2.34.0 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-12-14 11:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-06 10:13 [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Philippe Schenker 2021-12-06 10:13 ` [RFC PATCH 1/2] net: fec: make fec_reset_phy not only usable once Philippe Schenker 2021-12-06 13:13 ` Andrew Lunn 2021-12-06 10:13 ` [RFC PATCH 2/2] net: fec: reset phy in resume if it was powered down Philippe Schenker 2021-12-07 1:58 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got " Joakim Zhang 2021-12-10 13:51 ` Philippe Schenker 2021-12-11 13:01 ` [PATCH net-next] net: phy: perform a PHY reset on resume Francesco Dolcini 2021-12-11 14:15 ` Russell King (Oracle) 2021-12-11 14:57 ` Francesco Dolcini 2021-12-14 11:58 ` Francesco Dolcini 2021-12-13 4:40 ` Joakim Zhang 2021-12-13 10:57 ` Philippe Schenker 2021-12-13 4:39 ` [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down Joakim Zhang
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 NNTP newsgroup(s).