[PATCH 0/2] ARM: add support for pcb8309

[PATCH 0/2] ARM: add support for pcb8309

[Horatiu Vultur]

Add documentation and add the pcb8309.
It features a LAN9662 SoC with 2 internal copport ports and two SFP cages.

Horatiu Vultur (2):
  dt-bindings: arm: at91: add lan966 pcb8309 board
  ARM: dts: lan966x: add support for pcb8309

 .../devicetree/bindings/arm/atmel-at91.yaml   |   6 +
 arch/arm/boot/dts/Makefile                    |   3 +-
 arch/arm/boot/dts/lan966x-pcb8309.dts         | 189 ++++++++++++++++++
 3 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/lan966x-pcb8309.dts

-- 
2.33.0

[PATCH 1/2] dt-bindings: arm: at91: add lan966 pcb8309 board

[Horatiu Vultur]

Add documentation for Microchip LAN9662 PCB8309.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 Documentation/devicetree/bindings/arm/atmel-at91.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.yaml b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
index 4e495e03264b..9dc9ad81193a 100644
--- a/Documentation/devicetree/bindings/arm/atmel-at91.yaml
+++ b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
@@ -169,6 +169,12 @@ properties:
           - const: microchip,lan9662
           - const: microchip,lan966
 
+      - description: Microchip LAN9662 PCB8309 Evaluation Board.
+        items:
+          - const: microchip,lan9662-pcb8309
+          - const: microchip,lan9662
+          - const: microchip,lan966
+
       - description: Microchip LAN9668 PCB8290 Evaluation Board.
         items:
           - const: microchip,lan9668-pcb8290
-- 
2.33.0

[PATCH 2/2] ARM: dts: lan966x: add support for pcb8309

[Horatiu Vultur]

Add basic support for pcb8309. It is similar with pcb8291 with one big
difference that is having 2 SFP cages. Therefore it has 4 network ports.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 arch/arm/boot/dts/Makefile            |   3 +-
 arch/arm/boot/dts/lan966x-pcb8309.dts | 189 ++++++++++++++++++++++++++
 2 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/lan966x-pcb8309.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 184899808ee7..6a6166e3a405 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -772,7 +772,8 @@ dtb-$(CONFIG_SOC_IMXRT) += \
 dtb-$(CONFIG_SOC_LAN966) += \
 	lan966x-pcb8291.dtb \
 	lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb \
-	lan966x-kontron-kswitch-d10-mmt-8g.dtb
+	lan966x-kontron-kswitch-d10-mmt-8g.dtb \
+	lan966x-pcb8309.dtb
 dtb-$(CONFIG_SOC_LS1021A) += \
 	ls1021a-iot.dtb \
 	ls1021a-moxa-uc-8410a.dtb \
