devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board
@ 2023-06-16 11:06 Josua Mayer
  2023-06-16 11:06 ` [PATCH 1/3] arm64: dts: lx2160a: describe the SerDes block #2 Josua Mayer
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 11:06 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Josua Mayer, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Li Yang

Add support for the SolidRun LX2162A System on Module (SoM), and the
Clearfog evaluation board.

This patch-set introduces:
- dt node for lx2160a serdes block #2
- dtsi for lx2162a system on module
- dts for lx2162 clearfog

Firstly Please note that checkpatch was complaining about the EEPROMS:
- DT compatible string "st,24c02" appears un-documented
- DT compatible string "st,24c2048" appears un-documented
- DT compatible string "atmel,24c2048" appears un-documented
However to my eyes these *are* already documented in at24.yaml,
and are also used in existing device-tree.

Secondly the MAINTAINERS file has not been modified.
Is it required I add myself or *someone* for these new dts?

Josua Mayer (3):
  arm64: dts: lx2160a: describe the SerDes block #2
  dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board

 .../devicetree/bindings/arm/fsl.yaml          |   2 +
 arch/arm64/boot/dts/freescale/Makefile        |   1 +
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi |   6 +
 .../dts/freescale/fsl-lx2162a-clearfog.dts    | 369 ++++++++++++++++++
 .../dts/freescale/fsl-lx2162a-sr-som.dtsi     |  78 ++++
 5 files changed, 456 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Li Yang <leoyang.li@nxp.com>
-- 
2.35.3


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

* [PATCH 1/3] arm64: dts: lx2160a: describe the SerDes block #2
  2023-06-16 11:06 [PATCH 0/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board Josua Mayer
@ 2023-06-16 11:06 ` Josua Mayer
  2023-06-16 11:06 ` [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board Josua Mayer
  2023-06-16 11:06 ` [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 " Josua Mayer
  2 siblings, 0 replies; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 11:06 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Josua Mayer, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Li Yang

Add description for the LX2160A second SerDes block.
It is functionally identical to the first one already added in:

3cbe93a "arch: arm64: dts: lx2160a: describe the SerDes block #1"

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index ea6a94b57aeb..2e68c05181dd 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -626,6 +626,12 @@ serdes_1: phy@1ea0000 {
 			#phy-cells = <1>;
 		};
 
+		serdes_2: phy@1eb0000 {
+			compatible = "fsl,lynx-28g";
+			reg = <0x0 0x1eb0000 0x0 0x1e30>;
+			#phy-cells = <1>;
+		};
+
 		crypto: crypto@8000000 {
 			compatible = "fsl,sec-v5.0", "fsl,sec-v4.0";
 			fsl,sec-era = <10>;
-- 
2.35.3


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

* [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 11:06 [PATCH 0/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board Josua Mayer
  2023-06-16 11:06 ` [PATCH 1/3] arm64: dts: lx2160a: describe the SerDes block #2 Josua Mayer
@ 2023-06-16 11:06 ` Josua Mayer
  2023-06-16 11:36   ` Krzysztof Kozlowski
  2023-06-16 11:06 ` [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 " Josua Mayer
  2 siblings, 1 reply; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 11:06 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Josua Mayer, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler, Andreas Kemnade

Add DT compatible for SolidRun LX2162A SoM and Clearfog board.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 15d411084065..438a4ece8157 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -1373,9 +1373,11 @@ properties:
       - description: SolidRun LX2160A based Boards
         items:
           - enum:
+              - solidrun,clearfog
               - solidrun,clearfog-cx
               - solidrun,honeycomb
           - const: solidrun,lx2160a-cex7
+          - const: solidrun,lx2162a-som
           - const: fsl,lx2160a
 
       - description: S32G2 based Boards
-- 
2.35.3


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

* [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board
  2023-06-16 11:06 [PATCH 0/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board Josua Mayer
  2023-06-16 11:06 ` [PATCH 1/3] arm64: dts: lx2160a: describe the SerDes block #2 Josua Mayer
  2023-06-16 11:06 ` [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board Josua Mayer
@ 2023-06-16 11:06 ` Josua Mayer
  2023-06-16 11:39   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 11:06 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Josua Mayer, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Li Yang

Add support for the SolidRun LX2162A System on Module (SoM), and the
Clearfog evaluation board.

The SoM has few software-controllable features:
- AR8035 Ethernet PHY
- eMMC
- SPI Flash
- fan controller
- various eeproms

The Clearfog evaluation board provides:
- microSD connector
- USB-A
- 2x 10Gbps SFP+
- 2x 25Gbps SFP+ with a retimer
- 8x 2.5Gbps RJ45
- 2x mPCI (assembly option / disables 2xRJ45)

The 8x RJ45 ports are connected with an 8-port PHY: Marvell 88E2580
supporting up to 5Gbps, while SoC and magnetics are limited to 2.5Gbps.

However 2500 speed is untested due to documentation and drivier
limitations. To avoid confusion the phy nodes have been explicitly
limited to 1000 for now.

The PCI nodes are disabled, but explicitly added to mark that this board
can have pci.
It is expected that the bootloader will patch the status property
"okay" and disable 2x RJ45 ports, according to active serdes configuration.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/freescale/Makefile        |   1 +
 .../dts/freescale/fsl-lx2162a-clearfog.dts    | 369 ++++++++++++++++++
 .../dts/freescale/fsl-lx2162a-sr-som.dtsi     |  78 ++++
 3 files changed, 448 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index ef7d17aef58f..b4fb5044d1c7 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-honeycomb.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-qds.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-clearfog.dtb
 
 fsl-ls1028a-qds-13bb-dtbs := fsl-ls1028a-qds.dtb fsl-ls1028a-qds-13bb.dtbo
 fsl-ls1028a-qds-65bb-dtbs := fsl-ls1028a-qds.dtb fsl-ls1028a-qds-65bb.dtbo
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts b/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
new file mode 100644
index 000000000000..550693b74e19
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+//
+// Device Tree file for LX2162A Clearfog
+//
+// Copyright 2023 Josua Mayer <josua@solid-run.com>
+
+/dts-v1/;
+
+#include "fsl-lx2160a.dtsi"
+#include "fsl-lx2162a-sr-som.dtsi"
+
+/ {
+	model = "SolidRun LX2162A Clearfog";
+	compatible = "solidrun,clearfog", "solidrun,lx2162a-som", "fsl,lx2160a";
+
+	aliases {
+		crypto = &crypto;
+		i2c0 = &i2c0;
+		i2c1 = &i2c2;
+		i2c2 = &i2c4;
+		i2c3 = &sfp_i2c0;
+		i2c4 = &sfp_i2c1;
+		i2c5 = &sfp_i2c2;
+		i2c6 = &sfp_i2c3;
+		i2c7 = &mpcie1_i2c;
+		i2c8 = &mpcie0_i2c;
+		i2c9 = &pcieclk_i2c;
+		mmc0 = &esdhc0;
+		mmc1 = &esdhc1;
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led_sfp_at: led-sfp-at {
+			gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* PROC_IRQ5 */
+			default-state = "off";
+		};
+		led_sfp_ab: led-sfp-ab {
+			gpios = <&gpio2 11 GPIO_ACTIVE_HIGH>; /* PROC_IRQ11 */
+			default-state = "off";
+		};
+		led_sfp_bt: led-sfp-bt {
+			gpios = <&gpio2 13 GPIO_ACTIVE_HIGH>; /* EVT1_B */
+			default-state = "off";
+		};
+		led_sfp_bb: led-sfp-bb {
+			gpios = <&gpio2 14 GPIO_ACTIVE_HIGH>; /* EVT2_B */
+			default-state = "off";
+		};
+	};
+
+	sfp_at: sfp-at {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp_i2c0>;
+		mod-def0-gpio = <&gpio2 16 GPIO_ACTIVE_LOW>; /* EVT4_B */
+		maximum-power-milliwatt = <2000>;
+	};
+
+	sfp_ab: sfp-ab {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp_i2c1>;
+		mod-def0-gpio = <&gpio2 1 GPIO_ACTIVE_LOW>; /* PROC_IRQ1 */
+		maximum-power-milliwatt = <2000>;
+	};
+
+	sfp_bt: sfp-bt {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp_i2c2>;
+		mod-def0-gpio = <&gpio2 10 GPIO_ACTIVE_LOW>; /* PROC_IRQ10 */
+		maximum-power-milliwatt = <2000>;
+	};
+
+	sfp_bb: sfp-bb {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp_i2c3>;
+		mod-def0-gpio = <&gpio2 15 GPIO_ACTIVE_LOW>; /* EVT3_B */
+		maximum-power-milliwatt = <2000>;
+	};
+};
+
+&i2c2 {
+	/* retimer: ds250df410@18 */
+
+	i2c-switch@70 {
+		compatible = "nxp,pca9546";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x70>;
+		i2c-mux-idle-disconnect;
+
+		sfp_i2c0: i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		sfp_i2c1: i2c@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+
+		sfp_i2c2: i2c@2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+		};
+
+		sfp_i2c3: i2c@3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+		};
+	};
+
+	i2c-switch@71 {
+		compatible = "nxp,pca9546";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x71>;
+		i2c-mux-idle-disconnect;
+
+		mpcie1_i2c: i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		mpcie0_i2c: i2c@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+
+		pcieclk_i2c: i2c@2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+
+			/* pcieclk: si53154@6b */
+		};
+	};
+};
+
+&dpmac3 {
+	sfp = <&sfp_at>;
+	managed = "in-band-status";
+	phys = <&serdes_1 7>;
+};
+
+&dpmac4 {
+	sfp = <&sfp_ab>;
+	managed = "in-band-status";
+	phys = <&serdes_1 6>;
+};
+
+&dpmac5 {
+	sfp = <&sfp_bt>;
+	managed = "in-band-status";
+	phys = <&serdes_1 5>;
+};
+
+&dpmac6 {
+	sfp = <&sfp_bb>;
+	managed = "in-band-status";
+	phys = <&serdes_1 4>;
+};
+
+&dpmac11 {
+	status = "okay";
+	phys = <&serdes_2 0>;
+	phy-handle = <&ethernet_phy2>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac12 {
+	status = "okay";
+	phys = <&serdes_2 1>;
+	phy-handle = <&ethernet_phy0>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac13 {
+	status = "okay";
+	phys = <&serdes_2 6>;
+	phy-handle = <&ethernet_phy5>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac14 {
+	status = "okay";
+	phys = <&serdes_2 7>;
+	phy-handle = <&ethernet_phy7>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac15 {
+	status = "okay";
+	phys = <&serdes_2 4>;
+	phy-handle = <&ethernet_phy3>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac16 {
+	status = "okay";
+	phys = <&serdes_2 5>;
+	phy-handle = <&ethernet_phy1>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac17 {
+	/* override connection to on-SoM phy */
+	/delete-property/ phy-handle;
+	/delete-property/ phy-connection-type;
+
+	status = "okay";
+	phys = <&serdes_2 2>;
+	phy-handle = <&ethernet_phy4>;
+	phy-connection-type = "rgmii";
+};
+
+&dpmac18 {
+	status = "okay";
+	phys = <&serdes_2 3>;
+	phy-handle = <&ethernet_phy6>;
+	phy-connection-type = "rgmii";
+};
+
+&emdio1 {
+	/*
+	 * SoM has a phy at address 1 connected to SoC Ethernet Controller 1.
+	 * It competes for WRIOP MAC17, and no connector has been wired.
+	 */
+	/delete-node/ ethernet-phy@1;
+
+	ethernet_phy0: mv88e2580@8 {
+		reg = <8>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy1: mv88e2580@9 {
+		reg = <9>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy2: mv88e2580@a {
+		reg = <10>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy3: mv88e2580@b {
+		reg = <11>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy4: mv88e2580@c {
+		reg = <12>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy5: mv88e2580@d {
+		reg = <13>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy6: mv88e2580@e {
+		reg = <14>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+
+	ethernet_phy7: mv88e2580@f {
+		reg = <15>;
+		compatible = "ethernet-phy-ieee802.3-c45";
+		max-speed = <1000>;
+	};
+};
+
+&esdhc0 {
+	status = "okay";
+	sd-uhs-sdr104;
+	sd-uhs-sdr50;
+	sd-uhs-sdr25;
+	sd-uhs-sdr12;
+};
+
+&pcie3 {
+	status = "disabled";
+};
+
+&pcie4 {
+	status = "disabled";
+};
+
+&pcs_mdio3 {
+	status = "okay";
+};
+
+&pcs_mdio4 {
+	status = "okay";
+};
+
+&pcs_mdio5 {
+	status = "okay";
+};
+
+&pcs_mdio6 {
+	status = "okay";
+};
+
+&pcs_mdio11 {
+	status = "okay";
+};
+
+&pcs_mdio12 {
+	status = "okay";
+};
+
+&pcs_mdio13 {
+	status = "okay";
+};
+
+&pcs_mdio14 {
+	status = "okay";
+};
+
+&pcs_mdio15 {
+	status = "okay";
+};
+
+&pcs_mdio16 {
+	status = "okay";
+};
+
+&pcs_mdio17 {
+	status = "okay";
+};
+
+&pcs_mdio18 {
+	status = "okay";
+};
+
+&serdes_1 {
+	status = "okay";
+};
+
+&serdes_2 {
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&usb0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
new file mode 100644
index 000000000000..49e03b6600d6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+//
+// Device Tree file for LX2162A-SOM
+//
+// Copyright 2021 Rabeeh Khoury <rabeeh@solid-run.com>
+// Copyright 2023 Josua Mayer <josua@solid-run.com>
+
+&crypto {
+	status = "okay";
+};
+
+&dpmac17 {
+	phy-handle = <&ethernet_phy0>;
+	phy-connection-type = "rgmii-id";
+};
+
+&emdio1 {
+	status = "okay";
+
+	ethernet_phy0: ethernet-phy@1 {
+		reg = <1>;
+	};
+};
+
+&esdhc1 {
+	status = "okay";
+	bus-width = <8>;
+	mmc-hs200-1_8v;
+	mmc-hs400-1_8v;
+};
+
+&fspi {
+	status = "okay";
+
+	flash@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "m25p80", "jedec,spi-nor";
+		m25p,fast-read;
+		spi-max-frequency = <50000000>;
+		reg = <0>;
+		/* The following setting enables 1-1-8 (CMD-ADDR-DATA) mode */
+		spi-rx-bus-width = <8>;
+		// spi-tx-bus-width = <8>;
+	};
+};
+
+&i2c0 {
+	status = "okay";
+
+	fan-controller@18 {
+		compatible = "ti,amc6821";
+		reg = <0x18>;
+		cooling-min-state = <0>;
+		cooling-max-state = <9>;
+		#cooling-cells = <2>;
+	};
+
+	ddr_spd: eeprom@51 {
+		reg = <0x57>;
+		compatible = "st,24c02", "atmel,24c02";
+		read-only;
+	};
+
+	config_eeprom: eeprom@57 {
+		reg = <0x57>;
+		compatible = "st,24c02", "atmel,24c02";
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	variable_eeprom: eeprom@54 {
+		reg = <0x54>;
+		compatible = "st,24c2048", "atmel,24c2048";
+	};
+};
-- 
2.35.3


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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 11:06 ` [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board Josua Mayer
@ 2023-06-16 11:36   ` Krzysztof Kozlowski
  2023-06-16 13:32     ` Josua Mayer
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16 11:36 UTC (permalink / raw)
  To: Josua Mayer, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler, Andreas Kemnade

On 16/06/2023 13:06, Josua Mayer wrote:
> Add DT compatible for SolidRun LX2162A SoM and Clearfog board.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 15d411084065..438a4ece8157 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -1373,9 +1373,11 @@ properties:
>        - description: SolidRun LX2160A based Boards
>          items:
>            - enum:
> +              - solidrun,clearfog
>                - solidrun,clearfog-cx
>                - solidrun,honeycomb
>            - const: solidrun,lx2160a-cex7
> +          - const: solidrun,lx2162a-som
>            - const: fsl,lx2160a

You change existing entries, breaking boards and changing the meaning,
without any explanation in commit msg. That's not how it is done. Please
provide rationale in commit msg.

Best regards,
Krzysztof


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

* Re: [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board
  2023-06-16 11:06 ` [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 " Josua Mayer
@ 2023-06-16 11:39   ` Krzysztof Kozlowski
  2023-06-16 12:57     ` Josua Mayer
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16 11:39 UTC (permalink / raw)
  To: Josua Mayer, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang

On 16/06/2023 13:06, Josua Mayer wrote:
> Add support for the SolidRun LX2162A System on Module (SoM), and the
> Clearfog evaluation board.
> 
> The SoM has few software-controllable features:
> - AR8035 Ethernet PHY
> - eMMC
> - SPI Flash
> - fan controller
> - various eeproms
> 

Thank you for your patch. There is something to discuss/improve.

> +
> +&i2c2 {
> +	/* retimer: ds250df410@18 */
> +
> +	i2c-switch@70 {
> +		compatible = "nxp,pca9546";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x70>;

reg is usually just after compatible.

...

> +
> +&emdio1 {
> +	/*
> +	 * SoM has a phy at address 1 connected to SoC Ethernet Controller 1.
> +	 * It competes for WRIOP MAC17, and no connector has been wired.
> +	 */
> +	/delete-node/ ethernet-phy@1;
> +
> +	ethernet_phy0: mv88e2580@8 {

Node names should be generic. See also explanation and list of examples
in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		reg = <8>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy1: mv88e2580@9 {
> +		reg = <9>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy2: mv88e2580@a {
> +		reg = <10>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy3: mv88e2580@b {
> +		reg = <11>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy4: mv88e2580@c {
> +		reg = <12>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy5: mv88e2580@d {
> +		reg = <13>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy6: mv88e2580@e {
> +		reg = <14>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +
> +	ethernet_phy7: mv88e2580@f {
> +		reg = <15>;
> +		compatible = "ethernet-phy-ieee802.3-c45";
> +		max-speed = <1000>;
> +	};
> +};
> +
> +&esdhc0 {
> +	status = "okay";
> +	sd-uhs-sdr104;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr25;
> +	sd-uhs-sdr12;
> +};
> +
> +&pcie3 {
> +	status = "disabled";
> +};
> +
> +&pcie4 {
> +	status = "disabled";
> +};
> +
> +&pcs_mdio3 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio4 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio5 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio6 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio11 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio12 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio13 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio14 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio15 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio16 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio17 {
> +	status = "okay";
> +};
> +
> +&pcs_mdio18 {
> +	status = "okay";
> +};
> +
> +&serdes_1 {
> +	status = "okay";
> +};
> +
> +&serdes_2 {
> +	status = "okay";
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&usb0 {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
> new file mode 100644
> index 000000000000..49e03b6600d6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +//
> +// Device Tree file for LX2162A-SOM
> +//
> +// Copyright 2021 Rabeeh Khoury <rabeeh@solid-run.com>
> +// Copyright 2023 Josua Mayer <josua@solid-run.com>
> +
> +&crypto {
> +	status = "okay";
> +};
> +
> +&dpmac17 {
> +	phy-handle = <&ethernet_phy0>;
> +	phy-connection-type = "rgmii-id";
> +};
> +
> +&emdio1 {
> +	status = "okay";
> +
> +	ethernet_phy0: ethernet-phy@1 {
> +		reg = <1>;
> +	};
> +};
> +
> +&esdhc1 {
> +	status = "okay";
> +	bus-width = <8>;
> +	mmc-hs200-1_8v;
> +	mmc-hs400-1_8v;
> +};
> +
> +&fspi {
> +	status = "okay";
> +
> +	flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "m25p80", "jedec,spi-nor";
> +		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
> +		reg = <0>;

Fix the order of properties. compatible is always first, then the reg.

> +		/* The following setting enables 1-1-8 (CMD-ADDR-DATA) mode */
> +		spi-rx-bus-width = <8>;
> +		// spi-tx-bus-width = <8>;

Don't add dead code without appropriate comment why it is dead.

> +	};
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	fan-controller@18 {
> +		compatible = "ti,amc6821";
> +		reg = <0x18>;
> +		cooling-min-state = <0>;
> +		cooling-max-state = <9>;
> +		#cooling-cells = <2>;
> +	};
> +
> +	ddr_spd: eeprom@51 {
> +		reg = <0x57>;
> +		compatible = "st,24c02", "atmel,24c02";

Ditto

> +		read-only;
> +	};
> +
> +	config_eeprom: eeprom@57 {
> +		reg = <0x57>;
> +		compatible = "st,24c02", "atmel,24c02";

Ditto

> +	};
> +};
> +
> +&i2c4 {
> +	status = "okay";
> +
> +	variable_eeprom: eeprom@54 {
> +		reg = <0x54>;
> +		compatible = "st,24c2048", "atmel,24c2048";

ditto

> +	};
> +};

Best regards,
Krzysztof


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

* Re: [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board
  2023-06-16 11:39   ` Krzysztof Kozlowski
@ 2023-06-16 12:57     ` Josua Mayer
  2023-06-16 16:59       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 12:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang

Hi Krzysztof,

Thank you for the comments!
Before sending a v2, I will:
- move all "reg" properties just after "compatible"
- give the phy nodes generic names
- remove dead code / review spi-nor-flash node

Am 16.06.23 um 14:39 schrieb Krzysztof Kozlowski:
> On 16/06/2023 13:06, Josua Mayer wrote:
>> Add support for the SolidRun LX2162A System on Module (SoM), and the
>> Clearfog evaluation board.
>>
>> The SoM has few software-controllable features:
>> - AR8035 Ethernet PHY
>> - eMMC
>> - SPI Flash
>> - fan controller
>> - various eeproms
>>
> Thank you for your patch. There is something to discuss/improve.
>
>> +
>> +&i2c2 {
>> +	/* retimer: ds250df410@18 */
>> +
>> +	i2c-switch@70 {
>> +		compatible = "nxp,pca9546";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		reg = <0x70>;
> reg is usually just after compatible.
>
> ...
>
>> +
>> +&emdio1 {
>> +	/*
>> +	 * SoM has a phy at address 1 connected to SoC Ethernet Controller 1.
>> +	 * It competes for WRIOP MAC17, and no connector has been wired.
>> +	 */
>> +	/delete-node/ ethernet-phy@1;
>> +
>> +	ethernet_phy0: mv88e2580@8 {
> Node names should be generic. See also explanation and list of examples
> in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
>> +		reg = <8>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy1: mv88e2580@9 {
>> +		reg = <9>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy2: mv88e2580@a {
>> +		reg = <10>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy3: mv88e2580@b {
>> +		reg = <11>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy4: mv88e2580@c {
>> +		reg = <12>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy5: mv88e2580@d {
>> +		reg = <13>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy6: mv88e2580@e {
>> +		reg = <14>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +
>> +	ethernet_phy7: mv88e2580@f {
>> +		reg = <15>;
>> +		compatible = "ethernet-phy-ieee802.3-c45";
>> +		max-speed = <1000>;
>> +	};
>> +};
>> +
>> +&esdhc0 {
>> +	status = "okay";
>> +	sd-uhs-sdr104;
>> +	sd-uhs-sdr50;
>> +	sd-uhs-sdr25;
>> +	sd-uhs-sdr12;
>> +};
>> +
>> +&pcie3 {
>> +	status = "disabled";
>> +};
>> +
>> +&pcie4 {
>> +	status = "disabled";
>> +};
>> +
>> +&pcs_mdio3 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio4 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio5 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio6 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio11 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio12 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio13 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio14 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio15 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio16 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio17 {
>> +	status = "okay";
>> +};
>> +
>> +&pcs_mdio18 {
>> +	status = "okay";
>> +};
>> +
>> +&serdes_1 {
>> +	status = "okay";
>> +};
>> +
>> +&serdes_2 {
>> +	status = "okay";
>> +};
>> +
>> +&uart0 {
>> +	status = "okay";
>> +};
>> +
>> +&usb0 {
>> +	status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
>> new file mode 100644
>> index 000000000000..49e03b6600d6
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
>> @@ -0,0 +1,78 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +//
>> +// Device Tree file for LX2162A-SOM
>> +//
>> +// Copyright 2021 Rabeeh Khoury <rabeeh@solid-run.com>
>> +// Copyright 2023 Josua Mayer <josua@solid-run.com>
>> +
>> +&crypto {
>> +	status = "okay";
>> +};
>> +
>> +&dpmac17 {
>> +	phy-handle = <&ethernet_phy0>;
>> +	phy-connection-type = "rgmii-id";
>> +};
>> +
>> +&emdio1 {
>> +	status = "okay";
>> +
>> +	ethernet_phy0: ethernet-phy@1 {
>> +		reg = <1>;
>> +	};
>> +};
>> +
>> +&esdhc1 {
>> +	status = "okay";
>> +	bus-width = <8>;
>> +	mmc-hs200-1_8v;
>> +	mmc-hs400-1_8v;
>> +};
>> +
>> +&fspi {
>> +	status = "okay";
>> +
>> +	flash@0 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "m25p80", "jedec,spi-nor";
>> +		m25p,fast-read;
>> +		spi-max-frequency = <50000000>;
>> +		reg = <0>;
> Fix the order of properties. compatible is always first, then the reg.
>
>> +		/* The following setting enables 1-1-8 (CMD-ADDR-DATA) mode */
>> +		spi-rx-bus-width = <8>;
>> +		// spi-tx-bus-width = <8>;
> Don't add dead code without appropriate comment why it is dead.
>
>> +	};
>> +};
>> +
>> +&i2c0 {
>> +	status = "okay";
>> +
>> +	fan-controller@18 {
>> +		compatible = "ti,amc6821";
>> +		reg = <0x18>;
>> +		cooling-min-state = <0>;
>> +		cooling-max-state = <9>;
>> +		#cooling-cells = <2>;
>> +	};
>> +
>> +	ddr_spd: eeprom@51 {
>> +		reg = <0x57>;
>> +		compatible = "st,24c02", "atmel,24c02";
> Ditto
>
>> +		read-only;
>> +	};
>> +
>> +	config_eeprom: eeprom@57 {
>> +		reg = <0x57>;
>> +		compatible = "st,24c02", "atmel,24c02";
> Ditto
>
>> +	};
>> +};
>> +
>> +&i2c4 {
>> +	status = "okay";
>> +
>> +	variable_eeprom: eeprom@54 {
>> +		reg = <0x54>;
>> +		compatible = "st,24c2048", "atmel,24c2048";
> ditto
>
>> +	};
>> +};
> Best regards,
> Krzysztof
>
sincerely
Josua Mayer

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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 11:36   ` Krzysztof Kozlowski
@ 2023-06-16 13:32     ` Josua Mayer
  2023-06-16 16:57       ` Andreas Kemnade
  2023-06-16 16:59       ` Krzysztof Kozlowski
  0 siblings, 2 replies; 13+ messages in thread
From: Josua Mayer @ 2023-06-16 13:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler, Andreas Kemnade

HI Krzysztof,

Am 16.06.23 um 14:36 schrieb Krzysztof Kozlowski:
> On 16/06/2023 13:06, Josua Mayer wrote:
>> Add DT compatible for SolidRun LX2162A SoM and Clearfog board.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>   Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>> index 15d411084065..438a4ece8157 100644
>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>> @@ -1373,9 +1373,11 @@ properties:
>>         - description: SolidRun LX2160A based Boards
>>           items:
>>             - enum:
>> +              - solidrun,clearfog
>>                 - solidrun,clearfog-cx
>>                 - solidrun,honeycomb
>>             - const: solidrun,lx2160a-cex7
>> +          - const: solidrun,lx2162a-som
>>             - const: fsl,lx2160a
> You change existing entries, breaking boards and changing the meaning,
> without any explanation in commit msg. That's not how it is done. Please
> provide rationale in commit msg.

I'm sorry. Given your comment I think I did not understand how these 
entries are supposed to work.
So perhaps you can provide some guidance based on my explanation?:

- NXP LX2162 is a smaller physical package of the same LX2160 SoC, with 
reduced IOs and some silicon blocks disabled.
- SolidRun LX2162 SoM is essentially a different form factor of LX2160 CEX
- SolidRun LX2162 Clearfog is the reference platform for the SoM. 
Despite it's naming similarity to clearfog-cx, it has a different 
feature set more similar to SolidRun Armada 388 Clearfog Pro

So I believed I could just add to the existing entry "SolidRun LX2160A 
based Boards" also the new LX2162 Board & SoM.
I see now that adding a fourth const messes upthe existing 3-part 
compatible for those already existing boards.

Please can you confirm if it would have been more correct to replace 
"const: solidrun,lx2160a-cex7" with an enum?:
enum:
   - solidrun,lx2160a-cex7
   - solidrun,lx2162a-som

Finally, is it okay to add a "solidrun,clearfog" given my explanation 
above, or should it be more specific "solidrun,lx2162a-clearfog"?

> Best regards,
> Krzysztof
Sincerely
Josua Mayer

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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 13:32     ` Josua Mayer
@ 2023-06-16 16:57       ` Andreas Kemnade
  2023-06-17 13:34         ` Josua Mayer
  2023-06-16 16:59       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 13+ messages in thread
From: Andreas Kemnade @ 2023-06-16 16:57 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler

On Fri, 16 Jun 2023 16:32:01 +0300
Josua Mayer <josua@solid-run.com> wrote:

> HI Krzysztof,
> 
> Am 16.06.23 um 14:36 schrieb Krzysztof Kozlowski:
> > On 16/06/2023 13:06, Josua Mayer wrote:  
> >> Add DT compatible for SolidRun LX2162A SoM and Clearfog board.
> >>
> >> Signed-off-by: Josua Mayer <josua@solid-run.com>
> >> ---
> >>   Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> >> index 15d411084065..438a4ece8157 100644
> >> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> >> @@ -1373,9 +1373,11 @@ properties:
> >>         - description: SolidRun LX2160A based Boards
> >>           items:
> >>             - enum:
> >> +              - solidrun,clearfog
> >>                 - solidrun,clearfog-cx
> >>                 - solidrun,honeycomb
> >>             - const: solidrun,lx2160a-cex7
> >> +          - const: solidrun,lx2162a-som
> >>             - const: fsl,lx2160a  
> > You change existing entries, breaking boards and changing the meaning,
> > without any explanation in commit msg. That's not how it is done. Please
> > provide rationale in commit msg.  
> 
> I'm sorry. Given your comment I think I did not understand how these 
> entries are supposed to work.
> So perhaps you can provide some guidance based on my explanation?:
> 

it breaks:
arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts
compatible = "solidrun,clearfog-cx",
                "solidrun,lx2160a-cex7", "fsl,lx2160a";

now you would require:
compatible = "solidrun,clearfog-cx",
                "solidrun,lx2160a-cex7", "solidrun,lx2162a-som", "fsl,lx2160a"

which is probably not what you want.

Regards,
Andreas

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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 13:32     ` Josua Mayer
  2023-06-16 16:57       ` Andreas Kemnade
@ 2023-06-16 16:59       ` Krzysztof Kozlowski
  2023-06-17 14:09         ` Josua Mayer
  1 sibling, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16 16:59 UTC (permalink / raw)
  To: Josua Mayer, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler, Andreas Kemnade

On 16/06/2023 15:32, Josua Mayer wrote:
> HI Krzysztof,
> 
> Am 16.06.23 um 14:36 schrieb Krzysztof Kozlowski:
>> On 16/06/2023 13:06, Josua Mayer wrote:
>>> Add DT compatible for SolidRun LX2162A SoM and Clearfog board.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>>   Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index 15d411084065..438a4ece8157 100644
>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> @@ -1373,9 +1373,11 @@ properties:
>>>         - description: SolidRun LX2160A based Boards
>>>           items:
>>>             - enum:
>>> +              - solidrun,clearfog
>>>                 - solidrun,clearfog-cx
>>>                 - solidrun,honeycomb
>>>             - const: solidrun,lx2160a-cex7
>>> +          - const: solidrun,lx2162a-som
>>>             - const: fsl,lx2160a
>> You change existing entries, breaking boards and changing the meaning,
>> without any explanation in commit msg. That's not how it is done. Please
>> provide rationale in commit msg.
> 
> I'm sorry. Given your comment I think I did not understand how these 
> entries are supposed to work.
> So perhaps you can provide some guidance based on my explanation?:
> 
> - NXP LX2162 is a smaller physical package of the same LX2160 SoC, with 
> reduced IOs and some silicon blocks disabled.
> - SolidRun LX2162 SoM is essentially a different form factor of LX2160 CEX
> - SolidRun LX2162 Clearfog is the reference platform for the SoM. 
> Despite it's naming similarity to clearfog-cx, it has a different 
> feature set more similar to SolidRun Armada 388 Clearfog Pro
> 
> So I believed I could just add to the existing entry "SolidRun LX2160A 
> based Boards" also the new LX2162 Board & SoM.

But you added much more, didn't you?

> I see now that adding a fourth const messes upthe existing 3-part 
> compatible for those already existing boards.
> 
> Please can you confirm if it would have been more correct to replace 
> "const: solidrun,lx2160a-cex7" with an enum?:
> enum:
>    - solidrun,lx2160a-cex7
>    - solidrun,lx2162a-som
> 
> Finally, is it okay to add a "solidrun,clearfog" given my explanation 
> above, or should it be more specific "solidrun,lx2162a-clearfog"?
> 

Test the binding and test DTS against it:
Please run `make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/

It might point you to answer.

Why do you make solidrun,honeycomb compatible with cex7 and som?

Best regards,
Krzysztof


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

* Re: [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board
  2023-06-16 12:57     ` Josua Mayer
@ 2023-06-16 16:59       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16 16:59 UTC (permalink / raw)
  To: Josua Mayer, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang

On 16/06/2023 14:57, Josua Mayer wrote:
> Hi Krzysztof,
> 
> Thank you for the comments!
> Before sending a v2, I will:
> - move all "reg" properties just after "compatible"
> - give the phy nodes generic names
> - remove dead code / review spi-nor-flash node
> 

You forgot testing.

Best regards,
Krzysztof


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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 16:57       ` Andreas Kemnade
@ 2023-06-17 13:34         ` Josua Mayer
  0 siblings, 0 replies; 13+ messages in thread
From: Josua Mayer @ 2023-06-17 13:34 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler

Hi Andreas,

Am 16.06.23 um 19:57 schrieb Andreas Kemnade:
> On Fri, 16 Jun 2023 16:32:01 +0300
> Josua Mayer <josua@solid-run.com> wrote:
>
>> HI Krzysztof,
>>
>> Am 16.06.23 um 14:36 schrieb Krzysztof Kozlowski:
>>> On 16/06/2023 13:06, Josua Mayer wrote:
>>>> Add DT compatible for SolidRun LX2162A SoM and Clearfog board.
>>>>
>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> index 15d411084065..438a4ece8157 100644
>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> @@ -1373,9 +1373,11 @@ properties:
>>>>          - description: SolidRun LX2160A based Boards
>>>>            items:
>>>>              - enum:
>>>> +              - solidrun,clearfog
>>>>                  - solidrun,clearfog-cx
>>>>                  - solidrun,honeycomb
>>>>              - const: solidrun,lx2160a-cex7
>>>> +          - const: solidrun,lx2162a-som
>>>>              - const: fsl,lx2160a
>>> You change existing entries, breaking boards and changing the meaning,
>>> without any explanation in commit msg. That's not how it is done. Please
>>> provide rationale in commit msg.
>> I'm sorry. Given your comment I think I did not understand how these
>> entries are supposed to work.
>> So perhaps you can provide some guidance based on my explanation?:
>>
> it breaks:
> arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts
> compatible = "solidrun,clearfog-cx",
>                  "solidrun,lx2160a-cex7", "fsl,lx2160a";
>
> now you would require:
> compatible = "solidrun,clearfog-cx",
>                  "solidrun,lx2160a-cex7", "solidrun,lx2162a-som", "fsl,lx2160a"
I see, thanks!
The more I look at it, the more logical this behaviour seems.
> which is probably not what you want.

Yep. I wanted to keep the 3 components, and allow forking in the middle:

"solidrun,<board>", "solidrun,<module>", "fsl,lx2160a"

This however creates many combinations that are undesirable.
Existing boards using com express don't suddenly become compatible with 
the new SoM.
Therefore I prepared a different approach vor v2 now.

> Regards,
> Andreas
- Josua Mayer

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

* Re: [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board
  2023-06-16 16:59       ` Krzysztof Kozlowski
@ 2023-06-17 14:09         ` Josua Mayer
  0 siblings, 0 replies; 13+ messages in thread
From: Josua Mayer @ 2023-06-17 14:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Li Yang, Marek Vasut, Fabio Estevam, Stefan Wahren,
	Frieder Schrempf, Marcel Ziswiler, Andreas Kemnade

Hi Krzysztof,

Am 16.06.23 um 19:59 schrieb Krzysztof Kozlowski:
> Test the binding and test DTS against it:
> Please run `make dtbs_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
This was helpful! I ran it a few times and studied the results.
Many complaints related to lx2160a.dtsi, some common among different 
layerscape based dts.
Finally some mistakes of my own.

v2 addresses all that were under my control, and a common one that I 
understood well enough.

> It might point you to answer.
>
> Why do you make solidrun,honeycomb compatible with cex7 and som?

Exactly - unintentional.


> Best regards,
> Krzysztof
- Josua Mayer

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

end of thread, other threads:[~2023-06-17 14:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-16 11:06 [PATCH 0/3] arm64: dts: freescale: Add support for LX2162 SoM & Clearfog Board Josua Mayer
2023-06-16 11:06 ` [PATCH 1/3] arm64: dts: lx2160a: describe the SerDes block #2 Josua Mayer
2023-06-16 11:06 ` [PATCH 2/3] dt-bindings: arm: Add SolidRun LX2162A SoM & Clearfog Board Josua Mayer
2023-06-16 11:36   ` Krzysztof Kozlowski
2023-06-16 13:32     ` Josua Mayer
2023-06-16 16:57       ` Andreas Kemnade
2023-06-17 13:34         ` Josua Mayer
2023-06-16 16:59       ` Krzysztof Kozlowski
2023-06-17 14:09         ` Josua Mayer
2023-06-16 11:06 ` [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 " Josua Mayer
2023-06-16 11:39   ` Krzysztof Kozlowski
2023-06-16 12:57     ` Josua Mayer
2023-06-16 16:59       ` Krzysztof Kozlowski

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).