Re: [PATCH v1 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 2 potential issue(s) to consider:
- [High] The `reg_sdhc1_vmmc` regulator incorrectly assigns a pinctrl node to `vin-supply` and is missing its `pinctrl-0` property.
- [Medium] `reg_ov5640_buf_en` toggles `GPIO0_21` during early boot, but its pinmux is incorrectly grouped into the `ov5640` camera node.
--

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

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

This commit adds device tree support for the Variscite Symphony carrier
board with the VAR-SOM-AM62 system on module. It includes configuration
for SD card, USB, Ethernet, camera, GPIO expander, and other interfaces.

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
> --- /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";
> +		vin-supply = <&pinctrl_sd1_vmmc>;

[Severity: High]
Does this incorrectly assign a pinctrl node to the vin-supply property?

If the regulator subsystem attempts to resolve this as a parent regulator,
will it fail and return -EPROBE_DEFER, preventing the SDHC1 controller
from probing?

Additionally, are the pinctrl properties (like pinctrl-0 and pinctrl-names)
missing from this regulator node entirely? Without them, will the enable
GPIO (GPIO0_30) fail to have its pin multiplexing configured correctly?

[ ... ]
> +	reg_ov5640_buf_en: regulator-camera-buf-en {
> +		compatible = "regulator-fixed";
> +		regulator-name = "ov5640_buf_en";
> +		gpios = <&main_gpio0 21 GPIO_ACTIVE_HIGH>;

[Severity: Medium]
Because this regulator is marked as regulator-boot-on, it will attempt to
drive GPIO0_21 during early boot. Does it lack its own pinctrl mapping
to ensure the pin is configured as a GPIO at that time?

[ ... ]
> +	ov5640: camera@3c {
> +		compatible = "ovti,ov5640";
> +		reg = <0x3c>;
> +		clocks = <&clk_ov5640_fixed>;
> +		clock-names = "xclk";
> +		AVDD-supply = <&reg_2p8v>;
> +		DOVDD-supply = <&reg_1p8v>;
> +		DVDD-supply = <&reg_1p5v>;
> +		powerdown-gpios = <&main_gpio0 10 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&main_gpio0 22 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_ov5640>;

Since pinctrl_ov5640 contains the pinmux configuration for GPIO0_21, but is
attached to the camera node instead of the regulator, will the pin remain
in an undefined state during early boot until the camera driver probes
much later? 

Should the pinmux for GPIO0_21 be moved to the reg_ov5640_buf_en regulator
node instead?

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