diff mbox series

[V2,05/10] mmc: sdhci-iproc: Set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN on BCM2711

Message ID 1628334401-6577-6-git-send-email-stefan.wahren@i2se.com
State New
Headers show
Series ARM: dts: Add Raspberry Pi CM4 & CM4 IO Board support | expand

Commit Message

Stefan Wahren Aug. 7, 2021, 11:06 a.m. UTC
From: Nicolas Saenz Julienne <nsaenz@kernel.org>

The controller doesn't seem to pick-up on clock changes, so set the
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency
directly from the clock.

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

Comments

Ulf Hansson Aug. 16, 2021, 1:59 p.m. UTC | #1
On Sat, 7 Aug 2021 at 13:07, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>

> From: Nicolas Saenz Julienne <nsaenz@kernel.org>

>

> The controller doesn't seem to pick-up on clock changes, so set the

> SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency

> directly from the clock.

>

> Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")

> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>

> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>


Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---

>  drivers/mmc/host/sdhci-iproc.c | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

>

> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c

> index 032bf85..e7565c6 100644

> --- a/drivers/mmc/host/sdhci-iproc.c

> +++ b/drivers/mmc/host/sdhci-iproc.c

> @@ -295,7 +295,8 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {

>  };

>

>  static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {

> -       .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,

> +       .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |

> +                 SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,

>         .ops = &sdhci_iproc_bcm2711_ops,

>  };

>

> --

> 2.7.4

>
Jeremy Linton Aug. 26, 2021, 6:36 a.m. UTC | #2
Hi,


On 8/7/21 6:06 AM, Stefan Wahren wrote:
> From: Nicolas Saenz Julienne <nsaenz@kernel.org>

> 

> The controller doesn't seem to pick-up on clock changes, so set the

> SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency

> directly from the clock.

> 

> Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")

> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>

> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

> ---

>   drivers/mmc/host/sdhci-iproc.c | 3 ++-

>   1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c

> index 032bf85..e7565c6 100644

> --- a/drivers/mmc/host/sdhci-iproc.c

> +++ b/drivers/mmc/host/sdhci-iproc.c

> @@ -295,7 +295,8 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {

>   };

>   

>   static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {

> -	.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,

> +	.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |

> +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,

>   	.ops = &sdhci_iproc_bcm2711_ops,

>   };


I just noticed that this got merged to rc7, and it breaks the ACPI based 
rpi's because it causes the 100Mhz max clock to be overridden to the 
return from sdhci_iproc_get_max_clock() which is 0 because there isn't a 
OF/DT based clock device.
Ulf Hansson Aug. 26, 2021, 9:22 a.m. UTC | #3
On Thu, 26 Aug 2021 at 08:36, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
>
> On 8/7/21 6:06 AM, Stefan Wahren wrote:
> > From: Nicolas Saenz Julienne <nsaenz@kernel.org>
> >
> > The controller doesn't seem to pick-up on clock changes, so set the
> > SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency
> > directly from the clock.
> >
> > Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> >   drivers/mmc/host/sdhci-iproc.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> > index 032bf85..e7565c6 100644
> > --- a/drivers/mmc/host/sdhci-iproc.c
> > +++ b/drivers/mmc/host/sdhci-iproc.c
> > @@ -295,7 +295,8 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
> >   };
> >
> >   static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
> > -     .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> > +     .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |
> > +               SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> >       .ops = &sdhci_iproc_bcm2711_ops,
> >   };
>
> I just noticed that this got merged to rc7, and it breaks the ACPI based
> rpi's because it causes the 100Mhz max clock to be overridden to the
> return from sdhci_iproc_get_max_clock() which is 0 because there isn't a
> OF/DT based clock device.

Thanks for reporting! I allow Stefan to respond in a day or two,
before I do a revert of it.

Kind regards
Uffe
Ulf Hansson Aug. 27, 2021, 2:44 p.m. UTC | #4
On Thu, 26 Aug 2021 at 13:18, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>

> Am 26.08.21 um 11:22 schrieb Ulf Hansson:

> > On Thu, 26 Aug 2021 at 08:36, Jeremy Linton <jeremy.linton@arm.com> wrote:

> >> Hi,

> >>

> >>

> >> On 8/7/21 6:06 AM, Stefan Wahren wrote:

> >>> From: Nicolas Saenz Julienne <nsaenz@kernel.org>

> >>>

> >>> The controller doesn't seem to pick-up on clock changes, so set the

> >>> SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency

> >>> directly from the clock.

> >>>

> >>> Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")

> >>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>

> >>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

> >>> ---

> >>>   drivers/mmc/host/sdhci-iproc.c | 3 ++-

> >>>   1 file changed, 2 insertions(+), 1 deletion(-)

> >>>

> >>> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c

> >>> index 032bf85..e7565c6 100644

> >>> --- a/drivers/mmc/host/sdhci-iproc.c

> >>> +++ b/drivers/mmc/host/sdhci-iproc.c

> >>> @@ -295,7 +295,8 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {

> >>>   };

> >>>

> >>>   static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {

> >>> -     .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,

> >>> +     .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |

> >>> +               SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,

> >>>       .ops = &sdhci_iproc_bcm2711_ops,

> >>>   };

> >> I just noticed that this got merged to rc7, and it breaks the ACPI based

> >> rpi's because it causes the 100Mhz max clock to be overridden to the

> >> return from sdhci_iproc_get_max_clock() which is 0 because there isn't a

> >> OF/DT based clock device.

> > Thanks for reporting! I allow Stefan to respond in a day or two,

> > before I do a revert of it.

>

> I'm fine with a revert.

>

> Thanks


Patch reverted and applied for fixes, thanks!

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 032bf85..e7565c6 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -295,7 +295,8 @@  static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
 };
 
 static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
-	.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
+	.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
 	.ops = &sdhci_iproc_bcm2711_ops,
 };