[PATCH V2 0/6] mmc: mmci: add get_datactrl_cfg callback

[PATCH V2 0/6] mmc: mmci: add get_datactrl_cfg callback

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch series adds get_datactrl_cfg callback in mmci_host_ops
to allow to get datactrl configuration specific at variant.

change V2:
-This V2 has been rebased on
"mmc: mmci: Cleanup some variant related code" series
-add helpers functions to define default datactrl value,
each variant could use these helpers to define datactrl
and adds their specific bits.
-use static in qcom and stm32
-regroup mmci_(ux500v2)variant_init in header file
to avoid checkpatch warning:
"WARNING: externs should be avoided in .c files"
-remove unused variant properties:
 "datactrl_dpsm_enable"
 "blksz_datactrl16"
 "blksz_datactrl4"

Ludovic Barre (6):
  mmc: mmci: add get_datactrl_cfg callback
  mmc: mmci: add helper functions to define datactrl value for variants
  mmc: mmci: define get_dctrl_cfg for legacy variant
  mmc: mmci: qcom: define get_dctrl_cfg
  mmc: mmci: stm32: define get_dctrl_cfg
  mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback

 drivers/mmc/host/mmci.c             | 100 ++++++++++++++++++++++--------------
 drivers/mmc/host/mmci.h             |  21 +++++---
 drivers/mmc/host/mmci_qcom_dml.c    |  13 +++++
 drivers/mmc/host/mmci_stm32_sdmmc.c |  19 +++++++
 4 files changed, 108 insertions(+), 45 deletions(-)

-- 
2.7.4

[PATCH V2 1/6] mmc: mmci: add get_datactrl_cfg callback

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds get_datactrl_cfg callback in mmci_host_ops
to allow to get datactrl configuration specific at variant.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 6bde28c..a59049b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -362,6 +362,7 @@ struct mmci_host_ops {
 			 bool next);
 	void (*unprep_data)(struct mmci_host *host, struct mmc_data *data,
 			    int err);
+	u32 (*get_datactrl_cfg)(struct mmci_host *host);
 	void (*get_next_data)(struct mmci_host *host, struct mmc_data *data);
 	int (*dma_setup)(struct mmci_host *host);
 	void (*dma_release)(struct mmci_host *host);
-- 
2.7.4

Re: [PATCH V2 1/6] mmc: mmci: add get_datactrl_cfg callback

[Ulf Hansson]

On Thu, 7 Mar 2019 at 17:39, Ludovic Barre <ludovic.Barre@st.com> wrote:
>
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch adds get_datactrl_cfg callback in mmci_host_ops
> to allow to get datactrl configuration specific at variant.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  drivers/mmc/host/mmci.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 6bde28c..a59049b 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -362,6 +362,7 @@ struct mmci_host_ops {
>                          bool next);
>         void (*unprep_data)(struct mmci_host *host, struct mmc_data *data,
>                             int err);
> +       u32 (*get_datactrl_cfg)(struct mmci_host *host);
>         void (*get_next_data)(struct mmci_host *host, struct mmc_data *data);
>         int (*dma_setup)(struct mmci_host *host);
>         void (*dma_release)(struct mmci_host *host);
> --
> 2.7.4
>

Perhaps squash this change with patch2, they seems to belong to each
other as they are both the preparation that is needed for the
following changes on top.

Kind regards
Uffe

[PATCH V2 2/6] mmc: mmci: add helper functions to define datactrl value for variants

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds default helper functions to define datactrl value.
Each variant could use these helpers to define datactrl and adds
their specific field.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++++
 drivers/mmc/host/mmci.h |  5 +++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9e6a2c1..28b76c5 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -983,6 +983,31 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
 	mmci_unprep_data(host, data, err);
 }
 
+u32 mmci_dctrl_blksz(struct mmci_host *host)
+{
+	return (ffs(host->data->blksz) - 1) << 4;
+}
+
+u32 mmci_dctrl_dir(struct mmci_host *host)
+{
+	return host->data->flags & MMC_DATA_READ ? MCI_DPSM_DIRECTION : 0;
+}
+
+u32 mmci_dctrl_ddr(struct mmci_host *host)
+{
+	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
+	    host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
+		return host->variant->datactrl_mask_ddrmode;
+	return 0;
+}
+
+u32 mmci_dctrl_sdio(struct mmci_host *host)
+{
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card))
+		return host->variant->datactrl_mask_sdio;
+	return 0;
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	struct variant_data *variant = host->variant;
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index a59049b..32ae41d 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -430,6 +430,11 @@ struct mmci_host {
 void mmci_write_clkreg(struct mmci_host *host, u32 clk);
 void mmci_write_pwrreg(struct mmci_host *host, u32 pwr);
 
+u32 mmci_dctrl_blksz(struct mmci_host *host);
+u32 mmci_dctrl_dir(struct mmci_host *host);
+u32 mmci_dctrl_ddr(struct mmci_host *host);
+u32 mmci_dctrl_sdio(struct mmci_host *host);
+
 #ifdef CONFIG_DMA_ENGINE
 int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
 			bool next);
