| Message ID | 6d3b1dce-7633-51a1-0556-97cd03304c2c@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: phy: micrel: set soft_reset callback to genphy_soft_reset for KSZ9031 | expand |
On 1/10/19 11:22 AM, Heiner Kallweit wrote: > So far genphy_soft_reset was used automatically if the PHY driver > didn't implement the soft_reset callback. This changed with the > mentioned commit and broke KSZ9031. To fix this configure the > KSZ9031 PHY driver to use genphy_soft_reset. > > Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") > Reported-by: Tony Lindgren <tony@atomide.com> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Thanks a lot for following up with the people at TI on this. We should really get a better understanding of what the issue is, and whether there is a possibly better workaround that could be developed which is not just as hard as a big hammer software reset. Maybe we can get some people/contacts at Micrel to help here. > --- > drivers/net/phy/micrel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index c33384710..7828d17f0 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { > .driver_data = &ksz9021_type, > .probe = kszphy_probe, > .config_init = ksz9031_config_init, > + .soft_reset = genphy_soft_reset, > .read_status = ksz9031_read_status, > .ack_interrupt = kszphy_ack_interrupt, > .config_intr = kszphy_config_intr, >
On 10.01.2019 20:24, Florian Fainelli wrote: > On 1/10/19 11:22 AM, Heiner Kallweit wrote: >> So far genphy_soft_reset was used automatically if the PHY driver >> didn't implement the soft_reset callback. This changed with the >> mentioned commit and broke KSZ9031. To fix this configure the >> KSZ9031 PHY driver to use genphy_soft_reset. >> >> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") >> Reported-by: Tony Lindgren <tony@atomide.com> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > Thanks a lot for following up with the people at TI on this. We should > really get a better understanding of what the issue is, and whether > there is a possibly better workaround that could be developed which is > not just as hard as a big hammer software reset. Maybe we can get some > people/contacts at Micrel to help here. > In ksz9031_config_init() quite some settings are done and IMO it's not that unusual that it takes a soft reset for such settings to become effective. But I agree it would be nice if somebody from the PHY vendor could comment on whether a soft reset is actually needed. Not sure who has a contact to Microchip. If the feedback takes time it may be better to apply the soft reset as fix and replace it later in case we have a less intrusive option. >> --- >> drivers/net/phy/micrel.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c >> index c33384710..7828d17f0 100644 >> --- a/drivers/net/phy/micrel.c >> +++ b/drivers/net/phy/micrel.c >> @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { >> .driver_data = &ksz9021_type, >> .probe = kszphy_probe, >> .config_init = ksz9031_config_init, >> + .soft_reset = genphy_soft_reset, >> .read_status = ksz9031_read_status, >> .ack_interrupt = kszphy_ack_interrupt, >> .config_intr = kszphy_config_intr, >> > >
* Heiner Kallweit <hkallweit1@gmail.com> [190110 19:22]: > So far genphy_soft_reset was used automatically if the PHY driver > didn't implement the soft_reset callback. This changed with the > mentioned commit and broke KSZ9031. To fix this configure the > KSZ9031 PHY driver to use genphy_soft_reset. > > Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") > Reported-by: Tony Lindgren <tony@atomide.com> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Thanks this fixes the regression for me: Tested-by: Tony Lindgren <tony@atomide.com> > --- > drivers/net/phy/micrel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index c33384710..7828d17f0 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { > .driver_data = &ksz9021_type, > .probe = kszphy_probe, > .config_init = ksz9031_config_init, > + .soft_reset = genphy_soft_reset, > .read_status = ksz9031_read_status, > .ack_interrupt = kszphy_ack_interrupt, > .config_intr = kszphy_config_intr, > -- > 2.20.1 >
* Heiner Kallweit <hkallweit1@gmail.com> [190110 19:41]: > On 10.01.2019 20:24, Florian Fainelli wrote: > > On 1/10/19 11:22 AM, Heiner Kallweit wrote: > >> So far genphy_soft_reset was used automatically if the PHY driver > >> didn't implement the soft_reset callback. This changed with the > >> mentioned commit and broke KSZ9031. To fix this configure the > >> KSZ9031 PHY driver to use genphy_soft_reset. > >> > >> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") > >> Reported-by: Tony Lindgren <tony@atomide.com> > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > > > Thanks a lot for following up with the people at TI on this. We should > > really get a better understanding of what the issue is, and whether > > there is a possibly better workaround that could be developed which is > > not just as hard as a big hammer software reset. Maybe we can get some > > people/contacts at Micrel to help here. > > > In ksz9031_config_init() quite some settings are done and IMO it's not > that unusual that it takes a soft reset for such settings to become > effective. But I agree it would be nice if somebody from the PHY vendor > could comment on whether a soft reset is actually needed. Not sure who > has a contact to Microchip. Adding Sekhar to Cc, maybe he has some contacts to ask. > If the feedback takes time it may be better to apply the soft reset as > fix and replace it later in case we have a less intrusive option. Yes please let's fix the regression first and then patch more later as needed. Regards, Tony > >> --- > >> drivers/net/phy/micrel.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > >> index c33384710..7828d17f0 100644 > >> --- a/drivers/net/phy/micrel.c > >> +++ b/drivers/net/phy/micrel.c > >> @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { > >> .driver_data = &ksz9021_type, > >> .probe = kszphy_probe, > >> .config_init = ksz9031_config_init, > >> + .soft_reset = genphy_soft_reset, > >> .read_status = ksz9031_read_status, > >> .ack_interrupt = kszphy_ack_interrupt, > >> .config_intr = kszphy_config_intr, > >> > > > > >
* Tony Lindgren <tony@atomide.com> [190110 13:27]: > * Heiner Kallweit <hkallweit1@gmail.com> [190110 19:41]: > > On 10.01.2019 20:24, Florian Fainelli wrote: > > > On 1/10/19 11:22 AM, Heiner Kallweit wrote: > > >> So far genphy_soft_reset was used automatically if the PHY driver > > >> didn't implement the soft_reset callback. This changed with the > > >> mentioned commit and broke KSZ9031. To fix this configure the > > >> KSZ9031 PHY driver to use genphy_soft_reset. > > >> > > >> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") > > >> Reported-by: Tony Lindgren <tony@atomide.com> > > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > > > > > Thanks a lot for following up with the people at TI on this. We should > > > really get a better understanding of what the issue is, and whether > > > there is a possibly better workaround that could be developed which is > > > not just as hard as a big hammer software reset. Maybe we can get some > > > people/contacts at Micrel to help here. > > > > > In ksz9031_config_init() quite some settings are done and IMO it's not > > that unusual that it takes a soft reset for such settings to become > > effective. But I agree it would be nice if somebody from the PHY vendor > > could comment on whether a soft reset is actually needed. Not sure who > > has a contact to Microchip. > > Adding Sekhar to Cc, maybe he has some contacts to ask. Oops.. Second attempt of adding Sekhar to Cc. > > If the feedback takes time it may be better to apply the soft reset as > > fix and replace it later in case we have a less intrusive option. > > Yes please let's fix the regression first and then patch > more later as needed. > > Regards, > > Tony > > > >> --- > > >> drivers/net/phy/micrel.c | 1 + > > >> 1 file changed, 1 insertion(+) > > >> > > >> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > > >> index c33384710..7828d17f0 100644 > > >> --- a/drivers/net/phy/micrel.c > > >> +++ b/drivers/net/phy/micrel.c > > >> @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { > > >> .driver_data = &ksz9021_type, > > >> .probe = kszphy_probe, > > >> .config_init = ksz9031_config_init, > > >> + .soft_reset = genphy_soft_reset, > > >> .read_status = ksz9031_read_status, > > >> .ack_interrupt = kszphy_ack_interrupt, > > >> .config_intr = kszphy_config_intr, > > >> > > > > > > > >
On 11/01/19 2:57 AM, Tony Lindgren wrote: > * Heiner Kallweit <hkallweit1@gmail.com> [190110 19:41]: >> On 10.01.2019 20:24, Florian Fainelli wrote: >>> On 1/10/19 11:22 AM, Heiner Kallweit wrote: >>>> So far genphy_soft_reset was used automatically if the PHY driver >>>> didn't implement the soft_reset callback. This changed with the >>>> mentioned commit and broke KSZ9031. To fix this configure the >>>> KSZ9031 PHY driver to use genphy_soft_reset. >>>> >>>> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") >>>> Reported-by: Tony Lindgren <tony@atomide.com> >>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> >>> Thanks a lot for following up with the people at TI on this. We should >>> really get a better understanding of what the issue is, and whether >>> there is a possibly better workaround that could be developed which is >>> not just as hard as a big hammer software reset. Maybe we can get some >>> people/contacts at Micrel to help here. >>> >> In ksz9031_config_init() quite some settings are done and IMO it's not >> that unusual that it takes a soft reset for such settings to become >> effective. But I agree it would be nice if somebody from the PHY vendor >> could comment on whether a soft reset is actually needed. Not sure who >> has a contact to Microchip. > > Adding Sekhar to Cc, maybe he has some contacts to ask. I dont have any direct contacts, but I will ask internally. > >> If the feedback takes time it may be better to apply the soft reset as >> fix and replace it later in case we have a less intrusive option. > > Yes please let's fix the regression first and then patch > more later as needed. +1 I justed tested AM437x GP EVM with 10 times cable plug-unplug and this fixes the regression. Tested-by: Sekhar Nori <nsekhar@ti.com> Thanks, Sekhar
On January 10, 2019 11:22:26 AM PST, Heiner Kallweit <hkallweit1@gmail.com> wrote: >So far genphy_soft_reset was used automatically if the PHY driver >didn't implement the soft_reset callback. This changed with the >mentioned commit and broke KSZ9031. To fix this configure the >KSZ9031 PHY driver to use genphy_soft_reset. > >Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") >Reported-by: Tony Lindgren <tony@atomide.com> >Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com> Date: Thu, 10 Jan 2019 20:22:26 +0100 > So far genphy_soft_reset was used automatically if the PHY driver > didn't implement the soft_reset callback. This changed with the > mentioned commit and broke KSZ9031. To fix this configure the > KSZ9031 PHY driver to use genphy_soft_reset. > > Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") > Reported-by: Tony Lindgren <tony@atomide.com> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index c33384710..7828d17f0 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -1070,6 +1070,7 @@ static struct phy_driver ksphy_driver[] = { .driver_data = &ksz9021_type, .probe = kszphy_probe, .config_init = ksz9031_config_init, + .soft_reset = genphy_soft_reset, .read_status = ksz9031_read_status, .ack_interrupt = kszphy_ack_interrupt, .config_intr = kszphy_config_intr,
So far genphy_soft_reset was used automatically if the PHY driver didn't implement the soft_reset callback. This changed with the mentioned commit and broke KSZ9031. To fix this configure the KSZ9031 PHY driver to use genphy_soft_reset. Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset") Reported-by: Tony Lindgren <tony@atomide.com> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/micrel.c | 1 + 1 file changed, 1 insertion(+)