[PATCH 0/6] mvebu: Network devicetree pin muxing

[PATCH 0/6] mvebu: Network devicetree pin muxing

[Ezequiel Garcia]

This series adds the proper devicetree pin muxing for the network interfaces
on all Armada 370 boards.

The MDIO interface uses pins MPP17 and MPP18, and so this is needed
on all devices using MDIO.

For the ethernet interfaces, only the boards connecting with RGMII
need the mux. On this SoC, the SGMII is available on dedicated SerDes lanes,
and so no MPP pin mux is required.

The only board which uses SGMII is the Armada 370 Reference Design board.

This has been tested on Armada 370 Mirabox only, using the recently submitted
mvneta support for barebox. The series is based on v3.16.

Ezequiel Garcia (6):
  ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC
  ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board
  ARM: mvebu: Add proper pin muxing on the Armada 370 DB board
  ARM: mvebu: Add proper pin muxing on Netgear ReadyNAS 102
  ARM: mvebu: Add proper pin muxing on Netgear ReadyNAS 104
  ARM: mvebu: Add proper pin muxing on Armada 370 RD board

 arch/arm/boot/dts/armada-370-db.dts            |  6 ++++++
 arch/arm/boot/dts/armada-370-mirabox.dts       |  6 ++++++
 arch/arm/boot/dts/armada-370-netgear-rn102.dts |  4 ++++
 arch/arm/boot/dts/armada-370-netgear-rn104.dts |  6 ++++++
 arch/arm/boot/dts/armada-370-rd.dts            |  4 ++++
 arch/arm/boot/dts/armada-370.dtsi              | 19 +++++++++++++++++++
 6 files changed, 45 insertions(+)

-- 
2.0.1

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Ezequiel Garcia]

This commit adds the required pin muxing for the two network interfaces
and the MDIO interface, to the Armada 370 SoC.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index 21b588b..b0700bf6 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -151,6 +151,25 @@
 						       "mpp62", "mpp60", "mpp58";
 					marvell,function = "audio";
 				};
+
+				mdio_pins: mdio_pins {
+					marvell,pins = "mpp17", "mpp18";
+					marvell,function = "ge";
+				};
+
+				ge0_pins: ge0_pins {
+					marvell,pins = "mpp5", "mpp6", "mpp7", "mpp8",
+						       "mpp9", "mpp10", "mpp11", "mpp12",
+						       "mpp13", "mpp14", "mpp15", "mpp16";
+					marvell,function = "ge0";
+				};
+
+				ge1_pins: ge1_pins {
+					marvell,pins = "mpp19", "mpp20", "mpp21", "mpp22",
+						       "mpp23", "mpp24", "mpp25", "mpp26",
+						       "mpp27", "mpp28", "mpp29", "mpp30";
+					marvell,function = "ge1";
+				};
 			};
 
 			gpio0: gpio at 18100 {
-- 
2.0.1

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Ezequiel Garcia]

