[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c25630ca-c791-287f-36a4-ebc0559d6541@linaro.org>
Date: Fri, 16 Jun 2023 13:39:38 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Josua Mayer <josua@...id-run.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc: Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Li Yang <leoyang.li@....com>
Subject: Re: [PATCH 3/3] arm64: dts: freescale: Add support for LX2162 SoM &
Clearfog Board
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@...id-run.com>
> +// Copyright 2023 Josua Mayer <josua@...id-run.com>
> +
> +&crypto {
> + status = "okay";
> +};
> +
> +&dpmac17 {
> + phy-handle = <ðernet_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
Powered by blists - more mailing lists