All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
@ 2020-09-09 21:54 ` Alexandru Gagniuc
  2020-09-10  7:52   ` Patrice CHOTARD
                     ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Alexandru Gagniuc @ 2020-09-09 21:54 UTC (permalink / raw)
  To: u-boot

mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
struct mmc_config from devicetree.
The same logic is duplicated in stm32_sdmmc2_probe(). Use
mmc_of_parse(), which is more generic.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
index 6d50356217..77871d5afc 100644
--- a/drivers/mmc/stm32_sdmmc2.c
+++ b/drivers/mmc/stm32_sdmmc2.c
@@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
 			     GPIOD_IS_IN);
 
 	cfg->f_min = 400000;
-	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
 	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 	cfg->name = "STM32 SD/MMC";
 
 	cfg->host_caps = 0;
-	if (cfg->f_max > 25000000)
-		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
-
-	switch (dev_read_u32_default(dev, "bus-width", 1)) {
-	case 8:
-		cfg->host_caps |= MMC_MODE_8BIT;
-		/* fall through */
-	case 4:
-		cfg->host_caps |= MMC_MODE_4BIT;
-		break;
-	case 1:
-		break;
-	default:
-		pr_err("invalid \"bus-width\" property, force to 1\n");
-	}
+	cfg->f_max = 52000000;
+	mmc_of_parse(dev, cfg);
 
 	upriv->mmc = &plat->mmc;
 
-- 
2.25.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
@ 2020-09-10  7:52   ` Patrice CHOTARD
  2020-09-10 16:04   ` Patrick DELAUNAY
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Patrice CHOTARD @ 2020-09-10  7:52 UTC (permalink / raw)
  To: u-boot

Hi Alexandru

On 9/9/20 11:54 PM, Alexandru Gagniuc wrote:
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
> struct mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use
> mmc_of_parse(), which is more generic.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> index 6d50356217..77871d5afc 100644
> --- a/drivers/mmc/stm32_sdmmc2.c
> +++ b/drivers/mmc/stm32_sdmmc2.c
> @@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>  			     GPIOD_IS_IN);
>  
>  	cfg->f_min = 400000;
> -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>  	cfg->name = "STM32 SD/MMC";
>  
>  	cfg->host_caps = 0;
> -	if (cfg->f_max > 25000000)
> -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> -
> -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> -	case 8:
> -		cfg->host_caps |= MMC_MODE_8BIT;
> -		/* fall through */
> -	case 4:
> -		cfg->host_caps |= MMC_MODE_4BIT;
> -		break;
> -	case 1:
> -		break;
> -	default:
> -		pr_err("invalid \"bus-width\" property, force to 1\n");
> -	}
> +	cfg->f_max = 52000000;
> +	mmc_of_parse(dev, cfg);
>  
>  	upriv->mmc = &plat->mmc;

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Patrice

Thanks

>  

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  2020-09-10  7:52   ` Patrice CHOTARD
@ 2020-09-10 16:04   ` Patrick DELAUNAY
  2020-09-10 20:10     ` Alex G.
  2020-09-11  7:49   ` Jaehoon Chung
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-09-10 16:04 UTC (permalink / raw)
  To: u-boot

Hi Alexandru,

> Sent: mercredi 9 septembre 2020 23:54
> To: uboot-stm32 at st-md-mailman.stormreply.com
> Cc: Alexandru Gagniuc <mr.nuke.me@gmail.com>; Patrick DELAUNAY
> <patrick.delaunay@st.com>; Patrice CHOTARD <patrice.chotard@st.com>; Peng
> Fan <peng.fan@nxp.com>; u-boot at lists.denx.de
> Subject: [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host
> capabilities
> Importance: High
> 
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of struct
> mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use mmc_of_parse(),
> which is more generic.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---

Thanks for the patch, I miss the addition of this new API.

>  drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c index
> 6d50356217..77871d5afc 100644
> --- a/drivers/mmc/stm32_sdmmc2.c
> +++ b/drivers/mmc/stm32_sdmmc2.c
> @@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>  			     GPIOD_IS_IN);
> 
>  	cfg->f_min = 400000;
> -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 |
> MMC_VDD_165_195;
>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>  	cfg->name = "STM32 SD/MMC";
> 
>  	cfg->host_caps = 0;
> -	if (cfg->f_max > 25000000)
> -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> -
> -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> -	case 8:
> -		cfg->host_caps |= MMC_MODE_8BIT;
> -		/* fall through */
> -	case 4:
> -		cfg->host_caps |= MMC_MODE_4BIT;
> -		break;
> -	case 1:
> -		break;
> -	default:
> -		pr_err("invalid \"bus-width\" property, force to 1\n");
> -	}
> +	cfg->f_max = 52000000;
> +	mmc_of_parse(dev, cfg);

Result of mmc_of_parse is not tested ?

I proposed:

+	ret = mmc_of_parse(dev, cfg);
+	if (ret)
+		return ret;

> 
>  	upriv->mmc = &plat->mmc;
> 
> --
> 2.25.4

I test this patch with a trace in stm32_sdmmc2_probe on STM32MP157C-EV1

Before the patch :
cfg->host_caps = 0x2000000e (SD card)
cfg->host_caps = 0x6000000e (eMMC)

After the patch:
cfg->host_caps = 0x300001e6 (SD card)
cfg->host_caps = 0x70004006 (eMMC)


I see 2 problem here :

