On Wednesday 13 January 2021 11:08:52 Russell King - ARM Linux admin wrote:
> On Wed, Jan 13, 2021 at 11:49:36AM +0100, Pali Rohár wrote:
> > On Monday 11 January 2021 06:00:44 Marek Behún wrote:
> > > @@ -1453,7 +1459,7 @@ static int sfp_sm_probe_phy(struct sfp *sfp, bool
> > > is_c45)
> > > struct phy_device *phy;
> > > int err;
> > >
> > > - phy = get_phy_device(sfp->i2c_mii, SFP_PHY_ADDR, is_c45);
> > > + phy = get_phy_device(sfp->i2c_mii, sfp->phy_addr, is_c45);
> > > if (phy == ERR_PTR(-ENODEV))
> > > return PTR_ERR(phy);
> > > if (IS_ERR(phy)) {
> > > @@ -1835,6 +1841,23 @@ static int sfp_sm_mod_probe(struct sfp *sfp, bool
> > > report)
> > >
> > > sfp->mdio_protocol = MDIO_I2C_DEFAULT;
> > >
> > > + sfp->phy_addr = SFP_PHY_ADDR;
> > > + sfp->module_t_wait = T_WAIT;
> > > +
> > > + if (((!memcmp(id.base.vendor_name, "OEM ", 16) ||
> > > + !memcmp(id.base.vendor_name, "Turris ", 16)) &&
> > > + (!memcmp(id.base.vendor_pn, "SFP-10G-T ", 16) ||
> > > + !memcmp(id.base.vendor_pn, "RTSFP-10", 8)))) {
> > > + sfp->mdio_protocol = MDIO_I2C_ROLLBALL;
> > > + sfp->phy_addr = SFP_PHY_ADDR_ROLLBALL;
> > > + sfp->module_t_wait = T_WAIT_ROLLBALL;
> > > +
> > > + /* RollBall SFPs may have wrong (zero) extended compliacne code
>
> Spelling error - "compliance"
>
> > > + * burned in EEPROM. For PHY probing we need the correct one.
> > > + */
> > > + id.base.extended_cc = SFF8024_ECC_10GBASE_T_SFI;
> >
> > Should not we rather in sfp_sm_probe_for_phy() function in "default"
> > section try to probe also for clause 45 PHY when clause 22 fails?
>
> Why? That's opening the possibilities for more problems - remember,
> the access method is vendor defined, and we already have the situation
> where I2C address 0x56 is used in two different styles that are
> indistinguishable:
>
> - Clause 22 write:
> Write register address, value high, value low.
> - Clause 22 read:
> Write register address.
> Read value high, low.
> - Clause 45 write:
> Write devad, register address high, register address low,
> value high, value low.
> - Clause 45 read:
> Write devad, register address high, register address low.
> Read value high, low.
>
> Look closely at the similarities of Clause 22 write and Clause 45
> read, you'll see that if you issue a clause 45 read to a SFP module
> that implements Clause 22, you actually end up issuing a write to it.
>
> Sending random MDIO cycles to a SFP is a really bad idea.
I see, thank you for explanation. Incorrect data in SFP EEPROM may cause
lot of other issues :-(