This commit adds the required pin muxing for the network interfaces and
the MDIO interface to be properly initialized. For instance, this makes
it possible for a bootloader to initialize and access the network interfaces.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-mirabox.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
index 097df7d8..c745256 100644
--- a/arch/arm/boot/dts/armada-370-mirabox.dts
+++ b/arch/arm/boot/dts/armada-370-mirabox.dts
@@ -91,6 +91,8 @@
 			};
 
 			mdio {
+				pinctrl-0 = <&mdio_pins>;
+				pinctrl-names = "default";
 				phy0: ethernet-phy at 0 {
 					reg = <0>;
 				};
@@ -100,11 +102,15 @@
 				};
 			};
 			ethernet at 70000 {
+				pinctrl-0 = <&ge0_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy0>;
 				phy-mode = "rgmii-id";
 			};
 			ethernet at 74000 {
+				pinctrl-0 = <&ge1_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy1>;
 				phy-mode = "rgmii-id";
-- 
2.0.1

[PATCH 3/6] ARM: mvebu: Add proper pin muxing on the Armada 370 DB board

[Ezequiel Garcia]

This commit adds the required pin muxing for the network interfaces and
the MDIO interface to be properly initialized. For instance, this makes
it possible for a bootloader to initialize and access the network interfaces

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-db.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index 416f4e5..ab57593 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -43,6 +43,8 @@
 			};
 
 			mdio {
+				pinctrl-0 = <&mdio_pins>;
+				pinctrl-names = "default";
 				phy0: ethernet-phy at 0 {
 					reg = <0>;
 				};
@@ -53,11 +55,15 @@
 			};
 
 			ethernet at 70000 {
+				pinctrl-0 = <&ge0_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy0>;
 				phy-mode = "rgmii-id";
 			};
 			ethernet at 74000 {
+				pinctrl-0 = <&ge1_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy1>;
 				phy-mode = "rgmii-id";
-- 
2.0.1

[PATCH 4/6] ARM: mvebu: Add proper pin muxing on Netgear ReadyNAS 102

[Ezequiel Garcia]

This commit adds the required pin muxing for the network interfaces and
the MDIO interface to be properly initialized. For instance, this makes
it possible for a bootloader to initialize and access the network interfaces

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-netgear-rn102.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-netgear-rn102.dts b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
index d6d572e..4b793da 100644
--- a/arch/arm/boot/dts/armada-370-netgear-rn102.dts
+++ b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
@@ -101,12 +101,16 @@
 			};
 
 			mdio {
+				pinctrl-0 = <&mdio_pins>;
+				pinctrl-names = "default";
 				phy0: ethernet-phy at 0 { /* Marvell 88E1318 */
 					reg = <0>;
 				};
 			};
 
 			ethernet at 74000 {
+				pinctrl-0 = <&ge1_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy0>;
 				phy-mode = "rgmii-id";
-- 
2.0.1

[PATCH 5/6] ARM: mvebu: Add proper pin muxing on Netgear ReadyNAS 104

[Ezequiel Garcia]

This commit adds the required pin muxing for the network interfaces and
the MDIO interface to be properly initialized. For instance, this makes
it possible for a bootloader to initialize and access the network interfaces

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-netgear-rn104.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-netgear-rn104.dts b/arch/arm/boot/dts/armada-370-netgear-rn104.dts
index c5fe8b5..f5fbd14 100644
--- a/arch/arm/boot/dts/armada-370-netgear-rn104.dts
+++ b/arch/arm/boot/dts/armada-370-netgear-rn104.dts
@@ -86,6 +86,8 @@
 			};
 
 			mdio {
+				pinctrl-0 = <&mdio_pins>;
+				pinctrl-names = "default";
 				phy0: ethernet-phy at 0 { /* Marvell 88E1318 */
 					reg = <0>;
 				};
@@ -96,12 +98,16 @@
 			};
 
 			ethernet at 70000 {
+				pinctrl-0 = <&ge0_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy0>;
 				phy-mode = "rgmii-id";
 			};
 
 			ethernet at 74000 {
+				pinctrl-0 = <&ge1_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy1>;
 				phy-mode = "rgmii-id";
-- 
2.0.1

[PATCH 6/6] ARM: mvebu: Add proper pin muxing on Armada 370 RD board

[Ezequiel Garcia]

This commit adds the required pin muxing for the network interfaces and
the MDIO interface to be properly initialized. For instance, this makes
it possible for a bootloader to initialize and access the network interfaces

Only the second network interface is pin muxed. The first network interface is
connected to the PHY using SGMII, which uses a dedicated SerDes lane.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-rd.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-rd.dts b/arch/arm/boot/dts/armada-370-rd.dts
index 4169f40..7fcd36c 100644
--- a/arch/arm/boot/dts/armada-370-rd.dts
+++ b/arch/arm/boot/dts/armada-370-rd.dts
@@ -59,6 +59,8 @@
 			};
 
 			mdio {
+				pinctrl-0 = <&mdio_pins>;
+				pinctrl-names = "default";
 				phy0: ethernet-phy at 0 {
 					reg = <0>;
 				};
@@ -74,6 +76,8 @@
 				phy-mode = "sgmii";
 			};
 			ethernet at 74000 {
+				pinctrl-0 = <&ge1_pins>;
+				pinctrl-names = "default";
 				status = "okay";
 				phy = <&phy1>;
 				phy-mode = "rgmii-id";
-- 
2.0.1

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Andrew Lunn]