1/ MMC_MODE_HS_52MHz = MMC_CAP(MMC_HS_52) is removed
    and the eMMC on EV1 use only at 26MHz instead 52MHz before the patch

	Mode: MMC High Speed (52MHz)
	=>
	Mode: MMC High Speed (26MHz)

The "cap-mmc-highspeed" is used in Linux for MMC HS at 26MHz or 52MHz

./Documentation/devicetree/bindings/mmc/mmc-controller.yaml:122: 
cap-mmc-highspeed:
    $ref: /schemas/types.yaml#/definitions/flag
    description:
      MMC high-speed timing is supported.

And in Linux mmc/core/mmc.c
	static void mmc_select_card_type(struct mmc_card *card)
	{
	.....

	if (caps & MMC_CAP_MMC_HIGHSPEED &&
	    card_type & EXT_CSD_CARD_TYPE_HS_26) {
		hs_max_dtr = MMC_HIGH_26_MAX_DTR;
		avail_type |= EXT_CSD_CARD_TYPE_HS_26;
	}

	if (caps & MMC_CAP_MMC_HIGHSPEED &&
	    card_type & EXT_CSD_CARD_TYPE_HS_52) {
		hs_max_dtr = MMC_HIGH_52_MAX_DTR;
		avail_type |= EXT_CSD_CARD_TYPE_HS_52;
	}
	....

include/linux/mmc/mmc.h:347:
	#define EXT_CSD_CARD_TYPE_HS_26	(1<<0)	/* Card can run at 26MHz */
	#define EXT_CSD_CARD_TYPE_HS_52	(1<<1)	/* Card can run@52MHz */

I think that it is a issue U-Boot, in mmc uclass in mmc_of_parse():

	if (dev_read_bool(dev, "cap-mmc-highspeed"))
-		cfg->host_caps |= MMC_CAP(MMC_HS);
+		cfg->host_caps |= MMC_CAP(MMC_HS) | MMC_CAP(MMC_HS_52);

In U-Boot this capability is splitted in 2 bits = one for 26MHz one for 52MHz
And  for card side it is managed on card side by mmc_get_capabilities()

include/mmc.h:254:
	#define EXT_CSD_CARD_TYPE_26	(1 << 0)	/* Card can run at 26MHz */
	#define EXT_CSD_CARD_TYPE_52	(1 << 1)	/* Card can run@52MHz */

A other solution is to only impact the sdmmc driver..... 
and to activate 52MHZ mode support is frequency is >= 26MHz

	cfg->f_max = 52000000;
	ret = mmc_of_parse(dev, cfg);
	if (ret)
		return ret;

	if ((cfg->host_caps & MMC_HS) && 
	    cfg->f_max > 26000000)
		cfg->host_caps |= MMC_HS_52;

but I prefer the previous generic solution in u-class.


2) some host caps can be added in device tree (they are supported by SOC and by Linux driver)
    but they are not supported by U-Boot driver, for example:

#define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
#define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
#define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
#define MMC_MODE_HS400_ES	MMC_CAP(MMC_HS_400_ES)

MMC_CAP(UHS_SDR12)
MMC_CAP(UHS_SDR25) 
MMC_CAP(UHS_SDR50)
MMC_CAP(UHS_SDR104)
MMC_CAP(UHS_DDR50)

