| 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
| ||
|
Message-ID: <b6bf0b53-8812-4099-b5d3-39e7fd264777@intel.com>
Date: Fri, 27 Jun 2025 16:19:24 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: "yangzh0906@...ndersoft.com" <yangzh0906@...ndersoft.com>, ulf.hansson
<ulf.hansson@...aro.org>, gordon.ge <gordon.ge@....ai>
CC: bst-upstream <bst-upstream@...ai.top>, linux-mmc
<linux-mmc@...r.kernel.org>, linux-arm-kernel
<linux-arm-kernel@...ts.infradead.org>, linux-kernel
<linux-kernel@...r.kernel.org>, geert+renesas <geert+renesas@...der.be>,
victor.shih <victor.shih@...esyslogic.com.tw>, shanchun1218
<shanchun1218@...il.com>, arnd <arnd@...db.de>, angelogioacchino.delregno
<angelogioacchino.delregno@...labora.com>, pbrobinson <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 27/06/2025 13:22, yangzh0906@...ndersoft.com wrote:
> Dear Mr. Hunter,
>
> Our platform supports 64-bit physical addressing, but the eMMC controller's SRAM-based DMA engine is constrained to a 32-bit address space.
> When using the standard SDHCI interface, which allocates DDR-based DMA buffers with 64-bit addresses, thedma_map_single() operation fails
> because the DMA engine cannot handle addresses beyond 32 bits.
SDHCI controllers can use 32-bit DMA or 64-bit DMA, however even with
64-bit DMA it is possible to restrict the DMA addresses to 32-bits
by setting a 32-bit DMA mask.
If the host controller capabilities indicate support for 64-bit DMA
but you want the driver to use 32-bit DMA, set SDHCI_QUIRK2_BROKEN_64_BIT_DMA.
However, if you want to use 64-bit DMA with only 32-bit DMA addresses
you can instead implement sdhci host op ->set_dma_mask() and in that
function dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))
>
> To resolve this hardware limitation, we implement a bounce buffer allocated via dma_alloc_coherent() to satisfy DMA addressing constraints.
The bounce buffer should not be needed to satisfy DMA addressing
constraints. It is used when SDHCI ADMA (scatter/gather) is broken.
Also please be aware that "top-posting" is discouraged, refer:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>
>> Best regards,
>> Albert
>>
>
> *From:* Adrian Hunter <mailto:adrian.hunter@...el.com>
> *Date:* 2025-06-16 19:19
> *To:* Albert Yang <mailto:yangzh0906@...ndersoft.com>; Ulf Hansson <mailto:ulf.hansson@...aro.org>; Ge Gordon <mailto:gordon.ge@....ai>
> *CC:* BST Linux Kernel Upstream Group <mailto:bst-upstream@...ai.top>; linux-mmc@...r.kernel.org <mailto:linux-mmc@...r.kernel.org>; linux-arm-kernel@...ts.infradead.org <mailto:linux-arm-kernel@...ts.infradead.org>; linux-kernel@...r.kernel.org <mailto:linux-kernel@...r.kernel.org>; Geert Uytterhoeven <mailto:geert+renesas@...der.be>; Victor Shih <mailto:victor.shih@...esyslogic.com.tw>; Shan-Chun Hung <mailto:shanchun1218@...il.com>; Arnd Bergmann <mailto:arnd@...db.de>; AngeloGioacchino Del Regno <mailto:angelogioacchino.delregno@...labora.com>; Peter Robinson <mailto:pbrobinson@...il.com>; Ben Chuang <mailto: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.
>
>
>
>
Powered by blists - more mailing lists