Re: [PATCH v5 8/8] mmc: sdhci-cadence: add Altera Agilex5 SD6HC support

[Philipp Zabel]

On Do, 2026-08-20 at 11:57 -0700, Tanmay Kathpalia wrote:
> The Altera Agilex5 SoC integrates a Cadence SD6HC controller that needs
> platform-specific configuration to operate correctly.
> 
> The SoC requires three named resets: "sdhc-reset", "combophy", and
> "sdmmc-ocp". All three are exclusive and must be asserted together before
> being released, so the SDHCI, SoftPHY, and OCP/AXI clock domains cross the
> reset boundary simultaneously. SoftPHY is shared with NAND at the SoC
> level, but only one of SDMMC or NAND is enabled on a given board.
> 
> The IOMMU maps DMA addresses within a 40-bit physical address space, so
> the DMA mask is capped at 40 bits to prevent allocation beyond the
> controller's reach.
> 
> The silicon requires the MULTIBLOCK_READ_ACMD12, CAP_CLOCK_BASE_BROKEN,
> PRESET_VALUE_BROKEN, and ACMD23_BROKEN quirks. Since
> CAP_CLOCK_BASE_BROKEN prevents reading the base clock from the
> capabilities register, the maximum clock is supplied from the platform
> clock instead.
> 
> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@xxxxxxxxxx>
> Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  drivers/mmc/host/sdhci-cadence-core.c | 85 +++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence-core.c b/drivers/mmc/host/sdhci-cadence-core.c
> index 18a5329f27db..e215f6a4c344 100644
> --- a/drivers/mmc/host/sdhci-cadence-core.c
> +++ b/drivers/mmc/host/sdhci-cadence-core.c
> @@ -7,6 +7,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/mmc/host.h>
> @@ -90,6 +91,7 @@ struct sdhci_cdns4_phy_cfg {
>  struct sdhci_cdns_drv_data {
>  	int (*init)(struct platform_device *pdev);
>  	const struct sdhci_pltfm_data pltfm_data;
> +	u64 dma_mask;
>  };
>  
>  static const struct sdhci_cdns4_phy_cfg sdhci_cdns4_phy_cfgs[] = {
> @@ -196,6 +198,23 @@ static unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
>  	return host->max_clk;
>  }
>  
> +static int sdhci_cdns_set_dma_mask(struct sdhci_host *host)
> +{
> +	const struct sdhci_cdns_drv_data *data;
> +	struct device *dev = mmc_dev(host->mmc);
> +	int ret;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data || !data->dma_mask)
> +		return 0;
> +
> +	ret = dma_set_mask_and_coherent(dev, data->dma_mask);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to set DMA mask\n");
> +
> +	return 0;
> +}
> +
>  static void sdhci_cdns_set_emmc_mode(struct sdhci_cdns_priv *priv, u32 mode)
>  {
>  	u32 tmp;
> @@ -462,6 +481,44 @@ static int elba_drv_init(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static int sdhci_cdns6_agilex5_init(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct reset_control_bulk_data resets[] = {
> +		{ .id = "sdhc-reset" },
> +		{ .id = "combophy" },
> +		{ .id = "sdmmc-ocp" },
> +	};
> +	int ret;
> +
> +	/*
> +	 * Assert SDHCI, SoftPHY (combophy), and SDMMC OCP/AXI resets together
> +	 * so their active periods overlap before all domains are released.
> +	 * SoftPHY is shared with NAND, but only one of SDMMC
> +	 * or NAND is enabled on a given board.
> +	 */
> +	ret = reset_control_bulk_get_exclusive(dev, ARRAY_SIZE(resets), resets);

Why not devm_reset_control_bulk_get_exclusive()?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get resets\n");
> +
> +	ret = reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to assert resets\n");
> +		goto out_put;
> +	}
> +
> +	/* Hold resets asserted long enough for all clock domains to capture. */
> +	usleep_range(10, 20);
> +
> +	ret = reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
> +	if (ret)
> +		dev_err_probe(dev, ret, "failed to deassert resets\n");
> +
> +out_put:
> +	reset_control_bulk_put(ARRAY_SIZE(resets), resets);

This happens to work as long as nothing else grabs the reset controls
afterwards, but it is technically incorrect. In the ret == 0 case the
resets are expected to stay deasserted for the lifetime of the device,
so we shouldn't give up control here.

regards
Philipp