I afraid (I don't sure) that this added caps change the mmc core behavior in U-Boot =
U-Boot try to select  the high speed mode even if not supported by driver....

The host_caps bitfield should be limited by the supported features in the driver  (or remove the unsupported features)

#define SDMMC_SUPPORTED_CAPS (MMC_MODE_1BIT | MMC_MODE_4BIT | MMC_MODE_8BIT | MMC_CAP_CD_ACTIVE_HIGH | MMC_CAP_NEEDS_POLL |  MMC_CAP_NONREMOVABLE | MMC_MODE_HS | MMC_MODE_HS_52MHz)
or
#define SDMMC_UNSUPPORTED_CAPS (MMC_MODE_DDR_52MHz | MMC_MODE_HS200 | MMC_MODE_HS400 | MMC_MODE_HS400_ES | UHS_CAPS)
 
	cfg->f_max = 52000000;
	ret = mmc_of_parse(dev, cfg);
	if (ret)
		return ret;

	cfg->host_caps &= SDMMC_SUPPORTED_CAPS;
or 
	cfg->host_caps |= ~SDMMC_UNSUPPORTED_CAPS;


Best Regards

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-10 16:04   ` Patrick DELAUNAY
@ 2020-09-10 20:10     ` Alex G.
  2020-09-11  9:00       ` Patrick DELAUNAY
  0 siblings, 1 reply; 18+ messages in thread
From: Alex G. @ 2020-09-10 20:10 UTC (permalink / raw)
  To: u-boot

On 9/10/20 11:04 AM, Patrick DELAUNAY wrote:
> Hi Alexandru,

Hi

[snip]

>> +	cfg->f_max = 52000000;
>> +	mmc_of_parse(dev, cfg);
> 
> Result of mmc_of_parse is not tested ?
> 
> I proposed:
> 
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;

You're right. I'll get that updated.


> I test this patch with a trace in stm32_sdmmc2_probe on STM32MP157C-EV1
[snip]
> I think that it is a issue U-Boot, in mmc uclass in mmc_of_parse():
> 
> 	if (dev_read_bool(dev, "cap-mmc-highspeed"))
> -		cfg->host_caps |= MMC_CAP(MMC_HS);
> +		cfg->host_caps |= MMC_CAP(MMC_HS) | MMC_CAP(MMC_HS_52);
> 
> In U-Boot this capability is splitted in 2 bits = one for 26MHz one for 52MHz
> And  for card side it is managed on card side by mmc_get_capabilities()

I agree. I am preparing a patch to address this.


[snip]
> 2) some host caps can be added in device tree (they are supported by SOC and by Linux driver)
>      but they are not supported by U-Boot driver, for example:
> 
> #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
> #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
> #define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
> #define MMC_MODE_HS400_ES	MMC_CAP(MMC_HS_400_ES)
> 
> 
> I afraid (I don't sure) that this added caps change the mmc core behavior in U-Boot =
> U-Boot try to select  the high speed mode even if not supported by driver....

Two issues here. The first is when devicetree enables an unsupported 
mode, say "mmc-hs400-1_2v". That's a devicetree bug, and not something 
we should fix in the code. Hypothetical exam: DT says
	bus-width = <8>;
but only four lines are routed on the board.

The second issue is what happens when somebody plugs in a UHS SD card? 
UHS support is not enabled by default in the stm32mp1 defconfigs, so the 
mmc core won't try to run it at UHS.

Now if somebody were to enable UHS manually:
	CONFIG_MMC_IO_VOLTAGE=y
	CONFIG_MMC_UHS_SUPPORT=y
Then yes, the UHS switch will be attempted, fail, and the card will not 
be detected.

To work around that, one could implement a .wait_dat0() that returns an 
error other than -ENOSYS. This would cause mmc_switch_voltage() to fail, 
making the mmc core to leave the card at default speeds.

My argument is that it takes conscious effort to break things in the 
first place, so it's not a situation we should worry about.


> The host_caps bitfield should be limited by the supported features in the driver  (or remove the unsupported features)
[snip]
> 	cfg->host_caps &= SDMMC_SUPPORTED_CAPS;
> or
> 	cfg->host_caps |= ~SDMMC_UNSUPPORTED_CAPS;

I think this sort of playing with the flags will cause more confusion. 
People would expect this to come from DT. For example, somebody sets 
"sd-uhs-ddr52" in devicetree. It's more confusing to check host_caps, 
and find that MMC_MODE_DDR_52MHz is not set than it is to see the driver 
trying to place the card in DDR52 and failing.

Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  2020-09-10  7:52   ` Patrice CHOTARD
  2020-09-10 16:04   ` Patrick DELAUNAY
@ 2020-09-11  7:49   ` Jaehoon Chung
  2020-09-11 11:46     ` Patrick DELAUNAY
  2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
  2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  4 siblings, 1 reply; 18+ messages in thread
From: Jaehoon Chung @ 2020-09-11  7:49 UTC (permalink / raw)
  To: u-boot

On 9/10/20 6:54 AM, Alexandru Gagniuc wrote:
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
> struct mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use
> mmc_of_parse(), which is more generic.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> index 6d50356217..77871d5afc 100644
> --- a/drivers/mmc/stm32_sdmmc2.c
> +++ b/drivers/mmc/stm32_sdmmc2.c
> @@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>  			     GPIOD_IS_IN);
>  
>  	cfg->f_min = 400000;
> -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>  	cfg->name = "STM32 SD/MMC";
>  
>  	cfg->host_caps = 0;
> -	if (cfg->f_max > 25000000)
> -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> -
> -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> -	case 8:
> -		cfg->host_caps |= MMC_MODE_8BIT;
> -		/* fall through */
> -	case 4:
> -		cfg->host_caps |= MMC_MODE_4BIT;
> -		break;
> -	case 1:
> -		break;
> -	default:
> -		pr_err("invalid \"bus-width\" property, force to 1\n");
> -	}
> +	cfg->f_max = 52000000;

cfg->f_max can be also removed?

Best Regards,
Jaehoon Chung

> +	mmc_of_parse(dev, cfg);
>  
>  	upriv->mmc = &plat->mmc;
>  
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-10 20:10     ` Alex G.
@ 2020-09-11  9:00       ` Patrick DELAUNAY
  0 siblings, 0 replies; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-09-11  9:00 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> From: Alex G. <mr.nuke.me@gmail.com>
> Sent: jeudi 10 septembre 2020 22:10
> 
> On 9/10/20 11:04 AM, Patrick DELAUNAY wrote:
> > Hi Alexandru,
> 
> Hi
> 
> [snip]
> 
> >> +	cfg->f_max = 52000000;
> >> +	mmc_of_parse(dev, cfg);
> >
> > Result of mmc_of_parse is not tested ?
> >
> > I proposed:
> >
> > +	ret = mmc_of_parse(dev, cfg);
> > +	if (ret)
> > +		return ret;
> 
> You're right. I'll get that updated.

Thanks;
 
> 
> > I test this patch with a trace in stm32_sdmmc2_probe on
> > STM32MP157C-EV1
> [snip]
> > I think that it is a issue U-Boot, in mmc uclass in mmc_of_parse():
> >
> > 	if (dev_read_bool(dev, "cap-mmc-highspeed"))
> > -		cfg->host_caps |= MMC_CAP(MMC_HS);
> > +		cfg->host_caps |= MMC_CAP(MMC_HS) |
> MMC_CAP(MMC_HS_52);
> >
> > In U-Boot this capability is splitted in 2 bits = one for 26MHz one
> > for 52MHz And  for card side it is managed on card side by
> > mmc_get_capabilities()
> 
> I agree. I am preparing a patch to address this.

Thanks;
 
> 
> [snip]
> > 2) some host caps can be added in device tree (they are supported by SOC and
> by Linux driver)
> >      but they are not supported by U-Boot driver, for example:
> >
> > #define MMC_MODE_DDR_52MHz	MMC_CAP(MMC_DDR_52)
> > #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
> > #define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
> > #define MMC_MODE_HS400_ES	MMC_CAP(MMC_HS_400_ES)
> >
> >
> > I afraid (I don't sure) that this added caps change the mmc core
> > behavior in U-Boot = U-Boot try to select  the high speed mode even if not
> supported by driver....
> 
> Two issues here. The first is when devicetree enables an unsupported mode, say
> "mmc-hs400-1_2v". That's a devicetree bug, and not something we should fix in
> the code. Hypothetical exam: DT says
> 	bus-width = <8>;
> but only four lines are routed on the board.

Yes it is a device tree issue when the mode is not supported by board / SOC.

But high mode is supported by the STM32MP15x SOC but only if additional
Operation are done (IO configuration to support higher speed) 

It is not supported in U-Boot driver (yet ?) but it is supported by Linux driver...

> The second issue is what happens when somebody plugs in a UHS SD card?
> UHS support is not enabled by default in the stm32mp1 defconfigs, so the mmc
> core won't try to run it at UHS.
> 
> Now if somebody were to enable UHS manually:
> 	CONFIG_MMC_IO_VOLTAGE=y
> 	CONFIG_MMC_UHS_SUPPORT=y
> Then yes, the UHS switch will be attempted, fail, and the card will not be detected.
> 
> To work around that, one could implement a .wait_dat0() that returns an error
> other than -ENOSYS. This would cause mmc_switch_voltage() to fail, making the
> mmc core to leave the card at default speeds.
> 
> My argument is that it takes conscious effort to break things in the first place, so
> it's not a situation we should worry about.
> 

Yes  we should have issue only if UHS defconfig was activated
(CONFIG_MMC_UHS_SUPPORT, CONFIG_MMC_HS*_SUPPORT)....

And it is not the case today in stm32*defconfig
And my proposal is protection (over).

> > The host_caps bitfield should be limited by the supported features in the driver
> (or remove the unsupported features)
> [snip]
> > 	cfg->host_caps &= SDMMC_SUPPORTED_CAPS;
> > or
> > 	cfg->host_caps |= ~SDMMC_UNSUPPORTED_CAPS;
> 
> I think this sort of playing with the flags will cause more confusion.
> People would expect this to come from DT. For example, somebody sets
> "sd-uhs-ddr52" in devicetree. It's more confusing to check host_caps,
> and find that MMC_MODE_DDR_52MHz is not set than it is to see the driver
> trying to place the card in DDR52 and failing.

- card_caps = CARD capacity

For you
- host_caps is SOC capacity (read from DT)

For me 
- host_caps is SOC capacity (read from DT) AND driver capacity

=> then in mmc u-class, the real capacity is  host_caps | card_caps

I already see this kind of limitation in one driver 
omap_hsmmc.c:1939:		cfg->host_caps &= ~fixups->unsupported_caps;

But I agree it is today an over protection on host_caps and it can be confusing
so you can forget this point....

> Alex

Regards

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-11  7:49   ` Jaehoon Chung
@ 2020-09-11 11:46     ` Patrick DELAUNAY
  2020-09-16  1:50       ` Jaehoon Chung
  0 siblings, 1 reply; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-09-11 11:46 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon

> From: Jaehoon Chung <jh80.chung@samsung.com>
> Sent: vendredi 11 septembre 2020 09:50
> 
> On 9/10/20 6:54 AM, Alexandru Gagniuc wrote:
> > mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
> > struct mmc_config from devicetree.
> > The same logic is duplicated in stm32_sdmmc2_probe(). Use
> > mmc_of_parse(), which is more generic.
> >
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > ---
> >  drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
> >  1 file changed, 2 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> > index 6d50356217..77871d5afc 100644
> > --- a/drivers/mmc/stm32_sdmmc2.c
> > +++ b/drivers/mmc/stm32_sdmmc2.c
> > @@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
> >  			     GPIOD_IS_IN);
> >
> >  	cfg->f_min = 400000;
> > -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
> >  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 |
> MMC_VDD_165_195;
> >  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
> >  	cfg->name = "STM32 SD/MMC";
> >
> >  	cfg->host_caps = 0;
> > -	if (cfg->f_max > 25000000)
> > -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> > -
> > -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> > -	case 8:
> > -		cfg->host_caps |= MMC_MODE_8BIT;
> > -		/* fall through */
> > -	case 4:
> > -		cfg->host_caps |= MMC_MODE_4BIT;
> > -		break;
> > -	case 1:
> > -		break;
> > -	default:
> > -		pr_err("invalid \"bus-width\" property, force to 1\n");
> > -	}
> > +	cfg->f_max = 52000000;
> 
> cfg->f_max can be also removed?
> 
> Best Regards,
> Jaehoon Chung

