| 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
| ||
|
Date: Mon, 1 Oct 2018 14:56:53 +0200
From: Ludovic BARRE <ludovic.barre@...com>
To: Ulf Hansson <ulf.hansson@...aro.org>
CC: Rob Herring <robh+dt@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...com>,
Benjamin Gaignard <benjamin.gaignard@...aro.org>,
Gerald Baeza <gerald.baeza@...com>,
Loic Pallardy <loic.pallardy@...com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
DTML <devicetree@...r.kernel.org>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH V2 27/27] mmc: mmci: add stm32 sdmmc variant
On 10/01/2018 11:31 AM, Ulf Hansson wrote:
> On 21 September 2018 at 11:46, Ludovic Barre <ludovic.Barre@...com> wrote:
>> From: Ludovic Barre <ludovic.barre@...com>
>>
>> This patch adds a stm32 sdmmc variant, rev 1.1.
>> Introduces a new Manufacturer id "0x53, ascii 'S' to define
>> new stm32 sdmmc family with clean range of amba
>> revision/configurations bits (corresponding to sdmmc_ver
>> register with major/minor fields).
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@...com>
>> ---
>> drivers/mmc/host/Kconfig | 10 ++
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/mmci.c | 25 ++++
>> drivers/mmc/host/mmci.h | 5 +
>> drivers/mmc/host/mmci_stm32_sdmmc.c | 282 ++++++++++++++++++++++++++++++++++++
>> 5 files changed, 323 insertions(+)
>> create mode 100644 drivers/mmc/host/mmci_stm32_sdmmc.c
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 5ab2eb0..e59671a 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -34,6 +34,16 @@ config MMC_QCOM_DML
>>
>> if unsure, say N.
>>
>> +config MMC_STM32_SDMMC
>> + bool "STMicroelectronics STM32 SDMMC Controller"
>> + depends on MMC_ARMMMCI
>> + default y
>> + help
>> + This selects the STMicroelectronics STM32 SDMMC host controller.
>> + If you have a STM32 sdmmc host with internal dma say Y or M here.
>> +
>> + If unsure, say N.
>> +
>> config MMC_PXA
>> tristate "Intel PXA25x/26x/27x Multimedia Card Interface support"
>> depends on ARCH_PXA
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index ce8398e..f14410f 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -6,6 +6,7 @@
>> obj-$(CONFIG_MMC_ARMMMCI) += armmmci.o
>> armmmci-y := mmci.o
>> armmmci-$(CONFIG_MMC_QCOM_DML) += mmci_qcom_dml.o
>> +armmmci-$(CONFIG_MMC_STM32_SDMMC) += mmci_stm32_sdmmc.o
>> obj-$(CONFIG_MMC_PXA) += pxamci.o
>> obj-$(CONFIG_MMC_MXC) += mxcmmc.o
>> obj-$(CONFIG_MMC_MXS) += mxs-mmc.o
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 4057456..ca2e483 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -254,6 +254,26 @@ static struct variant_data variant_stm32 = {
>> .init = mmci_variant_init,
>> };
>>
>> +static struct variant_data variant_stm32_sdmmc = {
>> + .fifosize = 16 * 4,
>> + .fifohalfsize = 8 * 4,
>> + .f_max = 208000000,
>> + .stm32_clkdiv = true,
>> + .reset = true,
>> + .cmdreg_cpsm_enable = MCI_CPSM_STM32_ENABLE,
>> + .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC,
>> + .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC,
>> + .cmdreg_srsp = MCI_CPSM_STM32_SRSP,
>> + .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS,
>> + .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK,
>> + .datactrl_first = true,
>> + .datacnt_useless = true,
>> + .datalength_bits = 25,
>> + .datactrl_blocksz = 14,
>> + .stm32_idmabsize_mask = GENMASK(12, 5),
>> + .init = sdmmc_variant_init,
>> +};
>> +
>> static struct variant_data variant_qcom = {
>> .fifosize = 16 * 4,
>> .fifohalfsize = 8 * 4,
>> @@ -2180,6 +2200,11 @@ static const struct amba_id mmci_ids[] = {
>> .mask = 0x00ffffff,
>> .data = &variant_stm32,
>> },
>> + {
>> + .id = 0x10153180,
>> + .mask = 0xf0ffffff,
>> + .data = &variant_stm32_sdmmc,
>> + },
>> /* Qualcomm variants */
>> {
>> .id = 0x00051180,
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index 017d9b8..581b9b1 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -305,6 +305,8 @@ struct mmci_host;
>> * register.
>> * @opendrain: bitmask identifying the OPENDRAIN bit inside MMCIPOWER register
>> * @reset: true if variant has need reset signal.
>> + * @dma_lli: true if variant has dma link list feature.
>> + * @stm32_idmabsize_mask: stm32 sdmmc idma buffer size.
>> */
>> struct variant_data {
>> unsigned int clkreg;
>> @@ -348,6 +350,8 @@ struct variant_data {
>> unsigned int irq_pio_mask;
>> u32 start_err;
>> u32 opendrain;
>> + bool dma_lli;
>> + u32 stm32_idmabsize_mask;
>
> What are these?
This property is specific for sdmmc variants:
sdmmc has a Internal DMA and the number bytes per buffer
could be different between sdmmc variants
(depend of SDMMC_IDMABSIZER register).
>
>> void (*init)(struct mmci_host *host);
>> };
>>
>> @@ -421,6 +425,7 @@ void mmci_write_clkreg(struct mmci_host *host, u32 clk);
>> void mmci_write_pwrreg(struct mmci_host *host, u32 pwr);
>>
>> void mmci_variant_init(struct mmci_host *host);
>> +void sdmmc_variant_init(struct mmci_host *host);
>>
>> int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
>> bool next);
>> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> new file mode 100644
>> index 0000000..cfbfc6f
>> --- /dev/null
>> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> @@ -0,0 +1,282 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
>> + * Author: Ludovic.barre@...com for STMicroelectronics.
>> + */
>> +#include <linux/delay.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/reset.h>
>> +#include <linux/scatterlist.h>
>> +#include "mmci.h"
>> +
>> +#define SDMMC_LLI_BUF_LEN PAGE_SIZE
>> +#define SDMMC_IDMA_BURST BIT(MMCI_STM32_IDMABNDT_SHIFT)
>> +
>> +struct sdmmc_lli_desc {
>> + u32 idmalar;
>> + u32 idmabase;
>> + u32 idmasize;
>> +};
>> +
>> +struct sdmmc_priv {
>> + dma_addr_t sg_dma;
>> + void *sg_cpu;
>> +};
>> +
>> +int sdmmc_idma_validate_data(struct mmci_host *host,
>> + struct mmc_data *data)
>> +{
>> + struct scatterlist *sg;
>> + int i;
>> +
>> + /*
>> + * idma has constraints on idmabase & idmasize for each element
>> + * excepted the last element which has no constraint on idmasize
>> + */
>> + for_each_sg(data->sg, sg, data->sg_len - 1, i) {
>> + if (!IS_ALIGNED(sg_dma_address(data->sg), sizeof(u32)) ||
>> + !IS_ALIGNED(sg_dma_len(data->sg), SDMMC_IDMA_BURST)) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unaligned scatterlist: ofst:%x length:%d\n",
>> + data->sg->offset, data->sg->length);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + if (!IS_ALIGNED(sg_dma_address(data->sg), sizeof(u32))) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unaligned last scatterlist: ofst:%x length:%d\n",
>> + data->sg->offset, data->sg->length);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int _sdmmc_idma_prep_data(struct mmci_host *host,
>> + struct mmc_data *data)
>> +{
>> + int n_elem;
>> +
>> + n_elem = dma_map_sg(mmc_dev(host->mmc),
>> + data->sg,
>> + data->sg_len,
>> + mmc_get_dma_dir(data));
>> +
>> + if (!n_elem) {
>> + dev_err(mmc_dev(host->mmc), "dma_map_sg failed\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sdmmc_idma_prep_data(struct mmci_host *host,
>> + struct mmc_data *data, bool next)
>> +{
>> + /* Check if job is already prepared. */
>> + if (!next && data->host_cookie == host->next_cookie)
>> + return 0;
>> +
>> + return _sdmmc_idma_prep_data(host, data);
>> +}
>> +
>> +static void sdmmc_idma_unprep_data(struct mmci_host *host,
>> + struct mmc_data *data, int err)
>> +{
>> + dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
>> + mmc_get_dma_dir(data));
>> +}
>
> The sdmmc_idma_prep_data() and sdmmc_idma_unprep_data(), seems very
> similar to what the mmci core driver needs to do in this regards.
>
> Can we perhaps avoid adding these callbacks altogether, but rather
> rely on common code in the mmci core driver?
Actually, these callbacks allow to manage prepare/unprepare of
dmaengine interface for mmci variant, (not needed for sdmmc which uses
an internal dma).
For Sdmmc, today there are no special case, just dma_map/unmap.
But in the future, I hope manage the lli list in these callback.
Only dma_map/unmap could be common, but the error management may
be complicated (in mmci variant).
Personally, I prefer keep prep_data/unprep_data mmci_host_ops
interfaces.
What do you suggest ?
>
> [...]
>
> Kind regards
> Uffe
>
Powered by blists - more mailing lists