Re: [PATCH 5/5] arm64: dts: freescale: Add support for bytesatwork i.MX93

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential
issues to consider:
- [High] arm64: dts: freescale: inverted GT911 GPIO polarity in
  touchscreen@5d nodes
- [Medium] arm64: dts: freescale: GPIO pinctrl groups assigned to wrong
  device nodes
- [Medium] arm64: dts: freescale: missing disable-over-current on
  usbotg1 and usbotg2
- [Low] arm64: dts: freescale: invalid ethernet-phy node names and
  deprecated property
- [Low] arm64: dts: freescale: underscores in pinctrl node names and
  non-generic gpio name
- [Low] arm64: dts: freescale: carrier board duplication between
  bytedevkit v1 and v2
--

commit 1273f35fed75113bbeefbcede52975d930b5ccbf
Author: Frank Obergfell <frank.obergfell@bytesatwork.ch>

arm64: dts: freescale: Add support for bytesatwork i.MX93

This commit adds devicetree support for the bytesatwork i.MX93 byteengine
module and the bytedevkit v1.0 and v2.0 carrier boards.

> diff --git a/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v1-0.dts b/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v1-0.dts
> new file mode 100644
> index 0000000000000..9662dd2c38e66
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v1-0.dts

[ ... ]

> +	reg_usb_otg1_vbus: regulator-otg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "USB1_VBUS";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio2 11 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_usb2_vbus: regulator-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "USB2_VBUS";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio2 18 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-always-on;
> +	};

[Severity: Medium]
Should pinctrl_usb1 and pinctrl_usb2 be assigned to reg_usb_otg1_vbus and
reg_usb2_vbus rather than to &usbotg1 and &usbotg2, with vbus-supply =
<&reg_usb2_vbus> added to &usbotg2?

On i.MX93, pinctrl-imx does not implement .gpio_request_enable, so a pad is
only muxed when the device node owning its pinctrl-0 property probes.
Because &usbotg1 depends on reg_usb_otg1_vbus and reg_usb2_vbus is marked
regulator-always-on, both regulators request and drive their GPIOs before
&usbotg1 or &usbotg2 probes.

Similarly, panel@0 under &dsi drives reset-gpios (<&gpio2 10>) in
ili9806e_probe() before acquiring its backlight, while that pad is muxed in
pinctrl_mipi_backlight on mipi_bl (in both v1-0 and v2-0), and the interrupt
pad for tca6418e@34 in imx93-byteengine.dtsi is muxed in pinctrl_lpi2c2
instead of on the GPIO expander node.

[ ... ]

> +		ethphy_qos: ethernet-phyqos@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			enet-phy-lane-no-swap;
> +			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> +			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> +			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> +			ti,dp83867-rxctrl-strap-quirk;
> +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> +		};
> +	};
> +};
> +
> +&fec {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&pinctrl_fec>;
> +	pinctrl-1 = <&pinctrl_fec_sleep>;
> +	phy-mode = "rgmii-id";
> +	phy-handle = <&ethphy_fec>;
> +	fsl,magic-packet;
> +	status = "okay";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clock-frequency = <5000000>;
> +
> +		ethphy_fec: ethernet-phyfec@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			enet-phy-lane-no-swap;
> +			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> +			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> +			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> +			ti,dp83867-rxctrl-strap-quirk;
> +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> +		};

[Severity: Low]
Should ethernet-phyqos@0 and ethernet-phyfec@0 (and ethernet-phyfec@c in
imx93-bytedevkit-v2-0.dts) be named ethernet-phy@0 and ethernet-phy@c?
Documentation/devicetree/bindings/net/ethernet-phy.yaml selects nodes using
^ethernet-phy(@[a-f0-9]+)?$, so these names cause dtbs_check to fail schema
matching for ethernet-phy-ieee802.3-c22.

Also, ti,fifo-depth is deprecated in ti,dp83867.yaml in favor of
tx-fifo-depth.

[ ... ]

