diff mbox series

[v2] net: sfp: handle 100G/25G active optical cables in sfp_parse_support

Message ID 20230814141739.25552-1-josua@solid-run.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v2] net: sfp: handle 100G/25G active optical cables in sfp_parse_support | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1332 this patch: 1332
netdev/cc_maintainers warning 1 maintainers not CCed: terrelln@fb.com
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1355 this patch: 1355
netdev/checkpatch warning WARNING: networking block comments don't use an empty /* line, use /* Comment...
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Josua Mayer Aug. 14, 2023, 2:17 p.m. UTC
Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC)
for active optical cables supporting 25G and 100G speeds.

Since the specification makes no statement about transmitter range, and
as the specific sfp module that had been tested features only 2m fiber -
short-range (SR) modes are selected.

The 100G speed is irrelevant because it would require multiple fibers /
multiple SFP28 modules combined under one netdev.
sfp-bus.c only handles a single module per netdev, so only 25Gbps modes
are selected.

sfp_parse_support already handles SFF8024_ECC_100GBASE_SR4_25GBASE_SR
with compatible properties, however that entry is a contradiction in
itself since with SFP(28) 100GBASE_SR4 is impossible - that would likely
be a mode for qsfp modules only.

Add a case for SFF8024_ECC_100G_25GAUI_C2M_AOC selecting 25gbase-r
interface mode and 25000baseSR link mode.
Also enforce SFP28 bitrate limits on the values read from sfp eeprom as
requested by Russell King.

Tested with fs.com S28-AO02 AOC SFP28 module.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
V1 -> V2: added separate case SFF8024_ECC_100G_25GAUI_C2M_AOC
V1 -> V2: added bitrate check for eeprom values

 drivers/net/phy/sfp-bus.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Simon Horman Aug. 15, 2023, 8:24 a.m. UTC | #1
On Mon, Aug 14, 2023 at 04:17:39PM +0200, Josua Mayer wrote:
> Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC)
> for active optical cables supporting 25G and 100G speeds.
> 
> Since the specification makes no statement about transmitter range, and
> as the specific sfp module that had been tested features only 2m fiber -
> short-range (SR) modes are selected.
> 
> The 100G speed is irrelevant because it would require multiple fibers /
> multiple SFP28 modules combined under one netdev.
> sfp-bus.c only handles a single module per netdev, so only 25Gbps modes
> are selected.
> 
> sfp_parse_support already handles SFF8024_ECC_100GBASE_SR4_25GBASE_SR
> with compatible properties, however that entry is a contradiction in
> itself since with SFP(28) 100GBASE_SR4 is impossible - that would likely
> be a mode for qsfp modules only.
> 
> Add a case for SFF8024_ECC_100G_25GAUI_C2M_AOC selecting 25gbase-r
> interface mode and 25000baseSR link mode.
> Also enforce SFP28 bitrate limits on the values read from sfp eeprom as
> requested by Russell King.
> 
> Tested with fs.com S28-AO02 AOC SFP28 module.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> V1 -> V2: added separate case SFF8024_ECC_100G_25GAUI_C2M_AOC
> V1 -> V2: added bitrate check for eeprom values
> 
>  drivers/net/phy/sfp-bus.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> index e8dd47bffe43..a4b0bb50e2eb 100644
> --- a/drivers/net/phy/sfp-bus.c
> +++ b/drivers/net/phy/sfp-bus.c
> @@ -258,6 +258,17 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
>  	switch (id->base.extended_cc) {
>  	case SFF8024_ECC_UNSPEC:
>  		break;
> +	case SFF8024_ECC_100G_25GAUI_C2M_AOC:
> +		if (br_min <= 28000 && br_max >= 25000) {
> +			/* 25GBASE-R, possibly with FEC */
> +			__set_bit(PHY_INTERFACE_MODE_25GBASER, interfaces);
> +			/*
> +			 * There is currently no link mode for 25000base
> +			 * with unspecified range, reuse SR.
> +			 */

Hi Josua,

a minor nit from my side: : if you have to re-spin for some other reason,
the multi-line comment style for Networking code is:

	/* This is
	 * something.
	 */

> +			phylink_set(modes, 25000baseSR_Full);
> +		}
> +		break;
>  	case SFF8024_ECC_100GBASE_SR4_25GBASE_SR:
>  		phylink_set(modes, 100000baseSR4_Full);
>  		phylink_set(modes, 25000baseSR_Full);
> -- 
> 2.35.3
> 
>
Jakub Kicinski Aug. 17, 2023, 4:30 p.m. UTC | #2
On Mon, 14 Aug 2023 16:17:39 +0200 Josua Mayer wrote:
> Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC)
> for active optical cables supporting 25G and 100G speeds.
> 
> Since the specification makes no statement about transmitter range, and
> as the specific sfp module that had been tested features only 2m fiber -
> short-range (SR) modes are selected.

FWIW this got marked as "changes requested" in patchwork by DaveM.
Since we didn't get an Ack from Russell, would you mind fixing
the comment style and reposting?
Josua Mayer Aug. 18, 2023, 10:47 a.m. UTC | #3
Hi Jakub,

Am 17.08.23 um 18:30 schrieb Jakub Kicinski:
> On Mon, 14 Aug 2023 16:17:39 +0200 Josua Mayer wrote:
>> Handle extended compliance code 0x1 (SFF8024_ECC_100G_25GAUI_C2M_AOC)
>> for active optical cables supporting 25G and 100G speeds.
>>
>> Since the specification makes no statement about transmitter range, and
>> as the specific sfp module that had been tested features only 2m fiber -
>> short-range (SR) modes are selected.
> FWIW this got marked as "changes requested" in patchwork by DaveM.
> Since we didn't get an Ack from Russell, would you mind fixing
> the comment style and reposting?
I will post a v3 soon!
diff mbox series

Patch

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index e8dd47bffe43..a4b0bb50e2eb 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -258,6 +258,17 @@  void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 	switch (id->base.extended_cc) {
 	case SFF8024_ECC_UNSPEC:
 		break;
+	case SFF8024_ECC_100G_25GAUI_C2M_AOC:
+		if (br_min <= 28000 && br_max >= 25000) {
+			/* 25GBASE-R, possibly with FEC */
+			__set_bit(PHY_INTERFACE_MODE_25GBASER, interfaces);
+			/*
+			 * There is currently no link mode for 25000base
+			 * with unspecified range, reuse SR.
+			 */
+			phylink_set(modes, 25000baseSR_Full);
+		}
+		break;
 	case SFF8024_ECC_100GBASE_SR4_25GBASE_SR:
 		phylink_set(modes, 100000baseSR4_Full);
 		phylink_set(modes, 25000baseSR_Full);