diff --git a/arch/arm/boot/dts/lan966x-pcb8309.dts b/arch/arm/boot/dts/lan966x-pcb8309.dts
new file mode 100644
index 000000000000..ef441195e8c1
--- /dev/null
+++ b/arch/arm/boot/dts/lan966x-pcb8309.dts
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * lan966x_pcb8309.dts - Device Tree file for PCB8309
+ */
+/dts-v1/;
+#include "lan966x.dtsi"
+#include "dt-bindings/phy/phy-lan966x-serdes.h"
+
+/ {
+	model = "Microchip EVB - LAN9662";
+	compatible = "microchip,lan9662-pcb8309", "microchip,lan9662", "microchip,lan966";
+
+	aliases {
+		serial0 = &usart3;
+		i2c102 = &i2c102;
+		i2c103 = &i2c103;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-restart {
+		compatible = "gpio-restart";
+		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
+		priority = <200>;
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		mux-controls = <&mux>;
+		i2c-parent = <&i2c4>;
+
+		i2c102: i2c-sfp@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c103: i2c-sfp@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&sgpio_out 11 0 GPIO_ACTIVE_HIGH>, /* p11b0 */
+			    <&sgpio_out 11 1 GPIO_ACTIVE_HIGH>; /* p11b1 */
+	};
+
+	sfp2: sfp2 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c102>;
+		tx-disable-gpios = <&sgpio_out 10 0 GPIO_ACTIVE_LOW>;
+		los-gpios = <&sgpio_in  2 0 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sgpio_in  2 1 GPIO_ACTIVE_LOW>;
+		tx-fault-gpios = <&sgpio_in  1 0 GPIO_ACTIVE_HIGH>;
+	};
+
+	sfp3: sfp3 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c103>;
+		tx-disable-gpios = <&sgpio_out 10 1 GPIO_ACTIVE_LOW>;
+		los-gpios = <&sgpio_in  3 0 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sgpio_in  3 1 GPIO_ACTIVE_LOW>;
+		tx-fault-gpios = <&sgpio_in  1 1 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&flx3 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
+	status = "okay";
+
+	usart3: serial@200 {
+		pinctrl-0 = <&fc3_b_pins>;
+		pinctrl-names = "default";
+		status = "okay";
+	};
+};
+
+&flx4 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
+	status = "okay";
+};
+
+&gpio {
+	fc3_b_pins: fc3-b-pins {
+		/* RXD, TXD */
+		pins = "GPIO_52", "GPIO_53";
+		function = "fc3_b";
+	};
+
+	fc4_b_pins: fc4-b-pins {
+		/* SCL, SDA */
+		pins = "GPIO_57", "GPIO_58";
+		function = "fc4_b";
+	};
+
+	sgpio_a_pins: sgpio-a-pins {
+		/* SCK, D0, D1, LD */
+		pins = "GPIO_32", "GPIO_33", "GPIO_34", "GPIO_35";
+		function = "sgpio_a";
+	};
+};
+
+&i2c4 {
+	compatible = "microchip,sam9x60-i2c";
+	reg = <0x600 0x200>;
+	interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clocks = <&nic_clk>;
+	pinctrl-0 = <&fc4_b_pins>;
+	pinctrl-names = "default";
+	i2c-analog-filter;
+	i2c-digital-filter;
+	i2c-digital-filter-width-ns = <35>;
+	i2c-sda-hold-time-ns = <1500>;
+	status = "okay";
+};
+
+&mdio1 {
+	status = "okay";
+};
+
+&phy0 {
+	status = "okay";
+};
+
+&phy1 {
+	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)>;
+};
+
+&port2 {
+	status = "okay";
+	sfp = <&sfp2>;
+	managed = "in-band-status";
+	phy-mode = "sgmii";
+	phys = <&serdes 2 SERDES6G(0)>;
+};
+
+&port3 {
+	status = "okay";
+	sfp = <&sfp3>;
+	managed = "in-band-status";
+	phy-mode = "sgmii";
+	phys = <&serdes 3 SERDES6G(1)>;
+};
+
+&serdes {
+	status = "okay";
+};
+
+&sgpio {
+	status = "okay";
+	pinctrl-0 = <&sgpio_a_pins>;
+	pinctrl-names = "default";
+	microchip,sgpio-port-ranges = <0 3>, <8 11>;
+	gpio@0 {
+		ngpios = <64>;
+	};
+	gpio@1 {
+		ngpios = <64>;
+	};
+};
+
+&switch {
+	status = "okay";
+};
-- 
2.33.0

RE: [PATCH 2/2] ARM: dts: lan966x: add support for pcb8309

[Kavyasree.Kotagiri]

