diff mbox series

[v1,2/2] sfp: add support for 100 base-x SFPs

Message ID 20210111130657.10703-3-bjarni.jonasson@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add 100 base-x mode | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 21 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Bjarni Jonasson Jan. 11, 2021, 1:06 p.m. UTC
Add support for 100Base-FX, 100Base-LX, 100Base-PX and 100Base-BX10 modules
This is needed for Sparx-5 switch.

Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com>
---
 drivers/net/phy/sfp-bus.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Russell King (Oracle) Jan. 11, 2021, 2:22 p.m. UTC | #1
On Mon, Jan 11, 2021 at 02:06:57PM +0100, Bjarni Jonasson wrote:
> Add support for 100Base-FX, 100Base-LX, 100Base-PX and 100Base-BX10 modules
> This is needed for Sparx-5 switch.
> 
> Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com>
> ---
>  drivers/net/phy/sfp-bus.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> index 58014feedf6c..b2a9ee3dd28e 100644
> --- a/drivers/net/phy/sfp-bus.c
> +++ b/drivers/net/phy/sfp-bus.c
> @@ -265,6 +265,12 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
>  	    br_min <= 1300 && br_max >= 1200)
>  		phylink_set(modes, 1000baseX_Full);
>  
> +	/* 100Base-FX, 100Base-LX, 100Base-PX, 100Base-BX10 */
> +	if (id->base.e100_base_fx || id->base.e100_base_lx)
> +		phylink_set(modes, 100baseFX_Full);
> +	if ((id->base.e_base_px || id->base.e_base_bx10) && br_nom == 100)
> +		phylink_set(modes, 100baseFX_Full);

Do you have any modules that identify as PX or BX10 modules? What if
their range of speeds covers 100M - you're only checking the nominal
speed here.

Note that this will likely conflict with changes I submitted over the
weekend, and it really needs to be done _before_ the comment about
"If we haven't discovered any modes", not below.
Bjarni Jonasson Jan. 12, 2021, 3:31 p.m. UTC | #2
On Mon, 2021-01-11 at 14:22 +0000, Russell King - ARM Linux admin
wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, Jan 11, 2021 at 02:06:57PM +0100, Bjarni Jonasson wrote:
> > Add support for 100Base-FX, 100Base-LX, 100Base-PX and 100Base-BX10 
> > modules
> > This is needed for Sparx-5 switch.
> > 
> > Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com>
> > ---
> >  drivers/net/phy/sfp-bus.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> > index 58014feedf6c..b2a9ee3dd28e 100644
> > --- a/drivers/net/phy/sfp-bus.c
> > +++ b/drivers/net/phy/sfp-bus.c
> > @@ -265,6 +265,12 @@ void sfp_parse_support(struct sfp_bus *bus,
> > const struct sfp_eeprom_id *id,
> >           br_min <= 1300 && br_max >= 1200)
> >               phylink_set(modes, 1000baseX_Full);
> > 
> > +     /* 100Base-FX, 100Base-LX, 100Base-PX, 100Base-BX10 */
> > +     if (id->base.e100_base_fx || id->base.e100_base_lx)
> > +             phylink_set(modes, 100baseFX_Full);
> > +     if ((id->base.e_base_px || id->base.e_base_bx10) && br_nom ==
> > 100)
> > +             phylink_set(modes, 100baseFX_Full);
> 
> Do you have any modules that identify as PX or BX10 modules? What if
> their range of speeds covers 100M - you're only checking the nominal
> speed here.

I have one module that is identified as BX10 (HP-SFP-100FX-J9054C), but
it seems that the PX should also be there according to the standard.

All 100fx modules I've tested had br_min == br_max == br_nom == 100 so
I really don't know what else to use.

> Note that this will likely conflict with changes I submitted over the
> weekend, and it really needs to be done _before_ the comment about
> "If we haven't discovered any modes", not below.

Not sure what you mean, the patch is above the comment (line 265 vs
345).  The patch is on top of 5.10, is that the issue?

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Rgds
--
Bjarni Jonasson
Microchip
Andrew Lunn Jan. 12, 2021, 3:39 p.m. UTC | #3
> Not sure what you mean, the patch is above the comment (line 265 vs
> 345).  The patch is on top of 5.10, is that the issue?

All networking patches for the next merge window should be against
net-next. Please see:

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

which talks about the different trees.

      Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 58014feedf6c..b2a9ee3dd28e 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -265,6 +265,12 @@  void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 	    br_min <= 1300 && br_max >= 1200)
 		phylink_set(modes, 1000baseX_Full);
 
+	/* 100Base-FX, 100Base-LX, 100Base-PX, 100Base-BX10 */
+	if (id->base.e100_base_fx || id->base.e100_base_lx)
+		phylink_set(modes, 100baseFX_Full);
+	if ((id->base.e_base_px || id->base.e_base_bx10) && br_nom == 100)
+		phylink_set(modes, 100baseFX_Full);
+
 	/* For active or passive cables, select the link modes
 	 * based on the bit rates and the cable compliance bytes.
 	 */
@@ -385,6 +391,9 @@  phy_interface_t sfp_select_interface(struct sfp_bus *bus,
 	if (phylink_test(link_modes, 1000baseX_Full))
 		return PHY_INTERFACE_MODE_1000BASEX;
 
+	if (phylink_test(link_modes, 100baseFX_Full))
+		return PHY_INTERFACE_MODE_100BASEX;
+
 	dev_warn(bus->sfp_dev, "Unable to ascertain link mode\n");
 
 	return PHY_INTERFACE_MODE_NA;