Re: [PATCH v2 2/3] arm64: dts: imx8mm-var-dart: add TPM support

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Sep 17, 2026 at 03:50:33PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] dts: incorrect reset-gpios polarity contradicts hardware description
> --
>
> commit 3fe00f46f78811d6ed3d86dcccba4b6f6cd29139
> Author: Stefano Radaelli <stefano.r@variscite.com>
>
> arm64: dts: imx8mm-var-dart: add TPM support
>
> This commit adds device tree support for the TPM module available on the
> Variscite DART-MX8M-MINI SOM.
>
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi
> > index c7a9eb23e5e88..953950e55bf99 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi
> > @@ -138,6 +138,16 @@ &i2c1 {
> >  	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >  	status = "okay";
> >
> > +	st33ktpm2xi2c_som: tpm@2e {
> > +		compatible = "st,st33ktpm2xi2c", "tcg,tpm-tis-i2c";
> > +		reg = <0x2e>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_tpm_reset>;
> > +		label = "tpm";
> > +		/* active-low TPM reset line. */
> > +		reset-gpios = <&gpio4 31 GPIO_ACTIVE_HIGH>;
>
> [Severity: Low]
> Does the device tree polarity match the actual hardware description here? The
> GPIO_ACTIVE_HIGH flag directly contradicts the inline comment that the
> physical line is active-low. Standard device tree practice requires the
> logical flag to match the physical active state (GPIO_ACTIVE_LOW).
>
> If this property is parsed by a bootloader (like U-Boot) or a future Linux
> driver update, wouldn't the reset line be driven to the wrong polarity (high
> for reset, low for de-assert), potentially holding the TPM in reset and
> breaking device initialization?

tpm_tis_core.c have not parser reset-gpios. ref

static int reset_gpio_assert(struct reset_controller_dev *rc, unsigned long id)
{
        struct reset_gpio_priv *priv = rc_to_reset_gpio(rc);

        return gpiod_set_value_cansleep(priv->reset, 1);
}

suppose it should be GPIO_ACTIVE_LOW to assert reset line

Frank

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789659585.git.stefano.r@variscite.com?part=2