| 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 |
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.
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
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 > > Kind regards > Uffe
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 --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, };