* [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe @ 2024-10-14 7:06 Sky Huang 2024-10-14 8:43 ` Marek Vasut 2025-05-23 22:26 ` Tom Rini 0 siblings, 2 replies; 8+ messages in thread From: Sky Huang @ 2024-10-14 7:06 UTC (permalink / raw) To: Joe Hershberger, Ramon Fried, Tom Rini, Marek Vasut, Eugeniu Rosca, Heinrich Schuchardt, Jacky Chou Cc: u-boot, gss_mtk_uboot_upstream, SkyLake.Huang From: "SkyLake.Huang" <skylake.huang@mediatek.com> Remove unnecessary CL22 phy reset before ethernet phy driver probe. Lots of ethernet phys requires driver to load firmware. Before that, CL22 phy reset may lead to malfunction. Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> --- drivers/net/phy/phy.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 716a1d46111..e6fed8c41d7 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, phy_interface_t interface) { - /* Soft Reset the PHY */ - phy_reset(phydev); if (phydev->dev && phydev->dev != dev) { printf("%s:%d is connected to %s. Reconnecting to %s\n", phydev->bus->name, phydev->addr, -- 2.45.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2024-10-14 7:06 [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe Sky Huang @ 2024-10-14 8:43 ` Marek Vasut 2025-08-05 18:00 ` Paul Kocialkowski 2025-05-23 22:26 ` Tom Rini 1 sibling, 1 reply; 8+ messages in thread From: Marek Vasut @ 2024-10-14 8:43 UTC (permalink / raw) To: Sky Huang, Joe Hershberger, Ramon Fried, Tom Rini, Marek Vasut, Eugeniu Rosca, Heinrich Schuchardt, Jacky Chou Cc: u-boot, gss_mtk_uboot_upstream On 10/14/24 9:06 AM, Sky Huang wrote: > From: "SkyLake.Huang" <skylake.huang@mediatek.com> > > Remove unnecessary CL22 phy reset before ethernet phy driver > probe. Lots of ethernet phys requires driver to load firmware. > Before that, CL22 phy reset may lead to malfunction. > > Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> > --- > drivers/net/phy/phy.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index 716a1d46111..e6fed8c41d7 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) > static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, > phy_interface_t interface) > { > - /* Soft Reset the PHY */ > - phy_reset(phydev); > if (phydev->dev && phydev->dev != dev) { > printf("%s:%d is connected to %s. Reconnecting to %s\n", > phydev->bus->name, phydev->addr, This needs clarification and likely has to be handled differently. Removing the reset would leave the PHY in potentially undefined state. Which PHY is affected by this ? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2024-10-14 8:43 ` Marek Vasut @ 2025-08-05 18:00 ` Paul Kocialkowski 2025-10-26 15:43 ` Tom Rini 0 siblings, 1 reply; 8+ messages in thread From: Paul Kocialkowski @ 2025-08-05 18:00 UTC (permalink / raw) To: Marek Vasut Cc: Sky Huang, Joe Hershberger, Ramon Fried, Tom Rini, Marek Vasut, Eugeniu Rosca, Heinrich Schuchardt, Jacky Chou, u-boot, gss_mtk_uboot_upstream [-- Attachment #1: Type: text/plain, Size: 2113 bytes --] Hi, Le Mon 14 Oct 24, 10:43, Marek Vasut a écrit : > On 10/14/24 9:06 AM, Sky Huang wrote: > > From: "SkyLake.Huang" <skylake.huang@mediatek.com> > > > > Remove unnecessary CL22 phy reset before ethernet phy driver > > probe. Lots of ethernet phys requires driver to load firmware. > > Before that, CL22 phy reset may lead to malfunction. > > > > Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> > > --- > > drivers/net/phy/phy.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > > index 716a1d46111..e6fed8c41d7 100644 > > --- a/drivers/net/phy/phy.c > > +++ b/drivers/net/phy/phy.c > > @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) > > static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, > > phy_interface_t interface) > > { > > - /* Soft Reset the PHY */ > > - phy_reset(phydev); > > if (phydev->dev && phydev->dev != dev) { > > printf("%s:%d is connected to %s. Reconnecting to %s\n", > > phydev->bus->name, phydev->addr, > > This needs clarification and likely has to be handled differently. Removing > the reset would leave the PHY in potentially undefined state. > > Which PHY is affected by this ? Good hunch. I just bisected down to this commit as I had some issues with my Allwinner V3 board using the internal CL22 PHY. The symptom is that the LEDS's polarity was inverted. Network still seems to work (although not tested beyond ping). This change can have very significant consequences in general, which were not explored at all in the commit. This may break many boards that do rely on that PHY reset, in various scenarios and for various reasons. I think it should be reverted as soon as possible to restore previous behavior. All the best, Paul -- Paul Kocialkowski, Independent contractor - sys-base - https://www.sys-base.io/ Free software developer - https://www.paulk.fr/ Expert in multimedia, graphics and embedded hardware support with Linux. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2025-08-05 18:00 ` Paul Kocialkowski @ 2025-10-26 15:43 ` Tom Rini 2025-10-26 15:56 ` Marek Vasut 0 siblings, 1 reply; 8+ messages in thread From: Tom Rini @ 2025-10-26 15:43 UTC (permalink / raw) To: Paul Kocialkowski, Marek Vasut, Jacky Chou, Sky Huang Cc: Joe Hershberger, Ramon Fried, Eugeniu Rosca, Heinrich Schuchardt, u-boot, gss_mtk_uboot_upstream [-- Attachment #1: Type: text/plain, Size: 2183 bytes --] On Tue, Aug 05, 2025 at 08:00:29PM +0200, Paul Kocialkowski wrote: > Hi, > > Le Mon 14 Oct 24, 10:43, Marek Vasut a écrit : > > On 10/14/24 9:06 AM, Sky Huang wrote: > > > From: "SkyLake.Huang" <skylake.huang@mediatek.com> > > > > > > Remove unnecessary CL22 phy reset before ethernet phy driver > > > probe. Lots of ethernet phys requires driver to load firmware. > > > Before that, CL22 phy reset may lead to malfunction. > > > > > > Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> > > > --- > > > drivers/net/phy/phy.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > > > index 716a1d46111..e6fed8c41d7 100644 > > > --- a/drivers/net/phy/phy.c > > > +++ b/drivers/net/phy/phy.c > > > @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) > > > static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, > > > phy_interface_t interface) > > > { > > > - /* Soft Reset the PHY */ > > > - phy_reset(phydev); > > > if (phydev->dev && phydev->dev != dev) { > > > printf("%s:%d is connected to %s. Reconnecting to %s\n", > > > phydev->bus->name, phydev->addr, > > > > This needs clarification and likely has to be handled differently. Removing > > the reset would leave the PHY in potentially undefined state. > > > > Which PHY is affected by this ? > > Good hunch. I just bisected down to this commit as I had some issues with my > Allwinner V3 board using the internal CL22 PHY. The symptom is that the LEDS's > polarity was inverted. Network still seems to work (although not tested beyond > ping). > > This change can have very significant consequences in general, which were not > explored at all in the commit. This may break many boards that do rely on that > PHY reset, in various scenarios and for various reasons. > > I think it should be reverted as soon as possible to restore previous behavior. I'm wondering if anyone has further comments here? If I do go and revert this what is then going to break on the MediaTek side for example? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2025-10-26 15:43 ` Tom Rini @ 2025-10-26 15:56 ` Marek Vasut 2025-10-26 17:32 ` Paul Kocialkowski 0 siblings, 1 reply; 8+ messages in thread From: Marek Vasut @ 2025-10-26 15:56 UTC (permalink / raw) To: Tom Rini, Paul Kocialkowski, Marek Vasut, Jacky Chou, Sky Huang Cc: Joe Hershberger, Ramon Fried, Eugeniu Rosca, Heinrich Schuchardt, u-boot, gss_mtk_uboot_upstream On 10/26/25 4:43 PM, Tom Rini wrote: > On Tue, Aug 05, 2025 at 08:00:29PM +0200, Paul Kocialkowski wrote: >> Hi, >> >> Le Mon 14 Oct 24, 10:43, Marek Vasut a écrit : >>> On 10/14/24 9:06 AM, Sky Huang wrote: >>>> From: "SkyLake.Huang" <skylake.huang@mediatek.com> >>>> >>>> Remove unnecessary CL22 phy reset before ethernet phy driver >>>> probe. Lots of ethernet phys requires driver to load firmware. >>>> Before that, CL22 phy reset may lead to malfunction. >>>> >>>> Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> >>>> --- >>>> drivers/net/phy/phy.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >>>> index 716a1d46111..e6fed8c41d7 100644 >>>> --- a/drivers/net/phy/phy.c >>>> +++ b/drivers/net/phy/phy.c >>>> @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) >>>> static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, >>>> phy_interface_t interface) >>>> { >>>> - /* Soft Reset the PHY */ >>>> - phy_reset(phydev); >>>> if (phydev->dev && phydev->dev != dev) { >>>> printf("%s:%d is connected to %s. Reconnecting to %s\n", >>>> phydev->bus->name, phydev->addr, >>> >>> This needs clarification and likely has to be handled differently. Removing >>> the reset would leave the PHY in potentially undefined state. >>> >>> Which PHY is affected by this ? >> >> Good hunch. I just bisected down to this commit as I had some issues with my >> Allwinner V3 board using the internal CL22 PHY. The symptom is that the LEDS's >> polarity was inverted. Network still seems to work (although not tested beyond >> ping). >> >> This change can have very significant consequences in general, which were not >> explored at all in the commit. This may break many boards that do rely on that >> PHY reset, in various scenarios and for various reasons. >> >> I think it should be reverted as soon as possible to restore previous behavior. > > I'm wondering if anyone has further comments here? If I do go and revert > this what is then going to break on the MediaTek side for example? Let's revert it, and if MTK stuff breaks, let's fix it properly then. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2025-10-26 15:56 ` Marek Vasut @ 2025-10-26 17:32 ` Paul Kocialkowski 2025-10-26 17:55 ` Tom Rini 0 siblings, 1 reply; 8+ messages in thread From: Paul Kocialkowski @ 2025-10-26 17:32 UTC (permalink / raw) To: Marek Vasut Cc: Tom Rini, Marek Vasut, Jacky Chou, Sky Huang, Joe Hershberger, Ramon Fried, Eugeniu Rosca, Heinrich Schuchardt, u-boot, gss_mtk_uboot_upstream [-- Attachment #1: Type: text/plain, Size: 3162 bytes --] Hi, On Sun 26 Oct 25, 16:56, Marek Vasut wrote: > On 10/26/25 4:43 PM, Tom Rini wrote: > > On Tue, Aug 05, 2025 at 08:00:29PM +0200, Paul Kocialkowski wrote: > > > Hi, > > > > > > Le Mon 14 Oct 24, 10:43, Marek Vasut a écrit : > > > > On 10/14/24 9:06 AM, Sky Huang wrote: > > > > > From: "SkyLake.Huang" <skylake.huang@mediatek.com> > > > > > > > > > > Remove unnecessary CL22 phy reset before ethernet phy driver > > > > > probe. Lots of ethernet phys requires driver to load firmware. > > > > > Before that, CL22 phy reset may lead to malfunction. > > > > > > > > > > Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> > > > > > --- > > > > > drivers/net/phy/phy.c | 2 -- > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > > > > > index 716a1d46111..e6fed8c41d7 100644 > > > > > --- a/drivers/net/phy/phy.c > > > > > +++ b/drivers/net/phy/phy.c > > > > > @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) > > > > > static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, > > > > > phy_interface_t interface) > > > > > { > > > > > - /* Soft Reset the PHY */ > > > > > - phy_reset(phydev); > > > > > if (phydev->dev && phydev->dev != dev) { > > > > > printf("%s:%d is connected to %s. Reconnecting to %s\n", > > > > > phydev->bus->name, phydev->addr, > > > > > > > > This needs clarification and likely has to be handled differently. Removing > > > > the reset would leave the PHY in potentially undefined state. > > > > > > > > Which PHY is affected by this ? > > > > > > Good hunch. I just bisected down to this commit as I had some issues with my > > > Allwinner V3 board using the internal CL22 PHY. The symptom is that the LEDS's > > > polarity was inverted. Network still seems to work (although not tested beyond > > > ping). > > > > > > This change can have very significant consequences in general, which were not > > > explored at all in the commit. This may break many boards that do rely on that > > > PHY reset, in various scenarios and for various reasons. > > > > > > I think it should be reverted as soon as possible to restore previous behavior. > > > > I'm wondering if anyone has further comments here? If I do go and revert > > this what is then going to break on the MediaTek side for example? > Let's revert it, and if MTK stuff breaks, let's fix it properly then. Agreed, keeping it only saves one setup but sacrifices many others. This is definitely not right. An immediate solution to support Mediatek would be to add a config option (enabled by default) for the PHY reset. Or maybe this could be described in the board file somehow (this is a policy decision so I don't think any approach based on device-tree would be relevant here). All the best, Paul -- Paul Kocialkowski, Independent contractor - sys-base - https://www.sys-base.io/ Free software developer - https://www.paulk.fr/ Expert in multimedia, graphics and embedded hardware support with Linux. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2025-10-26 17:32 ` Paul Kocialkowski @ 2025-10-26 17:55 ` Tom Rini 0 siblings, 0 replies; 8+ messages in thread From: Tom Rini @ 2025-10-26 17:55 UTC (permalink / raw) To: Paul Kocialkowski Cc: Marek Vasut, Marek Vasut, Jacky Chou, Sky Huang, Joe Hershberger, Ramon Fried, Eugeniu Rosca, Heinrich Schuchardt, u-boot, gss_mtk_uboot_upstream [-- Attachment #1: Type: text/plain, Size: 3228 bytes --] On Sun, Oct 26, 2025 at 06:32:23PM +0100, Paul Kocialkowski wrote: > Hi, > > On Sun 26 Oct 25, 16:56, Marek Vasut wrote: > > On 10/26/25 4:43 PM, Tom Rini wrote: > > > On Tue, Aug 05, 2025 at 08:00:29PM +0200, Paul Kocialkowski wrote: > > > > Hi, > > > > > > > > Le Mon 14 Oct 24, 10:43, Marek Vasut a écrit : > > > > > On 10/14/24 9:06 AM, Sky Huang wrote: > > > > > > From: "SkyLake.Huang" <skylake.huang@mediatek.com> > > > > > > > > > > > > Remove unnecessary CL22 phy reset before ethernet phy driver > > > > > > probe. Lots of ethernet phys requires driver to load firmware. > > > > > > Before that, CL22 phy reset may lead to malfunction. > > > > > > > > > > > > Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com> > > > > > > --- > > > > > > drivers/net/phy/phy.c | 2 -- > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > > > > > > index 716a1d46111..e6fed8c41d7 100644 > > > > > > --- a/drivers/net/phy/phy.c > > > > > > +++ b/drivers/net/phy/phy.c > > > > > > @@ -839,8 +839,6 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask) > > > > > > static void phy_connect_dev(struct phy_device *phydev, struct udevice *dev, > > > > > > phy_interface_t interface) > > > > > > { > > > > > > - /* Soft Reset the PHY */ > > > > > > - phy_reset(phydev); > > > > > > if (phydev->dev && phydev->dev != dev) { > > > > > > printf("%s:%d is connected to %s. Reconnecting to %s\n", > > > > > > phydev->bus->name, phydev->addr, > > > > > > > > > > This needs clarification and likely has to be handled differently. Removing > > > > > the reset would leave the PHY in potentially undefined state. > > > > > > > > > > Which PHY is affected by this ? > > > > > > > > Good hunch. I just bisected down to this commit as I had some issues with my > > > > Allwinner V3 board using the internal CL22 PHY. The symptom is that the LEDS's > > > > polarity was inverted. Network still seems to work (although not tested beyond > > > > ping). > > > > > > > > This change can have very significant consequences in general, which were not > > > > explored at all in the commit. This may break many boards that do rely on that > > > > PHY reset, in various scenarios and for various reasons. > > > > > > > > I think it should be reverted as soon as possible to restore previous behavior. > > > > > > I'm wondering if anyone has further comments here? If I do go and revert > > > this what is then going to break on the MediaTek side for example? > > Let's revert it, and if MTK stuff breaks, let's fix it properly then. > > Agreed, keeping it only saves one setup but sacrifices many others. This is > definitely not right. > > An immediate solution to support Mediatek would be to add a config > option (enabled by default) for the PHY reset. Or maybe this could be described > in the board file somehow (this is a policy decision so I don't think any > approach based on device-tree would be relevant here). OK, can you please post the revert and cc the MediaTek people, explaining what you ran in to? Thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe 2024-10-14 7:06 [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe Sky Huang 2024-10-14 8:43 ` Marek Vasut @ 2025-05-23 22:26 ` Tom Rini 1 sibling, 0 replies; 8+ messages in thread From: Tom Rini @ 2025-05-23 22:26 UTC (permalink / raw) To: Joe Hershberger, Ramon Fried, Marek Vasut, Eugeniu Rosca, Heinrich Schuchardt, Jacky Chou, Sky Huang Cc: u-boot, gss_mtk_uboot_upstream, SkyLake.Huang On Mon, 14 Oct 2024 15:06:11 +0800, Sky Huang wrote: > Remove unnecessary CL22 phy reset before ethernet phy driver > probe. Lots of ethernet phys requires driver to load firmware. > Before that, CL22 phy reset may lead to malfunction. > > Applied to u-boot/next, thanks! [1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe commit: bab54f5942c428be698216224fd10b91d974d4da -- Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-26 17:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-14 7:06 [PATCH 1/1] net: phy: Do not do CL22 phy reset before ethernet phy driver probe Sky Huang 2024-10-14 8:43 ` Marek Vasut 2025-08-05 18:00 ` Paul Kocialkowski 2025-10-26 15:43 ` Tom Rini 2025-10-26 15:56 ` Marek Vasut 2025-10-26 17:32 ` Paul Kocialkowski 2025-10-26 17:55 ` Tom Rini 2025-05-23 22:26 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.