On Fri, Aug 08, 2014 at 12:18:52PM -0300, Ezequiel Garcia wrote:
> This commit adds the required pin muxing for the network interfaces and
> the MDIO interface to be properly initialized. For instance, this makes
> it possible for a bootloader to initialize and access the network interfaces.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-370-mirabox.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
> index 097df7d8..c745256 100644
> --- a/arch/arm/boot/dts/armada-370-mirabox.dts
> +++ b/arch/arm/boot/dts/armada-370-mirabox.dts
> @@ -91,6 +91,8 @@
>  			};
>  
>  			mdio {
> +				pinctrl-0 = <&mdio_pins>;
> +				pinctrl-names = "default";
>  				phy0: ethernet-phy at 0 {
>  					reg = <0>;
>  				};
> @@ -100,11 +102,15 @@
>  				};
>  			};
>  			ethernet at 70000 {
> +				pinctrl-0 = <&ge0_pins>;
> +				pinctrl-names = "default";

Hi Ezequiel

Could you put these one level higher in the DT hierarchy? i.e. once in
armada-370.dtsi and not in every board .dts file?

	Andrew

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Ezequiel Garcia]

On 08 Aug 07:12 PM, Andrew Lunn wrote:
> On Fri, Aug 08, 2014 at 12:18:52PM -0300, Ezequiel Garcia wrote:
> > This commit adds the required pin muxing for the network interfaces and
> > the MDIO interface to be properly initialized. For instance, this makes
> > it possible for a bootloader to initialize and access the network interfaces.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  arch/arm/boot/dts/armada-370-mirabox.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
> > index 097df7d8..c745256 100644
> > --- a/arch/arm/boot/dts/armada-370-mirabox.dts
> > +++ b/arch/arm/boot/dts/armada-370-mirabox.dts
> > @@ -91,6 +91,8 @@
> >  			};
> >  
> >  			mdio {
> > +				pinctrl-0 = <&mdio_pins>;
> > +				pinctrl-names = "default";
> >  				phy0: ethernet-phy at 0 {
> >  					reg = <0>;
> >  				};
> > @@ -100,11 +102,15 @@
> >  				};
> >  			};
> >  			ethernet at 70000 {
> > +				pinctrl-0 = <&ge0_pins>;
> > +				pinctrl-names = "default";
> 
> Hi Ezequiel
> 
> Could you put these one level higher in the DT hierarchy? i.e. once in
> armada-370.dtsi and not in every board .dts file?
> 

Are you sure that's the correct approach?

As explained on the cover letter, these MAC MPP mux are not required in the
SGMII case. Moreover, this is not the only way to wire the MAC0 and MAC1
controllers, so I even considered pushing down the pinmux to the dts files!

The devicetree location of the pinmux is something that still confuses me,
so I'm more than open to directions here.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Andrew Lunn]

On Fri, Aug 08, 2014 at 03:21:56PM -0300, Ezequiel Garcia wrote:
> On 08 Aug 07:12 PM, Andrew Lunn wrote:
> > On Fri, Aug 08, 2014 at 12:18:52PM -0300, Ezequiel Garcia wrote:
> > > This commit adds the required pin muxing for the network interfaces and
> > > the MDIO interface to be properly initialized. For instance, this makes
> > > it possible for a bootloader to initialize and access the network interfaces.
> > > 
> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > > ---
> > >  arch/arm/boot/dts/armada-370-mirabox.dts | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
> > > index 097df7d8..c745256 100644
> > > --- a/arch/arm/boot/dts/armada-370-mirabox.dts
> > > +++ b/arch/arm/boot/dts/armada-370-mirabox.dts
> > > @@ -91,6 +91,8 @@
> > >  			};
> > >  
> > >  			mdio {
> > > +				pinctrl-0 = <&mdio_pins>;
> > > +				pinctrl-names = "default";
> > >  				phy0: ethernet-phy at 0 {
> > >  					reg = <0>;
> > >  				};
> > > @@ -100,11 +102,15 @@
> > >  				};
> > >  			};
> > >  			ethernet at 70000 {
> > > +				pinctrl-0 = <&ge0_pins>;
> > > +				pinctrl-names = "default";
> > 
> > Hi Ezequiel
> > 
> > Could you put these one level higher in the DT hierarchy? i.e. once in
> > armada-370.dtsi and not in every board .dts file?
> > 
> 
> Are you sure that's the correct approach?
> 
> As explained on the cover letter, these MAC MPP mux are not required in the
> SGMII case. Moreover, this is not the only way to wire the MAC0 and MAC1
> controllers, so I even considered pushing down the pinmux to the dts files!

Hi Ezequiel

At least at the moment, it seems like RGMII is the norm, and SGMII is
the exception. So having the default as RGMII probably makes
sense. Boards which don't require it can then override this in there
.dts file. If we see this changes with time, we can swap it around.

> The devicetree location of the pinmux is something that still confuses me,
> so I'm more than open to directions here.

Patch #1 comment is not really correct. It defines a number of pin
configuration nodes. These nodes can then be used via phandles.  What
might help is naming these differently. Name them
ge[01]_rgmii_pins. It is then clear when they should be used. You can
also add ge[0]_sgmii_pins, which would be an empty set. You can then
use ge[01]_rgmii_pins as the default in armada-370.dtsi and override
them with ge[0]_sgmii_pins for the 370 RD.

Experience from Kirkwood is that it also has multiple ways to wire up
MAC0 and MAC1, but in practice, nobody ever does. And if they did, it
can be overridden in the .dts file.

    Andrew

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Ezequiel Garcia]

