netdev - Re: [PATCH net-next 5/5] net: sfp: add support for multigig RollBall transceivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201029133800.GU1551@shell.armlinux.org.uk>
Date:   Thu, 29 Oct 2020 13:38:00 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Marek Behún <kabel@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 5/5] net: sfp: add support for multigig RollBall
 transceivers

On Wed, Oct 28, 2020 at 11:14:27PM +0100, Marek Behún wrote:
> This adds support for multigig copper SFP modules from RollBall/Hilink.
> These modules have a specific way to access clause 45 registers of the
> internal PHY.
> 
> We also need to wait at least 25 seconds after deasserting TX disable
> before accessing the PHY. The code waits for 30 seconds just to be sure.

Any ideas why it takes 25 seconds for the module to initialise - the
88x3310 startup is pretty fast in itself. However, it never amazes me
how broken SFP modules can be.

Extending T_WAIT is one way around this, and luckily I already catered
for the case where T_WAIT is extended beyond module_t_start_up.

Usual comment about line lengths...

> 
> Signed-off-by: Marek Behún <kabel@...nel.org>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Russell King <rmk+kernel@...linux.org.uk>
> ---
>  drivers/net/phy/sfp.c | 72 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 65 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index a392d5fc6ab4..379358f194ee 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -165,6 +165,7 @@ static const enum gpiod_flags gpio_flags[] = {
>   * on board (for a copper SFP) time to initialise.
>   */
>  #define T_WAIT			msecs_to_jiffies(50)
> +#define T_WAIT_LONG_PHY		msecs_to_jiffies(30000)

I think call this T_WAIT_ROLLBALL.

> @@ -1675,12 +1681,40 @@ static int sfp_cotsworks_fixup_check(struct sfp *sfp, struct sfp_eeprom_id *id)
>  	return 0;
>  }
>  
> +static int sfp_rollball_init_mdio(struct sfp *sfp)
> +{
> +	u8 page, password[4];
> +	int err;
> +
> +	page = 3;
> +
> +	err = sfp_write(sfp, true, SFP_PAGE, &page, 1);
> +	if (err != 1) {
> +		dev_err(sfp->dev, "Failed to set SFP page for RollBall MDIO access: %d\n", err);
> +		return err;
> +	}
> +
> +	password[0] = 0xff;
> +	password[1] = 0xff;
> +	password[2] = 0xff;
> +	password[3] = 0xff;
> +
> +	err = sfp_write(sfp, true, 0x7b, password, 4);
> +	if (err != 4) {
> +		dev_err(sfp->dev, "Failed to write password for RollBall MDIO access: %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}

I think this needs to be done in the MDIO driver - if we have userspace
or otherwise expand what we're doing, relying on page 3 remaining
selected will be very fragile.

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