Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10 support

From: Michael Walle
Date: Mon Mar 07 2022 - 07:17:16 EST


Am 2022-03-07 13:07, schrieb Claudiu.Beznea@xxxxxxxxxxxxx:
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@xxxxxxxxxxxxx:
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@xxxxxxxx>
---
 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