On 08 Aug 09:21 PM, Andrew Lunn wrote:
> On Fri, Aug 08, 2014 at 03:21:56PM -0300, Ezequiel Garcia wrote:
> > On 08 Aug 07:12 PM, Andrew Lunn wrote:
> > > On Fri, Aug 08, 2014 at 12:18:52PM -0300, Ezequiel Garcia wrote:
> > > > This commit adds the required pin muxing for the network interfaces and
> > > > the MDIO interface to be properly initialized. For instance, this makes
> > > > it possible for a bootloader to initialize and access the network interfaces.
> > > > 
> > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > > > ---
> > > >  arch/arm/boot/dts/armada-370-mirabox.dts | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
> > > > index 097df7d8..c745256 100644
> > > > --- a/arch/arm/boot/dts/armada-370-mirabox.dts
> > > > +++ b/arch/arm/boot/dts/armada-370-mirabox.dts
> > > > @@ -91,6 +91,8 @@
> > > >  			};
> > > >  
> > > >  			mdio {
> > > > +				pinctrl-0 = <&mdio_pins>;
> > > > +				pinctrl-names = "default";
> > > >  				phy0: ethernet-phy at 0 {
> > > >  					reg = <0>;
> > > >  				};
> > > > @@ -100,11 +102,15 @@
> > > >  				};
> > > >  			};
> > > >  			ethernet at 70000 {
> > > > +				pinctrl-0 = <&ge0_pins>;
> > > > +				pinctrl-names = "default";
> > > 
> > > Hi Ezequiel
> > > 
> > > Could you put these one level higher in the DT hierarchy? i.e. once in
> > > armada-370.dtsi and not in every board .dts file?
> > > 
> > 
> > Are you sure that's the correct approach?
> > 
> > As explained on the cover letter, these MAC MPP mux are not required in the
> > SGMII case. Moreover, this is not the only way to wire the MAC0 and MAC1
> > controllers, so I even considered pushing down the pinmux to the dts files!
> 
> Hi Ezequiel
> 
> At least at the moment, it seems like RGMII is the norm, and SGMII is
> the exception. So having the default as RGMII probably makes
> sense. Boards which don't require it can then override this in there
> .dts file. If we see this changes with time, we can swap it around.
> 

OK.

> > The devicetree location of the pinmux is something that still confuses me,
> > so I'm more than open to directions here.
> 
> Patch #1 comment is not really correct. It defines a number of pin
> configuration nodes. These nodes can then be used via phandles.  What
> might help is naming these differently. Name them
> ge[01]_rgmii_pins. It is then clear when they should be used. You can
> also add ge[0]_sgmii_pins, which would be an empty set. You can then
> use ge[01]_rgmii_pins as the default in armada-370.dtsi and override
> them with ge[0]_sgmii_pins for the 370 RD.
> 

OK.

> Experience from Kirkwood is that it also has multiple ways to wire up
> MAC0 and MAC1, but in practice, nobody ever does. And if they did, it
> can be overridden in the .dts file.
> 

The above sounds good, although I don't have a strong opinion.

If everybody is happy with the approach, I'll prepare a v2 changing the .dtsi
with RGMII as the default mux, and overriding it in the A370-RD dts.

Jason, Thomas, Greg: Any objections?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Fri,  8 Aug 2014 12:18:51 -0300, Ezequiel Garcia wrote:

> +				ge0_pins: ge0_pins {

Would it make sense to name that ge0_rgmii_pins ? And ditto for ge1 ?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Thomas Petazzoni]

Dear Andrew Lunn,

On Fri, 8 Aug 2014 21:21:06 +0200, Andrew Lunn wrote:

> At least at the moment, it seems like RGMII is the norm, and SGMII is
> the exception. So having the default as RGMII probably makes
> sense. Boards which don't require it can then override this in there
> .dts file. If we see this changes with time, we can swap it around.

I personally disagree with this approach. armada-370.dtsi is here to
describe what happens at the *SoC* level, not to factorize some random
board specific details that appear to be common between a certain
number of boards (but not all).

So I really, really, would prefer to keep the board-specific details
such as which pin muxing is done for Ethernet in each individual .dts
file.

.dtsi to describe the SoC, .dts to describe the board. It's clear and
simple for everyone to understand, especially for new comers.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Ezequiel Garcia]

