netdev - Re: [PATCH 07/10] dt-bindings: phy: add DT binding for Microsemi Ocelot SerDes muxing

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: <20180801081539.gxkviv6rnpwzoyxb@qschulz>
Date:   Wed, 1 Aug 2018 10:15:39 +0200
From:   Quentin Schulz <quentin.schulz@...tlin.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     alexandre.belloni@...tlin.com, ralf@...ux-mips.org,
        paul.burton@...s.com, jhogan@...nel.org, robh+dt@...nel.org,
        mark.rutland@....com, davem@...emloft.net, kishon@...com,
        andrew@...n.ch, linux-mips@...ux-mips.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, allan.nielsen@...rosemi.com,
        thomas.petazzoni@...tlin.com
Subject: Re: [PATCH 07/10] dt-bindings: phy: add DT binding for Microsemi
 Ocelot SerDes muxing

Hi Florian,

On Mon, Jul 30, 2018 at 02:39:35PM -0700, Florian Fainelli wrote:
> On 07/30/2018 05:43 AM, Quentin Schulz wrote:
> > Signed-off-by: Quentin Schulz <quentin.schulz@...tlin.com>
> > ---
> >  Documentation/devicetree/bindings/phy/phy-ocelot-serdes.txt | 42 +++++++-
> >  1 file changed, 42 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-ocelot-serdes.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/phy-ocelot-serdes.txt b/Documentation/devicetree/bindings/phy/phy-ocelot-serdes.txt
> > new file mode 100644
> > index 0000000..25b102d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-ocelot-serdes.txt
> > @@ -0,0 +1,42 @@
> > +Microsemi Ocelot SerDes muxing driver
> > +-------------------------------------
> > +
> > +On Microsemi Ocelot, there is a handful of registers in HSIO address
> > +space for setting up the SerDes to switch port muxing.
> > +
> > +A SerDes X can be "muxed" to work with switch port Y or Z for example.
> > +One specific SerDes can also be used as a PCIe interface.
> > +
> > +Hence, a SerDes represents an interface, be it an Ethernet or a PCIe one.
> > +
> > +There are two kinds of SerDes: SERDES1G supports 10/100Mbps in
> > +half/full-duplex and 1000Mbps in full-duplex mode while SERDES6G supports
> > +10/100Mbps in half/full-duplex and 1000/2500Mbps in full-duplex mode.
> > +
> > +Also, SERDES6G number (aka "macro") 0 is the only interface supporting
> > +QSGMII.
> > +
> > +Required properties:
> > +
> > +- compatible: should be "mscc,vsc7514-serdes"
> > +- #phy-cells : from the generic phy bindings, must be 3. The first number
> > +               defines the kind of Serdes (1 for SERDES1G_X, 6 for
> > +	       SERDES6G_X), the second defines the macros in the specified
> > +	       kind of Serdes (X for SERDES1G_X or SERDES6G_X) and the
> > +	       last one defines the input port to use for a given SerDes
> > +	       macro,
> 
> It would probably be more natural to reverse some of this and have the
> 1st cell be the input port, while the 2nd and 3rd cell are the serdes
> kind and the serdes macro type. Same comment as Andrew, can you please
> define the 2nd and 3rd cells possible values in a header file that you
> can include from both the DTS and the driver making use of that?

OK for a define for the DeviceTree part.

You want one set of defines for the values in the 2nd cell and one other
set of defines for the 3rd cell?

I'm fine with a define for the second value (which is basically the enum
serdes_type I've defined at the beginning of the serdes driver) but I
don't see the point of defining the index of the SerDes. What would it
look like?

enum serdes_type {
	SERDES1G = 1,
	SERDES6G = 6,
}

#define SERDES1G_0	0
#define SERDES1G_1	1
#define SERDES1G_2	2
#define SERDES6G_0	0
#define SERDES6G_1	1

Then, e.g.:

&port5 {
	phys = <&serdes 5 SERDES1G SERDES1G_0>
};

If you want a define for the pair (serdes_type, serdes_index), I don't
see how I could re-use it on the driver side but it makes more sense on the
DeviceTree side:

#define SERDES1G_0	1 0
#define SERDES1G_1	1 1
#define SERDES1G_2	1 2
#define SERDES6G_0	6 0
#define SERDES6G_1	6 1

Quentin

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)