> Add basic support for pcb8309. It is similar with pcb8291 with one big
> difference that is having 2 SFP cages. Therefore it has 4 network ports.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  arch/arm/boot/dts/Makefile            |   3 +-
>  arch/arm/boot/dts/lan966x-pcb8309.dts | 189
> ++++++++++++++++++++++++++
>  2 files changed, 191 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/lan966x-pcb8309.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 184899808ee7..6a6166e3a405 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -772,7 +772,8 @@ dtb-$(CONFIG_SOC_IMXRT) += \
>  dtb-$(CONFIG_SOC_LAN966) += \
>  	lan966x-pcb8291.dtb \
>  	lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb \
> -	lan966x-kontron-kswitch-d10-mmt-8g.dtb
> +	lan966x-kontron-kswitch-d10-mmt-8g.dtb \
> +	lan966x-pcb8309.dtb
>  dtb-$(CONFIG_SOC_LS1021A) += \
>  	ls1021a-iot.dtb \
>  	ls1021a-moxa-uc-8410a.dtb \
> diff --git a/arch/arm/boot/dts/lan966x-pcb8309.dts
> b/arch/arm/boot/dts/lan966x-pcb8309.dts
> new file mode 100644
> index 000000000000..ef441195e8c1
> --- /dev/null
> +++ b/arch/arm/boot/dts/lan966x-pcb8309.dts
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * lan966x_pcb8309.dts - Device Tree file for PCB8309
> + */
> +/dts-v1/;
> +#include "lan966x.dtsi"
> +#include "dt-bindings/phy/phy-lan966x-serdes.h"
> +
> +/ {
> +	model = "Microchip EVB - LAN9662";
> +	compatible = "microchip,lan9662-pcb8309", "microchip,lan9662",
> "microchip,lan966";
> +
> +	aliases {
> +		serial0 = &usart3;
> +		i2c102 = &i2c102;
> +		i2c103 = &i2c103;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	gpio-restart {
> +		compatible = "gpio-restart";
> +		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
> +		priority = <200>;
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		mux-controls = <&mux>;
> +		i2c-parent = <&i2c4>;
> +
> +		i2c102: i2c-sfp@1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;

Why do you need #address-cells and #size-cells here?
> +		};
> +
> +		i2c103: i2c-sfp@2 {
> +			reg = <2>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
Same here
> +		};
> +	};
> +
> +	mux: mux-controller {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&sgpio_out 11 0 GPIO_ACTIVE_HIGH>, /*
> p11b0 */
> +			    <&sgpio_out 11 1 GPIO_ACTIVE_HIGH>; /* p11b1
> */
> +	};
> +
> +	sfp2: sfp2 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c102>;
> +		tx-disable-gpios = <&sgpio_out 10 0 GPIO_ACTIVE_LOW>;
> +		los-gpios = <&sgpio_in  2 0 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&sgpio_in  2 1 GPIO_ACTIVE_LOW>;
> +		tx-fault-gpios = <&sgpio_in  1 0 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	sfp3: sfp3 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c103>;
> +		tx-disable-gpios = <&sgpio_out 10 1 GPIO_ACTIVE_LOW>;
> +		los-gpios = <&sgpio_in  3 0 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&sgpio_in  3 1 GPIO_ACTIVE_LOW>;
> +		tx-fault-gpios = <&sgpio_in  1 1 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&flx3 {
> +	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
> +	status = "okay";
> +
> +	usart3: serial@200 {
> +		pinctrl-0 = <&fc3_b_pins>;
> +		pinctrl-names = "default";
> +		status = "okay";
> +	};
> +};
> +
> +&flx4 {
> +	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
> +	status = "okay";
> +};
> +
> +&gpio {
> +	fc3_b_pins: fc3-b-pins {
> +		/* RXD, TXD */
> +		pins = "GPIO_52", "GPIO_53";
> +		function = "fc3_b";
> +	};
> +
> +	fc4_b_pins: fc4-b-pins {
> +		/* SCL, SDA */
> +		pins = "GPIO_57", "GPIO_58";
> +		function = "fc4_b";
> +	};
> +
> +	sgpio_a_pins: sgpio-a-pins {
> +		/* SCK, D0, D1, LD */
> +		pins = "GPIO_32", "GPIO_33", "GPIO_34", "GPIO_35";
> +		function = "sgpio_a";
> +	};
> +};
> +
> +&i2c4 {
> +	compatible = "microchip,sam9x60-i2c";
> +	reg = <0x600 0x200>;
> +	interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;

Same here 

> +	clocks = <&nic_clk>;
> +	pinctrl-0 = <&fc4_b_pins>;
> +	pinctrl-names = "default";
> +	i2c-analog-filter;
> +	i2c-digital-filter;
> +	i2c-digital-filter-width-ns = <35>;
> +	i2c-sda-hold-time-ns = <1500>;
> +	status = "okay";
> +};
> +
> +&mdio1 {
> +	status = "okay";
> +};
> +
> +&phy0 {
> +	status = "okay";
> +};
> +
> +&phy1 {
> +	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)>;
> +};
> +
> +&port2 {
> +	status = "okay";
> +	sfp = <&sfp2>;
> +	managed = "in-band-status";
> +	phy-mode = "sgmii";
> +	phys = <&serdes 2 SERDES6G(0)>;
> +};
> +
> +&port3 {
> +	status = "okay";
> +	sfp = <&sfp3>;
> +	managed = "in-band-status";
> +	phy-mode = "sgmii";
> +	phys = <&serdes 3 SERDES6G(1)>;
> +};
> +
> +&serdes {
> +	status = "okay";
> +};
> +
> +&sgpio {
> +	status = "okay";
> +	pinctrl-0 = <&sgpio_a_pins>;
> +	pinctrl-names = "default";
> +	microchip,sgpio-port-ranges = <0 3>, <8 11>;
> +	gpio@0 {
> +		ngpios = <64>;
> +	};
> +	gpio@1 {
> +		ngpios = <64>;
> +	};
> +};
> +
> +&switch {
> +	status = "okay";
> +};
> --
> 2.33.0

