Messages in this thread | | | Date | Fri, 29 Jul 2022 15:12:10 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net v1 1/1] net: dsa: microchip: don't try do read Gbit registers on non Gbit chips |
| |
On Fri, Jul 29, 2022 at 11:05:13AM +0200, Oleksij Rempel wrote: > On Thu, Jul 28, 2022 at 03:25:35PM +0200, Andrew Lunn wrote: > > On Thu, Jul 28, 2022 at 03:17:25PM +0200, Oleksij Rempel wrote: > > > Do not try to read not existing or wrong register on chips without > > > GBIT_SUPPORT. > > > > > > Fixes: c2e866911e25 ("net: dsa: microchip: break KSZ9477 DSA driver into two files") > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > --- > > > drivers/net/dsa/microchip/ksz9477.c | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > > > index c73bb6d383ad..f6bbd9646c85 100644 > > > --- a/drivers/net/dsa/microchip/ksz9477.c > > > +++ b/drivers/net/dsa/microchip/ksz9477.c > > > @@ -316,7 +316,13 @@ void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) > > > break; > > > } > > > } else { > > > - ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); > > > + /* No gigabit support. Do not read wrong registers. */ > > > + if (!(dev->features & GBIT_SUPPORT) && > > > + (reg == MII_CTRL1000 || reg == MII_ESTATUS || > > > + reg == MII_STAT1000)) > > > > Does this actually happen? > > > > If i remember this code correctly, it tries to make the oddly looking > > PHY look like a normal PHY. phylib is then used to drive the PHY? > > > > If i have that correct, why is phylib trying to read these registers? > > It should know there is no 1G support, and should skip them. > > It looks like currently undocumented silicon errata. According to the > data sheet, the BMSR_ESTATEN should not be set BMSR_ERCAP, but this bits > are set. > > The question is what is the proper place to implement it. There is same > PHYid for most KSZ switch PHYs, it is no possible to detect it by PHYid. > I have following options: > - add chips specific quirk in the ksz9477_r_phy(), just remove > BMSR_ESTATEN and BMSR_ERCAP. > - notify about errata over get_phy_flags and implement get_caps quirk in > the PHY driver.
I would do the first. The DSA driver is already doing some emulation of a normal PHY, so it seems odd to push a workaround into the PHY driver when it can be part of the emulation.
Andrew
| |