Message-ID: <20171031092911.GA9463@n2100.armlinux.org.uk>
Date:   Tue, 31 Oct 2017 09:29:12 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Baruch Siach <baruch@...s.co.il>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andrew Lunn <andrew@...n.ch>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        Antoine Tenart <antoine.tenart@...e-electrons.com>
Subject: Re: [PATCH net v4 2/3] dt-binding: net: sfp binding documentation

On Mon, Oct 30, 2017 at 08:13:42PM -0700, Florian Fainelli wrote:
> Hi Baruch,
> 
> On 09/07/2017 02:25 AM, Baruch Siach wrote:
> > diff --git a/Documentation/devicetree/bindings/net/sff,sfp.txt b/Documentation/devicetree/bindings/net/sff,sfp.txt
> > new file mode 100644
> > index 000000000000..60e970ce10ee
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/sff,sfp.txt
> > @@ -0,0 +1,76 @@
> > +Small Form Factor (SFF) Committee Small Form-factor Pluggable (SFP)
> > +Transceiver
> > +
> > +Required properties:
> > +
> > +- compatible : must be "sff,sfp"
> > +
> > +Optional Properties:
> > +
> > +- i2c-bus : phandle of an I2C bus controller for the SFP two wire serial
> > +  interface
> 
> What was the reasoning behind using this property instead of making the
> SFP a child of the i2c bus directly? Were you thinking that there could
> be systems where the SFP is not i2c-addresable, but another 2-wire
> protocol is used instead? This is not particularly wrong per-se I guess,
> but usually, the parent/child relationship should make that more obvious.

Here's what I said about this exact subject in August:

  What reg= value would you use to identify it?  There's no particular
  I2C bus address.  There's an EEPROM on the actual module, and there
  may be a PHY on the I2C bus (some PHYs include I2C as an alternative
  way to speak to them other than MDIO.)

  I2C couldn't probe these as they are effectively hotplugged.

  However, there's also the question about why it should be a child of
  the I2C bus - the I2C bus is just a means of communicating with and
  identifying the module.  You could equally argue that it should be
  a child of the GPIO controller, because that's how it's controlled.
  You could also argue that it should be a child of the ethernet
  interface, since that's the main data path.

The SFP support is basically a driver for a socket which has a
hot-pluggable device, which may be empty at the time of probing.
If it's empty, it just has an I2C bus, a bunch of GPIOs and a
couple of serdes lanes routed to it, which appear on pins on the
socket.  There is no device that can be detected or probed.

There is no I2C bus address of an empty socket.

When a module is plugged in, then we get some I2C devices appearing.
These can take one or more I2C bus addresses.  The I2C device
binding requires that all I2C child nodes specify the bus address
of their device.  Which address do you choose?

Bear in mind that when the socket is empty, as there's no devices
present, there wouldn't be a device for a driver to bind to, so
this driver, which deals with the hot-plugging of the socket has
no device to bind with.

Think of the "sff,sfp" driver as a driver for the /socket/ rather
than the /module/ in the socket.  The SFF specifications give the
requirements for the signals at the /socket/ and the driver
implements those requirements.

If drivers are required for the modules (because they have a PHY
on them) the normal Linux PHY drivers get used for that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Powered by blists - more mailing lists