Re: [PATCH 1/2] dt-bindings: arm: at91: add lan966 pcb8309 board

[Krzysztof Kozlowski]

On 20/07/2022 21:49, Horatiu Vultur wrote:
> Add documentation for Microchip LAN9662 PCB8309.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  Documentation/devicetree/bindings/arm/atmel-at91.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.yaml b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> index 4e495e03264b..9dc9ad81193a 100644
> --- a/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> +++ b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> @@ -169,6 +169,12 @@ properties:
>            - const: microchip,lan9662
>            - const: microchip,lan966
>  
> +      - description: Microchip LAN9662 PCB8309 Evaluation Board.
> +        items:
> +          - const: microchip,lan9662-pcb8309

This and other lan9662 boards should be just an enum. You grow the file
needlessly... Unless it is clear preference of maintainer. Other boards
follow normal enum approach, so it seems there is no such preference.


Best regards,
Krzysztof

Re: [PATCH 2/2] ARM: dts: lan966x: add support for pcb8309

[Claudiu.Beznea]

On 20.07.2022 22:49, Horatiu Vultur wrote:
> Add basic support for pcb8309. It is similar with pcb8291 with one big
> difference that is having 2 SFP cages. Therefore it has 4 network ports.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  arch/arm/boot/dts/Makefile            |   3 +-
>  arch/arm/boot/dts/lan966x-pcb8309.dts | 189 ++++++++++++++++++++++++++
>  2 files changed, 191 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/lan966x-pcb8309.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 184899808ee7..6a6166e3a405 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -772,7 +772,8 @@ dtb-$(CONFIG_SOC_IMXRT) += \
>  dtb-$(CONFIG_SOC_LAN966) += \
>  	lan966x-pcb8291.dtb \
>  	lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb \
> -	lan966x-kontron-kswitch-d10-mmt-8g.dtb
> +	lan966x-kontron-kswitch-d10-mmt-8g.dtb \
> +	lan966x-pcb8309.dtb
>  dtb-$(CONFIG_SOC_LS1021A) += \
>  	ls1021a-iot.dtb \
>  	ls1021a-moxa-uc-8410a.dtb \
> diff --git a/arch/arm/boot/dts/lan966x-pcb8309.dts b/arch/arm/boot/dts/lan966x-pcb8309.dts
> new file mode 100644
> index 000000000000..ef441195e8c1
> --- /dev/null
> +++ b/arch/arm/boot/dts/lan966x-pcb8309.dts
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * lan966x_pcb8309.dts - Device Tree file for PCB8309
> + */
> +/dts-v1/;
> +#include "lan966x.dtsi"
> +#include "dt-bindings/phy/phy-lan966x-serdes.h"
> +
> +/ {
> +	model = "Microchip EVB - LAN9662";
> +	compatible = "microchip,lan9662-pcb8309", "microchip,lan9662", "microchip,lan966";
> +
> +	aliases {
> +		serial0 = &usart3;
> +		i2c102 = &i2c102;
> +		i2c103 = &i2c103;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	gpio-restart {
> +		compatible = "gpio-restart";
> +		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
> +		priority = <200>;
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		mux-controls = <&mux>;
> +		i2c-parent = <&i2c4>;
> +
> +		i2c102: i2c-sfp@1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c103: i2c-sfp@2 {
> +			reg = <2>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +	};
> +
> +	mux: mux-controller {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&sgpio_out 11 0 GPIO_ACTIVE_HIGH>, /* p11b0 */
> +			    <&sgpio_out 11 1 GPIO_ACTIVE_HIGH>; /* p11b1 */
> +	};
> +
> +	sfp2: sfp2 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c102>;
> +		tx-disable-gpios = <&sgpio_out 10 0 GPIO_ACTIVE_LOW>;
> +		los-gpios = <&sgpio_in  2 0 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&sgpio_in  2 1 GPIO_ACTIVE_LOW>;
> +		tx-fault-gpios = <&sgpio_in  1 0 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	sfp3: sfp3 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c103>;
> +		tx-disable-gpios = <&sgpio_out 10 1 GPIO_ACTIVE_LOW>;
> +		los-gpios = <&sgpio_in  3 0 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&sgpio_in  3 1 GPIO_ACTIVE_LOW>;
> +		tx-fault-gpios = <&sgpio_in  1 1 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&flx3 {
> +	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
> +	status = "okay";
> +
> +	usart3: serial@200 {
> +		pinctrl-0 = <&fc3_b_pins>;
> +		pinctrl-names = "default";
> +		status = "okay";
> +	};
> +};
> +
> +&flx4 {
> +	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
> +	status = "okay";

Embed i2c4 node here as you did for usart3 above. It's easy to follow the
connection b/w flx node and it's enabled child.

> +};
> +
> +&gpio {
> +	fc3_b_pins: fc3-b-pins {
> +		/* RXD, TXD */
> +		pins = "GPIO_52", "GPIO_53";
> +		function = "fc3_b";
> +	};
> +
> +	fc4_b_pins: fc4-b-pins {
> +		/* SCL, SDA */
> +		pins = "GPIO_57", "GPIO_58";
> +		function = "fc4_b";
> +	};
> +
> +	sgpio_a_pins: sgpio-a-pins {
> +		/* SCK, D0, D1, LD */
> +		pins = "GPIO_32", "GPIO_33", "GPIO_34", "GPIO_35";
> +		function = "sgpio_a";
> +	};
> +};
> +
> +&i2c4 {
> +	compatible = "microchip,sam9x60-i2c";
> +	reg = <0x600 0x200>;
> +	interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	clocks = <&nic_clk>;
> +	pinctrl-0 = <&fc4_b_pins>;
> +	pinctrl-names = "default";
> +	i2c-analog-filter;
> +	i2c-digital-filter;
> +	i2c-digital-filter-width-ns = <35>;
> +	i2c-sda-hold-time-ns = <1500>;
> +	status = "okay";
> +};
> +
> +&mdio1 {
> +	status = "okay";
> +};
> +
> +&phy0 {
> +	status = "okay";
> +};
> +
> +&phy1 {
> +	status = "okay";
> +};
> +
> +&port0 {
> +	status = "okay";

Keep status at the end of node description for uniformity with the nodes
enabled above. Same for the rest of nodes below.

> +	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)>;
> +};
> +
> +&port2 {
> +	status = "okay";
> +	sfp = <&sfp2>;
> +	managed = "in-band-status";
> +	phy-mode = "sgmii";
> +	phys = <&serdes 2 SERDES6G(0)>;
> +};
> +
> +&port3 {
> +	status = "okay";
> +	sfp = <&sfp3>;
> +	managed = "in-band-status";
> +	phy-mode = "sgmii";
> +	phys = <&serdes 3 SERDES6G(1)>;
> +};
> +
> +&serdes {
> +	status = "okay";
> +};
> +
> +&sgpio {
> +	status = "okay";
> +	pinctrl-0 = <&sgpio_a_pins>;
> +	pinctrl-names = "default";
> +	microchip,sgpio-port-ranges = <0 3>, <8 11>;

Except this one. For this would be nice to have status here before
describing childs.

> +	gpio@0 {
> +		ngpios = <64>;
> +	};
> +	gpio@1 {
> +		ngpios = <64>;
> +	};
> +};
> +
> +&switch {
> +	status = "okay";
> +};

Re: [PATCH 2/2] ARM: dts: lan966x: add support for pcb8309

[Horatiu Vultur - M31836]

The 07/21/2022 06:08, Kavyasree Kotagiri - I30978 wrote:
> > +	i2c-mux {
> > +		compatible = "i2c-mux";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		mux-controls = <&mux>;
> > +		i2c-parent = <&i2c4>;
> > +
> > +		i2c102: i2c-sfp@1 {
> > +			reg = <1>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> 
> Why do you need #address-cells and #size-cells here?
> > +		};
> > +
> > +		i2c103: i2c-sfp@2 {
> > +			reg = <2>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> Same here
> > +		};
> > +	};

They are not needed. I will remove them in the next version.
> > +
> > +	mux: mux-controller {
> > +		compatible = "gpio-mux";
> > +		#mux-control-cells = <0>;
> > +
> > +		mux-gpios = <&sgpio_out 11 0 GPIO_ACTIVE_HIGH>, /*
> > p11b0 */
> > +			    <&sgpio_out 11 1 GPIO_ACTIVE_HIGH>; /* p11b1
> > */
> > +	};

...

> > +&i2c4 {
> > +	compatible = "microchip,sam9x60-i2c";
> > +	reg = <0x600 0x200>;
> > +	interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> 
> Same here 
> 

Also here.

-- 
/Horatiu

Re: [PATCH 2/2] ARM: dts: lan966x: add support for pcb8309

[Horatiu Vultur - M31836]

The 07/21/2022 07:39, Claudiu Beznea - M18063 wrote:
> > +&flx4 {
> > +	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
> > +	status = "okay";
> 
> Embed i2c4 node here as you did for usart3 above. It's easy to follow the
> connection b/w flx node and it's enabled child.

Yes, I will do that.

> 
> > +};
> > +
> > +&gpio {
> > +	fc3_b_pins: fc3-b-pins {
> > +		/* RXD, TXD */
> > +		pins = "GPIO_52", "GPIO_53";
> > +		function = "fc3_b";
> > +	};
> > +
> > +	fc4_b_pins: fc4-b-pins {
> > +		/* SCL, SDA */
> > +		pins = "GPIO_57", "GPIO_58";
> > +		function = "fc4_b";
> > +	};
> > +
> > +	sgpio_a_pins: sgpio-a-pins {
> > +		/* SCK, D0, D1, LD */
> > +		pins = "GPIO_32", "GPIO_33", "GPIO_34", "GPIO_35";
> > +		function = "sgpio_a";
> > +	};
> > +};
> > +
> > +&i2c4 {
> > +	compatible = "microchip,sam9x60-i2c";
> > +	reg = <0x600 0x200>;
> > +	interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	clocks = <&nic_clk>;
> > +	pinctrl-0 = <&fc4_b_pins>;
> > +	pinctrl-names = "default";
> > +	i2c-analog-filter;
> > +	i2c-digital-filter;
> > +	i2c-digital-filter-width-ns = <35>;
> > +	i2c-sda-hold-time-ns = <1500>;
> > +	status = "okay";
> > +};
> > +
> > +&mdio1 {
> > +	status = "okay";
> > +};
> > +
> > +&phy0 {
> > +	status = "okay";
> > +};
> > +
> > +&phy1 {
> > +	status = "okay";
> > +};
> > +
> > +&port0 {
> > +	status = "okay";
> 
> Keep status at the end of node description for uniformity with the nodes
> enabled above. Same for the rest of nodes below.

OK. I will do that in the next version.

> 
> > +	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)>;
> > +};
> > +
> > +&port2 {
> > +	status = "okay";
> > +	sfp = <&sfp2>;
> > +	managed = "in-band-status";
> > +	phy-mode = "sgmii";
> > +	phys = <&serdes 2 SERDES6G(0)>;
> > +};
> > +
> > +&port3 {
> > +	status = "okay";
> > +	sfp = <&sfp3>;
> > +	managed = "in-band-status";
> > +	phy-mode = "sgmii";
> > +	phys = <&serdes 3 SERDES6G(1)>;
> > +};
> > +
> > +&serdes {
> > +	status = "okay";
> > +};
> > +
> > +&sgpio {
> > +	status = "okay";
> > +	pinctrl-0 = <&sgpio_a_pins>;
> > +	pinctrl-names = "default";
> > +	microchip,sgpio-port-ranges = <0 3>, <8 11>;
> 
> Except this one. For this would be nice to have status here before
> describing childs.

OK. I will do that in the next version.

> 
> > +	gpio@0 {
> > +		ngpios = <64>;
> > +	};
> > +	gpio@1 {
> > +		ngpios = <64>;
> > +	};
> > +};
> > +
> > +&switch {
> > +	status = "okay";
> > +};
> 

-- 
/Horatiu

Re: [PATCH 1/2] dt-bindings: arm: at91: add lan966 pcb8309 board

[Horatiu Vultur]

The 07/21/2022 08:46, Krzysztof Kozlowski wrote:
> 
> On 20/07/2022 21:49, Horatiu Vultur wrote:
> > Add documentation for Microchip LAN9662 PCB8309.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  Documentation/devicetree/bindings/arm/atmel-at91.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.yaml b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> > index 4e495e03264b..9dc9ad81193a 100644
> > --- a/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> > +++ b/Documentation/devicetree/bindings/arm/atmel-at91.yaml
> > @@ -169,6 +169,12 @@ properties:
> >            - const: microchip,lan9662
> >            - const: microchip,lan966
> >
> > +      - description: Microchip LAN9662 PCB8309 Evaluation Board.
> > +        items:
> > +          - const: microchip,lan9662-pcb8309
> 
> This and other lan9662 boards should be just an enum. You grow the file
> needlessly... Unless it is clear preference of maintainer. Other boards
> follow normal enum approach, so it seems there is no such preference.

I can see your point. I will change it in the next version.

> 
> 
> Best regards,
> Krzysztof

-- 
/Horatiu