diff mbox series

[net-next,4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch

Message ID 20180323201117.8416-5-alexandre.belloni@bootlin.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series Microsemi Ocelot switch support | expand

Commit Message

Alexandre Belloni March 23, 2018, 8:11 p.m. UTC 
DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../devicetree/bindings/net/mscc-ocelot.txt        | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt

Comments

Andrew Lunn March 23, 2018, 9:01 p.m. UTC | #1 
On Fri, Mar 23, 2018 at 09:11:13PM +0100, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  .../devicetree/bindings/net/mscc-ocelot.txt        | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)
> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> +  entry in reg-names.
> +- reg-names: Must include the following entries:
> +  - "sys"
> +  - "rew"
> +  - "qs"
> +  - "hsio"
> +  - "qsys"
> +  - "ana"
> +  - "portX" with X from 0 to the number of last port index available on that
> +    switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> +  frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"
> +
> +Example:
> +
> +	switch@1010000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "mscc,ocelot-switch";
> +		reg = <0x1010000 0x10000>,
> +		      <0x1030000 0x10000>,
> +		      <0x1080000 0x100>,
> +		      <0x10d0000 0x10000>,
> +		      <0x11e0000 0x100>,
> +		      <0x11f0000 0x100>,
> +		      <0x1200000 0x100>,
> +		      <0x1210000 0x100>,
> +		      <0x1220000 0x100>,
> +		      <0x1230000 0x100>,
> +		      <0x1240000 0x100>,
> +		      <0x1250000 0x100>,
> +		      <0x1260000 0x100>,
> +		      <0x1270000 0x100>,
> +		      <0x1280000 0x100>,
> +		      <0x1800000 0x80000>,
> +		      <0x1880000 0x10000>;
> +		reg-names = "sys", "rew", "qs", "hsio", "port0",
> +			    "port1", "port2", "port3", "port4", "port5",
> +			    "port6", "port7", "port8", "port9", "port10",
> +			    "qsys", "ana";
> +		interrupts = <21 22>;
> +		interrupt-names = "xtr", "inj";
> +
> +		port0: port@0 {
> +			reg = <0>;
> +			phy-handle = <&phy0>;
> +		};
> +		port1: port@1 {
> +			reg = <1>;
> +			phy-handle = <&phy1>;
> +		};

Hi Alexandre

Is there anything else in the switch which in the future might need
child nodes? At the moment, you can do
for_each_available_child_of_node() and walk the ports. But if you do
need to add some other sorts of children in the future it gets
messy. With DSA, we have a ports {} container.

       Andrew
Florian Fainelli March 23, 2018, 9:11 p.m. UTC | #2 
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  .../devicetree/bindings/net/mscc-ocelot.txt        | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)
> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> +  entry in reg-names.
> +- reg-names: Must include the following entries:
> +  - "sys"
> +  - "rew"
> +  - "qs"
> +  - "hsio"
> +  - "qsys"
> +  - "ana"
> +  - "portX" with X from 0 to the number of last port index available on that
> +    switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> +  frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"

You are not documenting the "ports" subnode(s).Please move the
individual ports definition under a ports subnode, mainly for two reasons:

- it makes it easy at the .dtsi level to have all ports disabled by default

- this makes you strictly conforming to the DSA binding for Ethernet
switches and this is good for consistency (both parsing code and just
representation).
Rob Herring March 26, 2018, 10:25 p.m. UTC | #3 
On Fri, Mar 23, 2018 at 02:11:35PM -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> >  .../devicetree/bindings/net/mscc-ocelot.txt        | 62 ++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > new file mode 100644
> > index 000000000000..ee092a85b5a0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > @@ -0,0 +1,62 @@
> > +Microsemi Ocelot network Switch
> > +===============================
> > +
> > +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> > +VSC7514)
> > +
> > +Required properties:
> > +- compatible: Should be "mscc,ocelot-switch"
> > +- reg: Must contain an (offset, length) pair of the register set for each
> > +  entry in reg-names.
> > +- reg-names: Must include the following entries:
> > +  - "sys"
> > +  - "rew"
> > +  - "qs"
> > +  - "hsio"
> > +  - "qsys"
> > +  - "ana"
> > +  - "portX" with X from 0 to the number of last port index available on that
> > +    switch
> > +- interrupts: Should contain the switch interrupts for frame extraction and
> > +  frame injection
> > +- interrupt-names: should contain the interrupt names: "xtr", "inj"
> 
> You are not documenting the "ports" subnode(s).Please move the
> individual ports definition under a ports subnode, mainly for two reasons:
> 
> - it makes it easy at the .dtsi level to have all ports disabled by default
> 
> - this makes you strictly conforming to the DSA binding for Ethernet
> switches and this is good for consistency (both parsing code and just
> representation).

ports and port collide with the OF graph binding. It would be good if 
this moved to ethernet-port(s) or similar.

Rob
Rob Herring March 26, 2018, 10:25 p.m. UTC | #4 
On Fri, Mar 23, 2018 at 09:11:13PM +0100, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  .../devicetree/bindings/net/mscc-ocelot.txt        | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)

