Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Jul 2023 08:50:22 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
Cc:     netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
        linux-kernel@vger.kernel.org, andrew@lunn.ch,
        UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next 2/2] net: sfp: add quirk for FS's DAC10G SFP
 (SFPP-PC01)
Message-ID: <ZLo4vvAPXNy51ZRi@shell.armlinux.org.uk>
References: <20230721060057.2998-1-Raju.Lakkaraju@microchip.com>
 <20230721060057.2998-3-Raju.Lakkaraju@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230721060057.2998-3-Raju.Lakkaraju@microchip.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Fri, Jul 21, 2023 at 11:30:57AM +0530, Raju Lakkaraju wrote:
> Add a quirk for a DAC10G SFP that identifies itself as "FS" "SFPP-PC01".
> Add a quirk to enable the SGMII interface, modes 2500base-T, 1000base-T,
> 100base-T/Full and 100base-T/Half support.
> 
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> ---
>  drivers/net/phy/sfp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index ee049efdf71b..80d2680f08ab 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -421,6 +421,18 @@ static void sfp_quirk_oem_2_5g(const struct sfp_eeprom_id *id,
>  	sfp_quirk_disable_autoneg(id, modes, interfaces);
>  }
>  
> +static void sfp_quirk_fs_dac(const struct sfp_eeprom_id *id,
> +			     unsigned long *modes,
> +			     unsigned long *interfaces)
> +{
> +	/* Fiberstore(FS)'s DAC SFP (SFPP-PC01) */
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, modes);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, modes);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, modes);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, modes);

No. A DAC cable does _not_ give you twisted-pair ethernet which is what
baseT linkmodes are.

The description on fs.com states:

"The 10G SFP+ Passive Direct Attach Copper Twinax Cable is designed for
use in 10GBASE Ethernet."

That means it supports 10GBASE-CR, which is the link mode for 10 Gigabit
ethernet over a direct attach cable.

If it does work at 2.5G speeds, then the protocol that will be used for
that will be 2500base-X.

If it does work at 1G speeds, then the protocol that will be used for
that will be 1000base-X (which is the "standard" protocol for the host
connection for 1G SFPs.)

Going below that isn't technically possible, but can be done with SGMII
by forcing the link settings, but is there really any need to support
slower speeds? What possible valid reason could there be? A host not
supporting 1G speeds with a SFP cage would be utterly insane.

> +	__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);

I also disagree with this. Unless there is a PHY present, you can _not_
connect two hosts together that are using the SGMII protocol. The SGMII
"negotiation" protocol is _asymetric_, and it relies upon a PHY telling
the host what the link speed and duplex settings are. A host never
sends that information, so this will not "negotiate".

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