lkml.org 
[lkml]   [2020]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next] net: phy: sfp: Cotsworks SFF module EEPROM fixup
From
Date


On 7/15/2020 8:32 PM, Chris Healy wrote:
>
>
> On Wed, Jul 15, 2020 at 8:10 PM Florian Fainelli <f.fainelli@gmail.com
> <mailto:f.fainelli@gmail.com>> wrote:
>
>
>
> On 7/14/2020 10:59 AM, Chris Healy wrote:
> > Some Cotsworks SFF have invalid data in the first few bytes of the
> > module EEPROM.  This results in these modules not being detected as
> > valid modules.
> >
> > Address this by poking the correct EEPROM values into the module
> > EEPROM when the model/PN match and the existing module EEPROM contents
> > are not correct.
> >
> > Signed-off-by: Chris Healy <cphealy@gmail.com
> <mailto:cphealy@gmail.com>>
> > ---
> >  drivers/net/phy/sfp.c | 44
> +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 73c2969f11a4..2737d9b6b0ae 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -1632,10 +1632,43 @@ static int sfp_sm_mod_hpower(struct sfp
> *sfp, bool enable)
> >       return 0;
> >  }
> > 
> > +static int sfp_cotsworks_fixup_check(struct sfp *sfp, struct
> sfp_eeprom_id *id)
> > +{
> > +     u8 check;
> > +     int err;
> > +
> > +     if (id->base.phys_id != SFF8024_ID_SFF_8472 ||
> > +         id->base.phys_ext_id != SFP_PHYS_EXT_ID_SFP ||
> > +         id->base.connector != SFF8024_CONNECTOR_LC) {
> > +             dev_warn(sfp->dev, "Rewriting fiber module EEPROM
> with corrected values\n");
> > +             id->base.phys_id = SFF8024_ID_SFF_8472;
> > +             id->base.phys_ext_id = SFP_PHYS_EXT_ID_SFP;
> > +             id->base.connector = SFF8024_CONNECTOR_LC;
> > +             err = sfp_write(sfp, false, SFP_PHYS_ID, &id->base, 3);
> > +             if (err != 3) {
> > +                     dev_err(sfp->dev, "Failed to rewrite module
> EEPROM: %d\n", err);
> > +                     return err;
> > +             }
> > +
> > +             /* Cotsworks modules have been found to require a
> delay between write operations. */
> > +             mdelay(50);
> > +
> > +             /* Update base structure checksum */
> > +             check = sfp_check(&id->base, sizeof(id->base) - 1);
> > +             err = sfp_write(sfp, false, SFP_CC_BASE, &check, 1);
> > +             if (err != 1) {
> > +                     dev_err(sfp->dev, "Failed to update base
> structure checksum in fiber module EEPROM: %d\n", err);
> > +                     return err;
> > +             }
> > +     }
> > +     return 0;
> > +}
> > +
> >  static int sfp_sm_mod_probe(struct sfp *sfp, bool report)
> >  {
> >       /* SFP module inserted - read I2C data */
> >       struct sfp_eeprom_id id;
> > +     bool cotsworks_sfbg;
> >       bool cotsworks;
> >       u8 check;
> >       int ret;
> > @@ -1657,6 +1690,17 @@ static int sfp_sm_mod_probe(struct sfp
> *sfp, bool report)
> >        * serial number and date code.
> >        */
> >       cotsworks = !memcmp(id.base.vendor_name, "COTSWORKS       ",
> 16);
> > +     cotsworks_sfbg = !memcmp(id.base.vendor_pn, "SFBG", 4);
> > +
> > +     /* Cotsworks SFF module EEPROM do not always have valid phys_id,
> > +      * phys_ext_id, and connector bytes.  Rewrite SFF EEPROM
> bytes if
> > +      * Cotsworks PN matches and bytes are not correct.
> > +      */
> > +     if (cotsworks && cotsworks_sfbg) {
> > +             ret = sfp_cotsworks_fixup_check(sfp, &id);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
>
> So with the fixup you introduce, should we ever go into a situation
> where:
>
> EPROM extended structure checksum failure
>
> is printed?
>
>
> From what I've been told, Cotsworks had an ordering problem where both
> the base and extended checksums were being programmed before other
> fields were programmed during manufacturing resulting in both the base
> and extended checksums being incorrect.  (I've also heard that Cotsworks
> has resolved this issue late last year for all new units but units
> manufactured before late last year will have incorrect checksums.)
>
> Given that I was touching the base structure in this patch, I felt that
> updating the base checksum was warranted.  I did not consider updating
> the extended structure checksum as I wasn't changing anything else with
> the extended structure.  As such, we would still have an invalid
> extended structure checksum and get the associated error message.

That makes sense and thanks for providing the context here!
--
Florian

\
 
 \ /
  Last update: 2020-07-16 05:38    [W:0.058 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site