Re: [PATCH v1 5/9] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver

[Adrian Hunter <adrian.hunter@intel.com>]

On 28/05/2025 11:54, Albert Yang wrote:
> Add a driver for the DesignWare Mobile Storage Host Controller (DWCMSHC)
> SDHCI controller found in Black Sesame Technologies C1200 SoCs.
> 
> The driver provides specialized clock configuration, tuning, voltage
> switching, and power management for the BST DWCMSHC controller. It also
> includes support for eMMC boot and memory-mapped I/O for CRM registers.
> 
> Signed-off-by: Ge Gordon <gordon.ge@bst.ai>
> Signed-off-by: Albert Yang <yangzh0906@thundersoft.com>
> ---
>  drivers/mmc/host/Kconfig              |  11 +
>  drivers/mmc/host/Makefile             |   1 +
>  drivers/mmc/host/sdhci-of-bst-c1200.c | 920 ++++++++++++++++++++++++++
>  3 files changed, 932 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-of-bst-c1200.c

<SNIP>

> +static void bst_sdhci_allocate_bounce_buffer(struct sdhci_host *host)
> +{
> +	struct mmc_host *mmc = host->mmc;
> +	unsigned int max_blocks;
> +	unsigned int bounce_size;
> +	int ret;
> +
> +	/*
> +	 * Cap the bounce buffer at 64KB. Using a bigger bounce buffer
> +	 * has diminishing returns, this is probably because SD/MMC
> +	 * cards are usually optimized to handle this size of requests.
> +	 */
> +	bounce_size = SZ_32K;
> +	/*
> +	 * Adjust downwards to maximum request size if this is less
> +	 * than our segment size, else hammer down the maximum
> +	 * request size to the maximum buffer size.
> +	 */
> +	if (mmc->max_req_size < bounce_size)
> +		bounce_size = mmc->max_req_size;
> +	max_blocks = bounce_size / 512;
> +
> +	ret = of_reserved_mem_device_init_by_idx(mmc_dev(mmc), mmc_dev(mmc)->of_node, 0);
> +	if (ret) {
> +		dev_err(mmc_dev(mmc), "of_reserved_mem_device_init error\n");
> +		return;
> +	}
> +	host->bounce_buffer = dma_alloc_coherent(mmc_dev(mmc), bounce_size,
> +						 &host->bounce_addr, GFP_KERNEL);

sdhci uses dma_sync_single_for_device() and dma_sync_single_for_cpu()
with this buffer.  Does that really work?

> +
> +	ret = dma_mapping_error(mmc_dev(mmc), host->bounce_addr);
> +	if (ret) {
> +		devm_kfree(mmc_dev(mmc), host->bounce_buffer);
> +		host->bounce_buffer = NULL;
> +		/* Again fall back to max_segs == 1 */
> +		return;
> +	}
> +
> +	host->bounce_buffer_size = bounce_size;
> +
> +	/* Lie about this since we're bouncing */
> +	mmc->max_segs = max_blocks;
> +	mmc->max_seg_size = bounce_size;
> +	mmc->max_req_size = bounce_size;
> +
> +	pr_info("BST reallocate %s bounce up to %u segments into one, max segment size %u bytes\n",
> +		mmc_hostname(mmc), max_blocks, bounce_size);
> +}
> +
> +static int bst_sdhci_set_dma_mask(struct sdhci_host *host)

This is identical to sdhci_set_dma_mask() just just drop it.

> +{
> +	struct mmc_host *mmc = host->mmc;
> +	struct device *dev = mmc_dev(mmc);
> +	int ret = -EINVAL;
> +
> +	if (host->quirks2 & SDHCI_QUIRK2_BROKEN_64_BIT_DMA)
> +		host->flags &= ~SDHCI_USE_64_BIT_DMA;
> +
> +	/* Try 64-bit mask if hardware is capable  of it */
> +	if (host->flags & SDHCI_USE_64_BIT_DMA) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +		if (ret) {
> +			pr_warn("%s: Failed to set 64-bit DMA mask.\n",
> +				mmc_hostname(mmc));
> +			host->flags &= ~SDHCI_USE_64_BIT_DMA;
> +		}
> +	}
> +
> +	/* 32-bit mask as default & fallback */
> +	if (ret) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret)
> +			pr_warn("%s: Failed to set 32-bit DMA mask.\n",
> +				mmc_hostname(mmc));
> +	}
> +
> +	return ret;
> +}
> +
> +int bst_sdhci_setup_host(struct sdhci_host *host)

It is not acceptable for the driver to have its own copy of
sdhci_setup().

Please describe what you need to customize and why.