I don't think because " max-frequency" is optional in device tree (only "reg" is required)

Here 52MHz is a default value when it is absent in device tree
That avoids cfg->f_max = 0 after mmc_of_parse() call.

> 
> > +	mmc_of_parse(dev, cfg);
> >
> >  	upriv->mmc = &plat->mmc;
> >
> >

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
                     ` (2 preceding siblings ...)
  2020-09-11  7:49   ` Jaehoon Chung
@ 2020-09-15 19:51   ` Alexandru Gagniuc
  2020-10-02  9:16     ` Patrick DELAUNAY
                       ` (2 more replies)
  2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  4 siblings, 3 replies; 18+ messages in thread
From: Alexandru Gagniuc @ 2020-09-15 19:51 UTC (permalink / raw)
  To: u-boot

"cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no
additional flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
is used for MMC HS at both 26MHz and 52MHz.

Use the same approach and enable MMC_CAP(MMC_HS_52) host capability
when "cap-mmc-highspeed" is found in the devicetree. In the event an
MMC card doesn't support 52 MHz, it will be clocked at a speed based
on its EXT CSD, even on 52 MHz host controllers

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/mmc/mmc-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 90690c8d1e..6d2310eff3 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -198,7 +198,7 @@ int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
 	if (dev_read_bool(dev, "cap-sd-highspeed"))
 		cfg->host_caps |= MMC_CAP(SD_HS);
 	if (dev_read_bool(dev, "cap-mmc-highspeed"))
-		cfg->host_caps |= MMC_CAP(MMC_HS);
+		cfg->host_caps |= MMC_CAP(MMC_HS) | MMC_CAP(MMC_HS_52);
 	if (dev_read_bool(dev, "sd-uhs-sdr12"))
 		cfg->host_caps |= MMC_CAP(UHS_SDR12);
 	if (dev_read_bool(dev, "sd-uhs-sdr25"))
-- 
2.25.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
                     ` (3 preceding siblings ...)
  2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
