diff mbox series

[2/3] mmc: sdhci-iproc: Cap min clock frequency on BCM2711

Message ID 20210406104802.20898-3-nsaenz@kernel.org (mailing list archive)
State New, archived
Headers show
Series BCM2711 sdhci-iproc improvements | expand

Commit Message

Nicolas Saenz Julienne April 6, 2021, 10:48 a.m. UTC
There is a known bug on BCM2711's SDHCI core integration where the
controller will hang when the difference between the core clock and the
bus clock is too great. Specifically this can be reproduced under the
following conditions:

- No SD card plugged in, polling thread is running, probing cards at
  100KHz.
- BCM2711's core clock configured at 500MHz or more.

So set 200MHz as the minimum clock frequency available for that board.

For more information on the issue see this:
https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsaenz@kernel.org/T/#m11f2783a09b581da6b8a15f302625b43a6ecdeca

Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 drivers/mmc/host/sdhci-iproc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Stefan Wahren April 6, 2021, 5:59 p.m. UTC | #1
Hi Nicolas,

Am 06.04.21 um 12:48 schrieb Nicolas Saenz Julienne:
> There is a known bug on BCM2711's SDHCI core integration where the
> controller will hang when the difference between the core clock and the
> bus clock is too great. Specifically this can be reproduced under the
> following conditions:
>
> - No SD card plugged in, polling thread is running, probing cards at
>   100KHz.
> - BCM2711's core clock configured at 500MHz or more.
>
> So set 200MHz as the minimum clock frequency available for that board.
i think it should be 200 kHz?
>
> For more information on the issue see this:
> https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsaenz@kernel.org/T/#m11f2783a09b581da6b8a15f302625b43a6ecdeca
>
> Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  drivers/mmc/host/sdhci-iproc.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index ddeaf8e1f72f..1ef888e91f73 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -173,6 +173,23 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
>  		return pltfm_host->clock;
>  }
>  
> +/*
> + * There is a known bug on BCM2711's SDHCI core integration where the
> + * controller will hang when the difference between the core clock and the bus
> + * clock is too great. Specifically this can be reproduced under the following
> + * conditions:
> + *
> + *  - No SD card plugged in, polling thread is running, probing cards at
> + *    100KHz.
> + *  - BCM2711's core clock configured at 500MHz or more
> + *
> + * So we set 200MHz as the minimum clock frequency available for that SoC.
> + */
> +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> +{
> +	return 200000;
> +}
> +
>  static const struct sdhci_ops sdhci_iproc_ops = {
>  	.set_clock = sdhci_set_clock,
>  	.get_max_clock = sdhci_iproc_get_max_clock,
> @@ -271,6 +288,7 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
>  	.set_clock = sdhci_set_clock,
>  	.set_power = sdhci_set_power_and_bus_voltage,
>  	.get_max_clock = sdhci_iproc_get_max_clock,
> +	.get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.reset = sdhci_reset,
>  	.set_uhs_signaling = sdhci_set_uhs_signaling,
Nicolas Saenz Julienne April 7, 2021, 8:44 a.m. UTC | #2
On Tue, 2021-04-06 at 19:59 +0200, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 06.04.21 um 12:48 schrieb Nicolas Saenz Julienne:
> > There is a known bug on BCM2711's SDHCI core integration where the
> > controller will hang when the difference between the core clock and the
> > bus clock is too great. Specifically this can be reproduced under the
> > following conditions:
> > 
> > - No SD card plugged in, polling thread is running, probing cards at
> >   100KHz.
> > - BCM2711's core clock configured at 500MHz or more.
> > 
> > So set 200MHz as the minimum clock frequency available for that board.
> i think it should be 200 kHz?

Of course... Sorry for that.

Regards,
Nicolas

> > 
> > For more information on the issue see this:
> > https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsaenz@kernel.org/T/#m11f2783a09b581da6b8a15f302625b43a6ecdeca
> > 
> > Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  drivers/mmc/host/sdhci-iproc.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> > index ddeaf8e1f72f..1ef888e91f73 100644
> > --- a/drivers/mmc/host/sdhci-iproc.c
> > +++ b/drivers/mmc/host/sdhci-iproc.c
> > @@ -173,6 +173,23 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
> >  		return pltfm_host->clock;
> >  }
> >  
> > 
> > +/*
> > + * There is a known bug on BCM2711's SDHCI core integration where the
> > + * controller will hang when the difference between the core clock and the bus
> > + * clock is too great. Specifically this can be reproduced under the following
> > + * conditions:
> > + *
> > + *  - No SD card plugged in, polling thread is running, probing cards at
> > + *    100KHz.
> > + *  - BCM2711's core clock configured at 500MHz or more
> > + *
> > + * So we set 200MHz as the minimum clock frequency available for that SoC.
> > + */
> > +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> > +{
> > +	return 200000;
> > +}
> > +
> >  static const struct sdhci_ops sdhci_iproc_ops = {
> >  	.set_clock = sdhci_set_clock,
> >  	.get_max_clock = sdhci_iproc_get_max_clock,
> > @@ -271,6 +288,7 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
> >  	.set_clock = sdhci_set_clock,
> >  	.set_power = sdhci_set_power_and_bus_voltage,
> >  	.get_max_clock = sdhci_iproc_get_max_clock,
> > +	.get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
> >  	.set_bus_width = sdhci_set_bus_width,
> >  	.reset = sdhci_reset,
> >  	.set_uhs_signaling = sdhci_set_uhs_signaling,
Ivan T . Ivanov July 30, 2021, 6:44 a.m. UTC | #3
Hi Nicolas,

Quoting Nicolas Saenz Julienne (2021-04-07 11:44:39)
> On Tue, 2021-04-06 at 19:59 +0200, Stefan Wahren wrote:
> > Hi Nicolas,
> > 
> > Am 06.04.21 um 12:48 schrieb Nicolas Saenz Julienne:
> > > There is a known bug on BCM2711's SDHCI core integration where the
> > > controller will hang when the difference between the core clock and the
> > > bus clock is too great. Specifically this can be reproduced under the
> > > following conditions:
> > > 
> > > - No SD card plugged in, polling thread is running, probing cards at
> > >   100KHz.
> > > - BCM2711's core clock configured at 500MHz or more.
> > > 
> > > So set 200MHz as the minimum clock frequency available for that board.
> > i think it should be 200 kHz?
> 
> Of course... Sorry for that.
> 

What is the plan for this patch? 

Thanks,
Ivan
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..1ef888e91f73 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -173,6 +173,23 @@  static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
 		return pltfm_host->clock;
 }
 
+/*
+ * There is a known bug on BCM2711's SDHCI core integration where the
+ * controller will hang when the difference between the core clock and the bus
+ * clock is too great. Specifically this can be reproduced under the following
+ * conditions:
+ *
+ *  - No SD card plugged in, polling thread is running, probing cards at
+ *    100KHz.
+ *  - BCM2711's core clock configured at 500MHz or more
+ *
+ * So we set 200MHz as the minimum clock frequency available for that SoC.
+ */
+static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
+{
+	return 200000;
+}
+
 static const struct sdhci_ops sdhci_iproc_ops = {
 	.set_clock = sdhci_set_clock,
 	.get_max_clock = sdhci_iproc_get_max_clock,
@@ -271,6 +288,7 @@  static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
 	.set_clock = sdhci_set_clock,
 	.set_power = sdhci_set_power_and_bus_voltage,
 	.get_max_clock = sdhci_iproc_get_max_clock,
+	.get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,