On 09 Aug 05:21 PM, Thomas Petazzoni wrote:
> On Fri, 8 Aug 2014 21:21:06 +0200, Andrew Lunn wrote:
> 
> > At least at the moment, it seems like RGMII is the norm, and SGMII is
> > the exception. So having the default as RGMII probably makes
> > sense. Boards which don't require it can then override this in there
> > .dts file. If we see this changes with time, we can swap it around.
> 
> I personally disagree with this approach. armada-370.dtsi is here to
> describe what happens at the *SoC* level, not to factorize some random
> board specific details that appear to be common between a certain
> number of boards (but not all).
> 
> So I really, really, would prefer to keep the board-specific details
> such as which pin muxing is done for Ethernet in each individual .dts
> file.
> 
> .dtsi to describe the SoC, .dts to describe the board. It's clear and
> simple for everyone to understand, especially for new comers.
> 

Just to clarify, you say we should even have pin mux configuration in each dts,
or the configuration in the dtsi and the pin muxing of each device
in the dts?

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Jason Cooper]

Thomas,

On Sat, Aug 09, 2014 at 05:21:49PM +0200, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Fri, 8 Aug 2014 21:21:06 +0200, Andrew Lunn wrote:
> 
> > At least at the moment, it seems like RGMII is the norm, and SGMII is
> > the exception. So having the default as RGMII probably makes
> > sense. Boards which don't require it can then override this in there
> > .dts file. If we see this changes with time, we can swap it around.
> 
> I personally disagree with this approach. armada-370.dtsi is here to
> describe what happens at the *SoC* level, not to factorize some random
> board specific details that appear to be common between a certain
> number of boards (but not all).

It's my understanding that *possible* pinmux configurations are a
characteristic of the SoC.  Which configurations are used is an artifact
of the board.

I don't see the need to list all possible configurations in the SoC dtsi
file, but just the ones we have actually seen used.

