[PATCH 0/9] mmc: mmci: sdmmc: add sdr104 support

[PATCH 0/9] mmc: mmci: sdmmc: add sdr104 support

[Ludovic Barre]

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

[PATCH 1/9] mmc: mmci: sdmmc: replace sg_dma_xxx macros

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

[PATCH 2/9] mmc: mmci: sdmmc: rename sdmmc_priv struct to sdmmc_idma

[Ludovic Barre]

This patch renames sdmmc_priv struct to sdmmc_idma
which is assigned to host->dma_priv.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 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

[PATCH 3/9] mmc: mmci: add a reference at mmc_host_ops in mmci struct

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

Re: [PATCH 3/9] mmc: mmci: add a reference at mmc_host_ops in mmci struct

[Ulf Hansson]

On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
> ---
>  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

Re: [PATCH 3/9] mmc: mmci: add a reference at mmc_host_ops in mmci struct

[Ludovic BARRE]

Le 1/24/20 à 2:09 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
>> ---
>>   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
> 

[PATCH 4/9] mmc: mmci: add private pointer for variant

[Ludovic Barre]

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

[PATCH 5/9] dt-bindings: mmc: mmci: add delay block base register for sdmmc

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

Re: [PATCH 5/9] dt-bindings: mmc: mmci: add delay block base register for sdmmc

[Rob Herring]

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 <ludovic.barre@st.com>
> ---
>  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
> 

Re: [PATCH 5/9] dt-bindings: mmc: mmci: add delay block base register for sdmmc

[Ludovic BARRE]

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 <ludovic.barre@st.com>
>> ---
>>   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
>>

Re: [PATCH 5/9] dt-bindings: mmc: mmci: add delay block base register for sdmmc

[Rob Herring]

On Thu, Jan 16, 2020 at 3:21 AM Ludovic BARRE <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
> >> ---
> >>   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

Re: [PATCH 5/9] dt-bindings: mmc: mmci: add delay block base register for sdmmc

[Ludovic BARRE]

Le 1/16/20 à 3:33 PM, Rob Herring a écrit :
> On Thu, Jan 16, 2020 at 3:21 AM Ludovic BARRE <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
>>>> ---
>>>>    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
> 

[PATCH 6/9] mmc: mmci: sdmmc: add execute tuning with delay block

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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: Ludovic.barre@st.com 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

Re: [PATCH 6/9] mmc: mmci: sdmmc: add execute tuning with delay block

[Ulf Hansson]

On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
> ---
>  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: Ludovic.barre@st.com 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

Re: [PATCH 6/9] mmc: mmci: sdmmc: add execute tuning with delay block

[Ludovic BARRE]

hi Ulf

Le 1/24/20 à 2:10 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
>> ---
>>   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: Ludovic.barre@st.com 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
> 

[PATCH 7/9] mmc: mmci: add volt_switch callbacks

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

Re: [PATCH 7/9] mmc: mmci: add volt_switch callbacks

[Ulf Hansson]

On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
> ---
>  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

Re: [PATCH 7/9] mmc: mmci: add volt_switch callbacks

[Ludovic BARRE]

hi Ulf

Le 1/24/20 à 2:12 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
>> ---
>>   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
> 

[PATCH 8/9] mmc: mmci: sdmmc: add voltage switch functions

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

Re: [PATCH 8/9] mmc: mmci: sdmmc: add voltage switch functions

[Ulf Hansson]

On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
> ---
>  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

Re: [PATCH 8/9] mmc: mmci: sdmmc: add voltage switch functions

[Ludovic BARRE]

hi Ulf

Le 1/24/20 à 2:16 PM, Ulf Hansson a écrit :
> On Fri, 10 Jan 2020 at 14:49, Ludovic Barre <ludovic.barre@st.com> 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 <ludovic.barre@st.com>
>> ---
>>   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
> 

[PATCH 9/9] mmc: mmci: add sdmmc variant revision 2.0

[Ludovic Barre]

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 <ludovic.barre@st.com>
---
 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

Re: [PATCH 0/9] mmc: mmci: sdmmc: add sdr104 support

[Ludovic BARRE]

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(-)
> 

Re: [PATCH 0/9] mmc: mmci: sdmmc: add sdr104 support

[Ulf Hansson]

On Fri, 24 Jan 2020 at 13:55, Ludovic BARRE <ludovic.barre@st.com> 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

Re: [PATCH 0/9] mmc: mmci: sdmmc: add sdr104 support

[Ludovic BARRE]

hi Ulf

Le 1/24/20 à 2:19 PM, Ulf Hansson a écrit :
> On Fri, 24 Jan 2020 at 13:55, Ludovic BARRE <ludovic.barre@st.com> 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
>