| Message ID | 20210322185816.27582-5-nsaenz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | BCM2711's sdhci-iproc CMD timeouts | expand |
1st line of commit should be ARM: dts: bcm2711 On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote: > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) > seems to interfere with the VPU clock when setup at frequencies bigger > than 500MHz (a pretty common case). This ends up causing unwarranted > SDHCI CMD hangs when no SD card is present. > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > --- > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > index 3b4ab947492a..9aa8408d9960 100644 > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > @@ -257,6 +257,12 @@ &emmc2 { > vqmmc-supply = <&sd_io_1v8_reg>; > vmmc-supply = <&sd_vcc_reg>; > broken-cd; > + /* > + * Force the frequency to 150MHz as the default 100MHz seems to > + * interfere with the VPU clock when setup at frequencies bigger than > + * 500MHz, causing unwarranted CMD hangs. > + */ > + clock-frequency = <150000000>; > status = "okay"; > }; > >
On Mon, 2021-03-22 at 12:06 -0700, Scott Branden wrote:
> 1st line of commit should be ARM: dts: bcm2711
Ouch, of course... Sorry for that.
Regards,
Nicolas
Hi Nicolas, Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) > seems to interfere with the VPU clock when setup at frequencies bigger > than 500MHz (a pretty common case). This ends up causing unwarranted > SDHCI CMD hangs when no SD card is present. > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > --- > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > index 3b4ab947492a..9aa8408d9960 100644 > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > @@ -257,6 +257,12 @@ &emmc2 { > vqmmc-supply = <&sd_io_1v8_reg>; > vmmc-supply = <&sd_vcc_reg>; > broken-cd; > + /* > + * Force the frequency to 150MHz as the default 100MHz seems to > + * interfere with the VPU clock when setup at frequencies bigger than > + * 500MHz, causing unwarranted CMD hangs. > + */ > + clock-frequency = <150000000>; i don't want to bike-shed here, but is there any chance to solve this in clk-bcm2835 in a less hacky way? Anyway thank for taking care of this. Best regards > status = "okay"; > }; >
Hi Stefan, On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote: > Hi Nicolas, > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) > > seems to interfere with the VPU clock when setup at frequencies bigger > > than 500MHz (a pretty common case). This ends up causing unwarranted > > SDHCI CMD hangs when no SD card is present. > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > > --- > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > index 3b4ab947492a..9aa8408d9960 100644 > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > @@ -257,6 +257,12 @@ &emmc2 { > > vqmmc-supply = <&sd_io_1v8_reg>; > > vmmc-supply = <&sd_vcc_reg>; > > broken-cd; > > + /* > > + * Force the frequency to 150MHz as the default 100MHz seems to > > + * interfere with the VPU clock when setup at frequencies bigger than > > + * 500MHz, causing unwarranted CMD hangs. > > + */ > > + clock-frequency = <150000000>; > > i don't want to bike-shed here, but is there any chance to solve this in > clk-bcm2835 in a less hacky way? What do you have in mind? All I can think of is adding some kind of heuristic to the clock's prepare() callback. That said, I don't feel it would be a better solution than this. Regards, Nicolas
Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne: > Hi Stefan, > > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote: >> Hi Nicolas, >> >> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: >>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> >>> >>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) >>> seems to interfere with the VPU clock when setup at frequencies bigger >>> than 500MHz (a pretty common case). This ends up causing unwarranted >>> SDHCI CMD hangs when no SD card is present. >>> >>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> >>> --- >>> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>> index 3b4ab947492a..9aa8408d9960 100644 >>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>> @@ -257,6 +257,12 @@ &emmc2 { >>> vqmmc-supply = <&sd_io_1v8_reg>; >>> vmmc-supply = <&sd_vcc_reg>; >>> broken-cd; >>> + /* >>> + * Force the frequency to 150MHz as the default 100MHz seems to >>> + * interfere with the VPU clock when setup at frequencies bigger than >>> + * 500MHz, causing unwarranted CMD hangs. >>> + */ >>> + clock-frequency = <150000000>; >> i don't want to bike-shed here, but is there any chance to solve this in >> clk-bcm2835 in a less hacky way? > What do you have in mind? Sorry, nothing specific. > > All I can think of is adding some kind of heuristic to the clock's prepare() > callback. That said, I don't feel it would be a better solution than this. Based on my limited knowledge and an old SD card specification, all possibly connected devices could have different frequencies. So my concern here is, that in case we limit the frequency to a specific value we could break things just to suppress a warning. More background information about this issue would be helpful. Best regards
On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote: > Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne: > > Hi Stefan, > > > > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: > > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > > > > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) > > > > seems to interfere with the VPU clock when setup at frequencies bigger > > > > than 500MHz (a pretty common case). This ends up causing unwarranted > > > > SDHCI CMD hangs when no SD card is present. > > > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > > > > --- > > > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > index 3b4ab947492a..9aa8408d9960 100644 > > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > @@ -257,6 +257,12 @@ &emmc2 { > > > > vqmmc-supply = <&sd_io_1v8_reg>; > > > > vmmc-supply = <&sd_vcc_reg>; > > > > broken-cd; > > > > + /* > > > > + * Force the frequency to 150MHz as the default 100MHz seems to > > > > + * interfere with the VPU clock when setup at frequencies bigger than > > > > + * 500MHz, causing unwarranted CMD hangs. > > > > + */ > > > > + clock-frequency = <150000000>; > > > i don't want to bike-shed here, but is there any chance to solve this in > > > clk-bcm2835 in a less hacky way? > > What do you have in mind? > Sorry, nothing specific. > > > > All I can think of is adding some kind of heuristic to the clock's prepare() > > callback. That said, I don't feel it would be a better solution than this. > > Based on my limited knowledge and an old SD card specification, all > possibly connected devices could have different frequencies. So my > concern here is, that in case we limit the frequency to a specific value > we could break things just to suppress a warning. SDHCI should be able to handle up to 233MHz IIRC, and there are divisors available, it depends on the implementation but the worst kind provide /2^n. Not perfect, but good enough for things to work. Now, I've been having a deeper look into how clocks are handled, and found two new clues: - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN, that is, the controller isn't properly identifying the clock frequency fed into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI expert, so it's possible changing frequencies also needs a special operation to recalculate this variable. But this was making all internal calculations wrong when paired with this series. - With this flag set SDHCI's core now properly calculates divisor values based on whatever clock frequency I set in DT. And guess what, the issue reappears even when running on 150MHz. It turns out, as I had some debugging enabled, the issue only happens when the controller is configured at 100KHz (that only happens while running the card detect thread). So, I can now do this (note that for card detection try to communicate with the card starting at 400KHz down to 100KHz in 100KHz steps): ----->8----- diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c index 536c382e2486..e5a5de63f347 100644 --- a/drivers/mmc/host/sdhci-iproc.c +++ b/drivers/mmc/host/sdhci-iproc.c @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host) return pltfm_host->clock; } +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,13 +276,15 @@ 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, }; ----->8----- Which is rather nicer than what this series introduces. But I can't still explain why configuring the controller at 100KHz is causing the hangs (while having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz from the polling frequency list is going to break support for older SD cards. Regards, Nicolas
Am 26.03.21 um 17:17 schrieb Nicolas Saenz Julienne: > On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote: >> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne: >>> Hi Stefan, >>> >>> On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote: >>>> Hi Nicolas, >>>> >>>> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: >>>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> >>>>> >>>>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) >>>>> seems to interfere with the VPU clock when setup at frequencies bigger >>>>> than 500MHz (a pretty common case). This ends up causing unwarranted >>>>> SDHCI CMD hangs when no SD card is present. >>>>> >>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> >>>>> --- >>>>> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ >>>>> 1 file changed, 6 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>>>> index 3b4ab947492a..9aa8408d9960 100644 >>>>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts >>>>> @@ -257,6 +257,12 @@ &emmc2 { >>>>> vqmmc-supply = <&sd_io_1v8_reg>; >>>>> vmmc-supply = <&sd_vcc_reg>; >>>>> broken-cd; >>>>> + /* >>>>> + * Force the frequency to 150MHz as the default 100MHz seems to >>>>> + * interfere with the VPU clock when setup at frequencies bigger than >>>>> + * 500MHz, causing unwarranted CMD hangs. >>>>> + */ >>>>> + clock-frequency = <150000000>; >>>> i don't want to bike-shed here, but is there any chance to solve this in >>>> clk-bcm2835 in a less hacky way? >>> What do you have in mind? >> Sorry, nothing specific. >>> All I can think of is adding some kind of heuristic to the clock's prepare() >>> callback. That said, I don't feel it would be a better solution than this. >> Based on my limited knowledge and an old SD card specification, all >> possibly connected devices could have different frequencies. So my >> concern here is, that in case we limit the frequency to a specific value >> we could break things just to suppress a warning. > SDHCI should be able to handle up to 233MHz IIRC, and there are divisors > available, it depends on the implementation but the worst kind provide /2^n. > Not perfect, but good enough for things to work. > > Now, I've been having a deeper look into how clocks are handled, and found two > new clues: > > - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN, > that is, the controller isn't properly identifying the clock frequency fed > into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI > expert, so it's possible changing frequencies also needs a special operation > to recalculate this variable. But this was making all internal calculations > wrong when paired with this series. > > - With this flag set SDHCI's core now properly calculates divisor values based > on whatever clock frequency I set in DT. And guess what, the issue reappears > even when running on 150MHz. It turns out, as I had some debugging enabled, > the issue only happens when the controller is configured at 100KHz (that > only happens while running the card detect thread). > > So, I can now do this (note that for card detection try to communicate with the > card starting at 400KHz down to 100KHz in 100KHz steps): > > ----->8----- > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index 536c382e2486..e5a5de63f347 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host) > return pltfm_host->clock; > } > > +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,13 +276,15 @@ 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, > }; > > ----->8----- > > Which is rather nicer than what this series introduces. But I can't still > explain why configuring the controller at 100KHz is causing the hangs (while > having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz > from the polling frequency list is going to break support for older SD cards. Thanks for your investigations. Yes, this looks much cleaner to me. > > Regards, > Nicolas > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Nicolas, Sorry, I just noticed this thread. This is a known bug in some newer Arasan cores. The problem happens when the difference between the core clock and the bus clock is too great. Limiting the clock to 200KHz minimum should be a good fix. In my experience, it's only eMMC that needs the clock to be retried below 400KHz and not SD or SDIO. That's because the CMD signal for eMMC starts out as open-drain during identification and the size of the pull-up on the CMD signal can require the <400KHz clock. Once eMMC is out of identification mode the CMD signal is switched to push-pull and can run at much higher clock rates. I don't think that SD and SDIO have any open-drain signals, so they shouldn't need to retry at slower clock speeds. I'm trying to get more detail on the bug, like the exact ratio of core clock to bus clock that causes the problem. When I first found this bug I was told that the failure would not happen at 200KHz, but we were using a 405MHz core clock. One other question. Why are you using polling for the SD card, this newer controller supports the interrupt driven "Card Inserted" signal and avoids wasting time polling? Al On Fri, Mar 26, 2021 at 12:17 PM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote: > > Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne: > > > Hi Stefan, > > > > > > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote: > > > > Hi Nicolas, > > > > > > > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne: > > > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > > > > > > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW) > > > > > seems to interfere with the VPU clock when setup at frequencies bigger > > > > > than 500MHz (a pretty common case). This ends up causing unwarranted > > > > > SDHCI CMD hangs when no SD card is present. > > > > > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > > > > > --- > > > > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > > index 3b4ab947492a..9aa8408d9960 100644 > > > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > > > > > @@ -257,6 +257,12 @@ &emmc2 { > > > > > vqmmc-supply = <&sd_io_1v8_reg>; > > > > > vmmc-supply = <&sd_vcc_reg>; > > > > > broken-cd; > > > > > + /* > > > > > + * Force the frequency to 150MHz as the default 100MHz seems to > > > > > + * interfere with the VPU clock when setup at frequencies bigger than > > > > > + * 500MHz, causing unwarranted CMD hangs. > > > > > + */ > > > > > + clock-frequency = <150000000>; > > > > i don't want to bike-shed here, but is there any chance to solve this in > > > > clk-bcm2835 in a less hacky way? > > > What do you have in mind? > > Sorry, nothing specific. > > > > > > All I can think of is adding some kind of heuristic to the clock's prepare() > > > callback. That said, I don't feel it would be a better solution than this. > > > > Based on my limited knowledge and an old SD card specification, all > > possibly connected devices could have different frequencies. So my > > concern here is, that in case we limit the frequency to a specific value > > we could break things just to suppress a warning. > > SDHCI should be able to handle up to 233MHz IIRC, and there are divisors > available, it depends on the implementation but the worst kind provide /2^n. > Not perfect, but good enough for things to work. > > Now, I've been having a deeper look into how clocks are handled, and found two > new clues: > > - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN, > that is, the controller isn't properly identifying the clock frequency fed > into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI > expert, so it's possible changing frequencies also needs a special operation > to recalculate this variable. But this was making all internal calculations > wrong when paired with this series. > > - With this flag set SDHCI's core now properly calculates divisor values based > on whatever clock frequency I set in DT. And guess what, the issue reappears > even when running on 150MHz. It turns out, as I had some debugging enabled, > the issue only happens when the controller is configured at 100KHz (that > only happens while running the card detect thread). > > So, I can now do this (note that for card detection try to communicate with the > card starting at 400KHz down to 100KHz in 100KHz steps): > > ----->8----- > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index 536c382e2486..e5a5de63f347 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host) > return pltfm_host->clock; > } > > +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,13 +276,15 @@ 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, > }; > > ----->8----- > > Which is rather nicer than what this series introduces. But I can't still > explain why configuring the controller at 100KHz is causing the hangs (while > having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz > from the polling frequency list is going to break support for older SD cards. > > Regards, > Nicolas > >
Hi Alan, On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote: > Nicolas, > > Sorry, I just noticed this thread. > This is a known bug in some newer Arasan cores. > The problem happens when the difference between the core clock and the bus > clock is too great. > Limiting the clock to 200KHz minimum should be a good fix. Great, that's what I was hoping to hear :). Out of curiosity, can you share more details on how the failure occurs? > In my experience, it's only eMMC that needs the clock to be retried > below 400KHz and not SD or SDIO. That's because the CMD signal for > eMMC starts out as open-drain during identification and the size of > the pull-up on the CMD signal can require the <400KHz clock. Once eMMC > is out of identification mode the CMD signal is switched to push-pull > and can run at much higher clock rates. Fair enough, I need to do some tests, some of the compute modules use an eMMC. > I don't think that SD and SDIO have any open-drain signals, so they > shouldn't need to retry at slower clock speeds. Noted. > I'm trying to get more detail on the bug, like the exact ratio of core > clock to bus clock that causes the problem. When I first found this > bug I was told that the failure would not happen at 200KHz, but we > were using a 405MHz core clock. That would be nice to have. > One other question. Why are you using polling for the SD card, this > newer controller supports the interrupt driven "Card Inserted" signal > and avoids wasting time polling? I believe the line isn't routed on RPi4. > Al
Nicolas, I got a better description of the failure and it looks like the bus clock needs to be limited to 300KHz for a 500MHz core clock. What's happening is that an internal reset sequence is needed after a command timeout and the reset signal needs to be asserted for at least 2 ticks of the bus clock. This is done using a 12 bit counter clocked by the core clock. That means a 500MHz core clock produces a 122KHz reset signal which is too fast for 2 ticks of the 200KHz bus clock (100KHz) but is okay for the 300KHz (150Khz) bus clock. Al On Mon, Apr 5, 2021 at 4:45 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Alan, > > On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote: > > Nicolas, > > > > Sorry, I just noticed this thread. > > This is a known bug in some newer Arasan cores. > > The problem happens when the difference between the core clock and the bus > > clock is too great. > > Limiting the clock to 200KHz minimum should be a good fix. > > Great, that's what I was hoping to hear :). Out of curiosity, can you share > more details on how the failure occurs? > > > In my experience, it's only eMMC that needs the clock to be retried > > > below 400KHz and not SD or SDIO. That's because the CMD signal for > > eMMC starts out as open-drain during identification and the size of > > the pull-up on the CMD signal can require the <400KHz clock. Once eMMC > > is out of identification mode the CMD signal is switched to push-pull > > and can run at much higher clock rates. > > Fair enough, I need to do some tests, some of the compute modules use an eMMC. > > > I don't think that SD and SDIO have any open-drain signals, so they > > shouldn't need to retry at slower clock speeds. > > Noted. > > > I'm trying to get more detail on the bug, like the exact ratio of core > > clock to bus clock that causes the problem. When I first found this > > bug I was told that the failure would not happen at 200KHz, but we > > were using a 405MHz core clock. > > That would be nice to have. > > > One other question. Why are you using polling for the SD card, this > > newer controller supports the interrupt driven "Card Inserted" signal > > and avoids wasting time polling? > > I believe the line isn't routed on RPi4. > > > Al >
Hi Al, On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote: > Nicolas, > > I got a better description of the failure and it looks like the bus > clock needs to be limited to 300KHz for a 500MHz core clock. > What's happening is that an internal reset sequence is needed after a > command timeout and the reset signal needs to be asserted for at least > 2 ticks of the bus clock. This is done using a 12 bit counter clocked > by the core clock. That means a 500MHz core clock produces a 122KHz > reset signal which is too fast for 2 ticks of the 200KHz bus clock > (100KHz) but is okay for the 300KHz (150Khz) bus clock. Thanks for the info. I'll work something out. Regards, Nicolas
Hi again, On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote: > Nicolas, > > I got a better description of the failure and it looks like the bus > clock needs to be limited to 300KHz for a 500MHz core clock. > What's happening is that an internal reset sequence is needed after a > command timeout and the reset signal needs to be asserted for at least > 2 ticks of the bus clock. This is done using a 12 bit counter clocked > by the core clock. That means a 500MHz core clock produces a 122KHz > reset signal which is too fast for 2 ticks of the 200KHz bus clock > (100KHz) but is okay for the 300KHz (150Khz) bus clock. Is there any value in implementing this in a generic way? That is, will a SoC other than BCM2711 ever need this? Otherwise I can simply limit BCM2711's sdhci-iproc min clk frequency to 300KHz and call it a day. The alternative is something the likes of: min_clk = clk_get_rate(core_bus) >> 11; But it involves updating the bindings and DT. Regards, Nicolas
Hi Nicolas, Am 09.04.21 um 12:54 schrieb Nicolas Saenz Julienne: > Hi again, > > On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote: >> Nicolas, >> >> I got a better description of the failure and it looks like the bus >> clock needs to be limited to 300KHz for a 500MHz core clock. >> What's happening is that an internal reset sequence is needed after a >> command timeout and the reset signal needs to be asserted for at least >> 2 ticks of the bus clock. This is done using a 12 bit counter clocked >> by the core clock. That means a 500MHz core clock produces a 122KHz >> reset signal which is too fast for 2 ticks of the 200KHz bus clock >> (100KHz) but is okay for the 300KHz (150Khz) bus clock. > Is there any value in implementing this in a generic way? i don't have any idea which callback could manipulate the reset duration. Limiting the min clk frequency looks like the less invasive solution to me. Touching the DT isn't recommend. Best regards
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts index 3b4ab947492a..9aa8408d9960 100644 --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts @@ -257,6 +257,12 @@ &emmc2 { vqmmc-supply = <&sd_io_1v8_reg>; vmmc-supply = <&sd_vcc_reg>; broken-cd; + /* + * Force the frequency to 150MHz as the default 100MHz seems to + * interfere with the VPU clock when setup at frequencies bigger than + * 500MHz, causing unwarranted CMD hangs. + */ + clock-frequency = <150000000>; status = "okay"; };