> So I really, really, would prefer to keep the board-specific details
> such as which pin muxing is done for Ethernet in each individual .dts
> file.

So, wouldn't this be the phandle selection?

> .dtsi to describe the SoC, .dts to describe the board. It's clear and
> simple for everyone to understand, especially for new comers.

If you're strongly opposed to describing possible pinmux configurations
in the SoC dtsi file, how about an SoC-pinmux.dtsi?  I just see the
"Thar be dragons ahead" sign by describing pinmux in the board files.

Developers adding support for a new board will typically (for better or
for worse) start with a dts that's close (eg guruplug for dreamplug),
and edit it to fit their needs.  During the debugging process, if the
pinmux is described in the dts file, they might be more prone to tweak
it into a non-possible configuration for that SoC.  This is a lot less
likely if the possible pinmux configs are described in a dtsi file.  The
developer would be more likely to try different pinmux phandles in that
case.

fwiw, the possible pinmux configs are usually (always?) described in SoC
datasheets, so I think it logically belongs to the SoC dtsi.

thx,

Jason.

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Ezequiel Garcia]

On 09 Aug 05:19 PM, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Fri,  8 Aug 2014 12:18:51 -0300, Ezequiel Garcia wrote:
> 
> > +				ge0_pins: ge0_pins {
> 
> Would it make sense to name that ge0_rgmii_pins ? And ditto for ge1 ?
> 

Yup, I think it does.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Sebastian Hesselbarth]

On 08/08/2014 05:18 PM, Ezequiel Garcia wrote:
> This commit adds the required pin muxing for the two network interfaces
> and the MDIO interface, to the Armada 370 SoC.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-370.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> index 21b588b..b0700bf6 100644
> --- a/arch/arm/boot/dts/armada-370.dtsi
> +++ b/arch/arm/boot/dts/armada-370.dtsi
> @@ -151,6 +151,25 @@
>  						       "mpp62", "mpp60", "mpp58";
>  					marvell,function = "audio";
>  				};
> +
> +				mdio_pins: mdio_pins {

Ezequiel,

please keep me posted next time ;)

Above should be: mdio_pins: mdio-pins {
same for the ones below.

> +					marvell,pins = "mpp17", "mpp18";
> +					marvell,function = "ge";

As we haven't used it before, we could still rename "ge" function to
more approprate "mdio"... but it is ok.

Besides the comments,

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

> +				};
> +
> +				ge0_pins: ge0_pins {
> +					marvell,pins = "mpp5", "mpp6", "mpp7", "mpp8",
> +						       "mpp9", "mpp10", "mpp11", "mpp12",
> +						       "mpp13", "mpp14", "mpp15", "mpp16";
> +					marvell,function = "ge0";
> +				};
> +
> +				ge1_pins: ge1_pins {
> +					marvell,pins = "mpp19", "mpp20", "mpp21", "mpp22",
> +						       "mpp23", "mpp24", "mpp25", "mpp26",
> +						       "mpp27", "mpp28", "mpp29", "mpp30";
> +					marvell,function = "ge1";
> +				};
>  			};
>  
>  			gpio0: gpio at 18100 {
> 

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Sebastian Hesselbarth]

On 08/08/2014 05:18 PM, Ezequiel Garcia wrote:
> This commit adds the required pin muxing for the two network interfaces
> and the MDIO interface, to the Armada 370 SoC.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-370.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> index 21b588b..b0700bf6 100644
> --- a/arch/arm/boot/dts/armada-370.dtsi
> +++ b/arch/arm/boot/dts/armada-370.dtsi
> @@ -151,6 +151,25 @@
>  						       "mpp62", "mpp60", "mpp58";
>  					marvell,function = "audio";
>  				};
> +
> +				mdio_pins: mdio_pins {
> +					marvell,pins = "mpp17", "mpp18";
> +					marvell,function = "ge";
> +				};
> +
> +				ge0_pins: ge0_pins {

More nitpicking,

as stated by others it should be "ge{0,1]_rgmii_pins" not only because
it is rgmii, but because a370 also supports gmii for ge0.

Sebastian

> +					marvell,pins = "mpp5", "mpp6", "mpp7", "mpp8",
> +						       "mpp9", "mpp10", "mpp11", "mpp12",
> +						       "mpp13", "mpp14", "mpp15", "mpp16";
> +					marvell,function = "ge0";
> +				};
> +
> +				ge1_pins: ge1_pins {
> +					marvell,pins = "mpp19", "mpp20", "mpp21", "mpp22",
> +						       "mpp23", "mpp24", "mpp25", "mpp26",
> +						       "mpp27", "mpp28", "mpp29", "mpp30";
> +					marvell,function = "ge1";
> +				};
>  			};
>  
>  			gpio0: gpio at 18100 {
> 

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Thomas Petazzoni]

