linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support
@ 2022-03-03 16:03 Michael Walle
  2022-03-03 16:03 ` [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node Michael Walle
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add missing nodes for the flexcom blocks and a node for the SGPIO
block. Then add basic support for the Kontron KSwitch D10.

Microchip, please take a closer look at the compatible strings of
the newly added nodes.

Michael Walle (6):
  ARM: dts: lan966x: swap dma channels for crypto node
  ARM: dts: lan966x: add sgpio node
  ARM: dts: lan966x: add all flexcom usart nodes
  ARM: dts: lan966x: add flexcom SPI nodes
  ARM: dts: lan966x: add flexcom I2C nodes
  ARM: dts: lan966x: add basic Kontron KSwitch D10 support

 arch/arm/boot/dts/Makefile                    |   3 +-
 ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159 ++++++++++++
 arch/arm/boot/dts/lan966x.dtsi                | 228 +++++++++++++++++-
 3 files changed, 386 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts

-- 
2.30.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-04  8:21   ` Claudiu.Beznea
  2022-03-03 16:03 ` [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node Michael Walle
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

The YAML binding (crypto/atmel,at91sam9g46-aes.yaml) mandates the order
of the channels. Swap them to pass devicetree validation.

Fixes: 290deaa10c50 ("ARM: dts: add DT for lan966 SoC and 2-port board pcb8291")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index 7d2869648050..5e9cbc8cdcbc 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -114,9 +114,9 @@ aes: crypto@e004c000 {
 			compatible = "atmel,at91sam9g46-aes";
 			reg = <0xe004c000 0x100>;
 			interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
-			dmas = <&dma0 AT91_XDMAC_DT_PERID(13)>,
-			       <&dma0 AT91_XDMAC_DT_PERID(12)>;
-			dma-names = "rx", "tx";
+			dmas = <&dma0 AT91_XDMAC_DT_PERID(12)>,
+			       <&dma0 AT91_XDMAC_DT_PERID(13)>;
+			dma-names = "tx", "rx";
 			clocks = <&nic_clk>;
 			clock-names = "aes_clk";
 		};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
  2022-03-03 16:03 ` [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-04  8:24   ` Claudiu.Beznea
  2022-03-03 16:03 ` [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes Michael Walle
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add the device tree node for the SGPIO IP block reused from the
SparX-5. Keep the node disabled by default.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index 5e9cbc8cdcbc..a7d46a2ca058 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -223,6 +223,32 @@ gpio: pinctrl@e2004064 {
 			#interrupt-cells = <2>;
 		};
 
+		sgpio: gpio@e2004190 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "microchip,sparx5-sgpio";
+			clocks = <&sys_clk>;
+			reg = <0xe2004190 0x118>;
+			status = "disabled";
+
+			sgpio_in: gpio@0 {
+				reg = <0>;
+				compatible = "microchip,sparx5-sgpio-bank";
+				gpio-controller;
+				#gpio-cells = <3>;
+				interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-controller;
+				#interrupt-cells = <3>;
+			};
+
+			sgpio_out: gpio@1 {
+				compatible = "microchip,sparx5-sgpio-bank";
+				reg = <1>;
+				gpio-controller;
+				#gpio-cells = <3>;
+			};
+		};
+
 		gic: interrupt-controller@e8c11000 {
 			compatible = "arm,gic-400", "arm,cortex-a7-gic";
 			#interrupt-cells = <3>;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
  2022-03-03 16:03 ` [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node Michael Walle
  2022-03-03 16:03 ` [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-04  8:30   ` Claudiu.Beznea
  2022-03-03 16:03 ` [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes Michael Walle
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add all the usart nodes for the flexcom block. There was already
an usart node for the flexcom3 block. But it missed the DMA
channels. Although the DMA channels are specified, DMA is not
enabled by default because break detection doesn't work with DMA.

Keep the nodes disabled by default.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 55 ++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index a7d46a2ca058..bea69b6d2749 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
 			#size-cells = <1>;
 			ranges = <0x0 0xe0040000 0x800>;
 			status = "disabled";
+
+			usart0: serial@200 {
+				compatible = "atmel,at91sam9260-usart";
+				reg = <0x200 0x200>;
+				interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(2)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "usart";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		flx1: flexcom@e0044000 {
@@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
 			#size-cells = <1>;
 			ranges = <0x0 0xe0044000 0x800>;
 			status = "disabled";
+
+			usart1: serial@200 {
+				compatible = "atmel,at91sam9260-usart";
+				reg = <0x200 0x200>;
+				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(4)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "usart";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		trng: rng@e0048000 {
@@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
 			#size-cells = <1>;
 			ranges = <0x0 0xe0060000 0x800>;
 			status = "disabled";
+
+			usart2: serial@200 {
+				compatible = "atmel,at91sam9260-usart";
+				reg = <0x200 0x200>;
+				interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(6)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "usart";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		flx3: flexcom@e0064000 {
@@ -144,6 +183,9 @@ usart3: serial@200 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0x200 0x200>;
 				interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(8)>;
+				dma-names = "tx", "rx";
 				clocks = <&nic_clk>;
 				clock-names = "usart";
 				atmel,fifo-size = <32>;
@@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
 			#size-cells = <1>;
 			ranges = <0x0 0xe0070000 0x800>;
 			status = "disabled";
+
+			usart4: serial@200 {
+				compatible = "atmel,at91sam9260-usart";
+				reg = <0x200 0x200>;
+				interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(10)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "usart";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		timer0: timer@e008c000 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
                   ` (2 preceding siblings ...)
  2022-03-03 16:03 ` [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-04  8:30   ` Claudiu.Beznea
  2022-03-03 16:03 ` [PATCH v1 5/6] ARM: dts: lan966x: add flexcom I2C nodes Michael Walle
  2022-03-03 16:03 ` [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support Michael Walle
  5 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add all the SPI nodes for the flexcom IP block. Keep them
disabled by default.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 75 ++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index bea69b6d2749..0616927f1bb1 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -105,6 +105,21 @@ usart0: serial@200 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			spi0: spi@400 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "atmel,at91rm9200-spi";
+				reg = <0x400 0x200>;
+				interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(2)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "spi_clk";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		flx1: flexcom@e0044000 {
@@ -128,6 +143,21 @@ usart1: serial@200 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			spi1: spi@400 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "atmel,at91rm9200-spi";
+				reg = <0x400 0x200>;
+				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(4)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "spi_clk";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		trng: rng@e0048000 {
@@ -168,6 +198,21 @@ usart2: serial@200 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			spi2: spi@400 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "atmel,at91rm9200-spi";
+				reg = <0x400 0x200>;
+				interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(6)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "spi_clk";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		flx3: flexcom@e0064000 {
@@ -191,6 +236,21 @@ usart3: serial@200 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			spi3: spi@400 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "atmel,at91rm9200-spi";
+				reg = <0x400 0x200>;
+				interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(8)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "spi_clk";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		dma0: dma-controller@e0068000 {
@@ -233,6 +293,21 @@ usart4: serial@200 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			spi4: spi@400 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "atmel,at91rm9200-spi";
+				reg = <0x400 0x200>;
+				interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(10)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				clock-names = "spi_clk";
+				atmel,fifo-size = <32>;
+				status = "disabled";
+			};
 		};
 
 		timer0: timer@e008c000 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 5/6] ARM: dts: lan966x: add flexcom I2C nodes
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
                   ` (3 preceding siblings ...)
  2022-03-03 16:03 ` [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-03 16:03 ` [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support Michael Walle
  5 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add all I2C nodes of the flexcom IP blocks. The driver supports
FIFO, DMA or both combined. But the latter isn't working correctly.
Thus, skip the fifo-size property for now. DMA is doing single byte
reads in this case.

Keep the nodes disabled by default.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index 0616927f1bb1..c4aab7f65790 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -120,6 +120,19 @@ spi0: spi@400 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			i2c0: i2c@600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "microchip,sam9x60-i2c";
+				reg = <0x600 0x200>;
+				interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(2)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				status = "disabled";
+			};
 		};
 
 		flx1: flexcom@e0044000 {
@@ -158,6 +171,19 @@ spi1: spi@400 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			i2c1: i2c@600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "microchip,sam9x60-i2c";
+				reg = <0x600 0x200>;
+				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(4)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				status = "disabled";
+			};
 		};
 
 		trng: rng@e0048000 {
@@ -213,6 +239,19 @@ spi2: spi@400 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			i2c2: i2c@600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "microchip,sam9x60-i2c";
+				reg = <0x600 0x200>;
+				interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(6)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				status = "disabled";
+			};
 		};
 
 		flx3: flexcom@e0064000 {
@@ -251,6 +290,19 @@ spi3: spi@400 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			i2c3: i2c@600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "microchip,sam9x60-i2c";
+				reg = <0x600 0x200>;
+				interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(8)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				status = "disabled";
+			};
 		};
 
 		dma0: dma-controller@e0068000 {
@@ -308,6 +360,19 @@ spi4: spi@400 {
 				atmel,fifo-size = <32>;
 				status = "disabled";
 			};
+
+			i2c4: i2c@600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "microchip,sam9x60-i2c";
+				reg = <0x600 0x200>;
+				interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
+					   <&dma0 AT91_XDMAC_DT_PERID(10)>;
+				dma-names = "tx", "rx";
+				clocks = <&nic_clk>;
+				status = "disabled";
+			};
 		};
 
 		timer0: timer@e008c000 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
                   ` (4 preceding siblings ...)
  2022-03-03 16:03 ` [PATCH v1 5/6] ARM: dts: lan966x: add flexcom I2C nodes Michael Walle
@ 2022-03-03 16:03 ` Michael Walle
  2022-03-04  8:31   ` Claudiu.Beznea
  5 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-03 16:03 UTC (permalink / raw)
  To: Kavyasree Kotagiri, Nicolas Ferre
  Cc: Arnd Bergmann, Olof Johansson, soc, linux-arm-kernel, devicetree,
	linux-kernel, Rob Herring, Krzysztof Kozlowski,
	Alexandre Belloni, Claudiu Beznea, Michael Walle

Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
features 6 Gigabit copper ports and two SFP cages. For now the
following is working:
 - Kernel console
 - SFP cages I2C bus and mux
 - SPI
 - SGPIO
 - Watchdog

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/Makefile                    |   3 +-
 ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159 ++++++++++++++++++
 2 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 085c43649d44..86dd0f9804ee 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
 	imx7ulp-com.dtb \
 	imx7ulp-evk.dtb
 dtb-$(CONFIG_SOC_LAN966) += \
-	lan966x-pcb8291.dtb
+	lan966x-pcb8291.dtb \
+	lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
 dtb-$(CONFIG_SOC_LS1021A) += \
 	ls1021a-moxa-uc-8410a.dtb \
 	ls1021a-qds.dtb \
diff --git a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
new file mode 100644
index 000000000000..958678dec7ad
--- /dev/null
+++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
+ */
+
+/dts-v1/;
+#include "lan966x.dtsi"
+
+/ {
+	model = "Kontron KSwitch D10 MMT 6G-2GS";
+	compatible = "kontron,kswitch-d10-mmt-6g-2gs", "kontron,s1921",
+		     "microchip,lan9668", "microchip,lan966";
+
+	aliases {
+		serial0 = &usart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-restart {
+		compatible = "gpio-restart";
+		gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
+		priority = <200>;
+	};
+
+	i2cmux {
+		compatible = "i2c-mux-gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
+			    <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
+		i2c-parent = <&i2c4>;
+
+		i2c4_0: i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c4_1: i2c@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	sfp0: sfp0 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c4_0>;
+		los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
+		maximum-power-milliwatt = <2500>;
+		tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
+		tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
+		rate-select0-gpios = <&sgpio_out 2 0 GPIO_ACTIVE_HIGH>;
+		rate-select1-gpios = <&sgpio_out 2 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	sfp1: sfp1 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c4_1>;
+		los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
+		maximum-power-milliwatt = <2500>;
+		tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
+		tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
+		rate-select0-gpios = <&sgpio_out 2 2 GPIO_ACTIVE_HIGH>;
+		rate-select1-gpios = <&sgpio_out 2 3 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&flx0 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
+	status = "okay";
+};
+
+&flx3 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
+	status = "okay";
+};
+
+&flx4 {
+	atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
+	status = "okay";
+};
+
+&gpio {
+	usart0_pins: usart0-pins {
+		/* RXD, TXD */
+		pins = "GPIO_25", "GPIO_26";
+		function = "fc0_b";
+	};
+
+	sgpio_a_pins: sgpio-a-pins {
+		/* SCK, D0, D1, LD */
+		pins = "GPIO_32", "GPIO_33", "GPIO_34";
+		function = "sgpio_a";
+	};
+
+	sgpio_b_pins: sgpio-b-pins {
+		/* SCK, D0, D1, LD */
+		pins = "GPIO_64";
+		function = "sgpio_b";
+	};
+
+	fc3_b_pins: fc3-b-spi-pins {
+		/* SCK, MISO, MOSI */
+		pins = "GPIO_51", "GPIO_52", "GPIO_53";
+		function = "fc3_b";
+	};
+
+	fc4_b_pins: fc4-b-i2c-pins {
+		/* RXD, TXD */
+		pins = "GPIO_57", "GPIO_58";
+		function = "fc4_b";
+	};
+};
+
+&i2c4 {
+	pinctrl-0 = <&fc4_b_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&usart0 {
+	pinctrl-0 = <&usart0_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&sgpio {
+	pinctrl-0 = <&sgpio_a_pins>, <&sgpio_b_pins>;
+	pinctrl-names = "default";
+	bus-frequency = <8000000>;
+	/* arbitrary range because all GPIOs are in software mode */
+	microchip,sgpio-port-ranges = <0 11>;
+	status = "okay";
+};
+
+&sgpio_in {
+	ngpios = <128>;
+};
+
+&sgpio_out {
+	ngpios = <128>;
+};
+
+&spi3 {
+	pinctrl-0 = <&fc3_b_pins>;
+	pinctrl-names = "default";
+	cs-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+&watchdog {
+	status = "okay";
+};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node
  2022-03-03 16:03 ` [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node Michael Walle
@ 2022-03-04  8:21   ` Claudiu.Beznea
  0 siblings, 0 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-04  8:21 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The YAML binding (crypto/atmel,at91sam9g46-aes.yaml) mandates the order
> of the channels. Swap them to pass devicetree validation.
> 
> Fixes: 290deaa10c50 ("ARM: dts: add DT for lan966 SoC and 2-port board pcb8291")
> Signed-off-by: Michael Walle <michael@walle.cc>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

> ---
>  arch/arm/boot/dts/lan966x.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index 7d2869648050..5e9cbc8cdcbc 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -114,9 +114,9 @@ aes: crypto@e004c000 {
>                         compatible = "atmel,at91sam9g46-aes";
>                         reg = <0xe004c000 0x100>;
>                         interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> -                       dmas = <&dma0 AT91_XDMAC_DT_PERID(13)>,
> -                              <&dma0 AT91_XDMAC_DT_PERID(12)>;
> -                       dma-names = "rx", "tx";
> +                       dmas = <&dma0 AT91_XDMAC_DT_PERID(12)>,
> +                              <&dma0 AT91_XDMAC_DT_PERID(13)>;
> +                       dma-names = "tx", "rx";
>                         clocks = <&nic_clk>;
>                         clock-names = "aes_clk";
>                 };
> --
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node
  2022-03-03 16:03 ` [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node Michael Walle
@ 2022-03-04  8:24   ` Claudiu.Beznea
  0 siblings, 0 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-04  8:24 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add the device tree node for the SGPIO IP block reused from the
> SparX-5. Keep the node disabled by default.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  arch/arm/boot/dts/lan966x.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index 5e9cbc8cdcbc..a7d46a2ca058 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -223,6 +223,32 @@ gpio: pinctrl@e2004064 {
>                         #interrupt-cells = <2>;
>                 };
> 
> +               sgpio: gpio@e2004190 {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       compatible = "microchip,sparx5-sgpio";
> +                       clocks = <&sys_clk>;
> +                       reg = <0xe2004190 0x118>;
> +                       status = "disabled";
> +
> +                       sgpio_in: gpio@0 {
> +                               reg = <0>;
> +                               compatible = "microchip,sparx5-sgpio-bank";
> +                               gpio-controller;
> +                               #gpio-cells = <3>;
> +                               interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> +                               interrupt-controller;
> +                               #interrupt-cells = <3>;

Can you preserve the order or properties for this node

> +                       };
> +
> +                       sgpio_out: gpio@1 {
> +                               compatible = "microchip,sparx5-sgpio-bank";
> +                               reg = <1>;
> +                               gpio-controller;
> +                               #gpio-cells = <3>;

and this node. It would be easier to follow it.
As a note I see most of the nodes in this DT follows the order:
- compatible
- reg

For consistency with the rest of the nodes it would be good to keep the
same order here.

> +                       };
> +               };
> +
>                 gic: interrupt-controller@e8c11000 {
>                         compatible = "arm,gic-400", "arm,cortex-a7-gic";
>                         #interrupt-cells = <3>;
> --
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-03 16:03 ` [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes Michael Walle
@ 2022-03-04  8:30   ` Claudiu.Beznea
  2022-03-04 11:01     ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-04  8:30 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add all the usart nodes for the flexcom block. There was already
> an usart node for the flexcom3 block. But it missed the DMA
> channels. 

And it would be good to go though a different patch.

> Although the DMA channels are specified, DMA is not
> enabled by default because break detection doesn't work with DMA.
> 
> Keep the nodes disabled by default.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  arch/arm/boot/dts/lan966x.dtsi | 55 ++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index a7d46a2ca058..bea69b6d2749 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>                         #size-cells = <1>;
>                         ranges = <0x0 0xe0040000 0x800>;
>                         status = "disabled";
> +
> +                       usart0: serial@200 {
> +                               compatible = "atmel,at91sam9260-usart";

Are the usart blocks in lan966x 1:1 compatible with what is is sam9260? In
case not it may worth to have a new compatible here, for lan966x, such that
when new features will be implemented in usart driver for lan966x the old
DT (this one) will work with the new kernel implementation. Same for the
rest of the nodes added in this series.

> +                               reg = <0x200 0x200>;
> +                               interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(2)>;

Keep dma entries aligned.

> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "usart";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 flx1: flexcom@e0044000 {
> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
>                         #size-cells = <1>;
>                         ranges = <0x0 0xe0044000 0x800>;
>                         status = "disabled";
> +
> +                       usart1: serial@200 {
> +                               compatible = "atmel,at91sam9260-usart";
> +                               reg = <0x200 0x200>;
> +                               interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(4)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "usart";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 trng: rng@e0048000 {
> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
>                         #size-cells = <1>;
>                         ranges = <0x0 0xe0060000 0x800>;
>                         status = "disabled";
> +
> +                       usart2: serial@200 {
> +                               compatible = "atmel,at91sam9260-usart";
> +                               reg = <0x200 0x200>;
> +                               interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(6)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "usart";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 flx3: flexcom@e0064000 {
> @@ -144,6 +183,9 @@ usart3: serial@200 {
>                                 compatible = "atmel,at91sam9260-usart";
>                                 reg = <0x200 0x200>;
>                                 interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(8)>;
> +                               dma-names = "tx", "rx";
>                                 clocks = <&nic_clk>;
>                                 clock-names = "usart";
>                                 atmel,fifo-size = <32>;
> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
>                         #size-cells = <1>;
>                         ranges = <0x0 0xe0070000 0x800>;
>                         status = "disabled";
> +
> +                       usart4: serial@200 {
> +                               compatible = "atmel,at91sam9260-usart";
> +                               reg = <0x200 0x200>;
> +                               interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(10)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "usart";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 timer0: timer@e008c000 {
> --
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes
  2022-03-03 16:03 ` [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes Michael Walle
@ 2022-03-04  8:30   ` Claudiu.Beznea
  0 siblings, 0 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-04  8:30 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add all the SPI nodes for the flexcom IP block. Keep them
> disabled by default.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  arch/arm/boot/dts/lan966x.dtsi | 75 ++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index bea69b6d2749..0616927f1bb1 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -105,6 +105,21 @@ usart0: serial@200 {
>                                 atmel,fifo-size = <32>;
>                                 status = "disabled";
>                         };
> +
> +                       spi0: spi@400 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               compatible = "atmel,at91rm9200-spi";
> +                               reg = <0x400 0x200>;

try to keep the:
- compatible
- reg
first as all the other nodes in this DT. It would be simple to follow.

> +                               interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(2)>;

Align dma entries. Same for the rest of nodes added.

> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "spi_clk";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 flx1: flexcom@e0044000 {
> @@ -128,6 +143,21 @@ usart1: serial@200 {
>                                 atmel,fifo-size = <32>;
>                                 status = "disabled";
>                         };
> +
> +                       spi1: spi@400 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               compatible = "atmel,at91rm9200-spi";
> +                               reg = <0x400 0x200>;
> +                               interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(4)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "spi_clk";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 trng: rng@e0048000 {
> @@ -168,6 +198,21 @@ usart2: serial@200 {
>                                 atmel,fifo-size = <32>;
>                                 status = "disabled";
>                         };
> +
> +                       spi2: spi@400 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               compatible = "atmel,at91rm9200-spi";
> +                               reg = <0x400 0x200>;
> +                               interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(6)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "spi_clk";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 flx3: flexcom@e0064000 {
> @@ -191,6 +236,21 @@ usart3: serial@200 {
>                                 atmel,fifo-size = <32>;
>                                 status = "disabled";
>                         };
> +
> +                       spi3: spi@400 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               compatible = "atmel,at91rm9200-spi";
> +                               reg = <0x400 0x200>;
> +                               interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(8)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "spi_clk";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 dma0: dma-controller@e0068000 {
> @@ -233,6 +293,21 @@ usart4: serial@200 {
>                                 atmel,fifo-size = <32>;
>                                 status = "disabled";
>                         };
> +
> +                       spi4: spi@400 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               compatible = "atmel,at91rm9200-spi";
> +                               reg = <0x400 0x200>;
> +                               interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
> +                                          <&dma0 AT91_XDMAC_DT_PERID(10)>;
> +                               dma-names = "tx", "rx";
> +                               clocks = <&nic_clk>;
> +                               clock-names = "spi_clk";
> +                               atmel,fifo-size = <32>;
> +                               status = "disabled";
> +                       };
>                 };
> 
>                 timer0: timer@e008c000 {
> --
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-03 16:03 ` [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support Michael Walle
@ 2022-03-04  8:31   ` Claudiu.Beznea
  2022-03-04 11:15     ` Michael Walle
  2022-03-23  8:06     ` Tudor.Ambarus
  0 siblings, 2 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-04  8:31 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
> features 6 Gigabit copper ports and two SFP cages. For now the
> following is working:
>  - Kernel console
>  - SFP cages I2C bus and mux
>  - SPI
>  - SGPIO
>  - Watchdog
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  arch/arm/boot/dts/Makefile                    |   3 +-
>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159 ++++++++++++++++++
>  2 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 085c43649d44..86dd0f9804ee 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>         imx7ulp-com.dtb \
>         imx7ulp-evk.dtb
>  dtb-$(CONFIG_SOC_LAN966) += \
> -       lan966x-pcb8291.dtb
> +       lan966x-pcb8291.dtb \
> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>  dtb-$(CONFIG_SOC_LS1021A) += \
>         ls1021a-moxa-uc-8410a.dtb \
>         ls1021a-qds.dtb \
> diff --git a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
> new file mode 100644
> index 000000000000..958678dec7ad
> --- /dev/null
> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
> + */
> +
> +/dts-v1/;
> +#include "lan966x.dtsi"
> +
> +/ {
> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs", "kontron,s1921",
> +                    "microchip,lan9668", "microchip,lan966";
> +
> +       aliases {
> +               serial0 = &usart0;
> +       };
> +
> +       chosen {
> +               stdout-path = "serial0:115200n8";
> +       };
> +
> +       gpio-restart {
> +               compatible = "gpio-restart";
> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
> +               priority = <200>;
> +       };
> +
> +       i2cmux {
> +               compatible = "i2c-mux-gpio";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
> +               i2c-parent = <&i2c4>;
> +
> +               i2c4_0: i2c@1 {
> +                       reg = <1>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +               };
> +
> +               i2c4_1: i2c@2 {
> +                       reg = <2>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +               };
> +       };
> +
> +       sfp0: sfp0 {
> +               compatible = "sff,sfp";
> +               i2c-bus = <&i2c4_0>;
> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
> +               maximum-power-milliwatt = <2500>;
> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
> +               rate-select0-gpios = <&sgpio_out 2 0 GPIO_ACTIVE_HIGH>;
> +               rate-select1-gpios = <&sgpio_out 2 1 GPIO_ACTIVE_HIGH>;
> +       };
> +
> +       sfp1: sfp1 {
> +               compatible = "sff,sfp";
> +               i2c-bus = <&i2c4_1>;
> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
> +               maximum-power-milliwatt = <2500>;
> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
> +               rate-select0-gpios = <&sgpio_out 2 2 GPIO_ACTIVE_HIGH>;
> +               rate-select1-gpios = <&sgpio_out 2 3 GPIO_ACTIVE_HIGH>;
> +       };
> +};
> +
> +&flx0 {
> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
> +       status = "okay";
> +};
> +
> +&flx3 {
> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
> +       status = "okay";
> +};
> +
> +&flx4 {
> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
> +       status = "okay";
> +};

Although there is 1:1 mapping b/w ids of flexcoms and the embedded blocks
(flxX has usartX, i2cX, spiX) and there is nothing wrong with the approach
here I found a bit hard to follow if the correspondent embedded block
(i2c, spi, usart) is enabled or not.

> +
> +&gpio {
> +       usart0_pins: usart0-pins {
> +               /* RXD, TXD */
> +               pins = "GPIO_25", "GPIO_26";
> +               function = "fc0_b";
> +       };
> +
> +       sgpio_a_pins: sgpio-a-pins {
> +               /* SCK, D0, D1, LD */
> +               pins = "GPIO_32", "GPIO_33", "GPIO_34";
> +               function = "sgpio_a";
> +       };
> +
> +       sgpio_b_pins: sgpio-b-pins {
> +               /* SCK, D0, D1, LD */
> +               pins = "GPIO_64";
> +               function = "sgpio_b";
> +       };
> +
> +       fc3_b_pins: fc3-b-spi-pins {
> +               /* SCK, MISO, MOSI */
> +               pins = "GPIO_51", "GPIO_52", "GPIO_53";
> +               function = "fc3_b";
> +       };
> +
> +       fc4_b_pins: fc4-b-i2c-pins {
> +               /* RXD, TXD */
> +               pins = "GPIO_57", "GPIO_58";
> +               function = "fc4_b";
> +       };
> +};
> +
> +&i2c4 {
> +       pinctrl-0 = <&fc4_b_pins>;
> +       pinctrl-names = "default";
> +       status = "okay";
> +};
> +
> +&usart0 {
> +       pinctrl-0 = <&usart0_pins>;
> +       pinctrl-names = "default";
> +       status = "okay";
> +};
> +
> +&sgpio {
> +       pinctrl-0 = <&sgpio_a_pins>, <&sgpio_b_pins>;
> +       pinctrl-names = "default";
> +       bus-frequency = <8000000>;
> +       /* arbitrary range because all GPIOs are in software mode */
> +       microchip,sgpio-port-ranges = <0 11>;
> +       status = "okay";
> +};
> +
> +&sgpio_in {
> +       ngpios = <128>;
> +};
> +
> +&sgpio_out {
> +       ngpios = <128>;
> +};
> +
> +&spi3 {
> +       pinctrl-0 = <&fc3_b_pins>;
> +       pinctrl-names = "default";
> +       cs-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
> +       status = "okay";
> +};
> +
> +&watchdog {
> +       status = "okay";
> +};
> --
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-04  8:30   ` Claudiu.Beznea
@ 2022-03-04 11:01     ` Michael Walle
  2022-03-07 11:53       ` Claudiu.Beznea
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-04 11:01 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

Hi,

thanks for the quick review.

Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
> On 03.03.2022 18:03, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Add all the usart nodes for the flexcom block. There was already
>> an usart node for the flexcom3 block. But it missed the DMA
>> channels.
> 
> And it would be good to go though a different patch.

sure

>> Although the DMA channels are specified, DMA is not
>> enabled by default because break detection doesn't work with DMA.
>> 
>> Keep the nodes disabled by default.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  arch/arm/boot/dts/lan966x.dtsi | 55 
>> ++++++++++++++++++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/lan966x.dtsi 
>> b/arch/arm/boot/dts/lan966x.dtsi
>> index a7d46a2ca058..bea69b6d2749 100644
>> --- a/arch/arm/boot/dts/lan966x.dtsi
>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>                         #size-cells = <1>;
>>                         ranges = <0x0 0xe0040000 0x800>;
>>                         status = "disabled";
>> +
>> +                       usart0: serial@200 {
>> +                               compatible = 
>> "atmel,at91sam9260-usart";
> 
> Are the usart blocks in lan966x 1:1 compatible with what is is sam9260? 
> In
> case not it may worth to have a new compatible here, for lan966x, such 
> that
> when new features will be implemented in usart driver for lan966x the 
> old
> DT (this one) will work with the new kernel implementation.

During my review of the inital dtsi patch, I've asked the same question 
[1]
and I was told they are the same.

At least this exact usart compatible is already in this file. I was 
under
the impression, that was the least controversial compatible :)

But you'll need to tell me if they are the same or not, I don't have
any clue what microchip has reused. The only thing I can add is that
there is a version register within the IP blocks which is already used
in some drivers.

> Same for the rest of the nodes added in this series.
> 
>> +                               reg = <0x200 0x200>;
>> +                               interrupts = <GIC_SPI 48 
>> IRQ_TYPE_LEVEL_HIGH>;
>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
>> +                                          <&dma0 
>> AT91_XDMAC_DT_PERID(2)>;
> 
> Keep dma entries aligned.

Oh shoot. How embarrassing they are alligned when the tab width is 4 *g*
Thanks.

-michael

[1] 
https://lore.kernel.org/lkml/20220210123704.477826-1-michael@walle.cc/

>> +                               dma-names = "tx", "rx";
>> +                               clocks = <&nic_clk>;
>> +                               clock-names = "usart";
>> +                               atmel,fifo-size = <32>;
>> +                               status = "disabled";
>> +                       };
>>                 };
>> 
>>                 flx1: flexcom@e0044000 {
>> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
>>                         #size-cells = <1>;
>>                         ranges = <0x0 0xe0044000 0x800>;
>>                         status = "disabled";
>> +
>> +                       usart1: serial@200 {
>> +                               compatible = 
>> "atmel,at91sam9260-usart";
>> +                               reg = <0x200 0x200>;
>> +                               interrupts = <GIC_SPI 49 
>> IRQ_TYPE_LEVEL_HIGH>;
>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
>> +                                          <&dma0 
>> AT91_XDMAC_DT_PERID(4)>;
>> +                               dma-names = "tx", "rx";
>> +                               clocks = <&nic_clk>;
>> +                               clock-names = "usart";
>> +                               atmel,fifo-size = <32>;
>> +                               status = "disabled";
>> +                       };
>>                 };
>> 
>>                 trng: rng@e0048000 {
>> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
>>                         #size-cells = <1>;
>>                         ranges = <0x0 0xe0060000 0x800>;
>>                         status = "disabled";
>> +
>> +                       usart2: serial@200 {
>> +                               compatible = 
>> "atmel,at91sam9260-usart";
>> +                               reg = <0x200 0x200>;
>> +                               interrupts = <GIC_SPI 50 
>> IRQ_TYPE_LEVEL_HIGH>;
>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
>> +                                          <&dma0 
>> AT91_XDMAC_DT_PERID(6)>;
>> +                               dma-names = "tx", "rx";
>> +                               clocks = <&nic_clk>;
>> +                               clock-names = "usart";
>> +                               atmel,fifo-size = <32>;
>> +                               status = "disabled";
>> +                       };
>>                 };
>> 
>>                 flx3: flexcom@e0064000 {
>> @@ -144,6 +183,9 @@ usart3: serial@200 {
>>                                 compatible = 
>> "atmel,at91sam9260-usart";
>>                                 reg = <0x200 0x200>;
>>                                 interrupts = <GIC_SPI 51 
>> IRQ_TYPE_LEVEL_HIGH>;
>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
>> +                                          <&dma0 
>> AT91_XDMAC_DT_PERID(8)>;
>> +                               dma-names = "tx", "rx";
>>                                 clocks = <&nic_clk>;
>>                                 clock-names = "usart";
>>                                 atmel,fifo-size = <32>;
>> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
>>                         #size-cells = <1>;
>>                         ranges = <0x0 0xe0070000 0x800>;
>>                         status = "disabled";
>> +
>> +                       usart4: serial@200 {
>> +                               compatible = 
>> "atmel,at91sam9260-usart";
>> +                               reg = <0x200 0x200>;
>> +                               interrupts = <GIC_SPI 52 
>> IRQ_TYPE_LEVEL_HIGH>;
>> +                               dmas = <&dma0 
>> AT91_XDMAC_DT_PERID(11)>,
>> +                                          <&dma0 
>> AT91_XDMAC_DT_PERID(10)>;
>> +                               dma-names = "tx", "rx";
>> +                               clocks = <&nic_clk>;
>> +                               clock-names = "usart";
>> +                               atmel,fifo-size = <32>;
>> +                               status = "disabled";
>> +                       };
>>                 };
>> 
>>                 timer0: timer@e008c000 {
>> --
>> 2.30.2
>> 

-- 
-michael

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-04  8:31   ` Claudiu.Beznea
@ 2022-03-04 11:15     ` Michael Walle
  2022-03-07 12:07       ` Claudiu.Beznea
  2022-03-23  8:06     ` Tudor.Ambarus
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-04 11:15 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

Am 2022-03-04 09:31, schrieb Claudiu.Beznea@microchip.com:
> On 03.03.2022 18:03, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
>> features 6 Gigabit copper ports and two SFP cages. For now the
>> following is working:
>>  - Kernel console
>>  - SFP cages I2C bus and mux
>>  - SPI
>>  - SGPIO
>>  - Watchdog
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  arch/arm/boot/dts/Makefile                    |   3 +-
>>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159 
>> ++++++++++++++++++
>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>  create mode 100644 
>> arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> 
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 085c43649d44..86dd0f9804ee 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>>         imx7ulp-com.dtb \
>>         imx7ulp-evk.dtb
>>  dtb-$(CONFIG_SOC_LAN966) += \
>> -       lan966x-pcb8291.dtb
>> +       lan966x-pcb8291.dtb \
>> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>>  dtb-$(CONFIG_SOC_LS1021A) += \
>>         ls1021a-moxa-uc-8410a.dtb \
>>         ls1021a-qds.dtb \
>> diff --git 
>> a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts 
>> b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> new file mode 100644
>> index 000000000000..958678dec7ad
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> @@ -0,0 +1,159 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
>> + */
>> +
>> +/dts-v1/;
>> +#include "lan966x.dtsi"
>> +
>> +/ {
>> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
>> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs", 
>> "kontron,s1921",
>> +                    "microchip,lan9668", "microchip,lan966";
>> +
>> +       aliases {
>> +               serial0 = &usart0;
>> +       };
>> +
>> +       chosen {
>> +               stdout-path = "serial0:115200n8";
>> +       };
>> +
>> +       gpio-restart {
>> +               compatible = "gpio-restart";
>> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
>> +               priority = <200>;
>> +       };
>> +
>> +       i2cmux {
>> +               compatible = "i2c-mux-gpio";
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
>> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
>> +               i2c-parent = <&i2c4>;
>> +
>> +               i2c4_0: i2c@1 {
>> +                       reg = <1>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +               };
>> +
>> +               i2c4_1: i2c@2 {
>> +                       reg = <2>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +               };
>> +       };
>> +
>> +       sfp0: sfp0 {
>> +               compatible = "sff,sfp";
>> +               i2c-bus = <&i2c4_0>;
>> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
>> +               maximum-power-milliwatt = <2500>;
>> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
>> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
>> +               rate-select0-gpios = <&sgpio_out 2 0 
>> GPIO_ACTIVE_HIGH>;
>> +               rate-select1-gpios = <&sgpio_out 2 1 
>> GPIO_ACTIVE_HIGH>;
>> +       };
>> +
>> +       sfp1: sfp1 {
>> +               compatible = "sff,sfp";
>> +               i2c-bus = <&i2c4_1>;
>> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
>> +               maximum-power-milliwatt = <2500>;
>> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
>> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
>> +               rate-select0-gpios = <&sgpio_out 2 2 
>> GPIO_ACTIVE_HIGH>;
>> +               rate-select1-gpios = <&sgpio_out 2 3 
>> GPIO_ACTIVE_HIGH>;
>> +       };
>> +};
>> +
>> +&flx0 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>> +       status = "okay";
>> +};
>> +
>> +&flx3 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
>> +       status = "okay";
>> +};
>> +
>> +&flx4 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>> +       status = "okay";
>> +};
> 
> Although there is 1:1 mapping b/w ids of flexcoms and the embedded 
> blocks
> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the 
> approach
> here I found a bit hard to follow if the correspondent embedded block
> (i2c, spi, usart) is enabled or not.

I know and I had the same feeling, but I don't want to have the
subnodes (matched by name) in these nodes.  I.e. I want to avoid
something like:

&flx4 {
        atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
        status = "okay";

        i2c@600 {
               pinctrl-0 = <&fc4_b_pins>;
               pinctrl-names = "default";
               status = "okay";
        };
};

If someone renames the subnode in the dtsi, it might easily be
overlooked in the board files. Having the handle will raise an
error.

And because the node references should be sorted alphabetically
it will be cluttered around in the file. You could rename the
references to flx4_i2c though. But I don't know it its worth
the efforts. Let me know what you think.

-michael

> 
>> +
>> +&gpio {
>> +       usart0_pins: usart0-pins {
>> +               /* RXD, TXD */
>> +               pins = "GPIO_25", "GPIO_26";
>> +               function = "fc0_b";
>> +       };
>> +
>> +       sgpio_a_pins: sgpio-a-pins {
>> +               /* SCK, D0, D1, LD */
>> +               pins = "GPIO_32", "GPIO_33", "GPIO_34";
>> +               function = "sgpio_a";
>> +       };
>> +
>> +       sgpio_b_pins: sgpio-b-pins {
>> +               /* SCK, D0, D1, LD */
>> +               pins = "GPIO_64";
>> +               function = "sgpio_b";
>> +       };
>> +
>> +       fc3_b_pins: fc3-b-spi-pins {
>> +               /* SCK, MISO, MOSI */
>> +               pins = "GPIO_51", "GPIO_52", "GPIO_53";
>> +               function = "fc3_b";
>> +       };
>> +
>> +       fc4_b_pins: fc4-b-i2c-pins {
>> +               /* RXD, TXD */
>> +               pins = "GPIO_57", "GPIO_58";
>> +               function = "fc4_b";
>> +       };
>> +};
>> +
>> +&i2c4 {
>> +       pinctrl-0 = <&fc4_b_pins>;
>> +       pinctrl-names = "default";
>> +       status = "okay";
>> +};
>> +
>> +&usart0 {
>> +       pinctrl-0 = <&usart0_pins>;
>> +       pinctrl-names = "default";
>> +       status = "okay";
>> +};
>> +
>> +&sgpio {
>> +       pinctrl-0 = <&sgpio_a_pins>, <&sgpio_b_pins>;
>> +       pinctrl-names = "default";
>> +       bus-frequency = <8000000>;
>> +       /* arbitrary range because all GPIOs are in software mode */
>> +       microchip,sgpio-port-ranges = <0 11>;
>> +       status = "okay";
>> +};
>> +
>> +&sgpio_in {
>> +       ngpios = <128>;
>> +};
>> +
>> +&sgpio_out {
>> +       ngpios = <128>;
>> +};
>> +
>> +&spi3 {
>> +       pinctrl-0 = <&fc3_b_pins>;
>> +       pinctrl-names = "default";
>> +       cs-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
>> +       status = "okay";
>> +};
>> +
>> +&watchdog {
>> +       status = "okay";
>> +};
>> --
>> 2.30.2
>> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-04 11:01     ` Michael Walle
@ 2022-03-07 11:53       ` Claudiu.Beznea
  2022-03-07 12:04         ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-07 11:53 UTC (permalink / raw)
  To: michael
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

On 04.03.2022 13:01, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Hi,
> 
> thanks for the quick review.
> 
> Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
>> On 03.03.2022 18:03, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Add all the usart nodes for the flexcom block. There was already
>>> an usart node for the flexcom3 block. But it missed the DMA
>>> channels.
>>
>> And it would be good to go though a different patch.
> 
> sure
> 
>>> Although the DMA channels are specified, DMA is not
>>> enabled by default because break detection doesn't work with DMA.
>>>
>>> Keep the nodes disabled by default.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>> ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 55 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>> b/arch/arm/boot/dts/lan966x.dtsi
>>> index a7d46a2ca058..bea69b6d2749 100644
>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart0: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>
>> Are the usart blocks in lan966x 1:1 compatible with what is is sam9260?
>> In
>> case not it may worth to have a new compatible here, for lan966x, such
>> that
>> when new features will be implemented in usart driver for lan966x the
>> old
>> DT (this one) will work with the new kernel implementation.
> 
> During my review of the inital dtsi patch, I've asked the same question
> [1]
> and I was told they are the same.
> 
> At least this exact usart compatible is already in this file. I was
> under
> the impression, that was the least controversial compatible :)

OK.

> 
> But you'll need to tell me if they are the same or not, I don't have
> any clue what microchip has reused. 

From software point of view comparing registers should be good, as far as I
can tell. All AT91 datasheet should be available. I though you have checked
one against LAN966. At the moment I don't have a DS for LAN966. I'll find
one and have a look.

> The only thing I can add is that
> there is a version register within the IP blocks which is already used
> in some drivers.
> 
>> Same for the rest of the nodes added in this series.
>>
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 48
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(2)>;
>>
>> Keep dma entries aligned.
> 
> Oh shoot. How embarrassing they are alligned when the tab width is 4 *g*
> Thanks.
> 
> -michael
> 
> [1]
> https://lore.kernel.org/lkml/20220210123704.477826-1-michael@walle.cc/
> 
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 flx1: flexcom@e0044000 {
>>> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0044000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart1: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 49
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(4)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 trng: rng@e0048000 {
>>> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0060000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart2: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 50
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(6)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 flx3: flexcom@e0064000 {
>>> @@ -144,6 +183,9 @@ usart3: serial@200 {
>>>                                 compatible =
>>> "atmel,at91sam9260-usart";
>>>                                 reg = <0x200 0x200>;
>>>                                 interrupts = <GIC_SPI 51
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(8)>;
>>> +                               dma-names = "tx", "rx";
>>>                                 clocks = <&nic_clk>;
>>>                                 clock-names = "usart";
>>>                                 atmel,fifo-size = <32>;
>>> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0070000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart4: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 52
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0
>>> AT91_XDMAC_DT_PERID(11)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(10)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 timer0: timer@e008c000 {
>>> -- 
>>> 2.30.2
>>>
> 
> -- 
> -michael


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-07 11:53       ` Claudiu.Beznea
@ 2022-03-07 12:04         ` Michael Walle
  2022-03-18 12:17           ` Claudiu.Beznea
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-07 12:04 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

Am 2022-03-07 12:53, schrieb Claudiu.Beznea@microchip.com:
> On 04.03.2022 13:01, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the
>> content is safe
>> 
>> Hi,
>> 
>> thanks for the quick review.
>> 
>> Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you 
>>>> know
>>>> the content is safe
>>>> 
>>>> Add all the usart nodes for the flexcom block. There was already
>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>> channels.
>>> 
>>> And it would be good to go though a different patch.
>> 
>> sure
>> 
>>>> Although the DMA channels are specified, DMA is not
>>>> enabled by default because break detection doesn't work with DMA.
>>>> 
>>>> Keep the nodes disabled by default.
>>>> 
>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>> ---
>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>> ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 55 insertions(+)
>>>> 
>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>                         #size-cells = <1>;
>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>                         status = "disabled";
>>>> +
>>>> +                       usart0: serial@200 {
>>>> +                               compatible =
>>>> "atmel,at91sam9260-usart";
>>> 
>>> Are the usart blocks in lan966x 1:1 compatible with what is is 
>>> sam9260?
>>> In
>>> case not it may worth to have a new compatible here, for lan966x, 
>>> such
>>> that
>>> when new features will be implemented in usart driver for lan966x the
>>> old
>>> DT (this one) will work with the new kernel implementation.
>> 
>> During my review of the inital dtsi patch, I've asked the same 
>> question
>> [1]
>> and I was told they are the same.
>> 
>> At least this exact usart compatible is already in this file. I was
>> under
>> the impression, that was the least controversial compatible :)
> 
> OK.
> 
>> 
>> But you'll need to tell me if they are the same or not, I don't have
>> any clue what microchip has reused.
> 
> From software point of view comparing registers should be good, as far 
> as I
> can tell. All AT91 datasheet should be available. I though you have 
> checked
> one against LAN966. At the moment I don't have a DS for LAN966. I'll 
> find
> one and have a look.

So my train of thought was like: even if the registers are the same I
cannot be sure that it is the exact same IP and will behave the same.
Therefore, it is something only microchip can answer.

You can find the registers at
https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html

I'm not aware of any "classic" datasheet.

-michael

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-04 11:15     ` Michael Walle
@ 2022-03-07 12:07       ` Claudiu.Beznea
  2022-03-07 12:17         ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-07 12:07 UTC (permalink / raw)
  To: michael
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

On 04.03.2022 13:15, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Am 2022-03-04 09:31, schrieb Claudiu.Beznea@microchip.com:
>> On 03.03.2022 18:03, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
>>> features 6 Gigabit copper ports and two SFP cages. For now the
>>> following is working:
>>>  - Kernel console
>>>  - SFP cages I2C bus and mux
>>>  - SPI
>>>  - SGPIO
>>>  - Watchdog
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  arch/arm/boot/dts/Makefile                    |   3 +-
>>>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159
>>> ++++++++++++++++++
>>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>>  create mode 100644
>>> arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>
>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>> index 085c43649d44..86dd0f9804ee 100644
>>> --- a/arch/arm/boot/dts/Makefile
>>> +++ b/arch/arm/boot/dts/Makefile
>>> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>>>         imx7ulp-com.dtb \
>>>         imx7ulp-evk.dtb
>>>  dtb-$(CONFIG_SOC_LAN966) += \
>>> -       lan966x-pcb8291.dtb
>>> +       lan966x-pcb8291.dtb \
>>> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>>>  dtb-$(CONFIG_SOC_LS1021A) += \
>>>         ls1021a-moxa-uc-8410a.dtb \
>>>         ls1021a-qds.dtb \
>>> diff --git
>>> a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>> b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>> new file mode 100644
>>> index 000000000000..958678dec7ad
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>> @@ -0,0 +1,159 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "lan966x.dtsi"
>>> +
>>> +/ {
>>> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
>>> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs",
>>> "kontron,s1921",
>>> +                    "microchip,lan9668", "microchip,lan966";
>>> +
>>> +       aliases {
>>> +               serial0 = &usart0;
>>> +       };
>>> +
>>> +       chosen {
>>> +               stdout-path = "serial0:115200n8";
>>> +       };
>>> +
>>> +       gpio-restart {
>>> +               compatible = "gpio-restart";
>>> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
>>> +               priority = <200>;
>>> +       };
>>> +
>>> +       i2cmux {
>>> +               compatible = "i2c-mux-gpio";
>>> +               #address-cells = <1>;
>>> +               #size-cells = <0>;
>>> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
>>> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
>>> +               i2c-parent = <&i2c4>;
>>> +
>>> +               i2c4_0: i2c@1 {
>>> +                       reg = <1>;
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <0>;
>>> +               };
>>> +
>>> +               i2c4_1: i2c@2 {
>>> +                       reg = <2>;
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <0>;
>>> +               };
>>> +       };
>>> +
>>> +       sfp0: sfp0 {
>>> +               compatible = "sff,sfp";
>>> +               i2c-bus = <&i2c4_0>;
>>> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
>>> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
>>> +               maximum-power-milliwatt = <2500>;
>>> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
>>> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
>>> +               rate-select0-gpios = <&sgpio_out 2 0
>>> GPIO_ACTIVE_HIGH>;
>>> +               rate-select1-gpios = <&sgpio_out 2 1
>>> GPIO_ACTIVE_HIGH>;
>>> +       };
>>> +
>>> +       sfp1: sfp1 {
>>> +               compatible = "sff,sfp";
>>> +               i2c-bus = <&i2c4_1>;
>>> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
>>> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
>>> +               maximum-power-milliwatt = <2500>;
>>> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
>>> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
>>> +               rate-select0-gpios = <&sgpio_out 2 2
>>> GPIO_ACTIVE_HIGH>;
>>> +               rate-select1-gpios = <&sgpio_out 2 3
>>> GPIO_ACTIVE_HIGH>;
>>> +       };
>>> +};
>>> +
>>> +&flx0 {
>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&flx3 {
>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&flx4 {
>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>>> +       status = "okay";
>>> +};
>>
>> Although there is 1:1 mapping b/w ids of flexcoms and the embedded
>> blocks
>> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the
>> approach
>> here I found a bit hard to follow if the correspondent embedded block
>> (i2c, spi, usart) is enabled or not.
> 
> I know and I had the same feeling, but I don't want to have the
> subnodes (matched by name) in these nodes.  I.e. I want to avoid
> something like:
> 
> &flx4 {
>        atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>        status = "okay";
> 
>        i2c@600 {
>               pinctrl-0 = <&fc4_b_pins>;
>               pinctrl-names = "default";
>               status = "okay";
>        };
> };

All the other AT91 DTs are using the above format + the specific label in
front of the subnode, e.g:

	i2cX: i2c@600 {

> 
> If someone renames the subnode in the dtsi, it might easily be
> overlooked in the board files. Having the handle will raise an
> error.

If using label + node name as pointed above there will be an error thrown
for your scenario.

> 
> And because the node references should be sorted alphabetically
> it will be cluttered around in the file. You could rename the
> references to flx4_i2c though. But I don't know it its worth
> the efforts. Let me know what you think.
> 
> -michael
> 
>>
>>> +
>>> +&gpio {
>>> +       usart0_pins: usart0-pins {
>>> +               /* RXD, TXD */
>>> +               pins = "GPIO_25", "GPIO_26";
>>> +               function = "fc0_b";
>>> +       };
>>> +
>>> +       sgpio_a_pins: sgpio-a-pins {
>>> +               /* SCK, D0, D1, LD */
>>> +               pins = "GPIO_32", "GPIO_33", "GPIO_34";
>>> +               function = "sgpio_a";
>>> +       };
>>> +
>>> +       sgpio_b_pins: sgpio-b-pins {
>>> +               /* SCK, D0, D1, LD */
>>> +               pins = "GPIO_64";
>>> +               function = "sgpio_b";
>>> +       };
>>> +
>>> +       fc3_b_pins: fc3-b-spi-pins {
>>> +               /* SCK, MISO, MOSI */
>>> +               pins = "GPIO_51", "GPIO_52", "GPIO_53";
>>> +               function = "fc3_b";
>>> +       };
>>> +
>>> +       fc4_b_pins: fc4-b-i2c-pins {
>>> +               /* RXD, TXD */
>>> +               pins = "GPIO_57", "GPIO_58";
>>> +               function = "fc4_b";
>>> +       };
>>> +};
>>> +
>>> +&i2c4 {
>>> +       pinctrl-0 = <&fc4_b_pins>;
>>> +       pinctrl-names = "default";
>>> +       status = "okay";
>>> +};
>>> +
>>> +&usart0 {
>>> +       pinctrl-0 = <&usart0_pins>;
>>> +       pinctrl-names = "default";
>>> +       status = "okay";
>>> +};
>>> +
>>> +&sgpio {
>>> +       pinctrl-0 = <&sgpio_a_pins>, <&sgpio_b_pins>;
>>> +       pinctrl-names = "default";
>>> +       bus-frequency = <8000000>;
>>> +       /* arbitrary range because all GPIOs are in software mode */
>>> +       microchip,sgpio-port-ranges = <0 11>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&sgpio_in {
>>> +       ngpios = <128>;
>>> +};
>>> +
>>> +&sgpio_out {
>>> +       ngpios = <128>;
>>> +};
>>> +
>>> +&spi3 {
>>> +       pinctrl-0 = <&fc3_b_pins>;
>>> +       pinctrl-names = "default";
>>> +       cs-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&watchdog {
>>> +       status = "okay";
>>> +};
>>> -- 
>>> 2.30.2
>>>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-07 12:07       ` Claudiu.Beznea
@ 2022-03-07 12:17         ` Michael Walle
  2022-03-18 12:26           ` Claudiu.Beznea
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-07 12:17 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

Am 2022-03-07 13:07, schrieb Claudiu.Beznea@microchip.com:
> On 04.03.2022 13:15, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the
>> content is safe
>> 
>> Am 2022-03-04 09:31, schrieb Claudiu.Beznea@microchip.com:
>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you 
>>>> know
>>>> the content is safe
>>>> 
>>>> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
>>>> features 6 Gigabit copper ports and two SFP cages. For now the
>>>> following is working:
>>>>  - Kernel console
>>>>  - SFP cages I2C bus and mux
>>>>  - SPI
>>>>  - SGPIO
>>>>  - Watchdog
>>>> 
>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>> ---
>>>>  arch/arm/boot/dts/Makefile                    |   3 +-
>>>>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159
>>>> ++++++++++++++++++
>>>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>>>  create mode 100644
>>>> arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>> 
>>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>>> index 085c43649d44..86dd0f9804ee 100644
>>>> --- a/arch/arm/boot/dts/Makefile
>>>> +++ b/arch/arm/boot/dts/Makefile
>>>> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>>>>         imx7ulp-com.dtb \
>>>>         imx7ulp-evk.dtb
>>>>  dtb-$(CONFIG_SOC_LAN966) += \
>>>> -       lan966x-pcb8291.dtb
>>>> +       lan966x-pcb8291.dtb \
>>>> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>>>>  dtb-$(CONFIG_SOC_LS1021A) += \
>>>>         ls1021a-moxa-uc-8410a.dtb \
>>>>         ls1021a-qds.dtb \
>>>> diff --git
>>>> a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>> b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>> new file mode 100644
>>>> index 000000000000..958678dec7ad
>>>> --- /dev/null
>>>> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>> @@ -0,0 +1,159 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +#include "lan966x.dtsi"
>>>> +
>>>> +/ {
>>>> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
>>>> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs",
>>>> "kontron,s1921",
>>>> +                    "microchip,lan9668", "microchip,lan966";
>>>> +
>>>> +       aliases {
>>>> +               serial0 = &usart0;
>>>> +       };
>>>> +
>>>> +       chosen {
>>>> +               stdout-path = "serial0:115200n8";
>>>> +       };
>>>> +
>>>> +       gpio-restart {
>>>> +               compatible = "gpio-restart";
>>>> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
>>>> +               priority = <200>;
>>>> +       };
>>>> +
>>>> +       i2cmux {
>>>> +               compatible = "i2c-mux-gpio";
>>>> +               #address-cells = <1>;
>>>> +               #size-cells = <0>;
>>>> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
>>>> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
>>>> +               i2c-parent = <&i2c4>;
>>>> +
>>>> +               i2c4_0: i2c@1 {
>>>> +                       reg = <1>;
>>>> +                       #address-cells = <1>;
>>>> +                       #size-cells = <0>;
>>>> +               };
>>>> +
>>>> +               i2c4_1: i2c@2 {
>>>> +                       reg = <2>;
>>>> +                       #address-cells = <1>;
>>>> +                       #size-cells = <0>;
>>>> +               };
>>>> +       };
>>>> +
>>>> +       sfp0: sfp0 {
>>>> +               compatible = "sff,sfp";
>>>> +               i2c-bus = <&i2c4_0>;
>>>> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
>>>> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
>>>> +               maximum-power-milliwatt = <2500>;
>>>> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
>>>> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
>>>> +               rate-select0-gpios = <&sgpio_out 2 0
>>>> GPIO_ACTIVE_HIGH>;
>>>> +               rate-select1-gpios = <&sgpio_out 2 1
>>>> GPIO_ACTIVE_HIGH>;
>>>> +       };
>>>> +
>>>> +       sfp1: sfp1 {
>>>> +               compatible = "sff,sfp";
>>>> +               i2c-bus = <&i2c4_1>;
>>>> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
>>>> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
>>>> +               maximum-power-milliwatt = <2500>;
>>>> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
>>>> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
>>>> +               rate-select0-gpios = <&sgpio_out 2 2
>>>> GPIO_ACTIVE_HIGH>;
>>>> +               rate-select1-gpios = <&sgpio_out 2 3
>>>> GPIO_ACTIVE_HIGH>;
>>>> +       };
>>>> +};
>>>> +
>>>> +&flx0 {
>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&flx3 {
>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&flx4 {
>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>>>> +       status = "okay";
>>>> +};
>>> 
>>> Although there is 1:1 mapping b/w ids of flexcoms and the embedded
>>> blocks
>>> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the
>>> approach
>>> here I found a bit hard to follow if the correspondent embedded block
>>> (i2c, spi, usart) is enabled or not.
>> 
>> I know and I had the same feeling, but I don't want to have the
>> subnodes (matched by name) in these nodes.  I.e. I want to avoid
>> something like:
>> 
>> &flx4 {
>>        atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>>        status = "okay";
>> 
>>        i2c@600 {
>>               pinctrl-0 = <&fc4_b_pins>;
>>               pinctrl-names = "default";
>>               status = "okay";
>>        };
>> };
> 
> All the other AT91 DTs are using the above format + the specific label 
> in
> front of the subnode, e.g:
> 
> 	i2cX: i2c@600 {
> 
>> 
>> If someone renames the subnode in the dtsi, it might easily be
>> overlooked in the board files. Having the handle will raise an
>> error.
> 
> If using label + node name as pointed above there will be an error 
> thrown
> for your scenario.

Fair enough. You'll get a duplicate reference error as long as
the reference itself isn't renamed either.

But to make it short, unless you force me too, I'd like
to keep the child node as is and not as a subnode of
the flexcom. I just don't want to repeat the name if
there is no reason and I live with the fact that they
are not near each other :)

That flexcom-mode could also be deduced from the enabled
children, btw.

>> And because the node references should be sorted alphabetically
>> it will be cluttered around in the file. You could rename the
>> references to flx4_i2c though. But I don't know it its worth
>> the efforts. Let me know what you think.

-michael

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-07 12:04         ` Michael Walle
@ 2022-03-18 12:17           ` Claudiu.Beznea
  2022-03-22 21:39             ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-18 12:17 UTC (permalink / raw)
  To: michael, Kavyasree.Kotagiri
  Cc: Nicolas.Ferre, arnd, olof, soc, linux-arm-kernel, devicetree,
	linux-kernel, robh+dt, krzysztof.kozlowski, alexandre.belloni

On 07.03.2022 14:04, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Am 2022-03-07 12:53, schrieb Claudiu.Beznea@microchip.com:
>> On 04.03.2022 13:01, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the
>>> content is safe
>>>
>>> Hi,
>>>
>>> thanks for the quick review.
>>>
>>> Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>> channels.
>>>>
>>>> And it would be good to go though a different patch.
>>>
>>> sure
>>>
>>>>> Although the DMA channels are specified, DMA is not
>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>
>>>>> Keep the nodes disabled by default.
>>>>>
>>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>>> ---
>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>> ++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 55 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>                         #size-cells = <1>;
>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>                         status = "disabled";
>>>>> +
>>>>> +                       usart0: serial@200 {
>>>>> +                               compatible =
>>>>> "atmel,at91sam9260-usart";
>>>>
>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>> sam9260?
>>>> In
>>>> case not it may worth to have a new compatible here, for lan966x,
>>>> such
>>>> that
>>>> when new features will be implemented in usart driver for lan966x the
>>>> old
>>>> DT (this one) will work with the new kernel implementation.
>>>
>>> During my review of the inital dtsi patch, I've asked the same
>>> question
>>> [1]
>>> and I was told they are the same.
>>>
>>> At least this exact usart compatible is already in this file. I was
>>> under
>>> the impression, that was the least controversial compatible :)
>>
>> OK.
>>
>>>
>>> But you'll need to tell me if they are the same or not, I don't have
>>> any clue what microchip has reused.
>>
>> From software point of view comparing registers should be good, as far
>> as I
>> can tell. All AT91 datasheet should be available. I though you have
>> checked
>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>> find
>> one and have a look.
> 
> So my train of thought was like: even if the registers are the same I
> cannot be sure that it is the exact same IP and will behave the same.
> Therefore, it is something only microchip can answer.
> 
> You can find the registers at
> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
> 
> I'm not aware of any "classic" datasheet.

You can find all AT91 datasheet on Microchip web site [1].

Simple register comparison b/w register mapping at [2] and SAMA5D2
datasheet [3] (which uses the same compatible),  SAM9X60 datasheet [3] and
SAMA7G5 datasheet (not public at the moment) brings up a difference at
register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I say
it needs a new compatible.

Kavya, could you confirm this?

Thank you,
Claudiu Beznea

[1] https://www.microchip.com/
[2] https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
[3] http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
[4]
https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf

> 
> -michael


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-07 12:17         ` Michael Walle
@ 2022-03-18 12:26           ` Claudiu.Beznea
  0 siblings, 0 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-18 12:26 UTC (permalink / raw)
  To: michael
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

On 07.03.2022 14:17, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Am 2022-03-07 13:07, schrieb Claudiu.Beznea@microchip.com:
>> On 04.03.2022 13:15, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the
>>> content is safe
>>>
>>> Am 2022-03-04 09:31, schrieb Claudiu.Beznea@microchip.com:
>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
>>>>> features 6 Gigabit copper ports and two SFP cages. For now the
>>>>> following is working:
>>>>>  - Kernel console
>>>>>  - SFP cages I2C bus and mux
>>>>>  - SPI
>>>>>  - SGPIO
>>>>>  - Watchdog
>>>>>
>>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>>> ---
>>>>>  arch/arm/boot/dts/Makefile                    |   3 +-
>>>>>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159
>>>>> ++++++++++++++++++
>>>>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>>>>  create mode 100644
>>>>> arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>>>> index 085c43649d44..86dd0f9804ee 100644
>>>>> --- a/arch/arm/boot/dts/Makefile
>>>>> +++ b/arch/arm/boot/dts/Makefile
>>>>> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>>>>>         imx7ulp-com.dtb \
>>>>>         imx7ulp-evk.dtb
>>>>>  dtb-$(CONFIG_SOC_LAN966) += \
>>>>> -       lan966x-pcb8291.dtb
>>>>> +       lan966x-pcb8291.dtb \
>>>>> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>>>>>  dtb-$(CONFIG_SOC_LS1021A) += \
>>>>>         ls1021a-moxa-uc-8410a.dtb \
>>>>>         ls1021a-qds.dtb \
>>>>> diff --git
>>>>> a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>>> b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>>> new file mode 100644
>>>>> index 000000000000..958678dec7ad
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>>>>> @@ -0,0 +1,159 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
>>>>> + */
>>>>> +
>>>>> +/dts-v1/;
>>>>> +#include "lan966x.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
>>>>> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs",
>>>>> "kontron,s1921",
>>>>> +                    "microchip,lan9668", "microchip,lan966";
>>>>> +
>>>>> +       aliases {
>>>>> +               serial0 = &usart0;
>>>>> +       };
>>>>> +
>>>>> +       chosen {
>>>>> +               stdout-path = "serial0:115200n8";
>>>>> +       };
>>>>> +
>>>>> +       gpio-restart {
>>>>> +               compatible = "gpio-restart";
>>>>> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
>>>>> +               priority = <200>;
>>>>> +       };
>>>>> +
>>>>> +       i2cmux {
>>>>> +               compatible = "i2c-mux-gpio";
>>>>> +               #address-cells = <1>;
>>>>> +               #size-cells = <0>;
>>>>> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
>>>>> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
>>>>> +               i2c-parent = <&i2c4>;
>>>>> +
>>>>> +               i2c4_0: i2c@1 {
>>>>> +                       reg = <1>;
>>>>> +                       #address-cells = <1>;
>>>>> +                       #size-cells = <0>;
>>>>> +               };
>>>>> +
>>>>> +               i2c4_1: i2c@2 {
>>>>> +                       reg = <2>;
>>>>> +                       #address-cells = <1>;
>>>>> +                       #size-cells = <0>;
>>>>> +               };
>>>>> +       };
>>>>> +
>>>>> +       sfp0: sfp0 {
>>>>> +               compatible = "sff,sfp";
>>>>> +               i2c-bus = <&i2c4_0>;
>>>>> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
>>>>> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
>>>>> +               maximum-power-milliwatt = <2500>;
>>>>> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
>>>>> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
>>>>> +               rate-select0-gpios = <&sgpio_out 2 0
>>>>> GPIO_ACTIVE_HIGH>;
>>>>> +               rate-select1-gpios = <&sgpio_out 2 1
>>>>> GPIO_ACTIVE_HIGH>;
>>>>> +       };
>>>>> +
>>>>> +       sfp1: sfp1 {
>>>>> +               compatible = "sff,sfp";
>>>>> +               i2c-bus = <&i2c4_1>;
>>>>> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
>>>>> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
>>>>> +               maximum-power-milliwatt = <2500>;
>>>>> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
>>>>> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
>>>>> +               rate-select0-gpios = <&sgpio_out 2 2
>>>>> GPIO_ACTIVE_HIGH>;
>>>>> +               rate-select1-gpios = <&sgpio_out 2 3
>>>>> GPIO_ACTIVE_HIGH>;
>>>>> +       };
>>>>> +};
>>>>> +
>>>>> +&flx0 {
>>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>>>>> +       status = "okay";
>>>>> +};
>>>>> +
>>>>> +&flx3 {
>>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
>>>>> +       status = "okay";
>>>>> +};
>>>>> +
>>>>> +&flx4 {
>>>>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>>>>> +       status = "okay";
>>>>> +};
>>>>
>>>> Although there is 1:1 mapping b/w ids of flexcoms and the embedded
>>>> blocks
>>>> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the
>>>> approach
>>>> here I found a bit hard to follow if the correspondent embedded block
>>>> (i2c, spi, usart) is enabled or not.
>>>
>>> I know and I had the same feeling, but I don't want to have the
>>> subnodes (matched by name) in these nodes.  I.e. I want to avoid
>>> something like:
>>>
>>> &flx4 {
>>>        atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>>>        status = "okay";
>>>
>>>        i2c@600 {
>>>               pinctrl-0 = <&fc4_b_pins>;
>>>               pinctrl-names = "default";
>>>               status = "okay";
>>>        };
>>> };
>>
>> All the other AT91 DTs are using the above format + the specific label
>> in
>> front of the subnode, e.g:
>>
>>       i2cX: i2c@600 {
>>
>>>
>>> If someone renames the subnode in the dtsi, it might easily be
>>> overlooked in the board files. Having the handle will raise an
>>> error.
>>
>> If using label + node name as pointed above there will be an error
>> thrown
>> for your scenario.
> 
> Fair enough. You'll get a duplicate reference error as long as
> the reference itself isn't renamed either.
> 
> But to make it short, unless you force me too, I'd like
> to keep the child node as is and not as a subnode of
> the flexcom. I just don't want to repeat the name if
> there is no reason and I live with the fact that they
> are not near each other :)

To me it easy to follow with the current approach adopted by all AT91
targets. If you know that all flexcomX subnodes have the X in their naming
(e.g. i2c3, spi3, uart3 are childs of flexcom3) then its easy to follow it
also. But 1st time opening the dts file you probably don't know this.

Thank you,
Claudiu Beznea

> 
> That flexcom-mode could also be deduced from the enabled
> children, btw.
> 
>>> And because the node references should be sorted alphabetically
>>> it will be cluttered around in the file. You could rename the
>>> references to flx4_i2c though. But I don't know it its worth
>>> the efforts. Let me know what you think.
> 
> -michael


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-18 12:17           ` Claudiu.Beznea
@ 2022-03-22 21:39             ` Michael Walle
  2022-03-24 16:32               ` Claudiu.Beznea
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2022-03-22 21:39 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

Am 2022-03-18 13:17, schrieb Claudiu.Beznea@microchip.com:
> On 07.03.2022 14:04, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the
>> content is safe
>> 
>> Am 2022-03-07 12:53, schrieb Claudiu.Beznea@microchip.com:
>>> On 04.03.2022 13:01, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you 
>>>> know
>>>> the
>>>> content is safe
>>>> 
>>>> Hi,
>>>> 
>>>> thanks for the quick review.
>>>> 
>>>> Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
>>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>> know
>>>>>> the content is safe
>>>>>> 
>>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>>> channels.
>>>>> 
>>>>> And it would be good to go though a different patch.
>>>> 
>>>> sure
>>>> 
>>>>>> Although the DMA channels are specified, DMA is not
>>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>> 
>>>>>> Keep the nodes disabled by default.
>>>>>> 
>>>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>>> ++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 55 insertions(+)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>>                         #size-cells = <1>;
>>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>>                         status = "disabled";
>>>>>> +
>>>>>> +                       usart0: serial@200 {
>>>>>> +                               compatible =
>>>>>> "atmel,at91sam9260-usart";
>>>>> 
>>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>>> sam9260?
>>>>> In
>>>>> case not it may worth to have a new compatible here, for lan966x,
>>>>> such
>>>>> that
>>>>> when new features will be implemented in usart driver for lan966x 
>>>>> the
>>>>> old
>>>>> DT (this one) will work with the new kernel implementation.
>>>> 
>>>> During my review of the inital dtsi patch, I've asked the same
>>>> question
>>>> [1]
>>>> and I was told they are the same.
>>>> 
>>>> At least this exact usart compatible is already in this file. I was
>>>> under
>>>> the impression, that was the least controversial compatible :)
>>> 
>>> OK.
>>> 
>>>> 
>>>> But you'll need to tell me if they are the same or not, I don't have
>>>> any clue what microchip has reused.
>>> 
>>> From software point of view comparing registers should be good, as 
>>> far
>>> as I
>>> can tell. All AT91 datasheet should be available. I though you have
>>> checked
>>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>>> find
>>> one and have a look.
>> 
>> So my train of thought was like: even if the registers are the same I
>> cannot be sure that it is the exact same IP and will behave the same.
>> Therefore, it is something only microchip can answer.
>> 
>> You can find the registers at
>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>> 
>> I'm not aware of any "classic" datasheet.
> 
> You can find all AT91 datasheet on Microchip web site [1].
> 
> Simple register comparison b/w register mapping at [2] and SAMA5D2
> datasheet [3] (which uses the same compatible),  SAM9X60 datasheet [3] 
> and
> SAMA7G5 datasheet (not public at the moment) brings up a difference at
> register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
> SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I 
> say
> it needs a new compatible.

I can't follow you here. These bits are already used in the current UART
driver and are supported on the LAN966X. So if anything, SAMA5D2, 
SAM9X60
and SAMA7G5 need a new compatible, no?

-michael

> Kavya, could you confirm this?
> 
> Thank you,
> Claudiu Beznea
> 
> [1] https://www.microchip.com/
> [2] 
> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
> [3] 
> http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
> [4]
> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf
> 
>> 
>> -michael

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support
  2022-03-04  8:31   ` Claudiu.Beznea
  2022-03-04 11:15     ` Michael Walle
@ 2022-03-23  8:06     ` Tudor.Ambarus
  1 sibling, 0 replies; 23+ messages in thread
From: Tudor.Ambarus @ 2022-03-23  8:06 UTC (permalink / raw)
  To: Claudiu.Beznea, michael, Kavyasree.Kotagiri, Nicolas.Ferre
  Cc: arnd, olof, soc, linux-arm-kernel, devicetree, linux-kernel,
	robh+dt, krzysztof.kozlowski, alexandre.belloni

On 3/4/22 10:31, Claudiu.Beznea@microchip.com wrote:
>> +&flx4 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>> +       status = "okay";
>> +};
> Although there is 1:1 mapping b/w ids of flexcoms and the embedded blocks
> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the approach
> here I found a bit hard to follow if the correspondent embedded block
> (i2c, spi, usart) is enabled or not.
> 

+1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes
  2022-03-22 21:39             ` Michael Walle
@ 2022-03-24 16:32               ` Claudiu.Beznea
  0 siblings, 0 replies; 23+ messages in thread
From: Claudiu.Beznea @ 2022-03-24 16:32 UTC (permalink / raw)
  To: michael
  Cc: Kavyasree.Kotagiri, Nicolas.Ferre, arnd, olof, soc,
	linux-arm-kernel, devicetree, linux-kernel, robh+dt,
	krzysztof.kozlowski, alexandre.belloni

On 22.03.2022 23:39, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Am 2022-03-18 13:17, schrieb Claudiu.Beznea@microchip.com:
>> On 07.03.2022 14:04, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the
>>> content is safe
>>>
>>> Am 2022-03-07 12:53, schrieb Claudiu.Beznea@microchip.com:
>>>> On 04.03.2022 13:01, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the
>>>>> content is safe
>>>>>
>>>>> Hi,
>>>>>
>>>>> thanks for the quick review.
>>>>>
>>>>> Am 2022-03-04 09:30, schrieb Claudiu.Beznea@microchip.com:
>>>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>>> know
>>>>>>> the content is safe
>>>>>>>
>>>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>>>> channels.
>>>>>>
>>>>>> And it would be good to go though a different patch.
>>>>>
>>>>> sure
>>>>>
>>>>>>> Although the DMA channels are specified, DMA is not
>>>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>>>
>>>>>>> Keep the nodes disabled by default.
>>>>>>>
>>>>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>>>>> ---
>>>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>>>> ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 55 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>>>                         #size-cells = <1>;
>>>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>>>                         status = "disabled";
>>>>>>> +
>>>>>>> +                       usart0: serial@200 {
>>>>>>> +                               compatible =
>>>>>>> "atmel,at91sam9260-usart";
>>>>>>
>>>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>>>> sam9260?
>>>>>> In
>>>>>> case not it may worth to have a new compatible here, for lan966x,
>>>>>> such
>>>>>> that
>>>>>> when new features will be implemented in usart driver for lan966x
>>>>>> the
>>>>>> old
>>>>>> DT (this one) will work with the new kernel implementation.
>>>>>
>>>>> During my review of the inital dtsi patch, I've asked the same
>>>>> question
>>>>> [1]
>>>>> and I was told they are the same.
>>>>>
>>>>> At least this exact usart compatible is already in this file. I was
>>>>> under
>>>>> the impression, that was the least controversial compatible :)
>>>>
>>>> OK.
>>>>
>>>>>
>>>>> But you'll need to tell me if they are the same or not, I don't have
>>>>> any clue what microchip has reused.
>>>>
>>>> From software point of view comparing registers should be good, as
>>>> far
>>>> as I
>>>> can tell. All AT91 datasheet should be available. I though you have
>>>> checked
>>>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>>>> find
>>>> one and have a look.
>>>
>>> So my train of thought was like: even if the registers are the same I
>>> cannot be sure that it is the exact same IP and will behave the same.
>>> Therefore, it is something only microchip can answer.
>>>
>>> You can find the registers at
>>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>>>
>>> I'm not aware of any "classic" datasheet.
>>
>> You can find all AT91 datasheet on Microchip web site [1].
>>
>> Simple register comparison b/w register mapping at [2] and SAMA5D2
>> datasheet [3] (which uses the same compatible),  SAM9X60 datasheet [3]
>> and
>> SAMA7G5 datasheet (not public at the moment) brings up a difference at
>> register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
>> SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I
>> say
>> it needs a new compatible.
> 
> I can't follow you here. These bits are already used in the current UART
> driver

You're right, I haven't checked the driver.

> and are supported on the LAN966X. So if anything, SAMA5D2,
> SAM9X60
> and SAMA7G5 need a new compatible, no?

It seems that's true unless some errors in datasheet. I'll double check on
my side.

Thank you,
Claudiu Beznea

> 
> -michael
> 
>> Kavya, could you confirm this?
>>
>> Thank you,
>> Claudiu Beznea
>>
>> [1] https://www.microchip.com/
>> [2]
>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>> [3]
>> http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
>> [4]
>> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf
>>
>>
>>>
>>> -michael


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-03-24 16:32 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 16:03 [PATCH v1 0/6] ARM: dts: lan966x: dtsi improvements and KSwitch D10 support Michael Walle
2022-03-03 16:03 ` [PATCH v1 1/6] ARM: dts: lan966x: swap dma channels for crypto node Michael Walle
2022-03-04  8:21   ` Claudiu.Beznea
2022-03-03 16:03 ` [PATCH v1 2/6] ARM: dts: lan966x: add sgpio node Michael Walle
2022-03-04  8:24   ` Claudiu.Beznea
2022-03-03 16:03 ` [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes Michael Walle
2022-03-04  8:30   ` Claudiu.Beznea
2022-03-04 11:01     ` Michael Walle
2022-03-07 11:53       ` Claudiu.Beznea
2022-03-07 12:04         ` Michael Walle
2022-03-18 12:17           ` Claudiu.Beznea
2022-03-22 21:39             ` Michael Walle
2022-03-24 16:32               ` Claudiu.Beznea
2022-03-03 16:03 ` [PATCH v1 4/6] ARM: dts: lan966x: add flexcom SPI nodes Michael Walle
2022-03-04  8:30   ` Claudiu.Beznea
2022-03-03 16:03 ` [PATCH v1 5/6] ARM: dts: lan966x: add flexcom I2C nodes Michael Walle
2022-03-03 16:03 ` [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support Michael Walle
2022-03-04  8:31   ` Claudiu.Beznea
2022-03-04 11:15     ` Michael Walle
2022-03-07 12:07       ` Claudiu.Beznea
2022-03-07 12:17         ` Michael Walle
2022-03-18 12:26           ` Claudiu.Beznea
2022-03-23  8:06     ` Tudor.Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).