Re: [PATCH] mmc: host: dw-mmc-rockchip: avoid logspam when cd-broken

[Ulf Hansson]

On Tue, 1 Mar 2022 at 15:49, Peter Geis <pgwipeout@xxxxxxxxx> wrote:
>
> On Tue, Mar 1, 2022 at 7:46 AM Peter Geis <pgwipeout@xxxxxxxxx> wrote:
> >
> > On Tue, Mar 1, 2022 at 7:38 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
> > >
> > > On 2022-03-01 11:49, Peter Geis wrote:
> > > > On Tue, Mar 1, 2022 at 6:23 AM Robin Murphy <robin.murphy@xxxxxxx> 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