[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOMZO5CQNHHCUF+i3d710haMUyESeesp4nEs2_E74f1tmzuMZA@mail.gmail.com>
Date: Wed, 14 Aug 2024 12:11:28 -0300
From: Fabio Estevam <festevam@...il.com>
To: Tarang Raval <tarang.raval@...iconsignals.io>
Cc: shawnguo@...nel.org, krzk+dt@...nel.org, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>, devicetree@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: imx8mm-emtop-baseboard: Add Peripherals Support
Hi Tarang,
On Wed, Aug 14, 2024 at 11:51 AM Tarang Raval
<tarang.raval@...iconsignals.io> wrote:
>
> This adds the following peripherals support for the Emtop i.MX8M Mini Baseboard
> * Wi-Fi
> * Audio
> * SD card
> * RTC
> * CAN bus
> * USB OTG
>
> Signed-off-by: Tarang Raval <tarang.raval@...iconsignals.io>
> ---
> .../dts/freescale/imx8mm-emtop-baseboard.dts | 347 ++++++++++++++++++
> .../boot/dts/freescale/imx8mm-emtop-som.dtsi | 1 +
> 2 files changed, 348 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-emtop-baseboard.dts b/arch/arm64/boot/dts/freescale/imx8mm-emtop-baseboard.dts
> index 7d2cb74c64ee..5ce8f21a0b1b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-emtop-baseboard.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-emtop-baseboard.dts
> @@ -11,6 +11,129 @@ / {
> model = "Emtop Embedded Solutions i.MX8M Mini Baseboard V1";
> compatible = "ees,imx8mm-emtop-baseboard", "ees,imx8mm-emtop-som",
> "fsl,imx8mm";
> +
> + extcon_usb: extcon_usb1otg {
extcon_usb: extcon-usb1otg {
> + compatible = "linux,extcon-usb-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_extcon_usb>;
> + id-gpio = <&gpio1 10 GPIO_ACTIVE_HIGH>;
> + enable-gpio = <&gpio1 12 GPIO_ACTIVE_LOW>;
> + };
> +
> + modem_reset: modem-reset {
> + compatible = "gpio-reset";
This is not a valid compatible string.
> + regulators {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <0>;
Please remove this 'regulators' container and describe the regulators directly.
> + reg_audio: regulator-audio-vdd {
reg_audio: regulator-audio {
> + compatible = "regulator-fixed";
> + regulator-name = "wm8904_supply";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + reg_wifi_vmmc: regulator@1 {
This will trigger warnings...
reg_wifi_vmmc: regulator-wifi-vmmc {
> + compatible = "regulator-fixed";
> + regulator-name = "vmmc";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio2 10 GPIO_ACTIVE_HIGH>;
> + off-on-delay = <20000>;
> + startup-delay-us = <100>;
> + enable-active-high;
Move the enable-active-high right after the gpio.
> &fec1 {
> @@ -40,7 +163,130 @@ vddio: vddio-regulator {
> };
> };
>
> +&i2c3 {
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c3>;
> + status = "okay";
> +
> + rx8025: rtc@32 {
> + compatible = "rx8025";
> + reg = <0x32>;
> + };
> +
> + wm8904: wm8904@1a {
Node name should be generic
audio-code@1a {
> +&usdhc1 {
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + pinctrl-0 = <&pinctrl_usdhc1>, <&pinctrl_usdhc1_gpio>;
> + pinctrl-1 = <&pinctrl_usdhc1_100mhz>, <&pinctrl_usdhc1_gpio>;
> + pinctrl-2 = <&pinctrl_usdhc1_200mhz>, <&pinctrl_usdhc1_gpio>;
> + bus-width = <4>;
> + vmmc-supply = <®_wifi_vmmc>;
> + pm-ignore-notify;
This property does not exist upstream. Please remove it.
> +/* USBOTG */
> +&usbotg1 {
> + dr_mode = "otg";
> + extcon = <&extcon_usb>;
> + picophy,pre-emp-curr-control = <3>;
> + picophy,dc-vol-level-adjust = <7>;
These two properties do not exist upstream. Remove them.
> + pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
> + fsl,pins = <
> + MX8MM_IOMUXC_SD1_CLK_USDHC1_CLK 0x196
> + MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD 0x1d6
> + MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0 0x1d6
> + MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1 0x1d6
> + MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2 0x1d6
> + MX8MM_IOMUXC_SD1_DATA3_USDHC1_DATA3 0x1d6
> + >;
> + };
> + pinctrl_usdhc1_gpio: usdhc1grpgpio {
pinctrl_usdhc1_gpio: usdhc1gpiogrp {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-emtop-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-emtop-som.dtsi
> index 67d22d3768aa..9b188075921c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-emtop-som.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-emtop-som.dtsi
> @@ -182,6 +182,7 @@ pinctrl_gpio_led: emtop-gpio-led-grp {
> fsl,pins = <
> MX8MM_IOMUXC_NAND_READY_B_GPIO3_IO16 0x19
> MX8MM_IOMUXC_SAI3_RXC_GPIO4_IO29 0x19
> + MX8MM_IOMUXC_GPIO1_IO15_GPIO1_IO15 0x19 /* canbus_reset */
Adding a CAN transceiver reset GPIO into the gpio-led group looks strange.
Powered by blists - more mailing lists