@ 2020-09-15 19:51   ` Alexandru Gagniuc
  2020-10-02  9:20     ` Patrick DELAUNAY
                       ` (2 more replies)
  4 siblings, 3 replies; 18+ messages in thread
From: Alexandru Gagniuc @ 2020-09-15 19:51 UTC (permalink / raw)
  To: u-boot

mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
struct mmc_config from devicetree.
The same logic is duplicated in stm32_sdmmc2_probe(). Use
mmc_of_parse(), which is more generic.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes from v1:
 - Check the return value of mmc_of_parse().
 - The call to mmc_of_parse() is moved further up. This means we can just
   'return err' on error instead of exiting via goto.

 drivers/mmc/stm32_sdmmc2.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
index 6d50356217..a29657429c 100644
--- a/drivers/mmc/stm32_sdmmc2.c
+++ b/drivers/mmc/stm32_sdmmc2.c
@@ -653,6 +653,12 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
 	if (priv->base == FDT_ADDR_T_NONE)
 		return -EINVAL;
 
+	cfg->host_caps = 0;
+	cfg->f_max = 52000000;
+	ret = mmc_of_parse(dev, cfg);
+	if (ret < 0)
+		return ret;
+
 	if (dev_read_bool(dev, "st,neg-edge"))
 		priv->clk_reg_msk |= SDMMC_CLKCR_NEGEDGE;
 	if (dev_read_bool(dev, "st,sig-dir"))
@@ -676,28 +682,10 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
 			     GPIOD_IS_IN);
 
 	cfg->f_min = 400000;
-	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
 	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 	cfg->name = "STM32 SD/MMC";
 
-	cfg->host_caps = 0;
-	if (cfg->f_max > 25000000)
-		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
-
-	switch (dev_read_u32_default(dev, "bus-width", 1)) {
-	case 8:
-		cfg->host_caps |= MMC_MODE_8BIT;
-		/* fall through */
-	case 4:
-		cfg->host_caps |= MMC_MODE_4BIT;
-		break;
-	case 1:
-		break;
-	default:
-		pr_err("invalid \"bus-width\" property, force to 1\n");
-	}
-
 	upriv->mmc = &plat->mmc;
 
 	/* SDMMC init */
-- 
2.25.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-11 11:46     ` Patrick DELAUNAY
@ 2020-09-16  1:50       ` Jaehoon Chung
  0 siblings, 0 replies; 18+ messages in thread
From: Jaehoon Chung @ 2020-09-16  1:50 UTC (permalink / raw)
  To: u-boot

On 9/11/20 8:46 PM, Patrick DELAUNAY wrote:
> Hi Jaehoon
> 
>> From: Jaehoon Chung <jh80.chung@samsung.com>
>> Sent: vendredi 11 septembre 2020 09:50
>>
>> On 9/10/20 6:54 AM, Alexandru Gagniuc wrote:
>>> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
>>> struct mmc_config from devicetree.
>>> The same logic is duplicated in stm32_sdmmc2_probe(). Use
>>> mmc_of_parse(), which is more generic.
>>>
>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

>>> ---
>>>  drivers/mmc/stm32_sdmmc2.c | 18 ++----------------
>>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
>>> index 6d50356217..77871d5afc 100644
>>> --- a/drivers/mmc/stm32_sdmmc2.c
>>> +++ b/drivers/mmc/stm32_sdmmc2.c
>>> @@ -676,27 +676,13 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>>>  			     GPIOD_IS_IN);
>>>
>>>  	cfg->f_min = 400000;
>>> -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>>>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 |
>> MMC_VDD_165_195;
>>>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>>>  	cfg->name = "STM32 SD/MMC";
>>>
>>>  	cfg->host_caps = 0;
>>> -	if (cfg->f_max > 25000000)
>>> -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>>> -
>>> -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
>>> -	case 8:
>>> -		cfg->host_caps |= MMC_MODE_8BIT;
>>> -		/* fall through */
>>> -	case 4:
>>> -		cfg->host_caps |= MMC_MODE_4BIT;
>>> -		break;
>>> -	case 1:
>>> -		break;
>>> -	default:
>>> -		pr_err("invalid \"bus-width\" property, force to 1\n");
>>> -	}
>>> +	cfg->f_max = 52000000;
>>
>> cfg->f_max can be also removed?
>>
>> Best Regards,
>> Jaehoon Chung
> 
> I don't think because " max-frequency" is optional in device tree (only "reg" is required)
> 
> Here 52MHz is a default value when it is absent in device tree
> That avoids cfg->f_max = 0 after mmc_of_parse() call.
> 
>>
>>> +	mmc_of_parse(dev, cfg);
>>>
>>>  	upriv->mmc = &plat->mmc;
>>>
>>>
> 
> Patrick
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
@ 2020-10-02  9:16     ` Patrick DELAUNAY
  2020-10-02 12:54       ` Patrick DELAUNAY
  2020-10-02  9:33     ` Patrice CHOTARD
  2020-10-21  7:59     ` Patrick DELAUNAY
  2 siblings, 1 reply; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-10-02  9:16 UTC (permalink / raw)
  To: u-boot

Hi,

> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Sent: mardi 15 septembre 2020 21:52
> 
> "cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no additional
> flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
> is used for MMC HS at both 26MHz and 52MHz.
> 
> Use the same approach and enable MMC_CAP(MMC_HS_52) host capability
> when "cap-mmc-highspeed" is found in the devicetree. In the event an MMC card
> doesn't support 52 MHz, it will be clocked at a speed based on its EXT CSD, even
> on 52 MHz host controllers
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/mmc/mmc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index
> 90690c8d1e..6d2310eff3 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -198,7 +198,7 @@ int mmc_of_parse(struct udevice *dev, struct
> mmc_config *cfg)
>  	if (dev_read_bool(dev, "cap-sd-highspeed"))
>  		cfg->host_caps |= MMC_CAP(SD_HS);
>  	if (dev_read_bool(dev, "cap-mmc-highspeed"))
> -		cfg->host_caps |= MMC_CAP(MMC_HS);
> +		cfg->host_caps |= MMC_CAP(MMC_HS) |
> MMC_CAP(MMC_HS_52);
>  	if (dev_read_bool(dev, "sd-uhs-sdr12"))
>  		cfg->host_caps |= MMC_CAP(UHS_SDR12);
>  	if (dev_read_bool(dev, "sd-uhs-sdr25"))
> --
> 2.25.4

Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
Tested-by: Patrick Delaunay <patrick.delaunay@st.com>

Tested on STM32MP157C-EV1, for mmc 1 = emmc with patch [1]

	Mode: MMC High Speed (52MHz)

[1]: mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
     http://patchwork.ozlabs.org/project/uboot/patch/20200909215402.366561-1-mr.nuke.me at gmail.com/


Thanks

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
@ 2020-10-02  9:20     ` Patrick DELAUNAY
  2020-10-02  9:32     ` Patrice CHOTARD
  2020-10-21  9:06     ` Patrick DELAUNAY
  2 siblings, 0 replies; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-10-02  9:20 UTC (permalink / raw)
  To: u-boot

