[PATCH 0/3] ARM: dts: lan966x: Extend pcb8291.

[PATCH 0/3] ARM: dts: lan966x: Extend pcb8291.

[Horatiu Vultur]

Extend pcb8291 with different features like, add support to reset the board
using GPIO and add network support.

Horatiu Vultur (3):
  ARM: dts: lan966x: Add gpio-restart
  ARM: dts: lan966x: Disable can0 on pcb8291
  ARM: dts: lan966x: Enable network driver on pcb8291

 arch/arm/boot/dts/lan966x-pcb8291.dts | 45 ++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.33.0

[PATCH 1/3] ARM: dts: lan966x: Add gpio-restart

[Horatiu Vultur]

The pcb8291 can be rebooted by toggling the GPIO 56. Therefore enable
this in DT.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 arch/arm/boot/dts/lan966x-pcb8291.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
index d56d2054c38d..02fbf7371a8d 100644
--- a/arch/arm/boot/dts/lan966x-pcb8291.dts
+++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
@@ -16,6 +16,12 @@ chosen {
 	aliases {
 		serial0 = &usart3;
 	};
+
+	gpio-restart {
+		compatible = "gpio-restart";
+		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
+		priority = <200>;
+	};
 };
 
 &gpio {
-- 
2.33.0

Re: [PATCH 1/3] ARM: dts: lan966x: Add gpio-restart

[Claudiu.Beznea]

On 19.07.2022 00:29, Horatiu Vultur wrote:
> The pcb8291 can be rebooted by toggling the GPIO 56. Therefore enable
> this in DT.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  arch/arm/boot/dts/lan966x-pcb8291.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> index d56d2054c38d..02fbf7371a8d 100644
> --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> @@ -16,6 +16,12 @@ chosen {
>  	aliases {
>  		serial0 = &usart3;
>  	};
> +
> +	gpio-restart {
> +		compatible = "gpio-restart";
> +		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;

Could you, please, add also pinctrl-names, pinctrl-0 for this gpio?

> +		priority = <200>;
> +	};
>  };
>  
>  &gpio {

Re: [PATCH 1/3] ARM: dts: lan966x: Add gpio-restart

[Horatiu Vultur - M31836]

The 07/19/2022 07:52, Claudiu Beznea - M18063 wrote:
> On 19.07.2022 00:29, Horatiu Vultur wrote:
> > The pcb8291 can be rebooted by toggling the GPIO 56. Therefore enable
> > this in DT.
> > 
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  arch/arm/boot/dts/lan966x-pcb8291.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > index d56d2054c38d..02fbf7371a8d 100644
> > --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> > +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > @@ -16,6 +16,12 @@ chosen {
> >  	aliases {
> >  		serial0 = &usart3;
> >  	};
> > +
> > +	gpio-restart {
> > +		compatible = "gpio-restart";
> > +		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
> 
> Could you, please, add also pinctrl-names, pinctrl-0 for this gpio?

I am not sure I need to set pinctrl-names and pinctrl-0.
Because I don't need to setup any function for GPIO 56 to be able to
reset.
It is something similar sparx5 [1].

[1] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi#L10

> 
> > +		priority = <200>;
> > +	};
> >  };
> >  
> >  &gpio {
> 

-- 
/Horatiu

[PATCH 2/3] ARM: dts: lan966x: Disable can0 on pcb8291

[Horatiu Vultur]

On pcb8291, can0 and the network driver share some of the GPIOs so only
1 device can be active. Therefore disable can0 as we want to enable the
network driver.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 arch/arm/boot/dts/lan966x-pcb8291.dts | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
index 02fbf7371a8d..2cb532aa33f0 100644
--- a/arch/arm/boot/dts/lan966x-pcb8291.dts
+++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
@@ -30,18 +30,6 @@ fc3_b_pins: fc3-b-pins {
 		pins = "GPIO_52", "GPIO_53";
 		function = "fc3_b";
 	};
-
-	can0_b_pins:  can0-b-pins {
-		/* RX, TX */
-		pins = "GPIO_35", "GPIO_36";
-		function = "can0_b";
-	};
-};
-
-&can0 {
-	pinctrl-0 = <&can0_b_pins>;
-	pinctrl-names = "default";
-	status = "okay";
 };
 
 &flx3 {
-- 
2.33.0

Re: [PATCH 2/3] ARM: dts: lan966x: Disable can0 on pcb8291

[Claudiu.Beznea]

On 19.07.2022 00:29, Horatiu Vultur wrote:
> On pcb8291, can0 and the network driver share some of the GPIOs so only
> 1 device can be active. Therefore disable can0 as we want to enable the
> network driver.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  arch/arm/boot/dts/lan966x-pcb8291.dts | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> index 02fbf7371a8d..2cb532aa33f0 100644
> --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> @@ -30,18 +30,6 @@ fc3_b_pins: fc3-b-pins {
>  		pins = "GPIO_52", "GPIO_53";
>  		function = "fc3_b";
>  	};
> -
> -	can0_b_pins:  can0-b-pins {
> -		/* RX, TX */
> -		pins = "GPIO_35", "GPIO_36";
> -		function = "can0_b";
> -	};
> -};
> -
> -&can0 {
> -	pinctrl-0 = <&can0_b_pins>;
> -	pinctrl-names = "default";
> -	status = "okay";

You can just delete the status line here or change it to "disabled" and
leave the rest for reference. For evaluation one could just enable it here
afterwards. On the other AT91 specific boards we are using status =
"disabled"; and a short comment after like this:

	status = "disabled"; /* Conflict with gmac0. */

>  };
>  
>  &flx3 {

Re: [PATCH 2/3] ARM: dts: lan966x: Disable can0 on pcb8291

[Horatiu Vultur - M31836]

The 07/19/2022 07:55, Claudiu Beznea - M18063 wrote:
> On 19.07.2022 00:29, Horatiu Vultur wrote:
> > On pcb8291, can0 and the network driver share some of the GPIOs so only
> > 1 device can be active. Therefore disable can0 as we want to enable the
> > network driver.
> > 
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  arch/arm/boot/dts/lan966x-pcb8291.dts | 12 ------------
> >  1 file changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > index 02fbf7371a8d..2cb532aa33f0 100644
> > --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> > +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > @@ -30,18 +30,6 @@ fc3_b_pins: fc3-b-pins {
> >  		pins = "GPIO_52", "GPIO_53";
> >  		function = "fc3_b";
> >  	};
> > -
> > -	can0_b_pins:  can0-b-pins {
> > -		/* RX, TX */
> > -		pins = "GPIO_35", "GPIO_36";
> > -		function = "can0_b";
> > -	};
> > -};
> > -
> > -&can0 {
> > -	pinctrl-0 = <&can0_b_pins>;
> > -	pinctrl-names = "default";
> > -	status = "okay";
> 
> You can just delete the status line here or change it to "disabled" and
> leave the rest for reference. For evaluation one could just enable it here
> afterwards. On the other AT91 specific boards we are using status =
> "disabled"; and a short comment after like this:
> 
> 	status = "disabled"; /* Conflict with gmac0. */

That is a great idea. Yes I will do that.

> 
> >  };
> >  
> >  &flx3 {
> 

-- 
/Horatiu

[PATCH 3/3] ARM: dts: lan966x: Enable network driver on pcb8291

[Horatiu Vultur]

The pcb8291 has 2 ports that are connected to the internal ports of the
switch. Enable them in DT.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 arch/arm/boot/dts/lan966x-pcb8291.dts | 35 +++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
index 2cb532aa33f0..d890e6fcdbae 100644
--- a/arch/arm/boot/dts/lan966x-pcb8291.dts
+++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
@@ -4,6 +4,7 @@
  */
 /dts-v1/;
 #include "lan966x.dtsi"
+#include "dt-bindings/phy/phy-lan966x-serdes.h"
 
 / {
 	model = "Microchip EVB - LAN9662";
@@ -32,6 +33,40 @@ fc3_b_pins: fc3-b-pins {
 	};
 };
 
+&mdio1 {
+	status = "okay";
+};
+
+&phy0 {
+	status = "okay";
+};
+
+&phy1 {
+	status = "okay";
+};
+
+&switch {
+	status = "okay";
+};
+
+&serdes {
+	status = "okay";
+};
+
+&port0 {
+	status = "okay";
+	phy-handle = <&phy0>;
+	phy-mode = "gmii";
+	phys = <&serdes 0 CU(0)>;
+};
+
+&port1 {
+	status = "okay";
+	phy-handle = <&phy1>;
+	phy-mode = "gmii";
+	phys = <&serdes 1 CU(1)>;
+};
+
 &flx3 {
 	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
 	status = "okay";
-- 
2.33.0

Re: [PATCH 3/3] ARM: dts: lan966x: Enable network driver on pcb8291

[Claudiu.Beznea]

On 19.07.2022 00:29, Horatiu Vultur wrote:
> The pcb8291 has 2 ports that are connected to the internal ports of the
> switch. Enable them in DT.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  arch/arm/boot/dts/lan966x-pcb8291.dts | 35 +++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> index 2cb532aa33f0..d890e6fcdbae 100644
> --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> @@ -4,6 +4,7 @@
>   */
>  /dts-v1/;
>  #include "lan966x.dtsi"
> +#include "dt-bindings/phy/phy-lan966x-serdes.h"
>  
>  / {
>  	model = "Microchip EVB - LAN9662";
> @@ -32,6 +33,40 @@ fc3_b_pins: fc3-b-pins {
>  	};
>  };
>  
> +&mdio1 {
> +	status = "okay";
> +};
> +
> +&phy0 {
> +	status = "okay";
> +};
> +
> +&phy1 {
> +	status = "okay";
> +};
> +
> +&switch {
> +	status = "okay";
> +};
> +
> +&serdes {
> +	status = "okay";
> +};
> +
> +&port0 {
> +	status = "okay";
> +	phy-handle = <&phy0>;
> +	phy-mode = "gmii";
> +	phys = <&serdes 0 CU(0)>;
> +};
> +
> +&port1 {
> +	status = "okay";
> +	phy-handle = <&phy1>;
> +	phy-mode = "gmii";
> +	phys = <&serdes 1 CU(1)>;
> +};
> +

Although gpio node is not places where it should be we tend to keep all the
nodes sorted alphabetically. Could you place follow this rule for these new
nodes?

Thank you,
Claudiu Beznea

>  &flx3 {
>  	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>  	status = "okay";

Re: [PATCH 3/3] ARM: dts: lan966x: Enable network driver on pcb8291

[Horatiu Vultur - M31836]

The 07/19/2022 07:58, Claudiu Beznea - M18063 wrote:
> On 19.07.2022 00:29, Horatiu Vultur wrote:
> > The pcb8291 has 2 ports that are connected to the internal ports of the
> > switch. Enable them in DT.
> > 
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  arch/arm/boot/dts/lan966x-pcb8291.dts | 35 +++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/lan966x-pcb8291.dts b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > index 2cb532aa33f0..d890e6fcdbae 100644
> > --- a/arch/arm/boot/dts/lan966x-pcb8291.dts
> > +++ b/arch/arm/boot/dts/lan966x-pcb8291.dts
> > @@ -4,6 +4,7 @@
> >   */
> >  /dts-v1/;
> >  #include "lan966x.dtsi"
> > +#include "dt-bindings/phy/phy-lan966x-serdes.h"
> >  
> >  / {
> >  	model = "Microchip EVB - LAN9662";
> > @@ -32,6 +33,40 @@ fc3_b_pins: fc3-b-pins {
> >  	};
> >  };
> >  
> > +&mdio1 {
> > +	status = "okay";
> > +};
> > +
> > +&phy0 {
> > +	status = "okay";
> > +};
> > +
> > +&phy1 {
> > +	status = "okay";
> > +};
> > +
> > +&switch {
> > +	status = "okay";
> > +};
> > +
> > +&serdes {
> > +	status = "okay";
> > +};
> > +
> > +&port0 {
> > +	status = "okay";
> > +	phy-handle = <&phy0>;
> > +	phy-mode = "gmii";
> > +	phys = <&serdes 0 CU(0)>;
> > +};
> > +
> > +&port1 {
> > +	status = "okay";
> > +	phy-handle = <&phy1>;
> > +	phy-mode = "gmii";
> > +	phys = <&serdes 1 CU(1)>;
> > +};
> > +
> 
> Although gpio node is not places where it should be we tend to keep all the
> nodes sorted alphabetically. Could you place follow this rule for these new
> nodes?

Yes, I will update in the next version.

> 
> Thank you,
> Claudiu Beznea
> 
> >  &flx3 {
> >  	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
> >  	status = "okay";
> 

-- 
/Horatiu