Re: [PATCH v1 3/9] arm64: dts: agilex5: add Cadence SD6HC controller and SOCDK enablement

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Board-specific regulators are incorrectly defined in the generic SoC .dtsi file and placed inside a simple-bus.
- [Low] The Cadence SD6HC node includes an undocumented and unsupported `fifo-depth` property.
--

commit cd8114ca6cbb7c0bc2b1d26651ea445b3eef24ec
Author: Tanmay Kathpalia <tanmay.kathpalia@altera.com>

arm64: dts: agilex5: add Cadence SD6HC controller and SOCDK enablement

This commit adds an SD/MMC controller node backed by the Cadence SD6HC
to the Agilex5 SoC device tree. It also enables the controller on the
SOCDK board with high-speed and UHS-I modes.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
[ ... ]
> @@ -300,6 +300,44 @@ portb: gpio-controller@0 {
>  			};
>  		};
>  
> +		sd_emmc_power: regulator-fixed-3p3v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "card-power";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +			regulator-always-on;
> +		};
> +
> +		sd_io_1v8_reg: regulator-1p8v {
> +			compatible = "regulator-gpio";
> +			regulator-name = "sd-bus-io-power";
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <3300000>;
> +			states =	<1800000 0x1>,
> +					<3300000 0x0>;
> +		};

Should these board-specific regulators be defined in the generic SoC
dtsi file instead of the board dts file?

Placing them here forces these external board-level circuitry
definitions onto all future boards using this SoC.

Additionally, because they are placed inside a simple-bus without a reg
property, it looks like they will trigger simple_bus_reg compiler
warnings. The sd_io_1v8_reg node is also missing the mandatory gpios
property in the dtsi, which might cause dtbs_check schema validation
failures for any board that does not explicitly provide one.

> +
> +		emmc: mmc@10808000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "altr,agilex5-sd6hc", "cdns,sd6hc";
> +			reg = <0x10808000 0x1000>;
> +			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> +			fifo-depth = <0x800>;

Is the fifo-depth property supported by the Cadence SDHC binding?

The Cadence SDHC binding specifies unevaluatedProperties: false and
does not appear to define or support fifo-depth, which seems to be a
leftover from Synopsys DesignWare MMC bindings.

While the driver will ignore it, its presence might cause a dtbs_check
schema validation failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511202132.5597-1-tanmay.kathpalia@altera.com?part=3