To support the sdr104 mode, sdmmc variant needs:
-Hardware delay block support for sdmmc variant
with tuning procedure
-Voltage switch callbacks
-sdmmc revision 2.0
Ludovic Barre (9):
mmc: mmci: sdmmc: replace sg_dma_xxx macros
mmc: mmci: sdmmc: rename sdmmc_priv struct to sdmmc_idma
mmc: mmci: add a reference at mmc_host_ops in mmci struct
mmc: mmci: add private pointer for variant
dt-bindings: mmc: mmci: add delay block base register for sdmmc
mmc: mmci: sdmmc: add execute tuning with delay block
mmc: mmci: add volt_switch callbacks
mmc: mmci: sdmmc: add voltage switch functions
mmc: mmci: add sdmmc variant revision 2.0
.../devicetree/bindings/mmc/mmci.txt | 2 +
drivers/mmc/host/mmci.c | 39 ++++
drivers/mmc/host/mmci.h | 8 +
drivers/mmc/host/mmci_stm32_sdmmc.c | 199 +++++++++++++++++-
4 files changed, 241 insertions(+), 7 deletions(-)
--
2.17.1
The hardware delay block is used to align the sampling clock on
the data received by SDMMC. It is mandatory for SDMMC to
support the SDR104 mode. The delay block is used to generate
an output clock which is dephased from the input clock.
The phase of the output clock must be programmed by the execute
tuning interface.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 147 ++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index df08f6662431..10059fa19f4a 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -3,10 +3,13 @@
* Copyright (C) STMicroelectronics 2018 - All Rights Reserved
* Author: [email protected] for STMicroelectronics.
*/
+#include <linux/bitfield.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
#include <linux/mmc/host.h>
#include <linux/mmc/card.h>
+#include <linux/of_address.h>
#include <linux/reset.h>
#include <linux/scatterlist.h>
#include "mmci.h"
@@ -14,6 +17,20 @@
#define SDMMC_LLI_BUF_LEN PAGE_SIZE
#define SDMMC_IDMA_BURST BIT(MMCI_STM32_IDMABNDT_SHIFT)
+#define DLYB_CR 0x0
+#define DLYB_CR_DEN BIT(0)
+#define DLYB_CR_SEN BIT(1)
+
+#define DLYB_CFGR 0x4
+#define DLYB_CFGR_SEL_MASK GENMASK(3, 0)
+#define DLYB_CFGR_UNIT_MASK GENMASK(14, 8)
+#define DLYB_CFGR_LNG_MASK GENMASK(27, 16)
+#define DLYB_CFGR_LNGF BIT(31)
+
+#define DLYB_NB_DELAY 11
+#define DLYB_CFGR_SEL_MAX (DLYB_NB_DELAY + 1)
+#define DLYB_CFGR_UNIT_MAX 127
+
struct sdmmc_lli_desc {
u32 idmalar;
u32 idmabase;
@@ -25,6 +42,12 @@ struct sdmmc_idma {
void *sg_cpu;
};
+struct sdmmc_dlyb {
+ void __iomem *base;
+ u32 unit;
+ u32 max;
+};
+
static int sdmmc_idma_validate_data(struct mmci_host *host,
struct mmc_data *data)
{
@@ -226,12 +249,24 @@ static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
mmci_write_clkreg(host, clk);
}
+static void sdmmc_dlyb_input_ck(struct sdmmc_dlyb *dlyb)
+{
+ if (!dlyb || !dlyb->base)
+ return;
+
+ /* Output clock = Input clock */
+ writel_relaxed(0, dlyb->base + DLYB_CR);
+}
+
static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr)
{
struct mmc_ios ios = host->mmc->ios;
+ struct sdmmc_dlyb *dlyb = host->variant_priv;
pwr = host->pwr_reg_add;
+ sdmmc_dlyb_input_ck(dlyb);
+
if (ios.power_mode == MMC_POWER_OFF) {
/* Only a reset could power-off sdmmc */
reset_control_assert(host->rst);
@@ -323,6 +358,102 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
return true;
}
+static void sdmmc_dlyb_set_cfgr(struct sdmmc_dlyb *dlyb,
+ int unit, int phase, bool sampler)
+{
+ u32 cfgr;
+
+ writel_relaxed(DLYB_CR_SEN | DLYB_CR_DEN, dlyb->base + DLYB_CR);
+
+ cfgr = FIELD_PREP(DLYB_CFGR_UNIT_MASK, unit) |
+ FIELD_PREP(DLYB_CFGR_SEL_MASK, phase);
+ writel_relaxed(cfgr, dlyb->base + DLYB_CFGR);
+
+ if (!sampler)
+ writel_relaxed(DLYB_CR_DEN, dlyb->base + DLYB_CR);
+}
+
+static int sdmmc_dlyb_lng_tuning(struct mmci_host *host)
+{
+ struct sdmmc_dlyb *dlyb = host->variant_priv;
+ u32 cfgr;
+ int i, lng, ret;
+
+ for (i = 0; i <= DLYB_CFGR_UNIT_MAX; i++) {
+ sdmmc_dlyb_set_cfgr(dlyb, i, DLYB_CFGR_SEL_MAX, true);
+
+ ret = readl_relaxed_poll_timeout(dlyb->base + DLYB_CFGR, cfgr,
+ (cfgr & DLYB_CFGR_LNGF),
+ 1, 1000);
+ if (ret) {
+ dev_warn(mmc_dev(host->mmc),
+ "delay line cfg timeout unit:%d cfgr:%d\n",
+ i, cfgr);
+ continue;
+ }
+
+ lng = FIELD_GET(DLYB_CFGR_LNG_MASK, cfgr);
+ if (lng < BIT(DLYB_NB_DELAY) && lng > 0)
+ break;
+ }
+
+ if (i > DLYB_CFGR_UNIT_MAX)
+ return -EINVAL;
+
+ dlyb->unit = i;
+ dlyb->max = __fls(lng);
+
+ return 0;
+}
+
+static int sdmmc_dlyb_phase_tuning(struct mmci_host *host, u32 opcode)
+{
+ struct sdmmc_dlyb *dlyb = host->variant_priv;
+ int cur_len = 0, max_len = 0, end_of_len = 0;
+ int phase;
+
+ for (phase = 0; phase <= dlyb->max; phase++) {
+ sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
+
+ if (mmc_send_tuning(host->mmc, opcode, NULL)) {
+ cur_len = 0;
+ } else {
+ cur_len++;
+ if (cur_len > max_len) {
+ max_len = cur_len;
+ end_of_len = phase;
+ }
+ }
+ }
+
+ if (!max_len) {
+ dev_err(mmc_dev(host->mmc), "no tuning point found\n");
+ return -EINVAL;
+ }
+
+ phase = end_of_len - max_len / 2;
+ sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
+
+ dev_dbg(mmc_dev(host->mmc), "unit:%d max_dly:%d phase:%d\n",
+ dlyb->unit, dlyb->max, phase);
+
+ return 0;
+}
+
+static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
+{
+ struct mmci_host *host = mmc_priv(mmc);
+ struct sdmmc_dlyb *dlyb = host->variant_priv;
+
+ if (!dlyb || !dlyb->base)
+ return -EINVAL;
+
+ if (sdmmc_dlyb_lng_tuning(host))
+ return -EINVAL;
+
+ return sdmmc_dlyb_phase_tuning(host, opcode);
+}
+
static struct mmci_host_ops sdmmc_variant_ops = {
.validate_data = sdmmc_idma_validate_data,
.prep_data = sdmmc_idma_prep_data,
@@ -338,5 +469,21 @@ static struct mmci_host_ops sdmmc_variant_ops = {
void sdmmc_variant_init(struct mmci_host *host)
{
+ struct device_node *np = host->mmc->parent->of_node;
+ void __iomem *base_dlyb;
+ struct sdmmc_dlyb *dlyb;
+
host->ops = &sdmmc_variant_ops;
+
+ base_dlyb = devm_of_iomap(mmc_dev(host->mmc), np, 1, NULL);
+ if (IS_ERR(base_dlyb))
+ return;
+
+ dlyb = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dlyb), GFP_KERNEL);
+ if (!dlyb)
+ return;
+
+ dlyb->base = base_dlyb;
+ host->variant_priv = dlyb;
+ host->mmc_ops->execute_tuning = sdmmc_execute_tuning;
}
--
2.17.1
In variant init function, some references may be allocated for
variant specific usage. Add a private void* to mmci_host struct
allows at variant functions to access on this references by
mmci_host structure.
Signed-off-by: Ludovic Barre <[email protected]>
---
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 55acc0971a44..ddcdfb827996 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -410,6 +410,7 @@ struct mmci_host {
struct mmc_host_ops *mmc_ops;
struct mmci_host_ops *ops;
struct variant_data *variant;
+ void *variant_priv;
struct pinctrl *pinctrl;
struct pinctrl_state *pins_opendrain;
--
2.17.1
To prepare the voltage switch procedure, the VSWITCHEN bit must be
set before sending the cmd11.
To confirm completion of voltage switch, the VSWEND flag must be
checked.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.h | 4 +++
drivers/mmc/host/mmci_stm32_sdmmc.c | 40 ++++++++++++++++++++++++++++-
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index c04a144259a2..3634f98ad2d8 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -165,6 +165,7 @@
/* Extended status bits for the STM32 variants */
#define MCI_STM32_BUSYD0 BIT(20)
#define MCI_STM32_BUSYD0END BIT(21)
+#define MCI_STM32_VSWEND BIT(25)
#define MMCICLEAR 0x038
#define MCI_CMDCRCFAILCLR (1 << 0)
@@ -182,6 +183,9 @@
#define MCI_ST_SDIOITC (1 << 22)
#define MCI_ST_CEATAENDC (1 << 23)
#define MCI_ST_BUSYENDC (1 << 24)
+/* Extended clear bits for the STM32 variants */
+#define MCI_STM32_VSWENDC BIT(25)
+#define MCI_STM32_CKSTOPC BIT(26)
#define MMCIMASK0 0x03c
#define MCI_CMDCRCFAILMASK (1 << 0)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 10059fa19f4a..9f43cf947c5f 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -263,7 +263,9 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr)
struct mmc_ios ios = host->mmc->ios;
struct sdmmc_dlyb *dlyb = host->variant_priv;
- pwr = host->pwr_reg_add;
+ /* adds OF options and preserves voltage switch bits */
+ pwr = host->pwr_reg_add |
+ (host->pwr_reg & (MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
sdmmc_dlyb_input_ck(dlyb);
@@ -454,6 +456,40 @@ static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
return sdmmc_dlyb_phase_tuning(host, opcode);
}
+static void sdmmc_prep_vswitch(struct mmci_host *host)
+{
+ /* clear the voltage switch completion flag */
+ writel_relaxed(MCI_STM32_VSWENDC, host->base + MMCICLEAR);
+ /* enable Voltage switch procedure */
+ mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCHEN);
+}
+
+static int sdmmc_vswitch(struct mmci_host *host, struct mmc_ios *ios)
+{
+ unsigned long flags;
+ u32 status;
+ int ret = 0;
+
+ if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+ spin_lock_irqsave(&host->lock, flags);
+ mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCH);
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ /* wait voltage switch completion while 10ms */
+ ret = readl_relaxed_poll_timeout(host->base + MMCISTATUS,
+ status,
+ (status & MCI_STM32_VSWEND),
+ 10, 10000);
+
+ writel_relaxed(MCI_STM32_VSWENDC | MCI_STM32_CKSTOPC,
+ host->base + MMCICLEAR);
+ mmci_write_pwrreg(host, host->pwr_reg &
+ ~(MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
+ }
+
+ return ret;
+}
+
static struct mmci_host_ops sdmmc_variant_ops = {
.validate_data = sdmmc_idma_validate_data,
.prep_data = sdmmc_idma_prep_data,
@@ -465,6 +501,8 @@ static struct mmci_host_ops sdmmc_variant_ops = {
.set_clkreg = mmci_sdmmc_set_clkreg,
.set_pwrreg = mmci_sdmmc_set_pwrreg,
.busy_complete = sdmmc_busy_complete,
+ .prep_volt_switch = sdmmc_prep_vswitch,
+ .volt_switch = sdmmc_vswitch,
};
void sdmmc_variant_init(struct mmci_host *host)
--
2.17.1
This patch adds a sdmmc variant revision 2.0.
This revision is backward compatible with 1.1, and adds dma
link list support.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index d76a59c06cb0..2a570cbf6f69 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -275,6 +275,31 @@ static struct variant_data variant_stm32_sdmmc = {
.init = sdmmc_variant_init,
};
+static struct variant_data variant_stm32_sdmmcv2 = {
+ .fifosize = 16 * 4,
+ .fifohalfsize = 8 * 4,
+ .f_max = 208000000,
+ .stm32_clkdiv = 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,
+ .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP,
+ .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,
+ .datactrl_any_blocksz = true,
+ .stm32_idmabsize_mask = GENMASK(16, 5),
+ .dma_lli = true,
+ .busy_timeout = true,
+ .busy_detect_flag = MCI_STM32_BUSYD0,
+ .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
+ .init = sdmmc_variant_init,
+};
+
static struct variant_data variant_qcom = {
.fifosize = 16 * 4,
.fifohalfsize = 8 * 4,
@@ -2334,6 +2359,11 @@ static const struct amba_id mmci_ids[] = {
.mask = 0xf0ffffff,
.data = &variant_stm32_sdmmc,
},
+ {
+ .id = 0x00253180,
+ .mask = 0xf0ffffff,
+ .data = &variant_stm32_sdmmcv2,
+ },
/* Qualcomm variants */
{
.id = 0x00051180,
--
2.17.1
This patch adds 2 voltage switch callbacks in mmci_host_ops:
-prep_volt_switch allows to prepare voltage switch before to
sent the SD_SWITCH_VOLTAGE command (cmd11).
-volt_switch callback allows to define specific action after
regulator set voltage.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.c | 8 ++++++++
drivers/mmc/host/mmci.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 00b473f57047..d76a59c06cb0 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -22,6 +22,7 @@
#include <linux/mmc/pm.h>
#include <linux/mmc/host.h>
#include <linux/mmc/card.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/slot-gpio.h>
#include <linux/amba/bus.h>
#include <linux/clk.h>
@@ -1207,6 +1208,9 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
writel_relaxed(clks, host->base + MMCIDATATIMER);
}
+ if (host->ops->prep_volt_switch && cmd->opcode == SD_SWITCH_VOLTAGE)
+ host->ops->prep_volt_switch(host);
+
if (/*interrupt*/0)
c |= MCI_CPSM_INTERRUPT;
@@ -1820,6 +1824,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
{
+ struct mmci_host *host = mmc_priv(mmc);
int ret = 0;
if (!IS_ERR(mmc->supply.vqmmc)) {
@@ -1839,6 +1844,9 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
break;
}
+ if (!ret && host->ops && host->ops->volt_switch)
+ ret = host->ops->volt_switch(host, ios);
+
if (ret)
dev_warn(mmc_dev(mmc), "Voltage switch failed\n");
}
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index ddcdfb827996..c04a144259a2 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -377,6 +377,8 @@ struct mmci_host_ops {
void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
+ void (*prep_volt_switch)(struct mmci_host *host);
+ int (*volt_switch)(struct mmci_host *host, struct mmc_ios *ios);
};
struct mmci_host {
--
2.17.1
sg_dma_xxx should be used after a dma_map_sg call has been done
to get bus addresses of each of the SG entries and their lengths.
But mmci_host_ops validate_data can be called before dma_map_sg.
This patch replaces theses macros by sg->offset and sg->length
which are always defined.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index a4f7e8e689d3..6ccfbbc82c77 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -36,8 +36,8 @@ static int sdmmc_idma_validate_data(struct mmci_host *host,
* 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)) {
+ if (!IS_ALIGNED(data->sg->offset, sizeof(u32)) ||
+ !IS_ALIGNED(data->sg->length, SDMMC_IDMA_BURST)) {
dev_err(mmc_dev(host->mmc),
"unaligned scatterlist: ofst:%x length:%d\n",
data->sg->offset, data->sg->length);
@@ -45,7 +45,7 @@ static int sdmmc_idma_validate_data(struct mmci_host *host,
}
}
- if (!IS_ALIGNED(sg_dma_address(data->sg), sizeof(u32))) {
+ if (!IS_ALIGNED(data->sg->offset, sizeof(u32))) {
dev_err(mmc_dev(host->mmc),
"unaligned last scatterlist: ofst:%x length:%d\n",
data->sg->offset, data->sg->length);
--
2.17.1
This patch renames sdmmc_priv struct to sdmmc_idma
which is assigned to host->dma_priv.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 6ccfbbc82c77..df08f6662431 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -20,7 +20,7 @@ struct sdmmc_lli_desc {
u32 idmasize;
};
-struct sdmmc_priv {
+struct sdmmc_idma {
dma_addr_t sg_dma;
void *sg_cpu;
};
@@ -92,7 +92,7 @@ static void sdmmc_idma_unprep_data(struct mmci_host *host,
static int sdmmc_idma_setup(struct mmci_host *host)
{
- struct sdmmc_priv *idma;
+ struct sdmmc_idma *idma;
idma = devm_kzalloc(mmc_dev(host->mmc), sizeof(*idma), GFP_KERNEL);
if (!idma)
@@ -123,7 +123,7 @@ static int sdmmc_idma_setup(struct mmci_host *host)
static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
{
- struct sdmmc_priv *idma = host->dma_priv;
+ struct sdmmc_idma *idma = host->dma_priv;
struct sdmmc_lli_desc *desc = (struct sdmmc_lli_desc *)idma->sg_cpu;
struct mmc_data *data = host->data;
struct scatterlist *sg;
--
2.17.1
To support the sdr104 mode, the sdmmc variant has a
hardware delay block to manage the clock phase when sampling
data received by the card.
This patch adds a second base register (optional) for
sdmmc delay block.
Signed-off-by: Ludovic Barre <[email protected]>
---
Documentation/devicetree/bindings/mmc/mmci.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
index 6d3c626e017d..4ec921e4bf34 100644
--- a/Documentation/devicetree/bindings/mmc/mmci.txt
+++ b/Documentation/devicetree/bindings/mmc/mmci.txt
@@ -28,6 +28,8 @@ specific for ux500 variant:
- st,sig-pin-fbclk : feedback clock signal pin used.
specific for sdmmc variant:
+- reg : a second base register may be defined if a delay
+ block is present and used for tuning.
- st,sig-dir : signal direction polarity used for cmd, dat0 dat123.
- st,neg-edge : data & command phase relation, generated on
sd clock falling edge.
--
2.17.1
This patch adds mmc_host_ops pointer in mmci struct.
The variant init function may need to add a mmc_host_ops,
for example to add the execute_tuning support if this feature
is available.
Signed-off-by: Ludovic Barre <[email protected]>
---
drivers/mmc/host/mmci.c | 1 +
drivers/mmc/host/mmci.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7b13d66cbb21..00b473f57047 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1923,6 +1923,7 @@ static int mmci_probe(struct amba_device *dev,
host = mmc_priv(mmc);
host->mmc = mmc;
+ host->mmc_ops = &mmci_ops;
/*
* Some variant (STM32) doesn't have opendrain bit, nevertheless
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index ea6a0b5779d4..55acc0971a44 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -407,6 +407,7 @@ struct mmci_host {
u32 mask1_reg;
u8 vqmmc_enabled:1;
struct mmci_platform_data *plat;
+ struct mmc_host_ops *mmc_ops;
struct mmci_host_ops *ops;
struct variant_data *variant;
struct pinctrl *pinctrl;
--
2.17.1
On Fri, Jan 10, 2020 at 02:48:19PM +0100, Ludovic Barre wrote:
> To support the sdr104 mode, the sdmmc variant has a
> hardware delay block to manage the clock phase when sampling
> data received by the card.
>
> This patch adds a second base register (optional) for
> sdmmc delay block.
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/mmci.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
> index 6d3c626e017d..4ec921e4bf34 100644
> --- a/Documentation/devicetree/bindings/mmc/mmci.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmci.txt
> @@ -28,6 +28,8 @@ specific for ux500 variant:
> - st,sig-pin-fbclk : feedback clock signal pin used.
>
> specific for sdmmc variant:
> +- reg : a second base register may be defined if a delay
> + block is present and used for tuning.
Which compatibles have a 2nd reg entry?
> - st,sig-dir : signal direction polarity used for cmd, dat0 dat123.
> - st,neg-edge : data & command phase relation, generated on
> sd clock falling edge.
> --
> 2.17.1
>
Hi Rob
Le 1/15/20 à 3:56 PM, Rob Herring a écrit :
> On Fri, Jan 10, 2020 at 02:48:19PM +0100, Ludovic Barre wrote:
>> To support the sdr104 mode, the sdmmc variant has a
>> hardware delay block to manage the clock phase when sampling
>> data received by the card.
>>
>> This patch adds a second base register (optional) for
>> sdmmc delay block.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mmc/mmci.txt | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
>> index 6d3c626e017d..4ec921e4bf34 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmci.txt
>> +++ b/Documentation/devicetree/bindings/mmc/mmci.txt
>> @@ -28,6 +28,8 @@ specific for ux500 variant:
>> - st,sig-pin-fbclk : feedback clock signal pin used.
>>
>> specific for sdmmc variant:
>> +- reg : a second base register may be defined if a delay
>> + block is present and used for tuning.
>
> Which compatibles have a 2nd reg entry?
In fact, mmci driver is ARM Amba driver (arm,primecell) and has only one
compatible "arm,pl18x".
The variants are identified by primecell-periphid property
(discovered at runtime with HW block register or defined by
device tree property "arm,primecell-periphid").
The defaults "arm,pl18x" variants have only one base register,
but the SDMMC need a second base register for these
delay block registers.
example of sdmmc node:
sdmmc1: sdmmc@58005000 {
compatible = "arm,pl18x", "arm,primecell";
arm,primecell-periphid = <0x00253180>;
reg = <0x58005000 0x1000>, <0x58006000 0x1000>;
};
what do you advise?
>
>> - st,sig-dir : signal direction polarity used for cmd, dat0 dat123.
>> - st,neg-edge : data & command phase relation, generated on
>> sd clock falling edge.
>> --
>> 2.17.1
>>
On Thu, Jan 16, 2020 at 3:21 AM Ludovic BARRE <[email protected]> wrote:
>
> Hi Rob
>
> Le 1/15/20 à 3:56 PM, Rob Herring a écrit :
> > On Fri, Jan 10, 2020 at 02:48:19PM +0100, Ludovic Barre wrote:
> >> To support the sdr104 mode, the sdmmc variant has a
> >> hardware delay block to manage the clock phase when sampling
> >> data received by the card.
> >>
> >> This patch adds a second base register (optional) for
> >> sdmmc delay block.
> >>
> >> Signed-off-by: Ludovic Barre <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/mmc/mmci.txt | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
> >> index 6d3c626e017d..4ec921e4bf34 100644
> >> --- a/Documentation/devicetree/bindings/mmc/mmci.txt
> >> +++ b/Documentation/devicetree/bindings/mmc/mmci.txt
> >> @@ -28,6 +28,8 @@ specific for ux500 variant:
> >> - st,sig-pin-fbclk : feedback clock signal pin used.
> >>
> >> specific for sdmmc variant:
> >> +- reg : a second base register may be defined if a delay
> >> + block is present and used for tuning.
> >
> > Which compatibles have a 2nd reg entry?
>
> In fact, mmci driver is ARM Amba driver (arm,primecell) and has only one
> compatible "arm,pl18x".
> The variants are identified by primecell-periphid property
> (discovered at runtime with HW block register or defined by
> device tree property "arm,primecell-periphid").
>
> The defaults "arm,pl18x" variants have only one base register,
> but the SDMMC need a second base register for these
> delay block registers.
>
> example of sdmmc node:
> sdmmc1: sdmmc@58005000 {
> compatible = "arm,pl18x", "arm,primecell";
> arm,primecell-periphid = <0x00253180>;
> reg = <0x58005000 0x1000>, <0x58006000 0x1000>;
> };
>
> what do you advise?
I missed that this is a primecell block. Just give some indication
which variants have this 2nd range.
Rob
Le 1/16/20 à 3:33 PM, Rob Herring a écrit :
> On Thu, Jan 16, 2020 at 3:21 AM Ludovic BARRE <[email protected]> wrote:
>>
>> Hi Rob
>>
>> Le 1/15/20 à 3:56 PM, Rob Herring a écrit :
>>> On Fri, Jan 10, 2020 at 02:48:19PM +0100, Ludovic Barre wrote:
>>>> To support the sdr104 mode, the sdmmc variant has a
>>>> hardware delay block to manage the clock phase when sampling
>>>> data received by the card.
>>>>
>>>> This patch adds a second base register (optional) for
>>>> sdmmc delay block.
>>>>
>>>> Signed-off-by: Ludovic Barre <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/mmc/mmci.txt | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
>>>> index 6d3c626e017d..4ec921e4bf34 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/mmci.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/mmci.txt
>>>> @@ -28,6 +28,8 @@ specific for ux500 variant:
>>>> - st,sig-pin-fbclk : feedback clock signal pin used.
>>>>
>>>> specific for sdmmc variant:
>>>> +- reg : a second base register may be defined if a delay
>>>> + block is present and used for tuning.
>>>
>>> Which compatibles have a 2nd reg entry?
>>
>> In fact, mmci driver is ARM Amba driver (arm,primecell) and has only one
>> compatible "arm,pl18x".
>> The variants are identified by primecell-periphid property
>> (discovered at runtime with HW block register or defined by
>> device tree property "arm,primecell-periphid").
>>
>> The defaults "arm,pl18x" variants have only one base register,
>> but the SDMMC need a second base register for these
>> delay block registers.
>>
>> example of sdmmc node:
>> sdmmc1: sdmmc@58005000 {
>> compatible = "arm,pl18x", "arm,primecell";
>> arm,primecell-periphid = <0x00253180>;
>> reg = <0x58005000 0x1000>, <0x58006000 0x1000>;
>> };
>>
>> what do you advise?
>
> I missed that this is a primecell block. Just give some indication
> which variants have this 2nd range.
Thanks Rob.
I will add primecell id(s) concerned by this 2nd range.
> 0
> Rob
>
hi Ulf
Just a "gentleman ping" on this series
https://lkml.org/lkml/2020/1/10/392
Regards
Ludo
Le 1/10/20 à 2:48 PM, Ludovic Barre a écrit :
> To support the sdr104 mode, sdmmc variant needs:
> -Hardware delay block support for sdmmc variant
> with tuning procedure
> -Voltage switch callbacks
> -sdmmc revision 2.0
>
> Ludovic Barre (9):
> mmc: mmci: sdmmc: replace sg_dma_xxx macros
> mmc: mmci: sdmmc: rename sdmmc_priv struct to sdmmc_idma
> mmc: mmci: add a reference at mmc_host_ops in mmci struct
> mmc: mmci: add private pointer for variant
> dt-bindings: mmc: mmci: add delay block base register for sdmmc
> mmc: mmci: sdmmc: add execute tuning with delay block
> mmc: mmci: add volt_switch callbacks
> mmc: mmci: sdmmc: add voltage switch functions
> mmc: mmci: add sdmmc variant revision 2.0
>
> .../devicetree/bindings/mmc/mmci.txt | 2 +
> drivers/mmc/host/mmci.c | 39 ++++
> drivers/mmc/host/mmci.h | 8 +
> drivers/mmc/host/mmci_stm32_sdmmc.c | 199 +++++++++++++++++-
> 4 files changed, 241 insertions(+), 7 deletions(-)
>
On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>
> This patch adds mmc_host_ops pointer in mmci struct.
> The variant init function may need to add a mmc_host_ops,
> for example to add the execute_tuning support if this feature
> is available.
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> drivers/mmc/host/mmci.c | 1 +
> drivers/mmc/host/mmci.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 7b13d66cbb21..00b473f57047 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1923,6 +1923,7 @@ static int mmci_probe(struct amba_device *dev,
>
> host = mmc_priv(mmc);
> host->mmc = mmc;
> + host->mmc_ops = &mmci_ops;
Nitpick:
Can you please also move the assignment "mmc->ops = &mmci_ops;" to
this place as well, as I think these belongs together.
>
> /*
> * Some variant (STM32) doesn't have opendrain bit, nevertheless
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index ea6a0b5779d4..55acc0971a44 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -407,6 +407,7 @@ struct mmci_host {
> u32 mask1_reg;
> u8 vqmmc_enabled:1;
> struct mmci_platform_data *plat;
> + struct mmc_host_ops *mmc_ops;
> struct mmci_host_ops *ops;
> struct variant_data *variant;
> struct pinctrl *pinctrl;
> --
> 2.17.1
>
Kind regards
Uffe
On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>
> To prepare the voltage switch procedure, the VSWITCHEN bit must be
> set before sending the cmd11.
> To confirm completion of voltage switch, the VSWEND flag must be
> checked.
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> drivers/mmc/host/mmci.h | 4 +++
> drivers/mmc/host/mmci_stm32_sdmmc.c | 40 ++++++++++++++++++++++++++++-
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index c04a144259a2..3634f98ad2d8 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -165,6 +165,7 @@
> /* Extended status bits for the STM32 variants */
> #define MCI_STM32_BUSYD0 BIT(20)
> #define MCI_STM32_BUSYD0END BIT(21)
> +#define MCI_STM32_VSWEND BIT(25)
>
> #define MMCICLEAR 0x038
> #define MCI_CMDCRCFAILCLR (1 << 0)
> @@ -182,6 +183,9 @@
> #define MCI_ST_SDIOITC (1 << 22)
> #define MCI_ST_CEATAENDC (1 << 23)
> #define MCI_ST_BUSYENDC (1 << 24)
> +/* Extended clear bits for the STM32 variants */
> +#define MCI_STM32_VSWENDC BIT(25)
> +#define MCI_STM32_CKSTOPC BIT(26)
>
> #define MMCIMASK0 0x03c
> #define MCI_CMDCRCFAILMASK (1 << 0)
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index 10059fa19f4a..9f43cf947c5f 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -263,7 +263,9 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr)
> struct mmc_ios ios = host->mmc->ios;
> struct sdmmc_dlyb *dlyb = host->variant_priv;
>
> - pwr = host->pwr_reg_add;
> + /* adds OF options and preserves voltage switch bits */
> + pwr = host->pwr_reg_add |
> + (host->pwr_reg & (MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
>
> sdmmc_dlyb_input_ck(dlyb);
>
> @@ -454,6 +456,40 @@ static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> return sdmmc_dlyb_phase_tuning(host, opcode);
> }
>
> +static void sdmmc_prep_vswitch(struct mmci_host *host)
> +{
> + /* clear the voltage switch completion flag */
> + writel_relaxed(MCI_STM32_VSWENDC, host->base + MMCICLEAR);
> + /* enable Voltage switch procedure */
> + mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCHEN);
> +}
> +
> +static int sdmmc_vswitch(struct mmci_host *host, struct mmc_ios *ios)
> +{
> + unsigned long flags;
> + u32 status;
> + int ret = 0;
> +
> + if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> + spin_lock_irqsave(&host->lock, flags);
> + mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCH);
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + /* wait voltage switch completion while 10ms */
> + ret = readl_relaxed_poll_timeout(host->base + MMCISTATUS,
> + status,
> + (status & MCI_STM32_VSWEND),
> + 10, 10000);
> +
> + writel_relaxed(MCI_STM32_VSWENDC | MCI_STM32_CKSTOPC,
> + host->base + MMCICLEAR);
> + mmci_write_pwrreg(host, host->pwr_reg &
> + ~(MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
> + }
Don't you need to manage things when resetting to
MMC_SIGNAL_VOLTAGE_330, which for example happens during a card
removal or at system suspend/resume?
> +
> + return ret;
> +}
> +
> static struct mmci_host_ops sdmmc_variant_ops = {
> .validate_data = sdmmc_idma_validate_data,
> .prep_data = sdmmc_idma_prep_data,
> @@ -465,6 +501,8 @@ static struct mmci_host_ops sdmmc_variant_ops = {
> .set_clkreg = mmci_sdmmc_set_clkreg,
> .set_pwrreg = mmci_sdmmc_set_pwrreg,
> .busy_complete = sdmmc_busy_complete,
> + .prep_volt_switch = sdmmc_prep_vswitch,
> + .volt_switch = sdmmc_vswitch,
> };
>
> void sdmmc_variant_init(struct mmci_host *host)
> --
> 2.17.1
>
Kind regards
Uffe
On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>
> The hardware delay block is used to align the sampling clock on
> the data received by SDMMC. It is mandatory for SDMMC to
> support the SDR104 mode. The delay block is used to generate
> an output clock which is dephased from the input clock.
> The phase of the output clock must be programmed by the execute
> tuning interface.
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> drivers/mmc/host/mmci_stm32_sdmmc.c | 147 ++++++++++++++++++++++++++++
> 1 file changed, 147 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index df08f6662431..10059fa19f4a 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -3,10 +3,13 @@
> * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
> * Author: [email protected] for STMicroelectronics.
> */
> +#include <linux/bitfield.h>
> #include <linux/delay.h>
> #include <linux/dma-mapping.h>
> +#include <linux/iopoll.h>
> #include <linux/mmc/host.h>
> #include <linux/mmc/card.h>
> +#include <linux/of_address.h>
> #include <linux/reset.h>
> #include <linux/scatterlist.h>
> #include "mmci.h"
> @@ -14,6 +17,20 @@
> #define SDMMC_LLI_BUF_LEN PAGE_SIZE
> #define SDMMC_IDMA_BURST BIT(MMCI_STM32_IDMABNDT_SHIFT)
>
> +#define DLYB_CR 0x0
> +#define DLYB_CR_DEN BIT(0)
> +#define DLYB_CR_SEN BIT(1)
> +
> +#define DLYB_CFGR 0x4
> +#define DLYB_CFGR_SEL_MASK GENMASK(3, 0)
> +#define DLYB_CFGR_UNIT_MASK GENMASK(14, 8)
> +#define DLYB_CFGR_LNG_MASK GENMASK(27, 16)
> +#define DLYB_CFGR_LNGF BIT(31)
> +
> +#define DLYB_NB_DELAY 11
> +#define DLYB_CFGR_SEL_MAX (DLYB_NB_DELAY + 1)
> +#define DLYB_CFGR_UNIT_MAX 127
[...]
> +static int sdmmc_dlyb_lng_tuning(struct mmci_host *host)
> +{
> + struct sdmmc_dlyb *dlyb = host->variant_priv;
> + u32 cfgr;
> + int i, lng, ret;
> +
> + for (i = 0; i <= DLYB_CFGR_UNIT_MAX; i++) {
> + sdmmc_dlyb_set_cfgr(dlyb, i, DLYB_CFGR_SEL_MAX, true);
> +
> + ret = readl_relaxed_poll_timeout(dlyb->base + DLYB_CFGR, cfgr,
> + (cfgr & DLYB_CFGR_LNGF),
> + 1, 1000);
I suggest you introduce a define for this timeout, in the top of the file.
> + if (ret) {
> + dev_warn(mmc_dev(host->mmc),
> + "delay line cfg timeout unit:%d cfgr:%d\n",
> + i, cfgr);
> + continue;
> + }
> +
> + lng = FIELD_GET(DLYB_CFGR_LNG_MASK, cfgr);
> + if (lng < BIT(DLYB_NB_DELAY) && lng > 0)
> + break;
> + }
> +
> + if (i > DLYB_CFGR_UNIT_MAX)
> + return -EINVAL;
> +
> + dlyb->unit = i;
> + dlyb->max = __fls(lng);
> +
> + return 0;
> +}
> +
> +static int sdmmc_dlyb_phase_tuning(struct mmci_host *host, u32 opcode)
> +{
> + struct sdmmc_dlyb *dlyb = host->variant_priv;
> + int cur_len = 0, max_len = 0, end_of_len = 0;
> + int phase;
> +
> + for (phase = 0; phase <= dlyb->max; phase++) {
> + sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
> +
> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
> + cur_len = 0;
> + } else {
> + cur_len++;
> + if (cur_len > max_len) {
> + max_len = cur_len;
> + end_of_len = phase;
> + }
> + }
> + }
> +
> + if (!max_len) {
> + dev_err(mmc_dev(host->mmc), "no tuning point found\n");
> + return -EINVAL;
> + }
> +
> + phase = end_of_len - max_len / 2;
> + sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
> +
> + dev_dbg(mmc_dev(host->mmc), "unit:%d max_dly:%d phase:%d\n",
> + dlyb->unit, dlyb->max, phase);
> +
> + return 0;
> +}
> +
> +static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct mmci_host *host = mmc_priv(mmc);
> + struct sdmmc_dlyb *dlyb = host->variant_priv;
> +
> + if (!dlyb || !dlyb->base)
> + return -EINVAL;
> +
> + if (sdmmc_dlyb_lng_tuning(host))
> + return -EINVAL;
> +
> + return sdmmc_dlyb_phase_tuning(host, opcode);
What happens to the tuning registers when the controller device
becomes runtime suspended? Would it possible that the values gets lost
and then they need to be restored in runtime resume?
> +}
> +
> static struct mmci_host_ops sdmmc_variant_ops = {
> .validate_data = sdmmc_idma_validate_data,
> .prep_data = sdmmc_idma_prep_data,
> @@ -338,5 +469,21 @@ static struct mmci_host_ops sdmmc_variant_ops = {
>
> void sdmmc_variant_init(struct mmci_host *host)
> {
> + struct device_node *np = host->mmc->parent->of_node;
> + void __iomem *base_dlyb;
> + struct sdmmc_dlyb *dlyb;
> +
> host->ops = &sdmmc_variant_ops;
> +
> + base_dlyb = devm_of_iomap(mmc_dev(host->mmc), np, 1, NULL);
> + if (IS_ERR(base_dlyb))
> + return;
> +
> + dlyb = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dlyb), GFP_KERNEL);
> + if (!dlyb)
> + return;
> +
> + dlyb->base = base_dlyb;
> + host->variant_priv = dlyb;
> + host->mmc_ops->execute_tuning = sdmmc_execute_tuning;
> }
> --
> 2.17.1
>
Kind regards
Uffe
On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>
> This patch adds 2 voltage switch callbacks in mmci_host_ops:
> -prep_volt_switch allows to prepare voltage switch before to
> sent the SD_SWITCH_VOLTAGE command (cmd11).
> -volt_switch callback allows to define specific action after
> regulator set voltage.
I am fine with adding these callbacks, however I strongly suggest to
have a reference to "signal voltage" in the name of the callbacks. As
to avoid confusion for what there are used for.
Perhaps ->post_sig_volt_switch() and ->pre_sig_volt_switch() can work?
>
> Signed-off-by: Ludovic Barre <[email protected]>
> ---
> drivers/mmc/host/mmci.c | 8 ++++++++
> drivers/mmc/host/mmci.h | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 00b473f57047..d76a59c06cb0 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -22,6 +22,7 @@
> #include <linux/mmc/pm.h>
> #include <linux/mmc/host.h>
> #include <linux/mmc/card.h>
> +#include <linux/mmc/sd.h>
> #include <linux/mmc/slot-gpio.h>
> #include <linux/amba/bus.h>
> #include <linux/clk.h>
> @@ -1207,6 +1208,9 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
> writel_relaxed(clks, host->base + MMCIDATATIMER);
> }
>
> + if (host->ops->prep_volt_switch && cmd->opcode == SD_SWITCH_VOLTAGE)
> + host->ops->prep_volt_switch(host);
> +
> if (/*interrupt*/0)
> c |= MCI_CPSM_INTERRUPT;
>
> @@ -1820,6 +1824,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
>
> static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
> {
> + struct mmci_host *host = mmc_priv(mmc);
> int ret = 0;
>
> if (!IS_ERR(mmc->supply.vqmmc)) {
> @@ -1839,6 +1844,9 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
> break;
> }
>
> + if (!ret && host->ops && host->ops->volt_switch)
> + ret = host->ops->volt_switch(host, ios);
> +
> if (ret)
> dev_warn(mmc_dev(mmc), "Voltage switch failed\n");
> }
> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
> index ddcdfb827996..c04a144259a2 100644
> --- a/drivers/mmc/host/mmci.h
> +++ b/drivers/mmc/host/mmci.h
> @@ -377,6 +377,8 @@ struct mmci_host_ops {
> void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
> void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
> bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
> + void (*prep_volt_switch)(struct mmci_host *host);
> + int (*volt_switch)(struct mmci_host *host, struct mmc_ios *ios);
> };
>
> struct mmci_host {
> --
> 2.17.1
>
Kind regards
Uffe
On Fri, 24 Jan 2020 at 13:55, Ludovic BARRE <[email protected]> wrote:
>
> hi Ulf
>
> Just a "gentleman ping" on this series
> https://lkml.org/lkml/2020/1/10/392
I was just reviewing :-) Thanks for pinging!
One overall comment is that I think you can try to work a bit on the
changelogs. In some cases you described what the patch does, which is
good, but it may lack information about *why* the change is wanted.
Overall, the series looks nice.
Kind regards
Uffe
Le 1/24/20 à 2:09 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>>
>> This patch adds mmc_host_ops pointer in mmci struct.
>> The variant init function may need to add a mmc_host_ops,
>> for example to add the execute_tuning support if this feature
>> is available.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> drivers/mmc/host/mmci.c | 1 +
>> drivers/mmc/host/mmci.h | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 7b13d66cbb21..00b473f57047 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -1923,6 +1923,7 @@ static int mmci_probe(struct amba_device *dev,
>>
>> host = mmc_priv(mmc);
>> host->mmc = mmc;
>> + host->mmc_ops = &mmci_ops;
>
> Nitpick:
>
> Can you please also move the assignment "mmc->ops = &mmci_ops;" to
> this place as well, as I think these belongs together.
OK
>
>>
>> /*
>> * Some variant (STM32) doesn't have opendrain bit, nevertheless
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index ea6a0b5779d4..55acc0971a44 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -407,6 +407,7 @@ struct mmci_host {
>> u32 mask1_reg;
>> u8 vqmmc_enabled:1;
>> struct mmci_platform_data *plat;
>> + struct mmc_host_ops *mmc_ops;
>> struct mmci_host_ops *ops;
>> struct variant_data *variant;
>> struct pinctrl *pinctrl;
>> --
>> 2.17.1
>>
>
> Kind regards
> Uffe
>
hi Ulf
Le 1/24/20 à 2:10 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>>
>> The hardware delay block is used to align the sampling clock on
>> the data received by SDMMC. It is mandatory for SDMMC to
>> support the SDR104 mode. The delay block is used to generate
>> an output clock which is dephased from the input clock.
>> The phase of the output clock must be programmed by the execute
>> tuning interface.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> drivers/mmc/host/mmci_stm32_sdmmc.c | 147 ++++++++++++++++++++++++++++
>> 1 file changed, 147 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> index df08f6662431..10059fa19f4a 100644
>> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
>> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> @@ -3,10 +3,13 @@
>> * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
>> * Author: [email protected] for STMicroelectronics.
>> */
>> +#include <linux/bitfield.h>
>> #include <linux/delay.h>
>> #include <linux/dma-mapping.h>
>> +#include <linux/iopoll.h>
>> #include <linux/mmc/host.h>
>> #include <linux/mmc/card.h>
>> +#include <linux/of_address.h>
>> #include <linux/reset.h>
>> #include <linux/scatterlist.h>
>> #include "mmci.h"
>> @@ -14,6 +17,20 @@
>> #define SDMMC_LLI_BUF_LEN PAGE_SIZE
>> #define SDMMC_IDMA_BURST BIT(MMCI_STM32_IDMABNDT_SHIFT)
>>
>> +#define DLYB_CR 0x0
>> +#define DLYB_CR_DEN BIT(0)
>> +#define DLYB_CR_SEN BIT(1)
>> +
>> +#define DLYB_CFGR 0x4
>> +#define DLYB_CFGR_SEL_MASK GENMASK(3, 0)
>> +#define DLYB_CFGR_UNIT_MASK GENMASK(14, 8)
>> +#define DLYB_CFGR_LNG_MASK GENMASK(27, 16)
>> +#define DLYB_CFGR_LNGF BIT(31)
>> +
>> +#define DLYB_NB_DELAY 11
>> +#define DLYB_CFGR_SEL_MAX (DLYB_NB_DELAY + 1)
>> +#define DLYB_CFGR_UNIT_MAX 127
>
> [...]
>
>> +static int sdmmc_dlyb_lng_tuning(struct mmci_host *host)
>> +{
>> + struct sdmmc_dlyb *dlyb = host->variant_priv;
>> + u32 cfgr;
>> + int i, lng, ret;
>> +
>> + for (i = 0; i <= DLYB_CFGR_UNIT_MAX; i++) {
>> + sdmmc_dlyb_set_cfgr(dlyb, i, DLYB_CFGR_SEL_MAX, true);
>> +
>> + ret = readl_relaxed_poll_timeout(dlyb->base + DLYB_CFGR, cfgr,
>> + (cfgr & DLYB_CFGR_LNGF),
>> + 1, 1000);
>
> I suggest you introduce a define for this timeout, in the top of the file.
OK
>
>> + if (ret) {
>> + dev_warn(mmc_dev(host->mmc),
>> + "delay line cfg timeout unit:%d cfgr:%d\n",
>> + i, cfgr);
>> + continue;
>> + }
>> +
>> + lng = FIELD_GET(DLYB_CFGR_LNG_MASK, cfgr);
>> + if (lng < BIT(DLYB_NB_DELAY) && lng > 0)
>> + break;
>> + }
>> +
>> + if (i > DLYB_CFGR_UNIT_MAX)
>> + return -EINVAL;
>> +
>> + dlyb->unit = i;
>> + dlyb->max = __fls(lng);
>> +
>> + return 0;
>> +}
>> +
>> +static int sdmmc_dlyb_phase_tuning(struct mmci_host *host, u32 opcode)
>> +{
>> + struct sdmmc_dlyb *dlyb = host->variant_priv;
>> + int cur_len = 0, max_len = 0, end_of_len = 0;
>> + int phase;
>> +
>> + for (phase = 0; phase <= dlyb->max; phase++) {
>> + sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
>> +
>> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
>> + cur_len = 0;
>> + } else {
>> + cur_len++;
>> + if (cur_len > max_len) {
>> + max_len = cur_len;
>> + end_of_len = phase;
>> + }
>> + }
>> + }
>> +
>> + if (!max_len) {
>> + dev_err(mmc_dev(host->mmc), "no tuning point found\n");
>> + return -EINVAL;
>> + }
>> +
>> + phase = end_of_len - max_len / 2;
>> + sdmmc_dlyb_set_cfgr(dlyb, dlyb->unit, phase, false);
>> +
>> + dev_dbg(mmc_dev(host->mmc), "unit:%d max_dly:%d phase:%d\n",
>> + dlyb->unit, dlyb->max, phase);
>> +
>> + return 0;
>> +}
>> +
>> +static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> +{
>> + struct mmci_host *host = mmc_priv(mmc);
>> + struct sdmmc_dlyb *dlyb = host->variant_priv;
>> +
>> + if (!dlyb || !dlyb->base)
>> + return -EINVAL;
>> +
>> + if (sdmmc_dlyb_lng_tuning(host))
>> + return -EINVAL;
>> +
>> + return sdmmc_dlyb_phase_tuning(host, opcode);
>
> What happens to the tuning registers when the controller device
> becomes runtime suspended? Would it possible that the values gets lost
> and then they need to be restored in runtime resume?
when the device goes to runtime suspend:
-The sdmmc clock is disabled => sdmmc & dlyb registers are not accessible.
-The power domain of this blocks is not off, the register values are not
lost.
On runtime resume the clock is re-enabled and the registers
are accessible and with right values
Regards
Ludo
>
>> +}
>> +
>> static struct mmci_host_ops sdmmc_variant_ops = {
>> .validate_data = sdmmc_idma_validate_data,
>> .prep_data = sdmmc_idma_prep_data,
>> @@ -338,5 +469,21 @@ static struct mmci_host_ops sdmmc_variant_ops = {
>>
>> void sdmmc_variant_init(struct mmci_host *host)
>> {
>> + struct device_node *np = host->mmc->parent->of_node;
>> + void __iomem *base_dlyb;
>> + struct sdmmc_dlyb *dlyb;
>> +
>> host->ops = &sdmmc_variant_ops;
>> +
>> + base_dlyb = devm_of_iomap(mmc_dev(host->mmc), np, 1, NULL);
>> + if (IS_ERR(base_dlyb))
>> + return;
>> +
>> + dlyb = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dlyb), GFP_KERNEL);
>> + if (!dlyb)
>> + return;
>> +
>> + dlyb->base = base_dlyb;
>> + host->variant_priv = dlyb;
>> + host->mmc_ops->execute_tuning = sdmmc_execute_tuning;
>> }
>> --
>> 2.17.1
>>
>
> Kind regards
> Uffe
>
hi Ulf
Le 1/24/20 à 2:12 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>>
>> This patch adds 2 voltage switch callbacks in mmci_host_ops:
>> -prep_volt_switch allows to prepare voltage switch before to
>> sent the SD_SWITCH_VOLTAGE command (cmd11).
>> -volt_switch callback allows to define specific action after
>> regulator set voltage.
>
> I am fine with adding these callbacks, however I strongly suggest to
> have a reference to "signal voltage" in the name of the callbacks. As
> to avoid confusion for what there are used for.
>
> Perhaps ->post_sig_volt_switch() and ->pre_sig_volt_switch() can work?
>
sure, I change to post_sig_volt_switch and pre_sig_volt_switch.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> drivers/mmc/host/mmci.c | 8 ++++++++
>> drivers/mmc/host/mmci.h | 2 ++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 00b473f57047..d76a59c06cb0 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -22,6 +22,7 @@
>> #include <linux/mmc/pm.h>
>> #include <linux/mmc/host.h>
>> #include <linux/mmc/card.h>
>> +#include <linux/mmc/sd.h>
>> #include <linux/mmc/slot-gpio.h>
>> #include <linux/amba/bus.h>
>> #include <linux/clk.h>
>> @@ -1207,6 +1208,9 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
>> writel_relaxed(clks, host->base + MMCIDATATIMER);
>> }
>>
>> + if (host->ops->prep_volt_switch && cmd->opcode == SD_SWITCH_VOLTAGE)
>> + host->ops->prep_volt_switch(host);
>> +
>> if (/*interrupt*/0)
>> c |= MCI_CPSM_INTERRUPT;
>>
>> @@ -1820,6 +1824,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
>>
>> static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
>> {
>> + struct mmci_host *host = mmc_priv(mmc);
>> int ret = 0;
>>
>> if (!IS_ERR(mmc->supply.vqmmc)) {
>> @@ -1839,6 +1844,9 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
>> break;
>> }
>>
>> + if (!ret && host->ops && host->ops->volt_switch)
>> + ret = host->ops->volt_switch(host, ios);
>> +
>> if (ret)
>> dev_warn(mmc_dev(mmc), "Voltage switch failed\n");
>> }
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index ddcdfb827996..c04a144259a2 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -377,6 +377,8 @@ struct mmci_host_ops {
>> void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
>> void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
>> bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
>> + void (*prep_volt_switch)(struct mmci_host *host);
>> + int (*volt_switch)(struct mmci_host *host, struct mmc_ios *ios);
>> };
>>
>> struct mmci_host {
>> --
>> 2.17.1
>>
>
> Kind regards
> Uffe
>
hi Ulf
Le 1/24/20 à 2:16 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <[email protected]> wrote:
>>
>> To prepare the voltage switch procedure, the VSWITCHEN bit must be
>> set before sending the cmd11.
>> To confirm completion of voltage switch, the VSWEND flag must be
>> checked.
>>
>> Signed-off-by: Ludovic Barre <[email protected]>
>> ---
>> drivers/mmc/host/mmci.h | 4 +++
>> drivers/mmc/host/mmci_stm32_sdmmc.c | 40 ++++++++++++++++++++++++++++-
>> 2 files changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index c04a144259a2..3634f98ad2d8 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -165,6 +165,7 @@
>> /* Extended status bits for the STM32 variants */
>> #define MCI_STM32_BUSYD0 BIT(20)
>> #define MCI_STM32_BUSYD0END BIT(21)
>> +#define MCI_STM32_VSWEND BIT(25)
>>
>> #define MMCICLEAR 0x038
>> #define MCI_CMDCRCFAILCLR (1 << 0)
>> @@ -182,6 +183,9 @@
>> #define MCI_ST_SDIOITC (1 << 22)
>> #define MCI_ST_CEATAENDC (1 << 23)
>> #define MCI_ST_BUSYENDC (1 << 24)
>> +/* Extended clear bits for the STM32 variants */
>> +#define MCI_STM32_VSWENDC BIT(25)
>> +#define MCI_STM32_CKSTOPC BIT(26)
>>
>> #define MMCIMASK0 0x03c
>> #define MCI_CMDCRCFAILMASK (1 << 0)
>> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> index 10059fa19f4a..9f43cf947c5f 100644
>> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
>> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
>> @@ -263,7 +263,9 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr)
>> struct mmc_ios ios = host->mmc->ios;
>> struct sdmmc_dlyb *dlyb = host->variant_priv;
>>
>> - pwr = host->pwr_reg_add;
>> + /* adds OF options and preserves voltage switch bits */
>> + pwr = host->pwr_reg_add |
>> + (host->pwr_reg & (MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
>>
>> sdmmc_dlyb_input_ck(dlyb);
>>
>> @@ -454,6 +456,40 @@ static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> return sdmmc_dlyb_phase_tuning(host, opcode);
>> }
>>
>> +static void sdmmc_prep_vswitch(struct mmci_host *host)
>> +{
>> + /* clear the voltage switch completion flag */
>> + writel_relaxed(MCI_STM32_VSWENDC, host->base + MMCICLEAR);
>> + /* enable Voltage switch procedure */
>> + mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCHEN);
>> +}
>> +
>> +static int sdmmc_vswitch(struct mmci_host *host, struct mmc_ios *ios)
>> +{
>> + unsigned long flags;
>> + u32 status;
>> + int ret = 0;
>> +
>> + if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
>> + spin_lock_irqsave(&host->lock, flags);
>> + mmci_write_pwrreg(host, host->pwr_reg | MCI_STM32_VSWITCH);
>> + spin_unlock_irqrestore(&host->lock, flags);
>> +
>> + /* wait voltage switch completion while 10ms */
>> + ret = readl_relaxed_poll_timeout(host->base + MMCISTATUS,
>> + status,
>> + (status & MCI_STM32_VSWEND),
>> + 10, 10000);
>> +
>> + writel_relaxed(MCI_STM32_VSWENDC | MCI_STM32_CKSTOPC,
>> + host->base + MMCICLEAR);
>> + mmci_write_pwrreg(host, host->pwr_reg &
>> + ~(MCI_STM32_VSWITCHEN | MCI_STM32_VSWITCH));
>> + }
>
> Don't you need to manage things when resetting to
> MMC_SIGNAL_VOLTAGE_330, which for example happens during a card
> removal or at system suspend/resume?
>
The VSWITCH sequence is used only for 3.3V to 1.8V.
If there are: card remove | suspend/resume.
The power cycle of sdmmc must be reinitialised
and the reset is mandatory.
>> +
>> + return ret;
>> +}
>> +
>> static struct mmci_host_ops sdmmc_variant_ops = {
>> .validate_data = sdmmc_idma_validate_data,
>> .prep_data = sdmmc_idma_prep_data,
>> @@ -465,6 +501,8 @@ static struct mmci_host_ops sdmmc_variant_ops = {
>> .set_clkreg = mmci_sdmmc_set_clkreg,
>> .set_pwrreg = mmci_sdmmc_set_pwrreg,
>> .busy_complete = sdmmc_busy_complete,
>> + .prep_volt_switch = sdmmc_prep_vswitch,
>> + .volt_switch = sdmmc_vswitch,
>> };
>>
>> void sdmmc_variant_init(struct mmci_host *host)
>> --
>> 2.17.1
>>
>
> Kind regards
> Uffe
>
hi Ulf
Le 1/24/20 à 2:19 PM, Ulf Hansson a écrit :
> On Fri, 24 Jan 2020 at 13:55, Ludovic BARRE <[email protected]> wrote:
>>
>> hi Ulf
>>
>> Just a "gentleman ping" on this series
>> https://lkml.org/lkml/2020/1/10/392
>
> I was just reviewing :-) Thanks for pinging!
>
> One overall comment is that I think you can try to work a bit on the
> changelogs. In some cases you described what the patch does, which is
> good, but it may lack information about *why* the change is wanted.
Ok, I try to add a comment to *why*
>
> Overall, the series looks nice.
>
> Kind regards
> Uffe
>