| Message ID | 1628334401-6577-1-git-send-email-stefan.wahren@i2se.com |
|---|---|
| Headers | show |
| Series | ARM: dts: Add Raspberry Pi CM4 & CM4 IO Board support | expand |
On Sat, 7 Aug 2021 at 13:07, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > From: Nicolas Saenz Julienne <nsaenz@kernel.org> > > 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 > 100 kHz. > - BCM2711's core clock configured at 500MHz or more. > > So set 200 kHz 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> > 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 | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index cce390f..032bf85 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 > + * 100 kHz. > + * - BCM2711's core clock configured at 500MHz or more > + * > + * So we set 200kHz 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, > -- > 2.7.4 >
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 >
On 8/7/2021 1:06 PM, Stefan Wahren wrote: > This series add support for the Raspberry Pi Compute Module 4 and its > IO board. > > Changes in V2: > - drop emmc2bus patch as it affects userspace (thanks to Marc Zyngier) > - tested with CM4 and integrate sdhci patches from Nicolas (just include kHz fixups) > - add Rob's Acked-by > - add HS200 property to emmc2 node for a slight performance gain > - move antenna comment to the proper position Nicolas, I am assuming you will be picking up these patches and submit a BCM283x pull request with them. Thanks! > > Nicolas Saenz Julienne (2): > mmc: sdhci-iproc: Cap min clock frequency on BCM2711 > mmc: sdhci-iproc: Set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN on BCM2711 > > Stefan Wahren (8): > ARM: dts: bcm2711: fix MDIO #address- and #size-cells > ARM: dts: bcm2711-rpi-4-b: fix sd_io_1v8_reg regulator states > dt-bindings: display: bcm2835: add optional property power-domains > ARM: dts: bcm283x-rpi: Move Wifi/BT into separate dtsi > dt-bindings: arm: bcm2835: Add Raspberry Pi Compute Module 4 > ARM: dts: Add Raspberry Pi Compute Module 4 > ARM: dts: Add Raspberry Pi Compute Module 4 IO Board > arm64: dts: broadcom: Add reference to RPi CM4 IO Board > > .../devicetree/bindings/arm/bcm/bcm2835.yaml | 1 + > .../bindings/display/brcm,bcm2835-dsi0.yaml | 3 + > .../bindings/display/brcm,bcm2835-hdmi.yaml | 3 + > .../bindings/display/brcm,bcm2835-v3d.yaml | 3 + > .../bindings/display/brcm,bcm2835-vec.yaml | 3 + > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 42 ++----- > arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts | 138 +++++++++++++++++++++ > arch/arm/boot/dts/bcm2711-rpi-cm4.dtsi | 113 +++++++++++++++++ > arch/arm/boot/dts/bcm2711.dtsi | 4 +- > arch/arm/boot/dts/bcm2835-rpi-zero-w.dts | 31 ++--- > arch/arm/boot/dts/bcm2837-rpi-3-a-plus.dts | 36 ++---- > arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts | 36 ++---- > arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 36 ++---- > arch/arm/boot/dts/bcm283x-rpi-wifi-bt.dtsi | 34 +++++ > arch/arm64/boot/dts/broadcom/Makefile | 1 + > .../arm64/boot/dts/broadcom/bcm2711-rpi-cm4-io.dts | 2 + > drivers/mmc/host/sdhci-iproc.c | 21 +++- > 18 files changed, 366 insertions(+), 142 deletions(-) > create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts > create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4.dtsi > create mode 100644 arch/arm/boot/dts/bcm283x-rpi-wifi-bt.dtsi > create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-io.dts >
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
Hi Florian, On Sun, 2021-08-22 at 12:31 +0200, Florian Fainelli wrote: > > On 8/7/2021 1:06 PM, Stefan Wahren wrote: > > This series add support for the Raspberry Pi Compute Module 4 and its > > IO board. > > > > Changes in V2: > > - drop emmc2bus patch as it affects userspace (thanks to Marc Zyngier) > > - tested with CM4 and integrate sdhci patches from Nicolas (just include kHz fixups) > > - add Rob's Acked-by > > - add HS200 property to emmc2 node for a slight performance gain > > - move antenna comment to the proper position > > Nicolas, I am assuming you will be picking up these patches and submit a > BCM283x pull request with them. Thanks! Yes, sorry for the late replies. I'm just back from vacation. Regards, Nicolas
Hi Stefan, On Sat, 2021-08-07 at 13:06 +0200, Stefan Wahren wrote: > This series add support for the Raspberry Pi Compute Module 4 and its > IO board. Sorry for the late review. I finally got some time to review/test this properly. The devicetree patches look really good, dtbs_check's shows no new errors, and all peripherals work. The only thing I can't verify is PCIe as I have nothing to plug into it, but it should work. I'll apply the series (modulo the mmc patches) for-next soon. Thanks! > Changes in V2: > - drop emmc2bus patch as it affects userspace (thanks to Marc Zyngier) > - tested with CM4 and integrate sdhci patches from Nicolas (just include kHz fixups) > - add Rob's Acked-by > - add HS200 property to emmc2 node for a slight performance gain > - move antenna comment to the proper position > > Nicolas Saenz Julienne (2): > mmc: sdhci-iproc: Cap min clock frequency on BCM2711 > mmc: sdhci-iproc: Set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN on BCM2711 > > Stefan Wahren (8): > ARM: dts: bcm2711: fix MDIO #address- and #size-cells > ARM: dts: bcm2711-rpi-4-b: fix sd_io_1v8_reg regulator states > dt-bindings: display: bcm2835: add optional property power-domains > ARM: dts: bcm283x-rpi: Move Wifi/BT into separate dtsi > dt-bindings: arm: bcm2835: Add Raspberry Pi Compute Module 4 > ARM: dts: Add Raspberry Pi Compute Module 4 > ARM: dts: Add Raspberry Pi Compute Module 4 IO Board > arm64: dts: broadcom: Add reference to RPi CM4 IO Board > > .../devicetree/bindings/arm/bcm/bcm2835.yaml | 1 + > .../bindings/display/brcm,bcm2835-dsi0.yaml | 3 + > .../bindings/display/brcm,bcm2835-hdmi.yaml | 3 + > .../bindings/display/brcm,bcm2835-v3d.yaml | 3 + > .../bindings/display/brcm,bcm2835-vec.yaml | 3 + > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 42 ++----- > arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts | 138 +++++++++++++++++++++ > arch/arm/boot/dts/bcm2711-rpi-cm4.dtsi | 113 +++++++++++++++++ > arch/arm/boot/dts/bcm2711.dtsi | 4 +- > arch/arm/boot/dts/bcm2835-rpi-zero-w.dts | 31 ++--- > arch/arm/boot/dts/bcm2837-rpi-3-a-plus.dts | 36 ++---- > arch/arm/boot/dts/bcm2837-rpi-3-b-plus.dts | 36 ++---- > arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 36 ++---- > arch/arm/boot/dts/bcm283x-rpi-wifi-bt.dtsi | 34 +++++ > arch/arm64/boot/dts/broadcom/Makefile | 1 + > .../arm64/boot/dts/broadcom/bcm2711-rpi-cm4-io.dts | 2 + > drivers/mmc/host/sdhci-iproc.c | 21 +++- > 18 files changed, 366 insertions(+), 142 deletions(-) > create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts > create mode 100644 arch/arm/boot/dts/bcm2711-rpi-cm4.dtsi > create mode 100644 arch/arm/boot/dts/bcm283x-rpi-wifi-bt.dtsi > create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-cm4-io.dts >
On Sat, 2021-08-07 at 13:06 +0200, Stefan Wahren wrote: > This adds the matching carrier for Raspberry Pi Compute Module 4. > Instead of xHCI USB host controller there is just a USB 2.0 interface > connected to the DWC2 controller from the BCM2711. As a result > there is a free PCIe Gen 2 socket. Also there are 2 full-size HDMI 2.0 > connectors. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- A general note: the CM4 carrier board contains an RTC and a simple fan controller, which will need to be enabled further down the road. Regards, Nicolas
Hi Nicolas, Am 04.09.21 um 16:52 schrieb nicolas saenz julienne: > On Sat, 2021-08-07 at 13:06 +0200, Stefan Wahren wrote: >> This adds the matching carrier for Raspberry Pi Compute Module 4. >> Instead of xHCI USB host controller there is just a USB 2.0 interface >> connected to the DWC2 controller from the BCM2711. As a result >> there is a free PCIe Gen 2 socket. Also there are 2 full-size HDMI 2.0 >> connectors. >> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >> --- > A general note: the CM4 carrier board contains an RTC and a simple fan > controller, which will need to be enabled further down the road. yes i forgot to mention that i left out these parts because they are connected to the VideoCore I2C. But yes, this could be added later ... Best regards Stefan > > Regards, > Nicolas > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel