Re: [PATCH 06/19] mmc: cb710: Inform the mmc core about the maximum busy timeout

[Ulf Hansson <ulf.hansson@linaro.org>]

On Wed, 15 Apr 2020 at 09:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 14 Apr 2020 at 20:40, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >
> > On Tue, Apr 14, 2020 at 06:14:00PM +0200, Ulf Hansson wrote:
> > > Some commands uses R1B responses, which means the card may assert the DAT0
> > > line to signal busy for a period of time, after it has received the
> > > command. The mmc core normally specifies the busy period for the command in
> > > the cmd->busy_timeout. Ideally the driver should respect it, but that
> > > requires quite some update of the code, so let's defer that to someone with
> > > the HW at hand.
> > >
> > > Instead, let's inform the mmc core about the maximum supported busy timeout
> > > in ->max_busy_timeout during ->probe(). This value corresponds to the fixed
> > > ~2s timeout of the polling loop, implemented in cb710_wait_for_event(). In
> > > this way, we let the mmc core validate the needed timeout, which may lead
> > > to that it converts from a R1B into a R1 response and then use CMD13 to
> > > poll for busy completion.
> > >
> > > In other words, this change enables support for commands with longer busy
> > > periods than 2s, like erase (CMD38) for example.
> > [...]
> > > +     /*
> > > +      * In cb710_wait_for_event() we use a fixed timeout of ~2s, hence let's
> > > +      * inform the core about it. A future improvement should instead make
> > > +      * use of the cmd->busy_timeout.
> > > +      */
> > [...]
> >
> > I'm not sure whether the controller limits busy signalling time.
> > Since this is rare HW now, I would just pass the timeout to
> > cb710_wait_for_event() and see if someone reports a bug.
>
> Alright, let me try something like that. Thanks for your suggestion.

I looked a bit more closely at your suggestion, but unfortunately it
requires quite some changes to the code.

To pass a timeout and need to move cb710_wait_for_event() from using a
fixed number of polling loops into using "timeout_ms" value instead.
Although, rather than open coding yet another new polling loop and
perhaps introduce new errors, I prefer moving to readx_poll_timeout().
However, to do that, even more changes are needed around
cb710_check_event().

Once that is done, we also have cb710_wait_while_busy(), that uses a
similar polling loop as cb710_wait_for_event(). So then it seems
reasonable to convert this one too.

To conclude, for now, I am going to apply $subject patch as is. Then
we can always do the above conversion later on, if we see that there
is a need for it.

Kind regards
Uffe