Re: [PATCH v1 1/1] arm64: dts: imx95-var-dart-sonata: add CAN controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Usage of `gpio1` for SPI chip select may cause a system hang due to hardware firewall (TRDC) constraints.
- [Medium] Unused GPIO pin configuration in `pinctrl_can` due to missing `microchip,rx-int-gpios` property.
--

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

arm64: dts: imx95-var-dart-sonata: add CAN controller

This commit adds the MCP251xFD CAN controller to the Sonata carrier board
device tree. It updates the LPSPI7 chip selects and configures the
necessary pins and interrupts for the CAN node.

> diff --git a/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts b/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
> index 0f3d2e488f4a9..62133b40d64b2 100644
> --- a/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
> +++ b/arch/arm64/boot/dts/freescale/imx95-var-dart-sonata.dts
> @@ -280,7 +280,8 @@ st33ktpm2xi2c: tpm@2e {
>  &lpspi7 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_lpspi7>;
> -	cs-gpios = <&gpio2 4 GPIO_ACTIVE_LOW>;
> +	cs-gpios = <&gpio2 4 GPIO_ACTIVE_LOW>,
> +		   <&gpio1 7 GPIO_ACTIVE_LOW>;

[Severity: High]
Does this direct gpio1 access risk triggering a hardware firewall violation?

On i.MX95, gpio1 is located in the AONMIX domain. The device tree include
arch/arm64/boot/dts/freescale/imx95.dtsi documents that gpio1 is under
the exclusive control of the System Manager.

Referencing it in cs-gpios causes the SPI core to use the standard MMIO-based
gpio-mxc driver to access the controller directly. If the firewall
configuration isn't relaxed for the Non-Secure Linux domain on this board,
toggling this pin may result in an SError and kernel panic during boot.

[ ... ]

> @@ -306,6 +307,18 @@ ads7846: touchscreen@0 {
>  		ti,settle-delay-usec = /bits/ 16 <150>;
>  		ti,keep-vref-on;
>  	};
> +
> +	/* CAN controller */
> +	can0: can@1 {
> +		compatible = "microchip,mcp251xfd";
> +		reg = <1>;
> +		clocks = <&clk_osc_can0>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_can>;
> +		spi-max-frequency = <12000000>;
> +	};
>  };

[ ... ]

> @@ -442,6 +455,13 @@ IMX95_PAD_GPIO_IO37__GPIO5_IO_BIT17				0x31e
>  		>;
>  	};
>  
> +	pinctrl_can: cangrp {
> +		fsl,pins = <
> +			IMX95_PAD_GPIO_IO35__GPIO5_IO_BIT15				0x31e
> +			IMX95_PAD_GPIO_IO22__GPIO2_IO_BIT22				0x31e

[Severity: Medium]
Is IMX95_PAD_GPIO_IO22__GPIO2_IO_BIT22 intended to be used as a secondary
RX interrupt for the CAN controller?

It is configured here as an input, identical to the primary interrupt on
gpio5. However, the can0 node does not reference it.

If this pin is meant to act as the dedicated INT1 interrupt for the
MCP251xFD, the can0 node might be missing the microchip,rx-int-gpios
property. Without it, the driver falls back to single-interrupt mode,
which requires extra register reads to determine the interrupt source, and
leaves this pin's state unmanaged.

> +		>;
> +	};
> +

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