| Message ID | 20240701-b4-dp83869-sfp-v1-2-a71d6d0ad5f8@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: dp83869: Add support for downstream SFP cages | expand |
On Mon, Jul 01, 2024 at 10:51:04AM +0200, Romain Gantois wrote: > The DP83869 PHY requires a software restart after OP_MODE is changed in the > OP_MODE_DECODE register. > > Add this restart in dp83869_configure_mode(). > > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> > --- > drivers/net/phy/dp83869.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c > index f6b05e3a3173e..6bb9bb1c0e962 100644 > --- a/drivers/net/phy/dp83869.c > +++ b/drivers/net/phy/dp83869.c > @@ -786,6 +786,10 @@ static int dp83869_configure_mode(struct phy_device *phydev, Not directly this patch, but dp83869_configure_mode() has: ret = phy_write(phydev, MII_BMCR, MII_DP83869_BMCR_DEFAULT); where #define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ BMCR_FULLDPLX | \ BMCR_SPEED1000) When considering the previous patch, maybe BMCR_ANENABLE should be conditional on the mode being selected? Andrew
On lundi 1 juillet 2024 18:44:33 UTC+2 Andrew Lunn wrote: > On Mon, Jul 01, 2024 at 10:51:04AM +0200, Romain Gantois wrote: > > The DP83869 PHY requires a software restart after OP_MODE is changed in > > the > > OP_MODE_DECODE register. > > > > Add this restart in dp83869_configure_mode(). > > > > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> > > --- > > > > drivers/net/phy/dp83869.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c > > index f6b05e3a3173e..6bb9bb1c0e962 100644 > > --- a/drivers/net/phy/dp83869.c > > +++ b/drivers/net/phy/dp83869.c > > @@ -786,6 +786,10 @@ static int dp83869_configure_mode(struct phy_device > > *phydev, > Not directly this patch, but dp83869_configure_mode() has: > > ret = phy_write(phydev, MII_BMCR, MII_DP83869_BMCR_DEFAULT); > > where #define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ > BMCR_FULLDPLX | \ > BMCR_SPEED1000) > > When considering the previous patch, maybe BMCR_ANENABLE should be > conditional on the mode being selected? Indeed, this would definitely make sense. Thanks,
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c index f6b05e3a3173e..6bb9bb1c0e962 100644 --- a/drivers/net/phy/dp83869.c +++ b/drivers/net/phy/dp83869.c @@ -786,6 +786,10 @@ static int dp83869_configure_mode(struct phy_device *phydev, return -EINVAL; } + ret = phy_write(phydev, DP83869_CTRL, DP83869_SW_RESTART); + + usleep_range(10, 20); + return ret; }
The DP83869 PHY requires a software restart after OP_MODE is changed in the OP_MODE_DECODE register. Add this restart in dp83869_configure_mode(). Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> --- drivers/net/phy/dp83869.c | 4 ++++ 1 file changed, 4 insertions(+)