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

Hi Krzysztof,

Thanks for the review.

On 6/29/2026 12:36 PM, Krzysztof Kozlowski wrote:
> On Sat, Jun 27, 2026 at 01:14:48PM -0700, Tanmay Kathpalia wrote:
>> Add the Cadence SD6HC controller node to the Agilex5 SoC DTSI as a
>> shared SD/eMMC node, disabled by default. The controller integrates
>> with the system SMMU for IOMMU support and uses SDMCLK as the primary
>> clock source for PHY timing.
>>
>> On the SOCDK board, add a fixed 3.3V regulator for card power and a
>> GPIO-controlled regulator for I/O voltage switching between 1.8V and
>> 3.3V. Enable the controller for SD-only operation in 4-bit bus width
>> with high-speed and SDR104 UHS-I modes at 200 MHz.
>>
>> Signed-off-by: Tanmay Kathpalia <[email protected]>
>> ---
>>   .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 25 +++++++++++++++
>>   .../boot/dts/intel/socfpga_agilex5_socdk.dts  | 31 +++++++++++++++++++
>>   2 files changed, 56 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
>> index 02e62d954e94..f552aa0c1faa 100644
>> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
>> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
>> @@ -300,6 +300,31 @@ portb: gpio-controller@0 {
>>   			};
>>   		};
>>   
>> +		/*
>> +		 * Shared SD/eMMC controller node. On the SOCDK OOBE daughter-card
>> +		 * this is used for SD card operation; on the SOCDK eMMC daughter-card
>> +		 * it is configured for eMMC.
>> +		 */
>> +		emmc: mmc@10808000 {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
> Please follow DTS coding style for new code.

Ack, I'll update the DTS formatting to follow the coding style in v3.

>
>> +			compatible = "altr,agilex5-sd6hc", "cdns,sd6hc";
>> +			reg = <0x10808000 0x1000>;
>> +			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
>> +			resets = <&rst SDMMC_RESET>, <&rst COMBOPHY_RESET>, <&rst SDMMC_OCP_RESET>;
>> +			reset-names = "sdhc-reset", "combophy", "sdmmc-ocp";
>> +			/*
>> +			 * "ciu" (SDMCLK) is listed first so it is selected as the
>> +			 * primary clock by the SDHCI platform layer; the SD6HC PHY
>> +			 * timing calculations are derived from this clock rate.
>> +			 */
>> +			clocks = <&clkmgr AGILEX5_SDMCLK>, <&clkmgr AGILEX5_L4_MP_CLK>;
>> +			clock-names = "ciu", "biu";
>> +			iommus = <&smmu 5>;
>> +			dma-coherent;
>> +			status = "disabled";
>> +		};
>> +
>>   		nand: nand-controller@10b80000 {
>>   			compatible = "cdns,hp-nfc";
>>   			reg = <0x10b80000 0x10000>,
>> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts b/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
>> index 262bb3e8e5c7..c56f46721bb0 100644
>> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
>> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts
>> @@ -34,6 +34,24 @@ memory@80000000 {
>>   		/* We expect the bootloader to fill in the reg */
>>   		reg = <0x0 0x80000000 0x0 0x0>;
>>   	};
>> +
>> +	vmmc_reg: regulator-fixed-3p3v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc-sd";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	vqmmc_io_reg: regulator-1p8v {
>> +		compatible = "regulator-gpio";
>> +		regulator-name = "vqmmc-io";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		states =	<1800000 0x1>,
> There is only one space after '='.

I'll fix the formatting in v3.

>
>> +				<3300000 0x0>;
>> +		gpios = <&portb 3 GPIO_ACTIVE_HIGH>;
>> +	};
>>   };
> Best regards,
> Krzysztof
>