lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<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 = <&reg_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

Powered by Openwall GNU/*/Linux Powered by OpenVZ