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!