Dear Jason Cooper,

On Sat, 9 Aug 2014 11:57:56 -0400, Jason Cooper wrote:

> > I personally disagree with this approach. armada-370.dtsi is here to
> > describe what happens at the *SoC* level, not to factorize some random
> > board specific details that appear to be common between a certain
> > number of boards (but not all).
> 
> It's my understanding that *possible* pinmux configurations are a
> characteristic of the SoC.  Which configurations are used is an artifact
> of the board.
> 
> I don't see the need to list all possible configurations in the SoC dtsi
> file, but just the ones we have actually seen used.

Absolutely.

> > So I really, really, would prefer to keep the board-specific details
> > such as which pin muxing is done for Ethernet in each individual .dts
> > file.
> 
> So, wouldn't this be the phandle selection?

Yes.

> > .dtsi to describe the SoC, .dts to describe the board. It's clear and
> > simple for everyone to understand, especially for new comers.
> 
> If you're strongly opposed to describing possible pinmux configurations
> in the SoC dtsi file, how about an SoC-pinmux.dtsi?  I just see the
> "Thar be dragons ahead" sign by describing pinmux in the board files.

Not at all. Pinmux configurations in the .dtsi files are perfect, as
long as those pinmux configurations are related to "SoC" things, which
the RGMII pinmux configurations are.

However, Andrew was suggesting to move the phandle selection to
the .dtsi file, with the argument that most boards use RGMII, so it
should be the default. That's what I disagreed with.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/6] ARM: mvebu: Add proper pin muxing on Globalscale Mirabox board

[Andrew Lunn]

> However, Andrew was suggesting to move the phandle selection to
> the .dtsi file, with the argument that most boards use RGMII, so it
> should be the default. That's what I disagreed with.

I'm not too bothered. So place it in the board .dts file.

    Andrew

[PATCH 1/6] ARM: mvebu: Add required ethernet muxing for the Armada 370 SoC

[Ezequiel Garcia]

On 10 Aug 02:16 PM, Sebastian Hesselbarth wrote:
> On 08/08/2014 05:18 PM, Ezequiel Garcia wrote:
> > This commit adds the required pin muxing for the two network interfaces
> > and the MDIO interface, to the Armada 370 SoC.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  arch/arm/boot/dts/armada-370.dtsi | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> > index 21b588b..b0700bf6 100644
> > --- a/arch/arm/boot/dts/armada-370.dtsi
> > +++ b/arch/arm/boot/dts/armada-370.dtsi
> > @@ -151,6 +151,25 @@
> >  						       "mpp62", "mpp60", "mpp58";
> >  					marvell,function = "audio";
> >  				};
> > +
> > +				mdio_pins: mdio_pins {
> 
> Ezequiel,
> 
> please keep me posted next time ;)
> 

Ouch...

> Above should be: mdio_pins: mdio-pins {
> same for the ones below.
> 

OK.

> > +					marvell,pins = "mpp17", "mpp18";
> > +					marvell,function = "ge";
> 
> As we haven't used it before, we could still rename "ge" function to
> more approprate "mdio"... but it is ok.
>

FWIW, I'd rather leave it as it is. If we change the function name, we would
make backporting a PITA, just because of a nitpick.
 
> Besides the comments,
> 
> Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com