Messages in this thread | | | Date | Wed, 26 Jul 2023 10:53:17 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net-next v3] net: dsa: mv88e6xxx: Add erratum 3.14 for 88E6390X and 88E6190X |
| |
On Wed, Jul 26, 2023 at 11:49:35AM +0200, Ante Knezic wrote: > On Tue, 25 Jul 2023 18:49:19 +0100 Russell King (Oracle) wrote: > > Does the errata say that _all_ lanes need this treatment, even when > > they are not being used as a group (e.g. for XAUI) ? > > No, unfortunatelly errata says very little, I tried applying erratum only on the requested > lane of port 9/10 but this did not work out as expected and the issue was still visible. > I dont have the necessary HW to perform more tests on other lanes unfortunatelly. > > On Tue, 25 Jul 2023 18:49:19 +0100 Russell King (Oracle) wrote: > > On Tue, Jul 25, 2023 at 08:23:43PM +0300, Vladimir Oltean wrote: > > > On Fri, Jul 21, 2023 at 12:26:18PM +0200, Ante Knezic wrote: > > > > diff --git a/drivers/net/dsa/mv88e6xxx/pcs-639x.c b/drivers/net/dsa/mv88e6xxx/pcs-639x.c > > > > index 98dd49dac421..50b14804c360 100644 > > > > --- a/drivers/net/dsa/mv88e6xxx/pcs-639x.c > > > > +++ b/drivers/net/dsa/mv88e6xxx/pcs-639x.c > > > > @@ -20,6 +20,7 @@ struct mv88e639x_pcs { > > > > struct mdio_device mdio; > > > > struct phylink_pcs sgmii_pcs; > > > > struct phylink_pcs xg_pcs; > > > > + struct mv88e6xxx_chip *chip; > > > > bool erratum_3_14; > > ... > > > > > static int mv88e639x_sgmii_pcs_post_config(struct phylink_pcs *pcs, > > > > phy_interface_t interface) > > > > { > > > > struct mv88e639x_pcs *mpcs = sgmii_pcs_to_mv88e639x_pcs(pcs); > > > > + struct mv88e6xxx_chip *chip = mpcs->chip; > > > > > > > > mv88e639x_sgmii_pcs_control_pwr(mpcs, true); > > > > > > > > + if (chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6190X || > > > > + chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6390X) > > > > + mv88e6390_erratum_3_14(mpcs); > > > > int err; > > ... > > if (mpcs->erratum_3_14) { > > err = mv88e6390_erratum_3_14(mpcs); > > if (err) > > dev_err(mpcs->mdio.dev.parent, > > "failed to apply erratum 3.14: %pe\n", > > ERR_PTR(err)); > > } > > > > So you propose to ditch the chip ptr from the mpcs and add a bool variable instead. But > isn't this too general - the errata applies only to 6190X and 6390X, other devices > might (and probably do) have errata 3.14 as something completely different? Possible new changes > (new errata, fixes etc) in the pcs-xxx.c might benefit from having a chip ptr more than > using a bool variable "just" for one errata found on two device types?
As a longer term goal, I would like to move the pcs drivers out of mv88e6xxx and into drivers/net/pcs, so I want to minimise the use of the "chip" pointer in the drivers. That's why I coded them the way I have, as almost entirely stand-alone implementations that make no use of the hardware accessors provided by the 88e6xxx core.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
| |