[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87980643-44b4-4df9-9eb7-1583b5074bdd@ysoft.com>
Date: Wed, 25 Sep 2024 13:30:31 +0200
From: Michal Vokáč <michal.vokac@...ft.com>
To: Marco Felsch <m.felsch@...gutronix.de>
Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Fabio Estevam <festevam@...il.com>, Michael Walle <mwalle@...nel.org>,
devicetree@...r.kernel.org, Alexander Stein
<alexander.stein@...tq-group.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Mathieu Othacehe <m.othacehe@...il.com>,
Hugo Villeneuve <hvilleneuve@...onoff.com>, linux-kernel@...r.kernel.org,
Hiago De Franco <hiago.franco@...adex.com>,
Herburger <gregor.herburger@...tq-group.com>,
Petr Benes <petr.benes@...ft.com>, linux-usb@...r.kernel.org,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Joao Paulo Goncalves <joao.goncalves@...adex.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, imx@...ts.linux.dev,
Shawn Guo <shawnguo@...nel.org>, linux-arm-kernel@...ts.infradead.org,
Frank Li <Frank.li@....com>
Subject: Re: [PATCH v4 2/2] arm64: dts: imx: Add imx8mp-iota2-lumpy board
On 24. 09. 24 19:37, Marco Felsch wrote:
> Hi Frank,
>
> On 24-09-24, Frank Li wrote:
>> On Tue, Sep 24, 2024 at 12:39:41PM +0200, Michal Vokáč wrote:
>>> The IOTA2 Lumpy board is based on the i.MX8MPlus EVK.
>>>
>>> Basic features are:
>>> - 4GB LPDDR4
>>> - 64GB eMMC
>>> - 2x 1GB Ethernet
>>> - USB 3.0 Type-C dual role port, without power delivery
>>> - USB 3.0 Type-A host port
>>> - RGB LED - PWM driven
>>> - speaker - PWM driven
>>> - RTC with super capacitor backup
>>>
>>> Signed-off-by: Michal Vokáč <michal.vokac@...ft.com>
>>> ---
>>> v4:
>>> - Moved the iomuxc node to the end of the file.
>>> - Moved the bus-width and non-removeable properties below
>>> the pinctrl-* properties in &usdhc3 node.
>>> - Moved the fsl,ext-reset-output below the pinctrl-* properties
>>> in &wdog1 node.
>>> v3:
>>> - Dropped pinctrl-names property from &usb_dwc3_1 node.
>>> v2:
>>> - Dropped unused property from pwm4 node.
>>> - Sorted all nodes and properties using dt-format tool from Frank.
>>>
>>> arch/arm64/boot/dts/freescale/Makefile | 1 +
>>> .../boot/dts/freescale/imx8mp-iota2-lumpy.dts | 423 ++++++++++++++++++
>>
>> Suggest use https://github.com/lznuaa/dt-format
>> sort node. any issue, let me know.
>
> Thanks for the link :) would be nice to have this script to be part of
> the kernel. The script follows the rules in [1] I'm just used to have
> common properties like pinctrl-* in front of the device specific
> properties e.g. "enable-active-high". But this rule is not part of [1]
> so I can't blame the script.
>
> Regards,
> Marco
>
> [1] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
Thank you for the review Frank & Marco.
I quickly went through the file again and found another few properties
that could be better ordered according to the kernel documentation [1].
>>> 2 files changed, 424 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
>>> index 9d3df8b218a2..aa26a50b7bb4 100644
>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>> @@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
>>> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-iota2-lumpy.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-navqp.dtb
>>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts b/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts
>>> new file mode 100644
>>> index 000000000000..9eb58e818dc7
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-iota2-lumpy.dts
>>> @@ -0,0 +1,423 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright 2023 Y Soft
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "imx8mp.dtsi"
>>> +
>>> +/ {
>>> + compatible = "ysoft,imx8mp-iota2-lumpy", "fsl,imx8mp";
>>> + model = "Y Soft i.MX8MPlus IOTA2 Lumpy board";
>>> +
>>> + beeper {
>>> + compatible = "pwm-beeper";
>>> + pwms = <&pwm4 0 500000 0>;
>>> + };
>>> +
>>> + chosen {
>>> + stdout-path = &uart2;
>>> + };
>>> +
>>> + gpio_keys: gpio-keys {
>>> + compatible = "gpio-keys";
>>> + pinctrl-0 = <&pinctrl_gpio_keys>;
>>> + pinctrl-names = "default";
>>> +
>>> + button-reset {
>>> + gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
>>> + label = "Factory RESET";
>>> + linux,code = <BTN_0>;
>>> + };
>>> + };
>>> +
>>> + reg_usb_host: regulator-usb-host {
>>> + compatible = "regulator-fixed";
>>> + enable-active-high;
>>> + gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
The enable-active-high and gpio should go bellow regulator-*.
>>> + pinctrl-0 = <&pinctrl_usb_host_vbus>;
>>> + pinctrl-names = "default";
>>> + regulator-max-microvolt = <5000000>;
>>> + regulator-min-microvolt = <5000000>;
>>> + regulator-name = "usb-host";
>>> + };
>>> +
>>> + memory@...00000 {
>>> + reg = <0x0 0x40000000 0 0x80000000>,
>>> + <0x1 0x00000000 0 0x80000000>;
>>> + device_type = "memory";
>>> + };
>>> +};
>>> +
>>> +&A53_0 {
>>> + cpu-supply = <®_arm>;
>>> +};
>>> +
>>> +&A53_1 {
>>> + cpu-supply = <®_arm>;
>>> +};
>>> +
>>> +&A53_2 {
>>> + cpu-supply = <®_arm>;
>>> +};
>>> +
>>> +&A53_3 {
>>> + cpu-supply = <®_arm>;
>>> +};
>>> +
>>> +&eqos {
>>> + phy-handle = <ðphy0>;
>>> + phy-mode = "rgmii-id";
>>> + pinctrl-0 = <&pinctrl_eqos>;
>>> + pinctrl-names = "default";
>>> + status = "okay";
>>> +
>>> + mdio {
>>> + compatible = "snps,dwmac-mdio";
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + ethphy0: ethernet-phy@0 {
>>> + reg = <0>;
>>> + interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
>>> + interrupt-parent = <&gpio3>;
>>> + micrel,led-mode = <0>;
The micrel,* is a vendor specific property. It should go bellow the reset-*.
>>> + pinctrl-0 = <&pinctrl_ethphy0>;
>>> + pinctrl-names = "default";
>>> + reset-assert-us = <1000>;
>>> + reset-deassert-us = <1000>;
>>> + reset-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
>>> + };
>>> + };
>>> +};
>>> +
>>> +&fec {
>>> + fsl,magic-packet;
>>> + phy-handle = <ðphy1>;
>>> + phy-mode = "rgmii-id";
>>> + pinctrl-0 = <&pinctrl_fec>;
>>> + pinctrl-names = "default";
>>> + status = "okay";
>>> +
>>> + mdio {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + ethphy1: ethernet-phy@0 {
>>> + reg = <0>;
>>> + interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
>>> + interrupt-parent = <&gpio3>;
>>> + micrel,led-mode = <0>;
Same as above, micrel,* should go bellow common properties.
I will send a v5 with these fixed.
Michal
>>> + pinctrl-0 = <&pinctrl_ethphy1>;
>>> + pinctrl-names = "default";
>>> + reset-assert-us = <1000>;
>>> + reset-deassert-us = <1000>;
>>> + reset-gpios = <&gpio3 20 GPIO_ACTIVE_LOW>;
>>> + };
>>> + };
>>> +};
>>> +
Powered by blists - more mailing lists