The dw_mmc-rockchip driver drops a large amound of logspam constantly
when the cd-broken flag is enabled.
Set the warning to be debug ratelimited in this case.
Signed-off-by: Peter Geis <[email protected]>
---
drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 95d0ec0f5f3a..d0ebf0afa42a 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
cclkin = ios->clock * RK3288_CLKGEN_DIV;
ret = clk_set_rate(host->ciu_clk, cclkin);
- if (ret)
- dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+ if (ret) {
+ /* this screams when card detection is broken */
+ if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
+ dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
+ else
+ dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+ }
bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
if (bus_hz != host->bus_hz) {
--
2.25.1
On 2022-02-28 22:36, Peter Geis wrote:
> The dw_mmc-rockchip driver drops a large amound of logspam constantly
> when the cd-broken flag is enabled.
> Set the warning to be debug ratelimited in this case.
Isn't this just papering over some fundamental problem with the clock?
If it's failing to set the expected rate for communicating with a card,
then presumably that's an issue for correct operation in general? The
fact that polling for a card makes a lot more of that communication
happen seems unrelated :/
Robin.
> Signed-off-by: Peter Geis <[email protected]>
> ---
> drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index 95d0ec0f5f3a..d0ebf0afa42a 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> cclkin = ios->clock * RK3288_CLKGEN_DIV;
>
> ret = clk_set_rate(host->ciu_clk, cclkin);
> - if (ret)
> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> + if (ret) {
> + /* this screams when card detection is broken */
> + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> + else
> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> + }
>
> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> if (bus_hz != host->bus_hz) {
On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
>
> On 2022-02-28 22:36, Peter Geis wrote:
> > The dw_mmc-rockchip driver drops a large amound of logspam constantly
> > when the cd-broken flag is enabled.
> > Set the warning to be debug ratelimited in this case.
>
> Isn't this just papering over some fundamental problem with the clock?
> If it's failing to set the expected rate for communicating with a card,
> then presumably that's an issue for correct operation in general? The
> fact that polling for a card makes a lot more of that communication
> happen seems unrelated :/
Good Morning,
This only happens when a card is not inserted, so communication cannot happen.
I found it while lighting off the SoQuartz module.
As it is pin compatible with the RPi CM4, and the CM4 does not have a
card detect line, sdmmc is non functional without cd-broken.
This led to the fun spew when there wasn't a card inserted as this
function is called every poll tick.
Thanks,
Peter
>
> Robin.
>
> > Signed-off-by: Peter Geis <[email protected]>
> > ---
> > drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> > index 95d0ec0f5f3a..d0ebf0afa42a 100644
> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> > cclkin = ios->clock * RK3288_CLKGEN_DIV;
> >
> > ret = clk_set_rate(host->ciu_clk, cclkin);
> > - if (ret)
> > - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + if (ret) {
> > + /* this screams when card detection is broken */
> > + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> > + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + else
> > + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + }
> >
> > bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> > if (bus_hz != host->bus_hz) {
On Tue, Mar 1, 2022 at 7:46 AM Peter Geis <[email protected]> wrote:
>
> On Tue, Mar 1, 2022 at 7:38 AM Robin Murphy <[email protected]> wrote:
> >
> > On 2022-03-01 11:49, Peter Geis wrote:
> > > On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
> > >>
> > >> On 2022-02-28 22:36, Peter Geis wrote:
> > >>> The dw_mmc-rockchip driver drops a large amound of logspam constantly
> > >>> when the cd-broken flag is enabled.
> > >>> Set the warning to be debug ratelimited in this case.
> > >>
> > >> Isn't this just papering over some fundamental problem with the clock?
> > >> If it's failing to set the expected rate for communicating with a card,
> > >> then presumably that's an issue for correct operation in general? The
> > >> fact that polling for a card makes a lot more of that communication
> > >> happen seems unrelated :/
> > >
> > > Good Morning,
> > >
> > > This only happens when a card is not inserted, so communication cannot happen.
> >
> > Well, I suppose there's a philosophical question in there about whether
> > shouting into the void counts as "communication", but AFAIR what the
> > polling function does is power up the controller, send a command, and
> > see if it gets a response.
> >
> > If the clock can't be set to the proper rate for low-speed discovery,
> > some or all cards may not be detected properly. Conversely if it is
> > already at a slow enough rate for discovery but can't be set higher once
> > a proper communication mode has been established, data transfer
> > performance will be terrible. Either way, it is not OK in general for
> > clk_set_rate() to fail, hence the warning. You have a clock driver problem.
>
> Alright, I'll look into this.
> It seems only extremely low clock speeds fail and I know rockchip
> chips have a hard time with extremely low clock rates.
> I'll trace out where the failure is happening.
Okay, I hope you can provide me a direction to go from here, because
it looks like it's doing exactly what it should do in this situation.
mmc core is requesting a rate (200k/100k).
clk core tries to find a parent to provide a clock that low and fails,
because the lowest possible parent is 750k.
clk_sdmmc(x) is listed as no-div, so it can't go any lower.
It seems to me that this error is sane, because other results of
einval you want to catch.
But einval in this case is fine, because
The thing that strikes me weird is currently clk_core thinks the
lowest possible freq here is 0, when in actuality it should be 750k,
am I correct here?
The mmc controller has an internal divider, so if my line of thinking
is correct here we should be more flexible here and request a rate
that's acceptable rather than just failing if it doesn't work.
But that's based on my limited understanding of how mmc core is
requesting this and what it expects in return.
>
> Thanks!
>
> >
> > Cheers,
> > Robin.
> >
> > > I found it while lighting off the SoQuartz module.
> > > As it is pin compatible with the RPi CM4, and the CM4 does not have a
> > > card detect line, sdmmc is non functional without cd-broken.
> > > This led to the fun spew when there wasn't a card inserted as this
> > > function is called every poll tick.
> > >
> > > Thanks,
> > > Peter
> > >
> > >>
> > >> Robin.
> > >>
> > >>> Signed-off-by: Peter Geis <[email protected]>
> > >>> ---
> > >>> drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> > >>> 1 file changed, 7 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> > >>> index 95d0ec0f5f3a..d0ebf0afa42a 100644
> > >>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > >>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > >>> @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> > >>> cclkin = ios->clock * RK3288_CLKGEN_DIV;
> > >>>
> > >>> ret = clk_set_rate(host->ciu_clk, cclkin);
> > >>> - if (ret)
> > >>> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > >>> + if (ret) {
> > >>> + /* this screams when card detection is broken */
> > >>> + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> > >>> + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> > >>> + else
> > >>> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > >>> + }
> > >>>
> > >>> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> > >>> if (bus_hz != host->bus_hz) {
On 2022-03-01 11:49, Peter Geis wrote:
> On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
>>
>> On 2022-02-28 22:36, Peter Geis wrote:
>>> The dw_mmc-rockchip driver drops a large amound of logspam constantly
>>> when the cd-broken flag is enabled.
>>> Set the warning to be debug ratelimited in this case.
>>
>> Isn't this just papering over some fundamental problem with the clock?
>> If it's failing to set the expected rate for communicating with a card,
>> then presumably that's an issue for correct operation in general? The
>> fact that polling for a card makes a lot more of that communication
>> happen seems unrelated :/
>
> Good Morning,
>
> This only happens when a card is not inserted, so communication cannot happen.
Well, I suppose there's a philosophical question in there about whether
shouting into the void counts as "communication", but AFAIR what the
polling function does is power up the controller, send a command, and
see if it gets a response.
If the clock can't be set to the proper rate for low-speed discovery,
some or all cards may not be detected properly. Conversely if it is
already at a slow enough rate for discovery but can't be set higher once
a proper communication mode has been established, data transfer
performance will be terrible. Either way, it is not OK in general for
clk_set_rate() to fail, hence the warning. You have a clock driver problem.
Cheers,
Robin.
> I found it while lighting off the SoQuartz module.
> As it is pin compatible with the RPi CM4, and the CM4 does not have a
> card detect line, sdmmc is non functional without cd-broken.
> This led to the fun spew when there wasn't a card inserted as this
> function is called every poll tick.
>
> Thanks,
> Peter
>
>>
>> Robin.
>>
>>> Signed-off-by: Peter Geis <[email protected]>
>>> ---
>>> drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>>> index 95d0ec0f5f3a..d0ebf0afa42a 100644
>>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>>> @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> cclkin = ios->clock * RK3288_CLKGEN_DIV;
>>>
>>> ret = clk_set_rate(host->ciu_clk, cclkin);
>>> - if (ret)
>>> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
>>> + if (ret) {
>>> + /* this screams when card detection is broken */
>>> + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
>>> + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
>>> + else
>>> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
>>> + }
>>>
>>> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
>>> if (bus_hz != host->bus_hz) {
On 2022-03-01 14:49, Peter Geis wrote:
> On Tue, Mar 1, 2022 at 7:46 AM Peter Geis <[email protected]> wrote:
>>
>> On Tue, Mar 1, 2022 at 7:38 AM Robin Murphy <[email protected]> wrote:
>>>
>>> On 2022-03-01 11:49, Peter Geis wrote:
>>>> On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
>>>>>
>>>>> On 2022-02-28 22:36, Peter Geis wrote:
>>>>>> The dw_mmc-rockchip driver drops a large amound of logspam constantly
>>>>>> when the cd-broken flag is enabled.
>>>>>> Set the warning to be debug ratelimited in this case.
>>>>>
>>>>> Isn't this just papering over some fundamental problem with the clock?
>>>>> If it's failing to set the expected rate for communicating with a card,
>>>>> then presumably that's an issue for correct operation in general? The
>>>>> fact that polling for a card makes a lot more of that communication
>>>>> happen seems unrelated :/
>>>>
>>>> Good Morning,
>>>>
>>>> This only happens when a card is not inserted, so communication cannot happen.
>>>
>>> Well, I suppose there's a philosophical question in there about whether
>>> shouting into the void counts as "communication", but AFAIR what the
>>> polling function does is power up the controller, send a command, and
>>> see if it gets a response.
>>>
>>> If the clock can't be set to the proper rate for low-speed discovery,
>>> some or all cards may not be detected properly. Conversely if it is
>>> already at a slow enough rate for discovery but can't be set higher once
>>> a proper communication mode has been established, data transfer
>>> performance will be terrible. Either way, it is not OK in general for
>>> clk_set_rate() to fail, hence the warning. You have a clock driver problem.
>>
>> Alright, I'll look into this.
>> It seems only extremely low clock speeds fail and I know rockchip
>> chips have a hard time with extremely low clock rates.
>> I'll trace out where the failure is happening.
>
> Okay, I hope you can provide me a direction to go from here, because
> it looks like it's doing exactly what it should do in this situation.
> mmc core is requesting a rate (200k/100k).
> clk core tries to find a parent to provide a clock that low and fails,
> because the lowest possible parent is 750k.
> clk_sdmmc(x) is listed as no-div, so it can't go any lower.
>
> It seems to me that this error is sane, because other results of
> einval you want to catch.
> But einval in this case is fine, because
> The thing that strikes me weird is currently clk_core thinks the
> lowest possible freq here is 0, when in actuality it should be 750k,
> am I correct here?
> The mmc controller has an internal divider, so if my line of thinking
> is correct here we should be more flexible here and request a rate
> that's acceptable rather than just failing if it doesn't work.
> But that's based on my limited understanding of how mmc core is
> requesting this and what it expects in return.
The downstream solution appears to be just to clamp the rate for
detection[1][2]. Not sure whether it's feasible to try to be cleverer
with the local divider to settle on a more in-spec rate for the final
output :/
Robin.
[1]
https://github.com/JeffyCN/mirrors/commit/d80d5062b22f9c4a559401bdb7b2727c4ced36c0
[2]
https://github.com/JeffyCN/mirrors/commit/3f26edfb2392df25efc361ad0a9f41d0917e40ee
On Tue, Mar 1, 2022 at 7:38 AM Robin Murphy <[email protected]> wrote:
>
> On 2022-03-01 11:49, Peter Geis wrote:
> > On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
> >>
> >> On 2022-02-28 22:36, Peter Geis wrote:
> >>> The dw_mmc-rockchip driver drops a large amound of logspam constantly
> >>> when the cd-broken flag is enabled.
> >>> Set the warning to be debug ratelimited in this case.
> >>
> >> Isn't this just papering over some fundamental problem with the clock?
> >> If it's failing to set the expected rate for communicating with a card,
> >> then presumably that's an issue for correct operation in general? The
> >> fact that polling for a card makes a lot more of that communication
> >> happen seems unrelated :/
> >
> > Good Morning,
> >
> > This only happens when a card is not inserted, so communication cannot happen.
>
> Well, I suppose there's a philosophical question in there about whether
> shouting into the void counts as "communication", but AFAIR what the
> polling function does is power up the controller, send a command, and
> see if it gets a response.
>
> If the clock can't be set to the proper rate for low-speed discovery,
> some or all cards may not be detected properly. Conversely if it is
> already at a slow enough rate for discovery but can't be set higher once
> a proper communication mode has been established, data transfer
> performance will be terrible. Either way, it is not OK in general for
> clk_set_rate() to fail, hence the warning. You have a clock driver problem.
Alright, I'll look into this.
It seems only extremely low clock speeds fail and I know rockchip
chips have a hard time with extremely low clock rates.
I'll trace out where the failure is happening.
Thanks!
>
> Cheers,
> Robin.
>
> > I found it while lighting off the SoQuartz module.
> > As it is pin compatible with the RPi CM4, and the CM4 does not have a
> > card detect line, sdmmc is non functional without cd-broken.
> > This led to the fun spew when there wasn't a card inserted as this
> > function is called every poll tick.
> >
> > Thanks,
> > Peter
> >
> >>
> >> Robin.
> >>
> >>> Signed-off-by: Peter Geis <[email protected]>
> >>> ---
> >>> drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> >>> 1 file changed, 7 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> >>> index 95d0ec0f5f3a..d0ebf0afa42a 100644
> >>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> >>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> >>> @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> >>> cclkin = ios->clock * RK3288_CLKGEN_DIV;
> >>>
> >>> ret = clk_set_rate(host->ciu_clk, cclkin);
> >>> - if (ret)
> >>> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> >>> + if (ret) {
> >>> + /* this screams when card detection is broken */
> >>> + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> >>> + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> >>> + else
> >>> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> >>> + }
> >>>
> >>> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> >>> if (bus_hz != host->bus_hz) {
Hi Peter,
在 2022/3/1 6:36, Peter Geis 写道:
> The dw_mmc-rockchip driver drops a large amound of logspam constantly
> when the cd-broken flag is enabled.
> Set the warning to be debug ratelimited in this case.
>
May I know which platform did you use?
> Signed-off-by: Peter Geis <[email protected]>
> ---
> drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index 95d0ec0f5f3a..d0ebf0afa42a 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> cclkin = ios->clock * RK3288_CLKGEN_DIV;
>
> ret = clk_set_rate(host->ciu_clk, cclkin);
> - if (ret)
> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> + if (ret) {
> + /* this screams when card detection is broken */
> + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> + else
> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> + }
>
> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> if (bus_hz != host->bus_hz) {
On Tue, Mar 1, 2022 at 8:04 PM Shawn Lin <[email protected]> wrote:
>
> Hi Peter,
>
> 在 2022/3/1 6:36, Peter Geis 写道:
> > The dw_mmc-rockchip driver drops a large amound of logspam constantly
> > when the cd-broken flag is enabled.
> > Set the warning to be debug ratelimited in this case.
> >
>
> May I know which platform did you use?
It's rk3566.
>
> > Signed-off-by: Peter Geis <[email protected]>
> > ---
> > drivers/mmc/host/dw_mmc-rockchip.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> > index 95d0ec0f5f3a..d0ebf0afa42a 100644
> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > @@ -50,8 +50,13 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> > cclkin = ios->clock * RK3288_CLKGEN_DIV;
> >
> > ret = clk_set_rate(host->ciu_clk, cclkin);
> > - if (ret)
> > - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + if (ret) {
> > + /* this screams when card detection is broken */
> > + if (host->slot->mmc->caps & MMC_CAP_NEEDS_POLL)
> > + dev_dbg_ratelimited(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + else
> > + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> > + }
> >
> > bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> > if (bus_hz != host->bus_hz) {
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
On Tue, 1 Mar 2022 at 15:49, Peter Geis <[email protected]> wrote:
>
> On Tue, Mar 1, 2022 at 7:46 AM Peter Geis <[email protected]> wrote:
> >
> > On Tue, Mar 1, 2022 at 7:38 AM Robin Murphy <[email protected]> wrote:
> > >
> > > On 2022-03-01 11:49, Peter Geis wrote:
> > > > On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <[email protected]> wrote:
> > > >>
> > > >> On 2022-02-28 22:36, Peter Geis wrote:
> > > >>> The dw_mmc-rockchip driver drops a large amound of logspam constantly
> > > >>> when the cd-broken flag is enabled.
> > > >>> Set the warning to be debug ratelimited in this case.
> > > >>
> > > >> Isn't this just papering over some fundamental problem with the clock?
> > > >> If it's failing to set the expected rate for communicating with a card,
> > > >> then presumably that's an issue for correct operation in general? The
> > > >> fact that polling for a card makes a lot more of that communication
> > > >> happen seems unrelated :/
> > > >
> > > > Good Morning,
> > > >
> > > > This only happens when a card is not inserted, so communication cannot happen.
> > >
> > > Well, I suppose there's a philosophical question in there about whether
> > > shouting into the void counts as "communication", but AFAIR what the
> > > polling function does is power up the controller, send a command, and
> > > see if it gets a response.
> > >
> > > If the clock can't be set to the proper rate for low-speed discovery,
> > > some or all cards may not be detected properly. Conversely if it is
> > > already at a slow enough rate for discovery but can't be set higher once
> > > a proper communication mode has been established, data transfer
> > > performance will be terrible. Either way, it is not OK in general for
> > > clk_set_rate() to fail, hence the warning. You have a clock driver problem.
> >
> > Alright, I'll look into this.
> > It seems only extremely low clock speeds fail and I know rockchip
> > chips have a hard time with extremely low clock rates.
> > I'll trace out where the failure is happening.
>
> Okay, I hope you can provide me a direction to go from here, because
> it looks like it's doing exactly what it should do in this situation.
> mmc core is requesting a rate (200k/100k).
> clk core tries to find a parent to provide a clock that low and fails,
> because the lowest possible parent is 750k.
> clk_sdmmc(x) is listed as no-div, so it can't go any lower.
>
> It seems to me that this error is sane, because other results of
> einval you want to catch.
> But einval in this case is fine, because
> The thing that strikes me weird is currently clk_core thinks the
> lowest possible freq here is 0, when in actuality it should be 750k,
> am I correct here?
> The mmc controller has an internal divider, so if my line of thinking
> is correct here we should be more flexible here and request a rate
> that's acceptable rather than just failing if it doesn't work.
> But that's based on my limited understanding of how mmc core is
> requesting this and what it expects in return.
The important point from the eMMC/SD/SDIO spec point of view, is that
the initialization of the cards starts at a maximum of 400KHz for the
bus interface clock.
In many cases, that requires a combination of the provided source
clock for the mmc controller to be decreased with some kind of an
internal divider managed by the controller itself.
Moreover, the mmc host driver shall set mmc->f_min in its
corresponding struct mmc_host during ->probe(), to inform the core
about its lowest possible rate that can be set - and this must not be
higher than 400KHz.
Hope that clarifies how things should work.
[...]
Kind regards
Uffe