Re: [PATCH V2 27/27] mmc: mmci: add stm32 sdmmc variant

From: Ludovic BARRE
Date: Mon Oct 01 2018 - 08:57:22 EST




On 10/01/2018 11:31 AM, Ulf Hansson wrote:
On 21 September 2018 at 11:46, Ludovic Barre <ludovic.Barre@xxxxxx> wrote:
From: Ludovic Barre <ludovic.barre@xxxxxx>

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@xxxxxx>
---
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@xxxxxx 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