From: Ludovic Barre <[email protected]>
This patch series adds get_datactrl_cfg callback in mmci_host_ops
to allow to get datactrl configuration specific at variant.
change V3:
-keep the common functions in mmci_start_data. define
function used by some variants like an helper
(example mmci_dctrl_blks used by mmci and sdmmc).
-To be consistent rename callback for ux500v2
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 (5):
mmc: mmci: add get_datactrl_cfg callback and helper functions
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 | 57 ++++++++++++++++---------------------
drivers/mmc/host/mmci.h | 21 +++++++++-----
drivers/mmc/host/mmci_qcom_dml.c | 6 ++++
drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++++++++++++
4 files changed, 63 insertions(+), 39 deletions(-)
--
2.7.4
From: Ludovic Barre <[email protected]>
This patch defines get_dctrl_cfg callback for legacy variants
whatever DMA_ENGINE configuration.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.c | 31 +++++++++++++++++++++++--------
drivers/mmc/host/mmci.h | 3 +++
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9e6a2c1..4041e36 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 = ux500v2_variant_init,
};
static struct variant_data variant_stm32 = {
@@ -617,6 +611,16 @@ 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);
}
+static u32 mmci_get_dctrl_cfg(struct mmci_host *host)
+{
+ return MCI_DPSM_ENABLE | mmci_dctrl_blksz(host);
+}
+
+static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host)
+{
+ return MCI_DPSM_ENABLE | (host->data->blksz << 16);
+}
+
/*
* All the DMA operation mode stuff goes inside this ifdef.
* This assumes that you have a generic DMA device interface,
@@ -941,6 +945,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 +953,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 ux500v2_variant_init(struct mmci_host *host)
+{
+ host->ops = &mmci_variant_ops;
+ host->ops->get_datactrl_cfg = 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 35c91d0..c614ea7 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 ux500v2_variant_init(struct mmci_host *host);
+
#ifdef CONFIG_MMC_QCOM_DML
void qcom_variant_init(struct mmci_host *host);
#else
--
2.7.4
From: Ludovic Barre <[email protected]>
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 <[email protected]>
---
drivers/mmc/host/mmci.h | 5 +++++
drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index c614ea7..fd24fdb 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..8e83ae6 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -265,10 +265,28 @@ 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);
+
+ 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
From: Ludovic Barre <[email protected]>
This patch allows to get datactrl configuration specific
at variant. This introduce more flexibility on datactlr
value.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.c | 26 ++------------------------
drivers/mmc/host/mmci.h | 7 -------
2 files changed, 2 insertions(+), 31 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4041e36..cd3a65c 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,
@@ -1004,7 +993,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);
@@ -1022,18 +1010,8 @@ 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);
+ datactrl |= host->data->flags & MMC_DATA_READ ? MCI_DPSM_DIRECTION : 0;
if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
u32 clk;
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index fd24fdb..a7bc557 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
From: Ludovic Barre <[email protected]>
This patch defines get_dctrl_cfg callback for qcom variant.
qcom variant has a specific block size definition.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci_qcom_dml.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index ccc1b18..3af396b 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c
@@ -188,9 +188,15 @@ static int qcom_dma_setup(struct mmci_host *host)
return 0;
}
+static u32 qcom_get_dctrl_cfg(struct mmci_host *host)
+{
+ return MCI_DPSM_ENABLE | (host->data->blksz << 4);
+}
+
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
From: Ludovic Barre <[email protected]>
This patch adds get_datactrl_cfg callback in mmci_host_ops
to allow to get datactrl configuration specific at variant.
Common helper function is defined and could be call by variant.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 6bde28c..35c91d0 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);
@@ -429,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);
+static inline u32 mmci_dctrl_blksz(struct mmci_host *host)
+{
+ return (ffs(host->data->blksz) - 1) << 4;
+}
+
#ifdef CONFIG_DMA_ENGINE
int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
bool next);
--
2.7.4
On Tue, 26 Mar 2019 at 10:11, Ludovic Barre <[email protected]> wrote:
>
> From: Ludovic Barre <[email protected]>
>
> This patch defines get_dctrl_cfg callback for legacy variants
> whatever DMA_ENGINE configuration.
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> drivers/mmc/host/mmci.c | 31 +++++++++++++++++++++++--------
> drivers/mmc/host/mmci.h | 3 +++
> 2 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 9e6a2c1..4041e36 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
Looks like you are moving the declaration to the header file. I would
rather keep it here as there is no point in sharing to another c-file
(at least not yet). The same applies for the new ux500v2 init
function.
Other than that, this looks good to me!
Kind regards
Uffe
On Tue, 26 Mar 2019 at 10:11, Ludovic Barre <[email protected]> wrote:
>
> From: Ludovic Barre <[email protected]>
>
> This patch series adds get_datactrl_cfg callback in mmci_host_ops
> to allow to get datactrl configuration specific at variant.
>
> change V3:
> -keep the common functions in mmci_start_data. define
> function used by some variants like an helper
> (example mmci_dctrl_blks used by mmci and sdmmc).
> -To be consistent rename callback for ux500v2
>
> 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 (5):
> mmc: mmci: add get_datactrl_cfg callback and helper functions
> 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 | 57 ++++++++++++++++---------------------
> drivers/mmc/host/mmci.h | 21 +++++++++-----
> drivers/mmc/host/mmci_qcom_dml.c | 6 ++++
> drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++++++++++++
> 4 files changed, 63 insertions(+), 39 deletions(-)
>
> --
> 2.7.4
>
Overall I think think this is step in right direction, nice work!
However, I also think there are more things that should be moved from
mmci_start_data() into the new ->get_datactrl_cfg() ops, such as the
SDIO things and the DDR mode things. Although, let's consider that as
improvements that can be made on top.
Besides the little comment I had on patch2, this looks good to me.
Kind regards
Uffe
On 3/26/19 6:46 PM, Ulf Hansson wrote:
> On Tue, 26 Mar 2019 at 10:11, Ludovic Barre <[email protected]> wrote:
>>
>> From: Ludovic Barre <[email protected]>
>>
>> This patch defines get_dctrl_cfg callback for legacy variants
>> whatever DMA_ENGINE configuration.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> drivers/mmc/host/mmci.c | 31 +++++++++++++++++++++++--------
>> drivers/mmc/host/mmci.h | 3 +++
>> 2 files changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 9e6a2c1..4041e36 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
>
> Looks like you are moving the declaration to the header file. I would
> rather keep it here as there is no point in sharing to another c-file
> (at least not yet). The same applies for the new ux500v2 init
> function.
no problem, I will resend a V4 to keep mmci and ux500v2 variant init in
the c file.
>
> Other than that, this looks good to me!
thanks
>
> Kind regards
> Uffe
>