Re: [External] Re: [PATCH v4 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 device tree

>> Add initial device tree support for the Lenovo ThinkEdge SE70, an=0A=
>> industrial edge gateway based on the NVIDIA Jetson Xavier NX module=0A=
>> (P3668-0001) with a custom carrier board.=0A=
>>=0A=
>> This initial submission includes:=0A=
>> - 40-pin expansion header pinmux configuration=0A=
>> - External Micro SD card slot with dedicated 3.3V regulator=0A=
>> - I2C bus for 40-pin header=0A=
>>=0A=
>> Static verification passed: dt_binding_check and dtbs compilation=0A=
>> complete without errors.=0A=
>>=0A=
>> Signed-off-by: Jiqi Li <[email protected]>=0A=
>> ---=0A=
>> Changes in v2:=0A=
>> - Remove unused input-related headers from baseboard DTSI=0A=
>>=0A=
>> Changes in v3:=0A=
>> - Merge baseboard content into main DTS, drop separate baseboard DTSI=0A=
>> - Use stdout-path instead of bootargs in chosen node=0A=
>>=0A=
>> Changes in v4:=0A=
>> - Rename regulator node from regulator-sd-3v3 to regulator-vdd-3v3-sd=0A=
>>   to follow the naming convention (requested by Mikko).=0A=
>> - Move regulator nodes to the bottom of the file and order=0A=
>>   alphabetically.=0A=
>> - Keep vdd_5v0_sys as it is referenced by USB and PMIC nodes in the=0A=
>>   SoC and SOM layers (required for compilation).=0A=
>> ---=0A=
>>  arch/arm64/boot/dts/nvidia/Makefile           |   1 +=0A=
>>  .../nvidia/tegra194-lenovo-thinkedge-se70.dts | 113 ++++++++++++++++++=
=0A=
>>  2 files changed, 114 insertions(+)=0A=
>>  create mode 100644 arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge=
-se70.dts=0A=
>>=0A=
>> diff --git a/arch/arm64/boot/dts/nvidia/Makefile b/arch/arm64/boot/dts/n=
vidia/Makefile=0A=
>> index 72c0cb5efa47..736a3f8a923f 100644=0A=
>> --- a/arch/arm64/boot/dts/nvidia/Makefile=0A=
>> +++ b/arch/arm64/boot/dts/nvidia/Makefile=0A=
>> @@ -29,6 +29,7 @@ dtb-$(CONFIG_ARCH_TEGRA_186_SOC) +=3D tegra186-p3509-0=
000+p3636-0001.dtb=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_194_SOC) +=3D tegra194-p2972-0000.dtb=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_194_SOC) +=3D tegra194-p3509-0000+p3668-0000.dt=
b=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_194_SOC) +=3D tegra194-p3509-0000+p3668-0001.dt=
b=0A=
>> +dtb-$(CONFIG_ARCH_TEGRA_194_SOC) +=3D tegra194-lenovo-thinkedge-se70.dt=
b=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_234_SOC) +=3D tegra234-sim-vdk.dtb=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_234_SOC) +=3D tegra234-p3737-0000+p3701-0000.dt=
b=0A=
>>  dtb-$(CONFIG_ARCH_TEGRA_234_SOC) +=3D tegra234-p3737-0000+p3701-0008.dt=
b=0A=
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.d=
ts b/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts=0A=
>> new file mode 100644=0A=
>> index 000000000000..02ac645a7784=0A=
>> --- /dev/null=0A=
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts=0A=
>> @@ -0,0 +1,113 @@=0A=
>> +// SPDX-License-Identifier: GPL-2.0=0A=
>> +/dts-v1/;=0A=
>> +=0A=
>> +#include "tegra194-p3668-0001.dtsi"=0A=
>> +=0A=
>> +#include <dt-bindings/gpio/tegra194-gpio.h>=0A=
>> +=0A=
>> +/ {=0A=
>> +     model =3D "Lenovo ThinkEdge SE70";=0A=
>> +     compatible =3D "lenovo,thinkedge-se70", "nvidia,p3668-0001", "nvid=
ia,tegra194";=0A=
>> +=0A=
>> +     chosen {=0A=
>> +             stdout-path =3D "serial0:115200n8";=0A=
>> +     };=0A=
>=0A=
>Unnecessary: the P3668-0001 dtsi already includes this.=0A=
>=0A=
>> +=0A=
>> +     bus@0 {=0A=
>> +             /* 40-pin expansion header custom pinmux configuration */=
=0A=
>> +             pinmux@2430000 {=0A=
>> +                     status =3D "okay";=0A=
>=0A=
>The pinmux node is already enabled.=0A=
>=0A=
>> +                     pinctrl-names =3D "default";=0A=
>> +                     pinctrl-0 =3D <&pinmux_hdr40>;=0A=
>> +=0A=
>> +                     pinmux_hdr40: pinmux-hdr40 {=0A=
>> +                             pin7 {=0A=
>> +                                     nvidia,pins =3D "aud_mclk_ps4";=0A=
>> +                                     nvidia,function =3D "aud";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_NO=
NE>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_DIS=
ABLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_DISABLE>;=0A=
>> +                             };=0A=
>> +                             pin11 {=0A=
>> +                                     nvidia,pins =3D "uart1_rts_pr4";=
=0A=
>> +                                     nvidia,function =3D "uarta";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_NO=
NE>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_DIS=
ABLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_DISABLE>;=0A=
>> +                             };=0A=
>> +                             pin12 {=0A=
>> +                                     nvidia,pins =3D "dap5_sclk_pt5";=
=0A=
>> +                                     nvidia,function =3D "i2s5";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_DO=
WN>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_DIS=
ABLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_ENABLE>;=0A=
>> +                             };=0A=
>> +                             pin35 {=0A=
>> +                                     nvidia,pins =3D "dap5_fs_pu0";=0A=
>> +                                     nvidia,function =3D "i2s5";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_DO=
WN>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_DIS=
ABLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_ENABLE>;=0A=
>> +                             };=0A=
>> +                             pin36 {=0A=
>> +                                     nvidia,pins =3D "uart1_cts_pr5";=
=0A=
>> +                                     nvidia,function =3D "uarta";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_UP=
>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_ENA=
BLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_ENABLE>;=0A=
>> +                             };=0A=
>> +                             pin38 {=0A=
>> +                                     nvidia,pins =3D "dap5_din_pt7";=0A=
>> +                                     nvidia,function =3D "i2s5";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_DO=
WN>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_ENA=
BLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_ENABLE>;=0A=
>> +                             };=0A=
>> +                             pin40 {=0A=
>> +                                     nvidia,pins =3D "dap5_dout_pt6";=
=0A=
>> +                                     nvidia,function =3D "i2s5";=0A=
>> +                                     nvidia,pull =3D <TEGRA_PIN_PULL_DO=
WN>;=0A=
>> +                                     nvidia,tristate =3D <TEGRA_PIN_DIS=
ABLE>;=0A=
>> +                                     nvidia,enable-input =3D <TEGRA_PIN=
_DISABLE>;=0A=
>> +                             };=0A=
>> +                     };=0A=
>> +             };=0A=
>> +=0A=
>> +             /* Configure i2c bus clock to 400kHz for carrier board per=
ipherals */=0A=
>> +             i2c@3160000 {=0A=
>> +                     clock-frequency =3D <400000>;=0A=
>> +                     status =3D "okay";=0A=
>=0A=
>This is also already enabled.=0A=
>=0A=
>> +             };=0A=
>> +=0A=
>> +             /* SDMMC3: External Micro SD card slot */=0A=
>> +             mmc@3440000 {=0A=
>> +                     bus-width =3D <4>;=0A=
>> +                     cd-gpios =3D <&gpio_aon TEGRA194_AON_GPIO(CC, 4) G=
PIO_ACTIVE_LOW>;=0A=
>> +                     disable-wp;=0A=
>> +                     no-1-8-v;=0A=
>> +                     vmmc-supply =3D <&vdd_3v3_sd>;=0A=
>> +                     status =3D "okay";=0A=
>> +             };=0A=
>=0A=
>Perhaps you want to add an mmc1 alias for this in /aliases.=0A=
>=0A=
>> +     };=0A=
>> +=0A=
>> +     /* Fixed 3.3V power supply regulator for external Micro SD slot */=
=0A=
>> +     vdd_3v3_sd: regulator-vdd-3v3-sd {=0A=
>> +             compatible =3D "regulator-fixed";=0A=
>> +             regulator-name =3D "VDD_3V3_SD";=0A=
>> +             regulator-min-microvolt =3D <3300000>;=0A=
>> +             regulator-max-microvolt =3D <3300000>;=0A=
>> +             gpio =3D <&gpio TEGRA194_MAIN_GPIO(Q, 5) GPIO_ACTIVE_HIGH>=
;=0A=
>> +             regulator-boot-on;=0A=
>> +             enable-active-high;=0A=
>> +     };=0A=
>> +=0A=
>> +     /* System 5V main supply, referenced by SOM and USB nodes */=0A=
>> +     vdd_5v0_sys: regulator-vdd-5v0-sys {=0A=
>> +             compatible =3D "regulator-fixed";=0A=
>> +             regulator-name =3D "VDD_5V_SYS";=0A=
>> +             regulator-min-microvolt =3D <5000000>;=0A=
>> +             regulator-max-microvolt =3D <5000000>;=0A=
>> +             regulator-always-on;=0A=
>> +             regulator-boot-on;=0A=
>> +     };=0A=
>> +};=0A=
>> --=0A=
>> 2.43.0=0A=
>>=0A=
>>=0A=
>=0A=
>So some nits, but overall:=0A=
>=0A=
>Reviewed-by: Mikko Perttunen <[email protected]>=0A=
>=0A=
>Thank you!=0A=
=0A=
Hi Mikko,=0A=
=0A=
Thanks for the review!  I've fixed the nits you pointed out:=0A=
- Dropped the redundant chosen node=0A=
- Removed unnecessary status=3D"okay"=0A=
- Added mmc1 alias=0A=
=0A=
I'll send v5 shortly.=0A=
=0A=
Best regards,=0A=
Jiqi Li=0A=