-- 
2.7.4

Re: [PATCH V2 2/6] mmc: mmci: add helper functions to define datactrl value for variants

[Ulf Hansson]

On Thu, 7 Mar 2019 at 17:39, Ludovic Barre <ludovic.Barre@st.com> wrote:
>
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch adds default helper functions to define datactrl value.
> Each variant could use these helpers to define datactrl and adds
> their specific field.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++++
>  drivers/mmc/host/mmci.h |  5 +++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 9e6a2c1..28b76c5 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -983,6 +983,31 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
>         mmci_unprep_data(host, data, err);
>  }
>
> +u32 mmci_dctrl_blksz(struct mmci_host *host)
> +{
> +       return (ffs(host->data->blksz) - 1) << 4;
> +}

Overall, this function and the other helpers added here, could be made
static inline functions, implemented in mmci.h.

If I understood Russell's point correctly, that's also we he suggested.

> +
> +u32 mmci_dctrl_dir(struct mmci_host *host)
> +{
> +       return host->data->flags & MMC_DATA_READ ? MCI_DPSM_DIRECTION : 0;

As pointed out by Russell, drop this and keep this in the common
mmci_start_data().

Or you need this for the stm32_sdmmc later? Then perhaps make that a
separate change on top.

> +}
> +
> +u32 mmci_dctrl_ddr(struct mmci_host *host)
> +{
> +       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
> +           host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
> +               return host->variant->datactrl_mask_ddrmode;

This seems a bit like an unnecessary helper. How about moving this
into the ux500v2 and qcom variants ->datactrl() callbacks instead,
thus we can also remove ->datactrl_mask_ddrmode from the variant
struct.

Does that make sense?

> +       return 0;
> +}
> +
> +u32 mmci_dctrl_sdio(struct mmci_host *host)
> +{
> +       if (host->mmc->card && mmc_card_sdio(host->mmc->card))
> +               return host->variant->datactrl_mask_sdio;
> +       return 0;
> +}
> +
>  static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
>  {
>         struct variant_data *variant = host->variant;
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index a59049b..32ae41d 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -430,6 +430,11 @@ struct mmci_host {
>  void mmci_write_clkreg(struct mmci_host *host, u32 clk);
>  void mmci_write_pwrreg(struct mmci_host *host, u32 pwr);
>
> +u32 mmci_dctrl_blksz(struct mmci_host *host);
> +u32 mmci_dctrl_dir(struct mmci_host *host);
> +u32 mmci_dctrl_ddr(struct mmci_host *host);
> +u32 mmci_dctrl_sdio(struct mmci_host *host);
> +
>  #ifdef CONFIG_DMA_ENGINE
>  int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
>                         bool next);
> --
> 2.7.4
>

Overall, I have to admit that it's tricky to fine a decent balance of
callbacks/helpers/variant-data. I guess we simply have to continue
evolve this, along the lines of what suggest here and just see how it
turns out. We can always patch the code on top if we find other/better
solutions along the road.

Kind regards
Uffe

[PATCH V2 3/6] mmc: mmci: define get_dctrl_cfg for legacy variant

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch defines get_dctrl_cfg callback for legacy variants
whatever DMA_ENGINE configuration.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 44 ++++++++++++++++++++++++++++++++++++--------
 drivers/mmc/host/mmci.h |  3 +++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 28b76c5..52f9dbf 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -46,12 +46,6 @@
 
 #define DRIVER_NAME "mmci-pl18x"
 
-#ifdef CONFIG_DMA_ENGINE
-static void mmci_variant_init(struct mmci_host *host);
-#else
-static inline void mmci_variant_init(struct mmci_host *host) {}
-#endif
-
 static unsigned int fmax = 515633;
 
 static struct variant_data variant_arm = {
@@ -231,7 +225,7 @@ static struct variant_data variant_ux500v2 = {
 	.irq_pio_mask		= MCI_IRQ_PIO_MASK,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
-	.init			= mmci_variant_init,
+	.init			= mmci_ux500v2_variant_init,
 };
 
 static struct variant_data variant_stm32 = {
@@ -617,6 +611,29 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
 	sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
 }
 
+u32 mmci_get_dctrl_cfg(struct mmci_host *host)
+{
+	u32 datactrl;
+
+	datactrl = MCI_DPSM_ENABLE;
+	datactrl |= mmci_dctrl_blksz(host) | mmci_dctrl_dir(host);
+	datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
+
+	return datactrl;
+}
+
+u32 mmci_ux500v2_get_dctrl_cfg(struct mmci_host *host)
+{
+	u32 datactrl;
+
+	datactrl = MCI_DPSM_ENABLE;
+	datactrl |= (host->data->blksz << 16);
+	datactrl |= mmci_dctrl_dir(host);
+	datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
+
+	return datactrl;
+}
+
 /*
  * All the DMA operation mode stuff goes inside this ifdef.
  * This assumes that you have a generic DMA device interface,
@@ -941,6 +958,7 @@ void mmci_dmae_unprep_data(struct mmci_host *host,
 static struct mmci_host_ops mmci_variant_ops = {
 	.prep_data = mmci_dmae_prep_data,
 	.unprep_data = mmci_dmae_unprep_data,
+	.get_datactrl_cfg = mmci_get_dctrl_cfg,
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = mmci_dmae_setup,
 	.dma_release = mmci_dmae_release,
@@ -948,12 +966,22 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.dma_finalize = mmci_dmae_finalize,
 	.dma_error = mmci_dmae_error,
 };
+#else
+static struct mmci_host_ops mmci_variant_ops = {
+	.get_datactrl_cfg = mmci_get_dctrl_cfg,
+}
+#endif
 
 void mmci_variant_init(struct mmci_host *host)
 {
 	host->ops = &mmci_variant_ops;
 }
-#endif
+
+void mmci_ux500v2_variant_init(struct mmci_host *host)
+{
+	host->ops = &mmci_variant_ops;
+	host->ops->get_datactrl_cfg = mmci_ux500v2_get_dctrl_cfg;
+}
 
 static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 32ae41d..bfc6e54 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -448,6 +448,9 @@ void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data);
 void mmci_dmae_error(struct mmci_host *host);
 #endif
 
+void mmci_variant_init(struct mmci_host *host);
+void mmci_ux500v2_variant_init(struct mmci_host *host);
+
 #ifdef CONFIG_MMC_QCOM_DML
 void qcom_variant_init(struct mmci_host *host);
 #else
-- 
2.7.4

Re: [PATCH V2 3/6] mmc: mmci: define get_dctrl_cfg for legacy variant

[Ulf Hansson]

On Thu, 7 Mar 2019 at 17:39, Ludovic Barre <ludovic.Barre@st.com> wrote:
>
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch defines get_dctrl_cfg callback for legacy variants
> whatever DMA_ENGINE configuration.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  drivers/mmc/host/mmci.c | 44 ++++++++++++++++++++++++++++++++++++--------
>  drivers/mmc/host/mmci.h |  3 +++
>  2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 28b76c5..52f9dbf 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -46,12 +46,6 @@
>
>  #define DRIVER_NAME "mmci-pl18x"
>
> -#ifdef CONFIG_DMA_ENGINE
> -static void mmci_variant_init(struct mmci_host *host);
> -#else
> -static inline void mmci_variant_init(struct mmci_host *host) {}
> -#endif
> -
>  static unsigned int fmax = 515633;
>
>  static struct variant_data variant_arm = {
> @@ -231,7 +225,7 @@ static struct variant_data variant_ux500v2 = {
>         .irq_pio_mask           = MCI_IRQ_PIO_MASK,
>         .start_err              = MCI_STARTBITERR,
>         .opendrain              = MCI_OD,
> -       .init                   = mmci_variant_init,
> +       .init                   = mmci_ux500v2_variant_init,
>  };
>
>  static struct variant_data variant_stm32 = {
> @@ -617,6 +611,29 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
>         sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
>  }
>
> +u32 mmci_get_dctrl_cfg(struct mmci_host *host)
> +{
> +       u32 datactrl;
> +
> +       datactrl = MCI_DPSM_ENABLE;
> +       datactrl |= mmci_dctrl_blksz(host) | mmci_dctrl_dir(host);
> +       datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
> +
> +       return datactrl;
> +}
> +
> +u32 mmci_ux500v2_get_dctrl_cfg(struct mmci_host *host)

To be consistent with other variant function names, let's drop the
"mmci_"* prefix.

> +{
> +       u32 datactrl;
> +
> +       datactrl = MCI_DPSM_ENABLE;
> +       datactrl |= (host->data->blksz << 16);
> +       datactrl |= mmci_dctrl_dir(host);
> +       datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
> +
> +       return datactrl;
> +}
> +
>  /*
>   * All the DMA operation mode stuff goes inside this ifdef.
>   * This assumes that you have a generic DMA device interface,
> @@ -941,6 +958,7 @@ void mmci_dmae_unprep_data(struct mmci_host *host,
>  static struct mmci_host_ops mmci_variant_ops = {
>         .prep_data = mmci_dmae_prep_data,
>         .unprep_data = mmci_dmae_unprep_data,
> +       .get_datactrl_cfg = mmci_get_dctrl_cfg,
>         .get_next_data = mmci_dmae_get_next_data,
>         .dma_setup = mmci_dmae_setup,
>         .dma_release = mmci_dmae_release,
> @@ -948,12 +966,22 @@ static struct mmci_host_ops mmci_variant_ops = {
>         .dma_finalize = mmci_dmae_finalize,
>         .dma_error = mmci_dmae_error,
>  };
> +#else
> +static struct mmci_host_ops mmci_variant_ops = {
> +       .get_datactrl_cfg = mmci_get_dctrl_cfg,
> +}
> +#endif
>
>  void mmci_variant_init(struct mmci_host *host)
>  {
>         host->ops = &mmci_variant_ops;
>  }
> -#endif
> +
> +void mmci_ux500v2_variant_init(struct mmci_host *host)
> +{
> +       host->ops = &mmci_variant_ops;
> +       host->ops->get_datactrl_cfg = mmci_ux500v2_get_dctrl_cfg;
> +}
>
>  static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index 32ae41d..bfc6e54 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -448,6 +448,9 @@ void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data);
>  void mmci_dmae_error(struct mmci_host *host);
>  #endif
>
> +void mmci_variant_init(struct mmci_host *host);
> +void mmci_ux500v2_variant_init(struct mmci_host *host);

Drop the "mmci_" prefix.

> +
>  #ifdef CONFIG_MMC_QCOM_DML
>  void qcom_variant_init(struct mmci_host *host);
>  #else
> --
> 2.7.4
>

Kind regards
Uffe

[PATCH V2 4/6] mmc: mmci: qcom: define get_dctrl_cfg

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch defines get_dctrl_cfg callback for qcom variant.
qcom variant has a specific block size definition.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci_qcom_dml.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index ccc1b18..f437ad1 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c
@@ -188,9 +188,22 @@ static int qcom_dma_setup(struct mmci_host *host)
 	return 0;
 }
 
+static u32 qcom_get_dctrl_cfg(struct mmci_host *host)
+{
+	u32 datactrl;
+
+	datactrl = MCI_DPSM_ENABLE;
+	datactrl |= (host->data->blksz << 4);
+	datactrl |= mmci_dctrl_dir(host);
+	datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
+
+	return datactrl;
+}
+
 static struct mmci_host_ops qcom_variant_ops = {
 	.prep_data = mmci_dmae_prep_data,
 	.unprep_data = mmci_dmae_unprep_data,
+	.get_datactrl_cfg = qcom_get_dctrl_cfg,
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = qcom_dma_setup,
 	.dma_release = mmci_dmae_release,
-- 
2.7.4

[PATCH V2 5/6] mmc: mmci: stm32: define get_dctrl_cfg

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch defines get_dctrl_cfg callback for sdmmc variant.
sdmmc variant has specific stm32 transfer modes.
sdmmc data transfer mode selection could be:
-Block data transfer ending on block count.
-SDIO multibyte data transfer.
-MMC Stream data transfer (not used).
-Block data transfer ending with STOP_TRANSMISSION command.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.h             |  5 +++++
 drivers/mmc/host/mmci_stm32_sdmmc.c | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index bfc6e54..4d33c4f 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -131,6 +131,11 @@
 /* Control register extensions in the Qualcomm versions */
 #define MCI_DPSM_QCOM_DATA_PEND	BIT(17)
 #define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20)
+/* Control register extensions in STM32 versions */
+#define MCI_DPSM_STM32_MODE_BLOCK	(0 << 2)
+#define MCI_DPSM_STM32_MODE_SDIO	(1 << 2)
+#define MCI_DPSM_STM32_MODE_STREAM	(2 << 2)
+#define MCI_DPSM_STM32_MODE_BLOCK_STOP	(3 << 2)
 
 #define MMCIDATACNT		0x030
 #define MMCISTATUS		0x034
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index cfbfc6f..4c710e1 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -265,10 +265,29 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr)
 	}
 }
 