> +	/* LVDS Touch */
> +	touchscreen@5d {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_lvds_touch>;
> +		compatible = "goodix,gt911";
> +		reg = <0x5d>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> +		irq-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
> +		reset-gpios = <&gpio2 21 GPIO_ACTIVE_LOW>;
> +		touchscreen-inverted-x;
> +		touchscreen-inverted-y;
> +	};
> +};
> +
> +&lpi2c2 {
> +	/* Mipi Touch */
> +	touchscreen@5d {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_mipi_touch>;
> +		compatible = "goodix,gt911";
> +		reg = <0x5d>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> +		irq-gpios = <&gpio2 21 GPIO_ACTIVE_LOW>;
> +		reset-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
> +	};
> +};

[Severity: High]
Does marking irq-gpios and reset-gpios as active-low in the touchscreen@5d
nodes (here and in imx93-bytedevkit-v2-0.dts) leave the gt911 held in reset
during probe?

In drivers/input/touchscreen/goodix.c, goodix_reset_no_int_sync() asserts
reset with gpiod_direction_output(ts->gpiod_rst, 0), drives the interrupt
line with gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14) to
strap the I2C address, and deasserts reset with
gpiod_direction_output(ts->gpiod_rst, 1).  With active-low polarity flags,
gpiolib inverts these logical levels, strapping the controller to address
0x14 instead of 0x5d and driving the physical reset line low when
goodix_i2c_test() runs.  Both properties should use active-high flags.

[ ... ]

> +&usbotg2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb2>;
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usbotg1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb1>;
> +	dr_mode = "otg";
> +	vbus-supply = <&reg_usb_otg1_vbus>;
> +	status = "okay";
> +};

[Severity: Medium]
Should &usbotg1 and &usbotg2 specify disable-over-current in both
imx93-bytedevkit-v1-0.dts and imx93-bytedevkit-v2-0.dts?

When disable-over-current is absent, usbmisc_imx7d_init() enables hardware
over-current detection, but the only i.MX93 over-current pads (enet1_td1 and
enet1_td2) are muxed to &eqos in pinctrl_eqos.

[ ... ]

> +	pinctrl_lvds_backlight: lvds_backlight-grp {
> +		fsl,pins = <
> +			MX93_PAD_GPIO_IO26__GPIO2_IO26		0x31e	/* LVDS Backlight */
> +		>;
> +	};
> +
> +	pinctrl_lvds_touch: lvds_touch-grp {
> +		fsl,pins = <
> +			MX93_PAD_GPIO_IO20__GPIO2_IO20		0x31e	/* LVDS Touch INT */
> +			MX93_PAD_GPIO_IO21__GPIO2_IO21		0x31e	/* LVDS Touch RST */
> +		>;
> +	};
> +
> +	pinctrl_mipi_touch: mipi_touch-grp {
> +		fsl,pins = <
> +			MX93_PAD_GPIO_IO21__GPIO2_IO21		0x31e	/* MIPI Touch INT */
> +			MX93_PAD_GPIO_IO20__GPIO2_IO20		0x31e	/* MIPI Touch RST */
> +		>;
> +	};
> +
> +	pinctrl_mipi_backlight: mipi_bl-grp {

[Severity: Low]
These pinctrl node names (and those in imx93-bytedevkit-v2-0.dts) contain
underscores, which triggers dtc node_name_chars_strict warnings when
building with W=1; hyphens should be used instead.  Also, tca6418e@34 in
imx93-byteengine.dtsi should use the generic node name gpio@34.

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v2-0.dts b/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v2-0.dts
> new file mode 100644
> index 0000000000000..52b38edfe5b62
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-bytedevkit-v2-0.dts
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2026 bytesatwork AG
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/net/ti-dp83867.h>
> +#include "imx93-byteengine.dtsi"

[Severity: Low]
This isn't a bug, but imx93-bytedevkit-v2-0.dts duplicates over 400 lines of
imx93-bytedevkit-v1-0.dts verbatim, differing only in the model string, the
&fec PHY address, and the &usbotg1 configuration.  Could the shared carrier
board description be factored into a common .dtsi (or included from one
board .dts into the other) so only the differences are specified?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109122250.720757-1-frank.obergfell@bytesatwork.ch?part=5