Hi,

> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Sent: mardi 15 septembre 2020 21:52
> 
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of struct
> mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use mmc_of_parse(),
> which is more generic.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes from v1:
>  - Check the return value of mmc_of_parse().
>  - The call to mmc_of_parse() is moved further up. This means we can just
>    'return err' on error instead of exiting via goto.
> 
>  drivers/mmc/stm32_sdmmc2.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 

Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>

Thanks

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  2020-10-02  9:20     ` Patrick DELAUNAY
@ 2020-10-02  9:32     ` Patrice CHOTARD
  2020-10-21  9:06     ` Patrick DELAUNAY
  2 siblings, 0 replies; 18+ messages in thread
From: Patrice CHOTARD @ 2020-10-02  9:32 UTC (permalink / raw)
  To: u-boot

Hi Alexandru

On 9/15/20 9:51 PM, Alexandru Gagniuc wrote:
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of
> struct mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use
> mmc_of_parse(), which is more generic.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes from v1:
>  - Check the return value of mmc_of_parse().
>  - The call to mmc_of_parse() is moved further up. This means we can just
>    'return err' on error instead of exiting via goto.
>
>  drivers/mmc/stm32_sdmmc2.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> index 6d50356217..a29657429c 100644
> --- a/drivers/mmc/stm32_sdmmc2.c
> +++ b/drivers/mmc/stm32_sdmmc2.c
> @@ -653,6 +653,12 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>  	if (priv->base == FDT_ADDR_T_NONE)
>  		return -EINVAL;
>  
> +	cfg->host_caps = 0;
> +	cfg->f_max = 52000000;
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (dev_read_bool(dev, "st,neg-edge"))
>  		priv->clk_reg_msk |= SDMMC_CLKCR_NEGEDGE;
>  	if (dev_read_bool(dev, "st,sig-dir"))
> @@ -676,28 +682,10 @@ static int stm32_sdmmc2_probe(struct udevice *dev)
>  			     GPIOD_IS_IN);
>  
>  	cfg->f_min = 400000;
> -	cfg->f_max = dev_read_u32_default(dev, "max-frequency", 52000000);
>  	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>  	cfg->name = "STM32 SD/MMC";
>  
> -	cfg->host_caps = 0;
> -	if (cfg->f_max > 25000000)
> -		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> -
> -	switch (dev_read_u32_default(dev, "bus-width", 1)) {
> -	case 8:
> -		cfg->host_caps |= MMC_MODE_8BIT;
> -		/* fall through */
> -	case 4:
> -		cfg->host_caps |= MMC_MODE_4BIT;
> -		break;
> -	case 1:
> -		break;
> -	default:
> -		pr_err("invalid \"bus-width\" property, force to 1\n");
> -	}
> -
>  	upriv->mmc = &plat->mmc;
>  
>  	/* SDMMC init */

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
  2020-10-02  9:16     ` Patrick DELAUNAY
