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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87619781-629b-4393-8c14-b34483a7c734@intel.com>
Date: Mon, 16 Jun 2025 14:19:01 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Albert Yang <yangzh0906@...ndersoft.com>, Ulf Hansson
	<ulf.hansson@...aro.org>, Ge Gordon <gordon.ge@....ai>
CC: BST Linux Kernel Upstream Group <bst-upstream@...ai.top>,
	<linux-mmc@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, Geert Uytterhoeven <geert+renesas@...der.be>,
	Victor Shih <victor.shih@...esyslogic.com.tw>, Shan-Chun Hung
	<shanchun1218@...il.com>, Arnd Bergmann <arnd@...db.de>, "AngeloGioacchino
 Del Regno" <angelogioacchino.delregno@...labora.com>, Peter Robinson
	<pbrobinson@...il.com>, Ben Chuang <ben.chuang@...esyslogic.com.tw>
Subject: Re: [PATCH v1 5/9] mmc: sdhci: add Black Sesame Technologies BST
 C1200 controller driver

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@....ai>
> Signed-off-by: Albert Yang <yangzh0906@...ndersoft.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.