From: Marek Behun <marek.behun@nic•cz>
To: Russell King - ARM Linux admin <linux@armlinux•org.uk>
Cc: netdev@vger•kernel.org
Subject: Re: [PATCH net-next] net: sfp: add some quirks for FreeTel direct attach modules
Date: Fri, 8 May 2020 18:27:06 +0200 [thread overview]
Message-ID: <20200508182706.06394c88@nic.cz> (raw)
In-Reply-To: <20200508152844.GV1551@shell.armlinux.org.uk>
On Fri, 8 May 2020 16:28:44 +0100
Russell King - ARM Linux admin <linux@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.behun@nic•cz>
> > Cc: Russell King <rmk+kernel@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.
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.
I will try to get another such cable and return to this.
Marek
next prev parent reply other threads:[~2020-05-08 16:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-07 13:21 [PATCH net-next] net: sfp: add some quirks for FreeTel direct attach modules Marek Behún
[not found] ` <CAOJe8K03_BFvfMHOUibi6OZLt28Z8Cn1katyhvJHP5b1rFZbUw@mail.gmail.com>
2020-05-08 15:02 ` Russell King - ARM Linux admin
2020-05-08 15:28 ` Russell King - ARM Linux admin
2020-05-08 16:27 ` Marek Behun [this message]
2020-05-08 18:00 ` Russell King - ARM Linux admin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200508182706.06394c88@nic.cz \
--to=marek.behun@nic$(echo .)cz \
--cc=linux@armlinux$(echo .)org.uk \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox