[PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Martin Schiller]

Add quirk for a copper SFP that identifies itself as "FS" "SFP-10GM-T".
It uses RollBall protocol to talk to the PHY and needs 4 sec wait before
probing the PHY.

Signed-off-by: Martin Schiller <ms@dev.tdt.de>
---
 drivers/net/phy/sfp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 9369f5297769..15284be4c38c 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -479,9 +479,10 @@ static const struct sfp_quirk sfp_quirks[] = {
 	// PHY.
 	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
 
-	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the PHY and
-	// needs 4 sec wait before probing the PHY.
+	// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to talk
+	// to the PHY and needs 4 sec wait before probing the PHY.
 	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
+	SFP_QUIRK_F("FS", "SFP-10GM-T", sfp_fixup_fs_2_5gt),
 
 	// Fiberstore GPON-ONU-34-20BI can operate at 2500base-X, but report 1.2GBd
 	// NRZ in their EEPROM
-- 
2.39.5

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Russell King (Oracle)]

On Wed, Feb 26, 2025 at 03:10:02PM +0100, Martin Schiller wrote:
> Add quirk for a copper SFP that identifies itself as "FS" "SFP-10GM-T".
> It uses RollBall protocol to talk to the PHY and needs 4 sec wait before
> probing the PHY.
> 
> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> ---
>  drivers/net/phy/sfp.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 9369f5297769..15284be4c38c 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -479,9 +479,10 @@ static const struct sfp_quirk sfp_quirks[] = {
>  	// PHY.
>  	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
>  
> -	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the PHY and
> -	// needs 4 sec wait before probing the PHY.
> +	// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to talk
> +	// to the PHY and needs 4 sec wait before probing the PHY.
>  	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
> +	SFP_QUIRK_F("FS", "SFP-10GM-T", sfp_fixup_fs_2_5gt),

Which makes sfp_fixup_fs_2_5gt mis-named. Please rename.

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

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Martin Schiller]

On 2025-02-26 15:38, Russell King (Oracle) wrote:
> On Wed, Feb 26, 2025 at 03:10:02PM +0100, Martin Schiller wrote:
>> Add quirk for a copper SFP that identifies itself as "FS" 
>> "SFP-10GM-T".
>> It uses RollBall protocol to talk to the PHY and needs 4 sec wait 
>> before
>> probing the PHY.
>> 
>> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
>> ---
>>  drivers/net/phy/sfp.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
>> index 9369f5297769..15284be4c38c 100644
>> --- a/drivers/net/phy/sfp.c
>> +++ b/drivers/net/phy/sfp.c
>> @@ -479,9 +479,10 @@ static const struct sfp_quirk sfp_quirks[] = {
>>  	// PHY.
>>  	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
>> 
>> -	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the PHY 
>> and
>> -	// needs 4 sec wait before probing the PHY.
>> +	// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to 
>> talk
>> +	// to the PHY and needs 4 sec wait before probing the PHY.
>>  	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
>> +	SFP_QUIRK_F("FS", "SFP-10GM-T", sfp_fixup_fs_2_5gt),
> 
> Which makes sfp_fixup_fs_2_5gt mis-named. Please rename.

OK, I'll rename it to sfp_fixup_rollball_wait.

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Kory Maincent]

On Wed, 26 Feb 2025 15:50:46 +0100
Martin Schiller <ms@dev.tdt.de> wrote:

> On 2025-02-26 15:38, Russell King (Oracle) wrote:
> > On Wed, Feb 26, 2025 at 03:10:02PM +0100, Martin Schiller wrote:  
> >> Add quirk for a copper SFP that identifies itself as "FS" 
> >> "SFP-10GM-T".
> >> It uses RollBall protocol to talk to the PHY and needs 4 sec wait 
> >> before
> >> probing the PHY.
> >> 
> >> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> >> ---
> >>  drivers/net/phy/sfp.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> >> index 9369f5297769..15284be4c38c 100644
> >> --- a/drivers/net/phy/sfp.c
> >> +++ b/drivers/net/phy/sfp.c
> >> @@ -479,9 +479,10 @@ static const struct sfp_quirk sfp_quirks[] = {
> >>  	// PHY.
> >>  	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
> >> 
> >> -	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the
> >> PHY and
> >> -	// needs 4 sec wait before probing the PHY.
> >> +	// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to 
> >> talk
> >> +	// to the PHY and needs 4 sec wait before probing the PHY.
> >>  	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
> >> +	SFP_QUIRK_F("FS", "SFP-10GM-T", sfp_fixup_fs_2_5gt),  
> > 
> > Which makes sfp_fixup_fs_2_5gt mis-named. Please rename.  
> 
> OK, I'll rename it to sfp_fixup_rollball_wait.

I would prefer sfp_fixup_fs_rollball_wait to keep the name of the manufacturer.
It can't be a generic fixup as other FSP could have other waiting time values
like the Turris RTSFP-10G which needs 25s.

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Martin Schiller]

On 2025-02-26 16:26, Kory Maincent wrote:
> On Wed, 26 Feb 2025 15:50:46 +0100
> Martin Schiller <ms@dev.tdt.de> wrote:
> 
>> On 2025-02-26 15:38, Russell King (Oracle) wrote:
>> > On Wed, Feb 26, 2025 at 03:10:02PM +0100, Martin Schiller wrote:
>> >> Add quirk for a copper SFP that identifies itself as "FS"
>> >> "SFP-10GM-T".
>> >> It uses RollBall protocol to talk to the PHY and needs 4 sec wait
>> >> before
>> >> probing the PHY.
>> >>
>> >> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
>> >> ---
>> >>  drivers/net/phy/sfp.c | 5 +++--
>> >>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
>> >> index 9369f5297769..15284be4c38c 100644
>> >> --- a/drivers/net/phy/sfp.c
>> >> +++ b/drivers/net/phy/sfp.c
>> >> @@ -479,9 +479,10 @@ static const struct sfp_quirk sfp_quirks[] = {
>> >>  	// PHY.
>> >>  	SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
>> >>
>> >> -	// Fiberstore SFP-2.5G-T uses Rollball protocol to talk to the
>> >> PHY and
>> >> -	// needs 4 sec wait before probing the PHY.
>> >> +	// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to
>> >> talk
>> >> +	// to the PHY and needs 4 sec wait before probing the PHY.
>> >>  	SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_fs_2_5gt),
>> >> +	SFP_QUIRK_F("FS", "SFP-10GM-T", sfp_fixup_fs_2_5gt),
>> >
>> > Which makes sfp_fixup_fs_2_5gt mis-named. Please rename.
>> 
>> OK, I'll rename it to sfp_fixup_rollball_wait.
> 
> I would prefer sfp_fixup_fs_rollball_wait to keep the name of the 
> manufacturer.
> It can't be a generic fixup as other FSP could have other waiting time 
> values
> like the Turris RTSFP-10G which needs 25s.

I think you're getting two things mixed up.
The phy still has 25 seconds to wake up. With sfp_fixup_rollball_wait
there simply is an additional 4s wait at the beginning before we start
searching for a phy.

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Kory Maincent]

On Wed, 26 Feb 2025 16:55:38 +0100
Martin Schiller <ms@dev.tdt.de> wrote:

> On 2025-02-26 16:26, Kory Maincent wrote:
> > On Wed, 26 Feb 2025 15:50:46 +0100
> > Martin Schiller <ms@dev.tdt.de> wrote:
> >   
> >> On 2025-02-26 15:38, Russell King (Oracle) wrote:  
>  [...]  
>  [...]  
>  [...]  
> >> 
> >> OK, I'll rename it to sfp_fixup_rollball_wait.  
> > 
> > I would prefer sfp_fixup_fs_rollball_wait to keep the name of the 
> > manufacturer.
> > It can't be a generic fixup as other FSP could have other waiting time 
> > values
> > like the Turris RTSFP-10G which needs 25s.  
> 
> I think you're getting two things mixed up.
> The phy still has 25 seconds to wake up. With sfp_fixup_rollball_wait
> there simply is an additional 4s wait at the beginning before we start
> searching for a phy.

Indeed you are right, I was looking in older Linux sources, sorry.
Still, the additional 4s wait seems relevant only for FS SFP, so it should
be included in the function naming to avoid confusion.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Martin Schiller]

On 2025-02-26 17:27, Kory Maincent wrote:
> On Wed, 26 Feb 2025 16:55:38 +0100
> Martin Schiller <ms@dev.tdt.de> wrote:
> 
>> On 2025-02-26 16:26, Kory Maincent wrote:
>> > On Wed, 26 Feb 2025 15:50:46 +0100
>> > Martin Schiller <ms@dev.tdt.de> wrote:
>> >
>> >> On 2025-02-26 15:38, Russell King (Oracle) wrote:
>>  [...]
>>  [...]
>>  [...]
>> >>
>> >> OK, I'll rename it to sfp_fixup_rollball_wait.
>> >
>> > I would prefer sfp_fixup_fs_rollball_wait to keep the name of the
>> > manufacturer.
>> > It can't be a generic fixup as other FSP could have other waiting time
>> > values
>> > like the Turris RTSFP-10G which needs 25s.
>> 
>> I think you're getting two things mixed up.
>> The phy still has 25 seconds to wake up. With sfp_fixup_rollball_wait
>> there simply is an additional 4s wait at the beginning before we start
>> searching for a phy.
> 
> Indeed you are right, I was looking in older Linux sources, sorry.
> Still, the additional 4s wait seems relevant only for FS SFP, so it 
> should
> be included in the function naming to avoid confusion.
> 

You may be right for the moment. But perhaps there will soon be SFP
modules from other manufacturers that also need this quirk.

There is also the function sfp_fixup_rollball_cc, which is currently
only used for modules with vendor string “OEM”. However, the function is
not called sfp_fixup_oem_rollball_cc.

Regards,
Martin

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Andrew Lunn]

On Wed, Feb 26, 2025 at 05:51:42PM +0100, Martin Schiller wrote:
> On 2025-02-26 17:27, Kory Maincent wrote:
> > On Wed, 26 Feb 2025 16:55:38 +0100
> > Martin Schiller <ms@dev.tdt.de> wrote:
> > 
> > > On 2025-02-26 16:26, Kory Maincent wrote:
> > > > On Wed, 26 Feb 2025 15:50:46 +0100
> > > > Martin Schiller <ms@dev.tdt.de> wrote:
> > > >
> > > >> On 2025-02-26 15:38, Russell King (Oracle) wrote:
> > >  [...]
> > >  [...]
> > >  [...]
> > > >>
> > > >> OK, I'll rename it to sfp_fixup_rollball_wait.
> > > >
> > > > I would prefer sfp_fixup_fs_rollball_wait to keep the name of the
> > > > manufacturer.
> > > > It can't be a generic fixup as other FSP could have other waiting time
> > > > values
> > > > like the Turris RTSFP-10G which needs 25s.
> > > 
> > > I think you're getting two things mixed up.
> > > The phy still has 25 seconds to wake up. With sfp_fixup_rollball_wait
> > > there simply is an additional 4s wait at the beginning before we start
> > > searching for a phy.
> > 
> > Indeed you are right, I was looking in older Linux sources, sorry.
> > Still, the additional 4s wait seems relevant only for FS SFP, so it
> > should
> > be included in the function naming to avoid confusion.
> > 
> 
> You may be right for the moment. But perhaps there will soon be SFP
> modules from other manufacturers that also need this quirk.

Since these are all kernel internal implementation details, we can
rename them any time we want. There is no kernel ABI involved. So
please use a name based on what we know now. If such a module does
appear sometime in the future, we can change the name at that point.

       Andrew

Re: [PATCH net-next] net: sfp: add quirk for FS SFP-10GM-T copper SFP+ module

[Kory Maincent]

On Wed, 26 Feb 2025 17:51:42 +0100
Martin Schiller <ms@dev.tdt.de> wrote:

> On 2025-02-26 17:27, Kory Maincent wrote:
> > On Wed, 26 Feb 2025 16:55:38 +0100
> > Martin Schiller <ms@dev.tdt.de> wrote:
> >   
> >> On 2025-02-26 16:26, Kory Maincent wrote:  
>  [...]  
>  [...]  
> >>  [...]
> >>  [...]
> >>  [...]  
>  [...]  
>  [...]  
> >> 
> >> I think you're getting two things mixed up.
> >> The phy still has 25 seconds to wake up. With sfp_fixup_rollball_wait
> >> there simply is an additional 4s wait at the beginning before we start
> >> searching for a phy.  
> > 
> > Indeed you are right, I was looking in older Linux sources, sorry.
> > Still, the additional 4s wait seems relevant only for FS SFP, so it 
> > should
> > be included in the function naming to avoid confusion.
> >   
> 
> You may be right for the moment. But perhaps there will soon be SFP
> modules from other manufacturers that also need this quirk.
> 
> There is also the function sfp_fixup_rollball_cc, which is currently
> only used for modules with vendor string “OEM”. However, the function is
> not called sfp_fixup_oem_rollball_cc.

Indeed. As you prefer then.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com