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

["Kathpalia, Tanmay" <tanmay.kathpalia@altera.com>]

On 5/13/2026 6:13 AM, sashiko-bot@kernel.org wrote:
> 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.

The sd_emmc_power and sd_io_1v8_reg regulators are
board-specific and will be moved to the board DTS file in v2. I
will also run dtbs_check schema validation to catch any
remaining issues before sending v2.
>
>> +
>> +		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.

fifo-depth is not supported by the Cadence SD6HC binding. It will
be removed in v2. I will verify with dtbs_check schema
validation before sending.