| Message ID | 20221003083705.22495-1-dan.scally@ideasonboard.com |
|---|---|
| Headers | show |
| Series | Debix Model A board devicetree | expand |
On 03/10/2022 10:37, Daniel Scally wrote: > Add a device tree file describing the Debix Model A board from > Polyhex Technology Co. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > arch/arm64/boot/dts/freescale/Makefile | 1 + > .../dts/freescale/imx8mp-debix-model-a.dts | 550 ++++++++++++++++++ > 2 files changed, 551 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile > index 238a83e5b8c6..f26e802cef82 100644 > --- a/arch/arm64/boot/dts/freescale/Makefile > +++ b/arch/arm64/boot/dts/freescale/Makefile > @@ -79,6 +79,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > new file mode 100644 > index 000000000000..6d0fa3930d0b > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > @@ -0,0 +1,550 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright 2019 NXP > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/leds/common.h> > +#include <dt-bindings/usb/pd.h> > + > +#include "imx8mp.dtsi" > + > +/ { > + model = "Polyhex Debix Model A (2GB) i.MX8MPlus board"; > + compatible = "polyhex,imx8mp-debix-modela2gb", "fsl,imx8mp"; > + > + chosen { > + stdout-path = &uart2; > + }; > + > + gpio-leds { > + compatible = "gpio-leds"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_gpio_led>; > + > + status-led { > + function = LED_FUNCTION_POWER; > + color = <LED_COLOR_ID_RED>; > + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>; > + default-state = "on"; > + }; > + }; > + > + reg_usdhc2_vmmc: regulator-usdhc2 { > + compatible = "regulator-fixed"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>; > + regulator-name = "VSD_3V3"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > +}; > + > +&A53_0 { > + cpu-supply = <&buck2>; > +}; > + > +&A53_1 { > + cpu-supply = <&buck2>; > +}; > + > +&A53_2 { > + cpu-supply = <&buck2>; > +}; > + > +&A53_3 { > + cpu-supply = <&buck2>; > +}; > + > +&eqos { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_eqos>; > + phy-connection-type = "rgmii-id"; > + phy-handle = <ðphy0>; > + status = "okay"; > + > + mdio { > + compatible = "snps,dwmac-mdio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethphy0: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>; > + reset-assert-us = <20>; > + reset-deassert-us = <200000>; > + }; > + }; > +}; > + > +&fec { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_fec>; > + phy-connection-type = "rgmii-id"; > + phy-handle = <ðphy1>; > + fsl,magic-packet; > + status = "okay"; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethphy1: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + reset-gpios = <&gpio4 19 GPIO_ACTIVE_LOW>; > + reset-assert-us = <10>; > + reset-deassert-us = <150>; > + }; > + }; > +}; > + > +&i2c1 { > + clock-frequency = <400000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c1>; > + status = "okay"; > + > + pmic@25 { > + reg = <0x25>; > + compatible = "nxp,pca9450c"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_pmic>; > + interrupt-parent = <&gpio1>; > + interrupts = <3 GPIO_ACTIVE_LOW>; Nope, wrong flag. This is a hint for us that you based your DTS on something ancient (e.g. broken downstream stuff). Please start from *scratch* and write your DTS based on a board taken from mainline. There is no point for us to repeat review for all the things we already fixed in mainline. > + > + regulators { > + buck1: BUCK1 { > + regulator-name = "BUCK1"; > + regulator-min-microvolt = <600000>; > + regulator-max-microvolt = <2187500>; > + regulator-boot-on; > + regulator-always-on; > + regulator-ramp-delay = <3125>; > + }; > + > + buck2: BUCK2 { > + regulator-name = "BUCK2"; > + regulator-min-microvolt = <600000>; > + regulator-max-microvolt = <2187500>; > + regulator-boot-on; > + regulator-always-on; > + regulator-ramp-delay = <3125>; > + nxp,dvs-run-voltage = <950000>; > + nxp,dvs-standby-voltage = <850000>; > + }; > + > + buck4: BUCK4{ > + regulator-name = "BUCK4"; > + regulator-min-microvolt = <600000>; > + regulator-max-microvolt = <3400000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + buck5: BUCK5{ > + regulator-name = "BUCK5"; > + regulator-min-microvolt = <600000>; > + regulator-max-microvolt = <3400000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + buck6: BUCK6 { > + regulator-name = "BUCK6"; > + regulator-min-microvolt = <600000>; > + regulator-max-microvolt = <3400000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo1: LDO1 { > + regulator-name = "LDO1"; > + regulator-min-microvolt = <1600000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo2: LDO2 { > + regulator-name = "LDO2"; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1150000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo3: LDO3 { > + regulator-name = "LDO3"; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo4: LDO4 { > + regulator-name = "LDO4"; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + ldo5: LDO5 { > + regulator-name = "LDO5"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + }; > + }; > +}; > + > +&i2c2 { > + clock-frequency = <100000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c2>; > + status = "okay"; > +}; > + > +&i2c3 { > + clock-frequency = <400000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c3>; > + status = "okay"; > +}; > + > +&i2c4 { > + clock-frequency = <100000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c4>; > + status = "okay"; > + > + eeprom@50 { > + compatible = "atmel,24c02"; > + reg = <0x50>; > + pagesize = <16>; > + }; > + > + hym8563@51 { Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Best regards, Krzysztof
Hi Krzysztof On 03/10/2022 10:26, Krzysztof Kozlowski wrote: > On 03/10/2022 10:37, Daniel Scally wrote: >> Add a device tree file describing the Debix Model A board from >> Polyhex Technology Co. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> arch/arm64/boot/dts/freescale/Makefile | 1 + >> .../dts/freescale/imx8mp-debix-model-a.dts | 550 ++++++++++++++++++ >> 2 files changed, 551 insertions(+) >> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >> >> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile >> index 238a83e5b8c6..f26e802cef82 100644 >> --- a/arch/arm64/boot/dts/freescale/Makefile >> +++ b/arch/arm64/boot/dts/freescale/Makefile >> @@ -79,6 +79,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb >> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb >> dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >> new file mode 100644 >> index 000000000000..6d0fa3930d0b >> --- /dev/null >> +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >> @@ -0,0 +1,550 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +/* >> + * Copyright 2019 NXP >> + */ >> + >> +/dts-v1/; >> + >> +#include <dt-bindings/gpio/gpio.h> >> +#include <dt-bindings/leds/common.h> >> +#include <dt-bindings/usb/pd.h> >> + >> +#include "imx8mp.dtsi" >> + >> +/ { >> + model = "Polyhex Debix Model A (2GB) i.MX8MPlus board"; >> + compatible = "polyhex,imx8mp-debix-modela2gb", "fsl,imx8mp"; >> + >> + chosen { >> + stdout-path = &uart2; >> + }; >> + >> + gpio-leds { >> + compatible = "gpio-leds"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_gpio_led>; >> + >> + status-led { >> + function = LED_FUNCTION_POWER; >> + color = <LED_COLOR_ID_RED>; >> + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>; >> + default-state = "on"; >> + }; >> + }; >> + >> + reg_usdhc2_vmmc: regulator-usdhc2 { >> + compatible = "regulator-fixed"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>; >> + regulator-name = "VSD_3V3"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>; >> + enable-active-high; >> + }; >> +}; >> + >> +&A53_0 { >> + cpu-supply = <&buck2>; >> +}; >> + >> +&A53_1 { >> + cpu-supply = <&buck2>; >> +}; >> + >> +&A53_2 { >> + cpu-supply = <&buck2>; >> +}; >> + >> +&A53_3 { >> + cpu-supply = <&buck2>; >> +}; >> + >> +&eqos { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_eqos>; >> + phy-connection-type = "rgmii-id"; >> + phy-handle = <ðphy0>; >> + status = "okay"; >> + >> + mdio { >> + compatible = "snps,dwmac-mdio"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + ethphy0: ethernet-phy@0 { >> + compatible = "ethernet-phy-ieee802.3-c22"; >> + reg = <0>; >> + reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>; >> + reset-assert-us = <20>; >> + reset-deassert-us = <200000>; >> + }; >> + }; >> +}; >> + >> +&fec { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_fec>; >> + phy-connection-type = "rgmii-id"; >> + phy-handle = <ðphy1>; >> + fsl,magic-packet; >> + status = "okay"; >> + >> + mdio { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + ethphy1: ethernet-phy@0 { >> + compatible = "ethernet-phy-ieee802.3-c22"; >> + reg = <0>; >> + reset-gpios = <&gpio4 19 GPIO_ACTIVE_LOW>; >> + reset-assert-us = <10>; >> + reset-deassert-us = <150>; >> + }; >> + }; >> +}; >> + >> +&i2c1 { >> + clock-frequency = <400000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c1>; >> + status = "okay"; >> + >> + pmic@25 { >> + reg = <0x25>; >> + compatible = "nxp,pca9450c"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_pmic>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <3 GPIO_ACTIVE_LOW>; > Nope, wrong flag. This is a hint for us that you based your DTS on > something ancient (e.g. broken downstream stuff). Please start from > *scratch* and write your DTS based on a board taken from mainline. > > There is no point for us to repeat review for all the things we already > fixed in mainline. Ack - I'll start it from scratch, thanks. > > >> + >> + regulators { >> + buck1: BUCK1 { >> + regulator-name = "BUCK1"; >> + regulator-min-microvolt = <600000>; >> + regulator-max-microvolt = <2187500>; >> + regulator-boot-on; >> + regulator-always-on; >> + regulator-ramp-delay = <3125>; >> + }; >> + >> + buck2: BUCK2 { >> + regulator-name = "BUCK2"; >> + regulator-min-microvolt = <600000>; >> + regulator-max-microvolt = <2187500>; >> + regulator-boot-on; >> + regulator-always-on; >> + regulator-ramp-delay = <3125>; >> + nxp,dvs-run-voltage = <950000>; >> + nxp,dvs-standby-voltage = <850000>; >> + }; >> + >> + buck4: BUCK4{ >> + regulator-name = "BUCK4"; >> + regulator-min-microvolt = <600000>; >> + regulator-max-microvolt = <3400000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + buck5: BUCK5{ >> + regulator-name = "BUCK5"; >> + regulator-min-microvolt = <600000>; >> + regulator-max-microvolt = <3400000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + buck6: BUCK6 { >> + regulator-name = "BUCK6"; >> + regulator-min-microvolt = <600000>; >> + regulator-max-microvolt = <3400000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + ldo1: LDO1 { >> + regulator-name = "LDO1"; >> + regulator-min-microvolt = <1600000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + ldo2: LDO2 { >> + regulator-name = "LDO2"; >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <1150000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + ldo3: LDO3 { >> + regulator-name = "LDO3"; >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + ldo4: LDO4 { >> + regulator-name = "LDO4"; >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + >> + ldo5: LDO5 { >> + regulator-name = "LDO5"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> + }; >> + }; >> +}; >> + >> +&i2c2 { >> + clock-frequency = <100000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c2>; >> + status = "okay"; >> +}; >> + >> +&i2c3 { >> + clock-frequency = <400000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c3>; >> + status = "okay"; >> +}; >> + >> +&i2c4 { >> + clock-frequency = <100000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c4>; >> + status = "okay"; >> + >> + eeprom@50 { >> + compatible = "atmel,24c02"; >> + reg = <0x50>; >> + pagesize = <16>; >> + }; >> + >> + hym8563@51 { > Node names should be generic. > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > Best regards, > Krzysztof >
On Mon, Oct 03, 2022 at 11:26:54AM +0200, Krzysztof Kozlowski wrote: > On 03/10/2022 10:37, Daniel Scally wrote: > > Add a device tree file describing the Debix Model A board from > > Polyhex Technology Co. > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > --- > > arch/arm64/boot/dts/freescale/Makefile | 1 + > > .../dts/freescale/imx8mp-debix-model-a.dts | 550 ++++++++++++++++++ > > 2 files changed, 551 insertions(+) > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile > > index 238a83e5b8c6..f26e802cef82 100644 > > --- a/arch/arm64/boot/dts/freescale/Makefile > > +++ b/arch/arm64/boot/dts/freescale/Makefile > > @@ -79,6 +79,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb > > dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > new file mode 100644 > > index 000000000000..6d0fa3930d0b > > --- /dev/null > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts > > @@ -0,0 +1,550 @@ > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > +/* > > + * Copyright 2019 NXP > > + */ > > + > > +/dts-v1/; > > + > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/leds/common.h> > > +#include <dt-bindings/usb/pd.h> > > + > > +#include "imx8mp.dtsi" > > + > > +/ { > > + model = "Polyhex Debix Model A (2GB) i.MX8MPlus board"; > > + compatible = "polyhex,imx8mp-debix-modela2gb", "fsl,imx8mp"; > > + > > + chosen { > > + stdout-path = &uart2; > > + }; > > + > > + gpio-leds { > > + compatible = "gpio-leds"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_gpio_led>; > > + > > + status-led { > > + function = LED_FUNCTION_POWER; > > + color = <LED_COLOR_ID_RED>; > > + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>; > > + default-state = "on"; > > + }; > > + }; > > + > > + reg_usdhc2_vmmc: regulator-usdhc2 { > > + compatible = "regulator-fixed"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>; > > + regulator-name = "VSD_3V3"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > +}; > > + > > +&A53_0 { > > + cpu-supply = <&buck2>; > > +}; > > + > > +&A53_1 { > > + cpu-supply = <&buck2>; > > +}; > > + > > +&A53_2 { > > + cpu-supply = <&buck2>; > > +}; > > + > > +&A53_3 { > > + cpu-supply = <&buck2>; > > +}; > > + > > +&eqos { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_eqos>; > > + phy-connection-type = "rgmii-id"; > > + phy-handle = <ðphy0>; > > + status = "okay"; > > + > > + mdio { > > + compatible = "snps,dwmac-mdio"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ethphy0: ethernet-phy@0 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <0>; > > + reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>; > > + reset-assert-us = <20>; > > + reset-deassert-us = <200000>; > > + }; > > + }; > > +}; > > + > > +&fec { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_fec>; > > + phy-connection-type = "rgmii-id"; > > + phy-handle = <ðphy1>; > > + fsl,magic-packet; > > + status = "okay"; > > + > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ethphy1: ethernet-phy@0 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <0>; > > + reset-gpios = <&gpio4 19 GPIO_ACTIVE_LOW>; > > + reset-assert-us = <10>; > > + reset-deassert-us = <150>; > > + }; > > + }; > > +}; > > + > > +&i2c1 { > > + clock-frequency = <400000>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_i2c1>; > > + status = "okay"; > > + > > + pmic@25 { > > + reg = <0x25>; > > + compatible = "nxp,pca9450c"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_pmic>; > > + interrupt-parent = <&gpio1>; > > + interrupts = <3 GPIO_ACTIVE_LOW>; > > Nope, wrong flag. This is a hint for us that you based your DTS on > something ancient (e.g. broken downstream stuff). Please start from > *scratch* and write your DTS based on a board taken from mainline. > > There is no point for us to repeat review for all the things we already > fixed in mainline. We can't fully do that, as we don't have access to the schematics for the board. What we can do, however, is to compare this .dts to others from mainline and investigate all differences to see if they can be bugs. > > + > > + regulators { > > + buck1: BUCK1 { > > + regulator-name = "BUCK1"; > > + regulator-min-microvolt = <600000>; > > + regulator-max-microvolt = <2187500>; > > + regulator-boot-on; > > + regulator-always-on; > > + regulator-ramp-delay = <3125>; > > + }; > > + > > + buck2: BUCK2 { > > + regulator-name = "BUCK2"; > > + regulator-min-microvolt = <600000>; > > + regulator-max-microvolt = <2187500>; > > + regulator-boot-on; > > + regulator-always-on; > > + regulator-ramp-delay = <3125>; > > + nxp,dvs-run-voltage = <950000>; > > + nxp,dvs-standby-voltage = <850000>; > > + }; > > + > > + buck4: BUCK4{ > > + regulator-name = "BUCK4"; > > + regulator-min-microvolt = <600000>; > > + regulator-max-microvolt = <3400000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + buck5: BUCK5{ > > + regulator-name = "BUCK5"; > > + regulator-min-microvolt = <600000>; > > + regulator-max-microvolt = <3400000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + buck6: BUCK6 { > > + regulator-name = "BUCK6"; > > + regulator-min-microvolt = <600000>; > > + regulator-max-microvolt = <3400000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + ldo1: LDO1 { > > + regulator-name = "LDO1"; > > + regulator-min-microvolt = <1600000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + ldo2: LDO2 { > > + regulator-name = "LDO2"; > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <1150000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + ldo3: LDO3 { > > + regulator-name = "LDO3"; > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + ldo4: LDO4 { > > + regulator-name = "LDO4"; > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + ldo5: LDO5 { > > + regulator-name = "LDO5"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + }; > > + }; > > +}; > > + > > +&i2c2 { > > + clock-frequency = <100000>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_i2c2>; > > + status = "okay"; > > +}; > > + > > +&i2c3 { > > + clock-frequency = <400000>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_i2c3>; > > + status = "okay"; > > +}; > > + > > +&i2c4 { > > + clock-frequency = <100000>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_i2c4>; > > + status = "okay"; > > + > > + eeprom@50 { > > + compatible = "atmel,24c02"; > > + reg = <0x50>; > > + pagesize = <16>; > > + }; > > + > > + hym8563@51 { > > Node names should be generic. > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
On 03/10/2022 14:06, Laurent Pinchart wrote: > On Mon, Oct 03, 2022 at 11:26:54AM +0200, Krzysztof Kozlowski wrote: >> On 03/10/2022 10:37, Daniel Scally wrote: >>> Add a device tree file describing the Debix Model A board from >>> Polyhex Technology Co. >>> >>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >>> --- >>> arch/arm64/boot/dts/freescale/Makefile | 1 + >>> .../dts/freescale/imx8mp-debix-model-a.dts | 550 ++++++++++++++++++ >>> 2 files changed, 551 insertions(+) >>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >>> >>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile >>> index 238a83e5b8c6..f26e802cef82 100644 >>> --- a/arch/arm64/boot/dts/freescale/Makefile >>> +++ b/arch/arm64/boot/dts/freescale/Makefile >>> @@ -79,6 +79,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mn-tqma8mqnl-mba8mx.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb >>> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-debix-model-a.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb >>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >>> new file mode 100644 >>> index 000000000000..6d0fa3930d0b >>> --- /dev/null >>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-debix-model-a.dts >>> @@ -0,0 +1,550 @@ >>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >>> +/* >>> + * Copyright 2019 NXP >>> + */ >>> + >>> +/dts-v1/; >>> + >>> +#include <dt-bindings/gpio/gpio.h> >>> +#include <dt-bindings/leds/common.h> >>> +#include <dt-bindings/usb/pd.h> >>> + >>> +#include "imx8mp.dtsi" >>> + >>> +/ { >>> + model = "Polyhex Debix Model A (2GB) i.MX8MPlus board"; >>> + compatible = "polyhex,imx8mp-debix-modela2gb", "fsl,imx8mp"; >>> + >>> + chosen { >>> + stdout-path = &uart2; >>> + }; >>> + >>> + gpio-leds { >>> + compatible = "gpio-leds"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_gpio_led>; >>> + >>> + status-led { >>> + function = LED_FUNCTION_POWER; >>> + color = <LED_COLOR_ID_RED>; >>> + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>; >>> + default-state = "on"; >>> + }; >>> + }; >>> + >>> + reg_usdhc2_vmmc: regulator-usdhc2 { >>> + compatible = "regulator-fixed"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>; >>> + regulator-name = "VSD_3V3"; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>; >>> + enable-active-high; >>> + }; >>> +}; >>> + >>> +&A53_0 { >>> + cpu-supply = <&buck2>; >>> +}; >>> + >>> +&A53_1 { >>> + cpu-supply = <&buck2>; >>> +}; >>> + >>> +&A53_2 { >>> + cpu-supply = <&buck2>; >>> +}; >>> + >>> +&A53_3 { >>> + cpu-supply = <&buck2>; >>> +}; >>> + >>> +&eqos { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_eqos>; >>> + phy-connection-type = "rgmii-id"; >>> + phy-handle = <ðphy0>; >>> + status = "okay"; >>> + >>> + mdio { >>> + compatible = "snps,dwmac-mdio"; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + ethphy0: ethernet-phy@0 { >>> + compatible = "ethernet-phy-ieee802.3-c22"; >>> + reg = <0>; >>> + reset-gpios = <&gpio4 18 GPIO_ACTIVE_LOW>; >>> + reset-assert-us = <20>; >>> + reset-deassert-us = <200000>; >>> + }; >>> + }; >>> +}; >>> + >>> +&fec { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_fec>; >>> + phy-connection-type = "rgmii-id"; >>> + phy-handle = <ðphy1>; >>> + fsl,magic-packet; >>> + status = "okay"; >>> + >>> + mdio { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + ethphy1: ethernet-phy@0 { >>> + compatible = "ethernet-phy-ieee802.3-c22"; >>> + reg = <0>; >>> + reset-gpios = <&gpio4 19 GPIO_ACTIVE_LOW>; >>> + reset-assert-us = <10>; >>> + reset-deassert-us = <150>; >>> + }; >>> + }; >>> +}; >>> + >>> +&i2c1 { >>> + clock-frequency = <400000>; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_i2c1>; >>> + status = "okay"; >>> + >>> + pmic@25 { >>> + reg = <0x25>; >>> + compatible = "nxp,pca9450c"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_pmic>; >>> + interrupt-parent = <&gpio1>; >>> + interrupts = <3 GPIO_ACTIVE_LOW>; >> >> Nope, wrong flag. This is a hint for us that you based your DTS on >> something ancient (e.g. broken downstream stuff). Please start from >> *scratch* and write your DTS based on a board taken from mainline. >> >> There is no point for us to repeat review for all the things we already >> fixed in mainline. > > We can't fully do that, as we don't have access to the schematics for > the board. What we can do, however, is to compare this .dts to others > from mainline and investigate all differences to see if they can be > bugs. The upstreaming process is rather to take a DTS from the mainline and customize it. Add stuff, remove something, change other nodes. Several pieces are similar between boards, like the PMIC. I think this patch was done in other way - entire PMIC was taken from downstream - therefore I recommend the previous approach. It does not mean that everything should be done from 0 but rather based on mainline DTS to avoid the same mistakes we already fixed. Many times... Best regards, Krzysztof