Re: [PATCH v3 2/2] arm64: dts: freescale: add initial device tree for TQMa8MPQS with i.MX8MP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Off-by-4 error in `gpio4` line names array assigns GPIO names to actively driven Ethernet pins
- [Medium] Redundant `gpio-hog` nodes for pins concurrently used as interrupts by drivers
- [Medium] Missing `status = "okay";` for `pwm2` and `pwm3` will cause backlight overlays to fail
--

commit 33a28c1f4850f44b57edd7bcc9794f6b7f5401a7
Author: Paul Gerber <paul.gerber@tq-group.com>

arm64: dts: freescale: add initial device tree for TQMa8MPQS with i.MX8MP

This adds support for TQMa8MPQS module on MB-SMARC-2 board.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi
> new file mode 100644
> index 0000000000000..570dfca3aa024
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi

[ ... ]

> +&gpio1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_gpio1>;
> +
> +	gpio-line-names = "SLEEP", "BATLOW#", "", "LID",
> +			  "", "GPIO10", "CHARGING#", "CHG_PRSNT#",
> +			  "PMIC_IRQ#", "ESPI_CS1_ALERT#", "USB1_OTG_ID", "USB2_OTG_ID",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "";
> +
> +	pmic_irq_hog: pmic-irq-hog {
> +		gpio-hog;
> +		gpios = <8 0>;
> +		input;
> +		line-name = "PMIC_IRQ#";
> +	};
> +};

[Severity: Medium]
Is it necessary to define gpio-hog nodes for pins that are already requested
as interrupts by their respective drivers (like the PCA9450C PMIC)?

Since gpio-line-names already correctly names the pin, doesn't hogging an
interrupt line redundantly tie up gpiolib resources and potentially conflict
with driver ownership?

(This question also applies to the enet0-int-hog, enet1-int-hog,
temp-event-hog, and rtc-int-hog nodes defined further below).

[ ... ]

> +&gpio4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_gpio4>, <&pinctrl_sdp>, <&pinctrl_espi_rst>;
> +
> +	gpio-line-names = "RTC_INT#", "GPIO8", "", "",
> +			  "GPIO9", "ESPI_RST#", "", "",
> +			  "", "", "", "",
> +			  "", "", "GBE0_SDP_DIR", "",
> +			  "GPIO7", "", "", "",
> +			  "", "", "", "",
> +			  "", "GPIO0", "GPIO1", "",
> +			  "GPIO2", "GPIO6", "", "";

[Severity: High]
Are GBE0_SDP_DIR and GPIO7 placed at the correct indices in this array?

They are currently at indices 14 and 16, which correspond to GPIO4_IO14 and
GPIO4_IO16. However, these are actively driven Ethernet lines (ENET1_RGMII_TD2
and ENET1_RGMII_TX_CTL) as multiplexed in pinctrl_fec.

Could this be an off-by-4 error, and should they instead be at indices 18 and
20, which are explicitly multiplexed as GPIOs? If exposed to userspace as
GPIOs, could manipulating these active Ethernet TX lines cause hardware
conflicts or disrupt network connectivity?

[ ... ]

> +&pwm2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm2>;
> +};
> +
> +&pwm3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm3>;
> +};

[Severity: Medium]
Since the backlight_lvds0 and backlight_lvds1 nodes on the baseboard rely on
&pwm3 and &pwm2, should these controllers have status = "okay"; added here?

If left disabled (as they are in the SoC dtsi), won't any overlays that enable
the panels silently fail when the backlight driver cannot acquire the disabled
PWM channels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522075656.841745-1-alexander.stein@ew.tq-group.com?part=2