On Fri, May 08, 2020 at 06:27:06PM +0200, Marek Behun wrote:
> On Fri, 8 May 2020 16:28:44 +0100
> Russell King - ARM Linux admin <li...@armlinux.org.uk> wrote:
>
> > On Thu, May 07, 2020 at 03:21:35PM +0200, Marek Behún wrote:
> > > FreeTel P.C30.2 and P.C30.3 may fail to report anything useful from
> > > their EEPROM. They report correct nominal bitrate of 10300 MBd, but do
> > > not report sfp_ct_passive nor sfp_ct_active in their ERPROM.
> > >
> > > These modules can also operate at 1000baseX and 2500baseX.
> > >
> > > Signed-off-by: Marek Behún <marek.be...@nic.cz>
> > > Cc: Russell King <rmk+ker...@armlinux.org.uk>
> > > ---
> > > drivers/net/phy/sfp-bus.c | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> > > index 6900c68260e0..f021709bedcc 100644
> > > --- a/drivers/net/phy/sfp-bus.c
> > > +++ b/drivers/net/phy/sfp-bus.c
> > > @@ -44,6 +44,14 @@ static void sfp_quirk_2500basex(const struct
> > > sfp_eeprom_id *id,
> > > phylink_set(modes, 2500baseX_Full);
> > > }
> > >
> > > +static void sfp_quirk_direct_attach_10g(const struct sfp_eeprom_id *id,
> > > + unsigned long *modes)
> > > +{
> > > + phylink_set(modes, 10000baseCR_Full);
> > > + phylink_set(modes, 2500baseX_Full);
> > > + phylink_set(modes, 1000baseX_Full);
> > > +}
> > > +
> > > static const struct sfp_quirk sfp_quirks[] = {
> > > {
> > > // Alcatel Lucent G-010S-P can operate at 2500base-X, but
> > > @@ -63,6 +71,18 @@ static const struct sfp_quirk sfp_quirks[] = {
> > > .vendor = "HUAWEI",
> > > .part = "MA5671A",
> > > .modes = sfp_quirk_2500basex,
> > > + }, {
> > > + // FreeTel P.C30.2 is a SFP+ direct attach that can operate at
> > > + // at 1000baseX, 2500baseX and 10000baseCR, but may report none
> > > + // of these in their EEPROM
> > > + .vendor = "FreeTel",
> > > + .part = "P.C30.2",
> > > + .modes = sfp_quirk_direct_attach_10g,
> > > + }, {
> > > + // same as previous
> > > + .vendor = "FreeTel",
> > > + .part = "P.C30.3",
> > > + .modes = sfp_quirk_direct_attach_10g,
> >
> > Looking at the EEPROM capabilities, it seems that these modules give
> > either:
> >
> > Transceiver codes : 0x01 0x00 0x00 0x00 0x00 0x04 0x80 0x00 0x00
> > Transceiver type : Infiniband: 1X Copper Passive
> > Transceiver type : Passive Cable
> > Transceiver type : FC: Twin Axial Pair (TW)
> > Encoding : 0x06 (64B/66B)
> > BR, Nominal : 10300MBd
> > Passive Cu cmplnce. : 0x01 (SFF-8431 appendix E) [SFF-8472 rev10.4 only]
> > BR margin, max : 0%
> > BR margin, min : 0%
> >
> > or:
> >
> > Transceiver codes : 0x00 0x00 0x00 0x00 0x00 0x04 0x80 0x00 0x00
> > Transceiver type : Passive Cable
> > Transceiver type : FC: Twin Axial Pair (TW)
> > Encoding : 0x06 (64B/66B)
> > BR, Nominal : 10300MBd
> > Passive Cu cmplnce. : 0x01 (SFF-8431 appendix E) [SFF-8472 rev10.4 only]
> > BR margin, max : 0%
> > BR margin, min : 0%
> >
> > These give ethtool capability mask of 000,00000600,0000e040, which
> > is:
> >
> > 2500baseX (bit 15)
> > 1000baseX (bit 41)
> > 10000baseCR (bit 42)
> >
> > 10000baseCR, 2500baseX and 1000baseX comes from:
> >
> > if ((id->base.sfp_ct_passive || id->base.sfp_ct_active) && br_nom) {
> > /* This may look odd, but some manufacturers use 12000MBd */
> > if (br_min <= 12000 && br_max >= 10300)
> > phylink_set(modes, 10000baseCR_Full);
> > if (br_min <= 3200 && br_max >= 3100)
> > phylink_set(modes, 2500baseX_Full);
> > if (br_min <= 1300 && br_max >= 1200)
> > phylink_set(modes, 1000baseX_Full);
> >
> > since id->base.sfp_ct_passive is true, and br_nom = br_max = 10300 and
> > br_min = 0.
> >
> > 10000baseCR will also come from:
> >
> > if (id->base.sfp_ct_passive) {
> > if (id->base.passive.sff8431_app_e)
> > phylink_set(modes, 10000baseCR_Full);
> > }
> >
> > You claimed in your patch description that sfp_ct_passive is not set,
> > but the EEPROM dumps contain:
> >
> > Transceiver type : Passive Cable
> >
> > which is correctly parsed by the kernel.
> >
> > So, I'm rather confused, and I don't see why this patch is needed.
> >
>
> Russell,
>
> something is wrong here, and it is my bad. I hope I didn't mix
> the EEPROM images from when I was playing with the contents, but it
> seems possible now :( I probably sent you modified images and lost the
> original ones.
Ah.
> The thing I know for sure is that it did not work when I got the
> cables and also that they had different contents inside - ie at least
> one side of one cable did not report ct_passive nor ct_active. And I
> think that they reported different things on each side.
One of your files did have a difference in the capabilities - it was
missing the Infiniband capability mentioned above.
> I will try to get another such cable and return to this.
Thanks.
One thing we could do is to augment the "guessing" towards the end of
sfp_parse_support() so that if we encounter a module indicating
support for 10300MBd and 64B/66B, we could set one of the 10G properties
so that at least we get some functionality from modules that report
no capabilities. See:
/* If we haven't discovered any modes that this module supports, try
* the encoding and bitrate to determine supported modes. Some BiDi
* modules (eg, 1310nm/1550nm) are not 1000BASE-BX compliant due to
* the differing wavelengths, so do not set any transceiver bits.
*/
if (bitmap_empty(modes, __ETHTOOL_LINK_MODE_MASK_NBITS)) {
/* If the encoding and bit rate allows 1000baseX */
if (id->base.encoding == SFF8024_ENCODING_8B10B && br_nom &&
br_min <= 1300 && br_max >= 1200)
phylink_set(modes, 1000baseX_Full);
}
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
