Re: [PATCH v2 3/3] arm64: dts: ti: var-som-am62: Add support for Variscite Symphony Board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `cs-gpios` property for `main_spi2` incorrectly specifies `GPIO_ACTIVE_HIGH` for a general-purpose SPI bus.
- [Low] The `reg_sdhc1_vmmc` regulator node is missing the `pinctrl-names` property and uses the deprecated `gpio` property.
- [Low] The `pca9534` GPIO expander node is missing the `interrupt-controller;` and `#interrupt-cells = <2>;` properties.
--

--- Patch [3]: [PATCH v2 3/3] arm64: dts: ti: var-som-am62: Add support for Variscite Symphony Board ---
commit 39365377c2ed764159841e605bd1eff65b2d9ad1
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: ti: var-som-am62: Add support for Variscite Symphony Board

Add device tree support for the Variscite Symphony carrier board with
the VAR-SOM-AM62 system on module.

Link: https://www.variscite.it/product/single-board-computers/symphony-board/

> diff --git a/arch/arm64/boot/dts/ti/k3-am625-var-som-symphony.dts b/arch/arm64/boot/dts/ti/k3-am625-var-som-symphony.dts
> new file mode 100644
> index 0000000000000..1aa50fa8809d1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-var-som-symphony.dts
[ ... ]
> +	reg_sdhc1_vmmc: regulator-sdhc1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "+V3.3_SD";
> +		pinctrl-0 = <&pinctrl_sd1_vmmc>;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		enable-active-high;
> +		gpio = <&main_gpio0 30 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Does this node need a pinctrl-names property? When pinctrl-0 is provided,
the DT schema requires pinctrl-names.

Also, should this use the gpios property instead of the deprecated gpio
property?

> +		bootph-all;
> +	};
[ ... ]
> +	/* GPIO expander */
> +	pca9534: gpio@20 {
> +		compatible = "nxp,pca9534";
> +		reg = <0x20>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pca9534>;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		interrupt-parent = <&main_gpio1>;
> +		interrupts = <30 IRQ_TYPE_EDGE_FALLING>;

[Severity: Low]
Should this node include interrupt-controller; and #interrupt-cells = <2>;
properties?

Without these, device tree overlays cannot use this expander as an
interrupt parent. The OF core will refuse to parse interrupts for any external
nodes that specify interrupt-parent = <&pca9534>.

[ ... ]
> +&main_spi2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_spi2>;
> +	ti,pindir-d0-out-d1-in;
> +	cs-gpios = <&main_gpio1 13 GPIO_ACTIVE_HIGH>;

[Severity: Medium]
Is GPIO_ACTIVE_HIGH correct for this general-purpose SPI bus chip select?

Standard SPI chip selects are almost universally active-low. By specifying
GPIO_ACTIVE_HIGH, the SPI core expects the physical chip select line to be
driven high to select the device, which could cause communication failures
for standard active-low devices attached to the bus.

> +	status = "okay";
> +};

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