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

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Frank,

Am Montag, 1. Juni 2026, 21:52:11 CEST schrieb Frank Li:
> On Fri, May 22, 2026 at 08:24:38AM +0000, sashiko-bot@kernel.org wrote:
> > 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.
> >
> ...
> >
> > > +&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?
> 
> Is AI said true?

It's actually true. This is an off-by-4 error. Fixed in next version.

> >
> > [ ... ]
> >
> > > +&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?
> 
> Make sense, in dtsi, pwm2 pwm3's status is disabled.

They are disabled on purpose, the same as for the backlights. Both the
backlight and pwm are supposed to be enabled in display overlay. I don't
see a problem here. I'll add the overlays in the next version as well, so
it's visible.

Best regards,
Alexander

> >
> > 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
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/