Re: [PATCH v1 1/4] arm64: dts: imx93-var-som-symphony: add TPM support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using a GPIO hog to enable the TPM device creates an unhandled dependency, leading to potential probe and suspend/resume failures.
- [Low] The node name and label for the PCAL6408 GPIO expander contain a typo, omitting the letter 'l'.
--

commit bcdb28f289b71a5c2629f6f0cbc1ebb8edd84640
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: imx93-var-som-symphony: add TPM support
    
This commit adds support for the ST33KTPM2XI2C TPM device on the Symphony
carrier board. It configures the TPM enable line as a GPIO hog on a
PCAL6408 expander located on the I2C bus.

> diff --git a/arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dts
> index c0842fb3cfa33..ea996a36b0228 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dts
> @@ -208,6 +208,26 @@ &lpi2c1 {
>  	sda-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  
> +	pca6408: gpio@21 {

[Severity: Low]
This isn't a bug, but the node name and label appear to contain a typo.
The commit message and compatible string identify the part as a PCAL6408,
but the label and node name omit the letter l.

> +		compatible = "nxp,pcal6408";
> +		reg = <0x21>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		status = "okay";
> +
> +		tpm-enable-hog {
> +			gpio-hog;
> +			gpios = <4 GPIO_ACTIVE_HIGH>;
> +			output-high;
> +			line-name = "tpm_en";
> +		};
> +	};

[Severity: High]
Does using a GPIO hog here create an unhandled probe dependency?

Because the device tree core ignores GPIO hog nodes when parsing
dependencies, it doesn't create an explicit supplier/consumer link via
fw_devlink. This means there is no guarantee the GPIO expander is probed
before the TPM device.

If the drivers are built as modules and loaded in parallel, and the TPM
driver happens to bind first, its probe function will attempt an I2C read.
Could that read fail because the TPM is still disabled, potentially
preventing the device from functioning?

> +	st33ktpm2xi2c: tpm@2e {
> +		compatible = "st,st33ktpm2xi2c", "tcg,tpm-tis-i2c";
> +		reg = <0x2e>;
> +	};
> +

[ ... ]

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