+static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host)
+{
+	u32 datactrl;
+
+	datactrl = mmci_dctrl_blksz(host) | mmci_dctrl_dir(host);
+	datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host);
+
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card) &&
+	    host->data->blocks == 1)
+		datactrl |= MCI_DPSM_STM32_MODE_SDIO;
+	else if (host->data->stop && !host->mrq->sbc)
+		datactrl |= MCI_DPSM_STM32_MODE_BLOCK_STOP;
+	else
+		datactrl |= MCI_DPSM_STM32_MODE_BLOCK;
+
+	return datactrl;
+}
+
 static struct mmci_host_ops sdmmc_variant_ops = {
 	.validate_data = sdmmc_idma_validate_data,
 	.prep_data = sdmmc_idma_prep_data,
 	.unprep_data = sdmmc_idma_unprep_data,
+	.get_datactrl_cfg = sdmmc_get_dctrl_cfg,
 	.dma_setup = sdmmc_idma_setup,
 	.dma_start = sdmmc_idma_start,
 	.dma_finalize = sdmmc_idma_finalize,
-- 
2.7.4

[PATCH V2 6/6] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch allows to get datactrl configuration specific
at variant. This introduce more flexibility on datactlr
value.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 31 +------------------------------
 drivers/mmc/host/mmci.h |  7 -------
 2 files changed, 1 insertion(+), 37 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 52f9dbf..dcee78f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -57,7 +57,6 @@ static struct variant_data variant_arm = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 16,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.reversed_irq_handling	= true,
@@ -77,7 +76,6 @@ static struct variant_data variant_arm_extended_fifo = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 16,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.mmcimask1		= true,
@@ -97,7 +95,6 @@ static struct variant_data variant_arm_extended_fifo_hwfc = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 16,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.mmcimask1		= true,
@@ -118,7 +115,6 @@ static struct variant_data variant_u300 = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 16,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
 	.st_sdio			= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
@@ -144,7 +140,6 @@ static struct variant_data variant_nomadik = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 24,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
 	.st_sdio		= true,
 	.st_clkdiv		= true,
@@ -173,7 +168,6 @@ static struct variant_data variant_ux500 = {
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.datalength_bits	= 24,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
 	.st_sdio		= true,
 	.st_clkdiv		= true,
@@ -207,11 +201,9 @@ static struct variant_data variant_ux500v2 = {
 	.datactrl_mask_ddrmode	= MCI_DPSM_ST_DDRMODE,
 	.datalength_bits	= 24,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
 	.st_sdio		= true,
 	.st_clkdiv		= true,
-	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.f_max			= 100000000,
 	.signal_direction	= true,
@@ -242,7 +234,6 @@ static struct variant_data variant_stm32 = {
 	.irq_pio_mask		= MCI_IRQ_PIO_MASK,
 	.datalength_bits	= 24,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
 	.st_sdio		= true,
 	.st_clkdiv		= true,
@@ -286,10 +277,8 @@ static struct variant_data variant_qcom = {
 	.cmdreg_srsp_crc	= MCI_CPSM_RESPONSE,
 	.cmdreg_srsp		= MCI_CPSM_RESPONSE,
 	.data_cmd_enable	= MCI_CPSM_QCOM_DATCMD,
-	.blksz_datactrl4	= true,
 	.datalength_bits	= 24,
 	.datactrl_blocksz	= 11,
-	.datactrl_dpsm_enable	= MCI_DPSM_ENABLE,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 208000000,
 	.explicit_mclk_control	= true,
@@ -1042,7 +1031,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	unsigned int datactrl, timeout, irqmask;
 	unsigned long long clks;
 	void __iomem *base;
-	int blksz_bits;
 
 	dev_dbg(mmc_dev(host->mmc), "blksz %04x blks %04x flags %08x\n",
 		data->blksz, data->blocks, data->flags);
@@ -1060,24 +1048,11 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(timeout, base + MMCIDATATIMER);
 	writel(host->size, base + MMCIDATALENGTH);
 
-	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
-
-	if (variant->blksz_datactrl16)
-		datactrl = variant->datactrl_dpsm_enable | (data->blksz << 16);
-	else if (variant->blksz_datactrl4)
-		datactrl = variant->datactrl_dpsm_enable | (data->blksz << 4);
-	else
-		datactrl = variant->datactrl_dpsm_enable | blksz_bits << 4;
-
-	if (data->flags & MMC_DATA_READ)
-		datactrl |= MCI_DPSM_DIRECTION;
+	datactrl = host->ops->get_datactrl_cfg(host);
 
 	if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
 		u32 clk;
 
-		datactrl |= variant->datactrl_mask_sdio;
-
 		/*
 		 * The ST Micro variant for SDIO small write transfers
 		 * needs to have clock H/W flow control disabled,
@@ -1094,10 +1069,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		mmci_write_clkreg(host, clk);
 	}
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 ||
-	    host->mmc->ios.timing == MMC_TIMING_MMC_DDR52)
-		datactrl |= variant->datactrl_mask_ddrmode;
-
 	/*
 	 * Attempt to use DMA operation mode, if this
 	 * should fail, fall back to PIO mode
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 4d33c4f..eefeeb0 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -280,12 +280,8 @@ struct mmci_host;
  * @st_clkdiv: true if using a ST-specific clock divider algorithm
  * @stm32_clkdiv: true if using a STM32-specific clock divider algorithm
  * @datactrl_mask_ddrmode: ddr mode mask in datactrl register.
- * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register
- * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl
- *		     register
  * @datactrl_mask_sdio: SDIO enable mask in datactrl register
  * @datactrl_blksz: block size in power of two
- * @datactrl_dpsm_enable: enable value for DPSM
  * @datactrl_first: true if data must be setup before send command
  * @datacnt_useless: true if you could not use datacnt register to read
  *		     remaining data
@@ -330,14 +326,11 @@ struct variant_data {
 	unsigned int		datactrl_mask_ddrmode;
 	unsigned int		datactrl_mask_sdio;
 	unsigned int		datactrl_blocksz;
-	unsigned int		datactrl_dpsm_enable;
 	u8			datactrl_first:1;
 	u8			datacnt_useless:1;
 	u8			st_sdio:1;
 	u8			st_clkdiv:1;
 	u8			stm32_clkdiv:1;
-	u8			blksz_datactrl16:1;
-	u8			blksz_datactrl4:1;
 	u32			pwrreg_powerup;
 	u32			f_max;
 	u8			signal_direction:1;
-- 
2.7.4

Re: [PATCH V2 6/6] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback

[Russell King - ARM Linux admin]

On Thu, Mar 07, 2019 at 05:39:02PM +0100, Ludovic Barre wrote:
> -	if (data->flags & MMC_DATA_READ)
> -		datactrl |= MCI_DPSM_DIRECTION;

Given that this is currently an invariant between all, it doesn't make
sense to have a separate public function and combine it into the
get_datactrl_cfg() implementations.  You may as well leave it in place
here, after you call get_datactrl_cfg().

> +	datactrl = host->ops->get_datactrl_cfg(host);

Otherwise, I don't see a problem with this, although it would be nice to
avoid the overhead of so many public functions, which could be done by
adding them as inline functions in mmci.h

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH V2 6/6] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback

[Ludovic BARRE]

hi Russell, Ulf

On 3/7/19 5:46 PM, Russell King - ARM Linux admin wrote:
> On Thu, Mar 07, 2019 at 05:39:02PM +0100, Ludovic Barre wrote:
>> -	if (data->flags & MMC_DATA_READ)
>> -		datactrl |= MCI_DPSM_DIRECTION;
> 
> Given that this is currently an invariant between all, it doesn't make
> sense to have a separate public function and combine it into the
> get_datactrl_cfg() implementations.  You may as well leave it in place
> here, after you call get_datactrl_cfg().
> 
>> +	datactrl = host->ops->get_datactrl_cfg(host);
> 
> Otherwise, I don't see a problem with this, although it would be nice to
> avoid the overhead of so many public functions, which could be done by
> adding them as inline functions in mmci.h
> 

To combine your comments (above and https://lkml.org/lkml/2019/3/6/318).
I could regroup mmci_dctrl_dir & mmci_dctrl_ddr & mmci_dctrl_sdio in a
common function mmci_dctrl_common and call by:
-Each get_datactrl_cfg variant
-Or in mmci_start_data

What do you prefer ?

Regards,
Ludo