@ 2020-10-02  9:33     ` Patrice CHOTARD
  2020-10-21  7:59     ` Patrick DELAUNAY
  2 siblings, 0 replies; 18+ messages in thread
From: Patrice CHOTARD @ 2020-10-02  9:33 UTC (permalink / raw)
  To: u-boot

Hi Alexndru

On 9/15/20 9:51 PM, Alexandru Gagniuc wrote:
> "cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no
> additional flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
> is used for MMC HS at both 26MHz and 52MHz.
>
> Use the same approach and enable MMC_CAP(MMC_HS_52) host capability
> when "cap-mmc-highspeed" is found in the devicetree. In the event an
> MMC card doesn't support 52 MHz, it will be clocked at a speed based
> on its EXT CSD, even on 52 MHz host controllers
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/mmc/mmc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 90690c8d1e..6d2310eff3 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -198,7 +198,7 @@ int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
>  	if (dev_read_bool(dev, "cap-sd-highspeed"))
>  		cfg->host_caps |= MMC_CAP(SD_HS);
>  	if (dev_read_bool(dev, "cap-mmc-highspeed"))
> -		cfg->host_caps |= MMC_CAP(MMC_HS);
> +		cfg->host_caps |= MMC_CAP(MMC_HS) | MMC_CAP(MMC_HS_52);
>  	if (dev_read_bool(dev, "sd-uhs-sdr12"))
>  		cfg->host_caps |= MMC_CAP(UHS_SDR12);
>  	if (dev_read_bool(dev, "sd-uhs-sdr25"))

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-10-02  9:16     ` Patrick DELAUNAY
@ 2020-10-02 12:54       ` Patrick DELAUNAY
  2020-10-22  2:11         ` Peng Fan
  0 siblings, 1 reply; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-10-02 12:54 UTC (permalink / raw)
  To: u-boot

Hi Peng,

> From: Uboot-stm32 <uboot-stm32-bounces@st-md-mailman.stormreply.com> On
> Behalf Of Patrick DELAUNAY
> 
> Hi,
> 
> > From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > Sent: mardi 15 septembre 2020 21:52
> >
> > "cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no
> > additional flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
> > is used for MMC HS at both 26MHz and 52MHz.
> >
> > Use the same approach and enable MMC_CAP(MMC_HS_52) host capability
> > when "cap-mmc-highspeed" is found in the devicetree. In the event an
> > MMC card doesn't support 52 MHz, it will be clocked at a speed based
> > on its EXT CSD, even on 52 MHz host controllers
> >
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > ---
> >  drivers/mmc/mmc-uclass.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
> Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> 
> Tested on STM32MP157C-EV1, for mmc 1 = emmc with patch [1]
> 
> 	Mode: MMC High Speed (52MHz)
> 
> [1]: mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
>      http://patchwork.ozlabs.org/project/uboot/patch/20200909215402.366561-1-
> mr.nuke.me at gmail.com/

Today this patch is delegate to me in patchwork even it is a mmc core patch:

http://patchwork.ozlabs.org/project/uboot/list/?series=201912

You are OK if I integrate this patch in my stm32 pull request for v2020.01-rc1 or
I delegate to you ?

> 
> Thanks
> 
> Patrick
> _______________________________________________
> Uboot-stm32 mailing list
> Uboot-stm32 at st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
  2020-10-02  9:16     ` Patrick DELAUNAY
  2020-10-02  9:33     ` Patrice CHOTARD
@ 2020-10-21  7:59     ` Patrick DELAUNAY
  2 siblings, 0 replies; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-10-21  7:59 UTC (permalink / raw)
  To: u-boot

Hi Alexandru,

> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Sent: mardi 15 septembre 2020 21:52
> To: uboot-stm32 at st-md-mailman.stormreply.com; peng.fan at nxp.com
> Cc: Alexandru Gagniuc <mr.nuke.me@gmail.com>; Patrick DELAUNAY
> <patrick.delaunay@st.com>; Patrice CHOTARD <patrice.chotard@st.com>; u-
> boot at lists.denx.de
> Subject: [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-
> mmc-highspeed"
> Importance: High
> 
> "cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no additional
> flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
> is used for MMC HS at both 26MHz and 52MHz.
> 
> Use the same approach and enable MMC_CAP(MMC_HS_52) host capability
> when "cap-mmc-highspeed" is found in the devicetree. In the event an MMC card
> doesn't support 52 MHz, it will be clocked at a speed based on its EXT CSD, even
> on 52 MHz host controllers
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/mmc/mmc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied to u-boot-stm/master, thanks!

Regards

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
  2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
  2020-10-02  9:20     ` Patrick DELAUNAY
  2020-10-02  9:32     ` Patrice CHOTARD
@ 2020-10-21  9:06     ` Patrick DELAUNAY
  2 siblings, 0 replies; 18+ messages in thread
From: Patrick DELAUNAY @ 2020-10-21  9:06 UTC (permalink / raw)
  To: u-boot

Hi Alexandru,

> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Sent: mardi 15 septembre 2020 21:52
> 
> mmc_of_parse() can populate the 'f_max' and 'host_caps' fields of struct
> mmc_config from devicetree.
> The same logic is duplicated in stm32_sdmmc2_probe(). Use mmc_of_parse(),
> which is more generic.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes from v1:
>  - Check the return value of mmc_of_parse().
>  - The call to mmc_of_parse() is moved further up. This means we can just
>    'return err' on error instead of exiting via goto.
> 
>  drivers/mmc/stm32_sdmmc2.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
>

Applied to u-boot-stm/master, thanks!

Regards

Patrick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed"
  2020-10-02 12:54       ` Patrick DELAUNAY
@ 2020-10-22  2:11         ` Peng Fan
  0 siblings, 0 replies; 18+ messages in thread
From: Peng Fan @ 2020-10-22  2:11 UTC (permalink / raw)
  To: u-boot

> Subject: RE: [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with
> "cap-mmc-highspeed"
> 
> Hi Peng,
> 
> > From: Uboot-stm32
> <uboot-stm32-bounces@st-md-mailman.stormreply.com>
> > On Behalf Of Patrick DELAUNAY
> >
> > Hi,
> >
> > > From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > Sent: mardi 15 septembre 2020 21:52
> > >
> > > "cap-mmc-highspeed" enables support for 26 MHz MMC, but there is no
> > > additional flag to enable 52 MHz MMC. In Linux. "cap-mmc-highspeed"
> > > is used for MMC HS at both 26MHz and 52MHz.
> > >
> > > Use the same approach and enable MMC_CAP(MMC_HS_52) host
> capability
> > > when "cap-mmc-highspeed" is found in the devicetree. In the event an
> > > MMC card doesn't support 52 MHz, it will be clocked at a speed based
> > > on its EXT CSD, even on 52 MHz host controllers
> > >
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > ---
> > >  drivers/mmc/mmc-uclass.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> >
> > Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
> > Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> >
> > Tested on STM32MP157C-EV1, for mmc 1 = emmc with patch [1]
> >
> > 	Mode: MMC High Speed (52MHz)
> >
> > [1]: mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities
> >
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> work.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20200909215402.366561-
> 1-&a
> >
> mp;data=02%7C01%7Cpeng.fan%40nxp.com%7C2242885f15fb4bc5a38508d
> 866d261c
> >
> e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63737240105383
> 8309&amp;
> >
> sdata=NJ1fDTquBTBOQVMdxYf3gTExxkNEHeZ9mnazXAKY0GQ%3D&amp;rese
> rved=0
> > mr.nuke.me at gmail.com/
> 
> Today this patch is delegate to me in patchwork even it is a mmc core patch:
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchw
> ork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D201912&amp;da
> ta=02%7C01%7Cpeng.fan%40nxp.com%7C2242885f15fb4bc5a38508d866d2
> 61ce%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63737240105
> 3848304&amp;sdata=J4b9Dy%2FnujRrpiYtaQbIq68w%2F2FfKlKoMheH8Mtae
> P8%3D&amp;reserved=0
> 
> You are OK if I integrate this patch in my stm32 pull request for v2020.01-rc1
> or I delegate to you ?

You could take that first. My PR will be later.

Thanks,
Peng.

> 
> >
> > Thanks
> >
> > Patrick
> > _______________________________________________
> > Uboot-stm32 mailing list
> > Uboot-stm32 at st-md-mailman.stormreply.com
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fst-m
> >
> d-mailman.stormreply.com%2Fmailman%2Flistinfo%2Fuboot-stm32&amp;dat
> a=0
> >
> 2%7C01%7Cpeng.fan%40nxp.com%7C2242885f15fb4bc5a38508d866d261ce
> %7C686ea
> >
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637372401053848304&amp;
> sdata=DDw
> >
> sEwf%2F%2B3JOeITkO2pnUjJFu4hpru5DqY5FUGNnXdM%3D&amp;reserved=
> 0

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-10-22  2:11 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200910005435epcas1p20d2f97d097717214bd2836fafdd140d9@epcas1p2.samsung.com>
2020-09-09 21:54 ` [PATCH] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
2020-09-10  7:52   ` Patrice CHOTARD
2020-09-10 16:04   ` Patrick DELAUNAY
2020-09-10 20:10     ` Alex G.
2020-09-11  9:00       ` Patrick DELAUNAY
2020-09-11  7:49   ` Jaehoon Chung
2020-09-11 11:46     ` Patrick DELAUNAY
2020-09-16  1:50       ` Jaehoon Chung
2020-09-15 19:51   ` [PATCH 1/2] mmc: mmc_of_parse: Enable 52 MHz support with "cap-mmc-highspeed" Alexandru Gagniuc
2020-10-02  9:16     ` Patrick DELAUNAY
2020-10-02 12:54       ` Patrick DELAUNAY
2020-10-22  2:11         ` Peng Fan
2020-10-02  9:33     ` Patrice CHOTARD
2020-10-21  7:59     ` Patrick DELAUNAY
2020-09-15 19:51   ` [PATCH v2 2/2] mmc: stm32_sdmmc2: Use mmc_of_parse() to read host capabilities Alexandru Gagniuc
2020-10-02  9:20     ` Patrick DELAUNAY
2020-10-02  9:32     ` Patrice CHOTARD
2020-10-21  9:06     ` Patrick DELAUNAY

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.