All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: "Marek Behún" <kabel@kernel.org>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	netdev@vger.kernel.org
Subject: Re: [PATCH russell-kings-net-queue v2 1/3] net: phy: mdio-i2c: support I2C MDIO protocol for RollBall SFP modules
Date: Tue, 20 Oct 2020 18:00:39 +0200	[thread overview]
Message-ID: <20201020160039.GI139700@lunn.ch> (raw)
In-Reply-To: <20201020150615.11969-2-kabel@kernel.org>

> This extends the mdio-i2c driver so that when SFP PHY address 17 is used
> (which in mdio-i2c terms corresponds to I2C address 0x51), then this
> different protocol is used for MDIO access.

Hi Marek

I don't see that being very scalable. What happens when the next SFP
comes along which has a different protocol at address 0x51. Since you
can identify the SFP via the EEPROM information, i would prefer you
explicitly tell it to use the rollball protocol when instantiating the
MDIO bus.

>   * I2C bus addresses 0x50 and 0x51 are normally an EEPROM, which is
>   * specified to be present in SFP modules.  These correspond with PHY
> - * addresses 16 and 17.  Disallow access to these "phy" addresses.
> + * addresses 16 and 17.  Disallow access to 0x50 "phy" address.
> + * Use RollBall protocol when accessing via the 0x51 address.
>   */
>  static bool i2c_mii_valid_phy_id(int phy_id)
>  {
> -	return phy_id != 0x10 && phy_id != 0x11;
> +	return phy_id != 0x10;
> +}

I'm not sure that is safe. It means that we will scan address 0x11 to
see if there is a PHY there. And if the SFP does have diagnostics
registers, that might be enough that phylib thinks there is a PHY
there.

I think you only need to allow access to 0x11 if rollball protocol has
been enabled.

     Andrew

  reply	other threads:[~2020-10-20 16:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 15:06 [PATCH russell-kings-net-queue v2 0/3] Support for RollBall 10G copper SFP modules Marek Behún
2020-10-20 15:06 ` [PATCH russell-kings-net-queue v2 1/3] net: phy: mdio-i2c: support I2C MDIO protocol for RollBall " Marek Behún
2020-10-20 16:00   ` Andrew Lunn [this message]
2020-10-20 16:15     ` Marek Behún
2020-10-20 15:06 ` [PATCH russell-kings-net-queue v2 2/3] net: phy: sfp: add support for multigig RollBall modules Marek Behún
2020-10-20 15:51   ` Andrew Lunn
2020-10-20 16:14     ` Marek Behún
2020-10-20 15:06 ` [PATCH russell-kings-net-queue v2 3/3] net: phylink: don't fail attaching phy on 1000base-x/2500base-x mode Marek Behún

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201020160039.GI139700@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=kabel@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.