What's the difference in these SoCs? You should probably have the 
part#'s in the compatible string.

> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> +  entry in reg-names.
> +- reg-names: Must include the following entries:
> +  - "sys"
> +  - "rew"
> +  - "qs"
> +  - "hsio"
> +  - "qsys"
> +  - "ana"
> +  - "portX" with X from 0 to the number of last port index available on that
> +    switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> +  frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"
> +
> +Example:
> +
> +	switch@1010000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "mscc,ocelot-switch";
> +		reg = <0x1010000 0x10000>,
> +		      <0x1030000 0x10000>,
> +		      <0x1080000 0x100>,
> +		      <0x10d0000 0x10000>,
> +		      <0x11e0000 0x100>,
> +		      <0x11f0000 0x100>,
> +		      <0x1200000 0x100>,
> +		      <0x1210000 0x100>,
> +		      <0x1220000 0x100>,
> +		      <0x1230000 0x100>,
> +		      <0x1240000 0x100>,
> +		      <0x1250000 0x100>,
> +		      <0x1260000 0x100>,
> +		      <0x1270000 0x100>,
> +		      <0x1280000 0x100>,
> +		      <0x1800000 0x80000>,
> +		      <0x1880000 0x10000>;
> +		reg-names = "sys", "rew", "qs", "hsio", "port0",
> +			    "port1", "port2", "port3", "port4", "port5",
> +			    "port6", "port7", "port8", "port9", "port10",
> +			    "qsys", "ana";
> +		interrupts = <21 22>;
> +		interrupt-names = "xtr", "inj";
> +
> +		port0: port@0 {
> +			reg = <0>;
> +			phy-handle = <&phy0>;
> +		};
> +		port1: port@1 {
> +			reg = <1>;
> +			phy-handle = <&phy1>;
> +		};
> +	};
> -- 
> 2.16.2
>
Andrew Lunn March 26, 2018, 10:50 p.m. UTC | #5 
> ports and port collide with the OF graph binding. It would be good if 
> this moved to ethernet-port(s) or similar.

Hi Rob

Well, we have been using port in DSA since March 2013. ports is a bit
newer, June 2016.

Changing DSA is not going to happen. But new switch bindings could use
ethernet-port(s). It just makes them inconsistent with existing switch
drivers.

       Andrew
Rob Herring March 27, 2018, 12:34 a.m. UTC | #6 
On Mon, Mar 26, 2018 at 5:50 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> ports and port collide with the OF graph binding. It would be good if
>> this moved to ethernet-port(s) or similar.
>
> Hi Rob
>
> Well, we have been using port in DSA since March 2013. ports is a bit
> newer, June 2016.

Yes, understood.

>
> Changing DSA is not going to happen. But new switch bindings could use
> ethernet-port(s). It just makes them inconsistent with existing switch
> drivers.

I'm not saying to change existing bindings, but evolve to something
that doesn't collide on new bindings if you don't have dependencies on
what the node names are. It's mainly so we can have something to key
off of to validate bindings better.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
new file mode 100644
index 000000000000..ee092a85b5a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
@@ -0,0 +1,62 @@ 
+Microsemi Ocelot network Switch
+===============================
+
+The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
+VSC7514)
+
+Required properties:
+- compatible: Should be "mscc,ocelot-switch"
+- reg: Must contain an (offset, length) pair of the register set for each
+  entry in reg-names.
+- reg-names: Must include the following entries:
+  - "sys"
+  - "rew"
+  - "qs"
+  - "hsio"
+  - "qsys"
+  - "ana"
+  - "portX" with X from 0 to the number of last port index available on that
+    switch
+- interrupts: Should contain the switch interrupts for frame extraction and
+  frame injection
+- interrupt-names: should contain the interrupt names: "xtr", "inj"
+
+Example:
+
+	switch@1010000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "mscc,ocelot-switch";
+		reg = <0x1010000 0x10000>,
+		      <0x1030000 0x10000>,
+		      <0x1080000 0x100>,
+		      <0x10d0000 0x10000>,
+		      <0x11e0000 0x100>,
+		      <0x11f0000 0x100>,
+		      <0x1200000 0x100>,
+		      <0x1210000 0x100>,
+		      <0x1220000 0x100>,
+		      <0x1230000 0x100>,
+		      <0x1240000 0x100>,
+		      <0x1250000 0x100>,
+		      <0x1260000 0x100>,
+		      <0x1270000 0x100>,
+		      <0x1280000 0x100>,
+		      <0x1800000 0x80000>,
+		      <0x1880000 0x10000>;
+		reg-names = "sys", "rew", "qs", "hsio", "port0",
+			    "port1", "port2", "port3", "port4", "port5",
+			    "port6", "port7", "port8", "port9", "port10",
+			    "qsys", "ana";
+		interrupts = <21 22>;
+		interrupt-names = "xtr", "inj";
+
+		port0: port@0 {
+			reg = <0>;
+			phy-handle = <&phy0>;
+		};
+		port1: port@1 {
+			reg = <1>;
+			phy-handle = <&phy1>;
+		};
+	};