Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

[Saravana Kannan]

I'm trying to understand the MMC suspend path a bit.

I looked at the commit message of 6fa79651cc808f68db6f6f297be5a950ccd5dffb.

IIUC, if MMC_CAP_WAIT_WHILE_BUSY is set then the mmc framework is
going to depend on the card_busy() op to ensure correctness instead of
using the S_A_TIMEOUT value from the card.

But I see a lot of mmc host drivers that implement card_busy() but
don't set the MMC_CAP_WAIT_WHILE_BUSY flag. That doesn't seem right to
me if my understanding is correct.

If it's supposed to be "we'll use card_busy() if
MMC_CAP_WAIT_WHILE_BUSY isn't set", then why do we have some mmc host
drivers that have both?

What am I misunderstanding?

-Saravana

Re: Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

[Ulf Hansson]

On Wed, 26 Jan 2022 at 04:46, Saravana Kannan <saravanak@google.com> wrote:
>
> I'm trying to understand the MMC suspend path a bit.
>
> I looked at the commit message of 6fa79651cc808f68db6f6f297be5a950ccd5dffb.
>
> IIUC, if MMC_CAP_WAIT_WHILE_BUSY is set then the mmc framework is
> going to depend on the card_busy() op to ensure correctness instead of
> using the S_A_TIMEOUT value from the card.

MMC_CAP_WAIT_WHILE_BUSY indicates whether the mmc controller supports
IRQ based busy detection completion. In other words, the mmc host
driver can receive an IRQ when busy signaling is completed on DAT0 by
the eMMC card.

However, to avoid waiting for the IRQ forever, there is a maximum
timeout that is specified by the mmc core, for the particular command
in question. For eMMC sleep, the S_A_TIMEOUT.

>
> But I see a lot of mmc host drivers that implement card_busy() but
> don't set the MMC_CAP_WAIT_WHILE_BUSY flag. That doesn't seem right to
> me if my understanding is correct.

That's perfectly okay. MMC_CAP_WAIT_WHILE_BUSY is IRQ based, while the
->card_busy() ops is used to poll for busy completion.

>
> If it's supposed to be "we'll use card_busy() if
> MMC_CAP_WAIT_WHILE_BUSY isn't set", then why do we have some mmc host
> drivers that have both?
>
> What am I misunderstanding?

There are some additional complexity for the corresponding code. This
has mostly ended up there because we also need to deal with mmc
controller's HW limitations around this feature.

For example, some mmc controllers have a HW limit for the length of
the timeout that can be set. If the needed timeout is longer than what
can be supported, we can't use IRQ based busy completion.

Did this make it more clear?

>
> -Saravana

Kind regards
Uffe

Re: Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

[Saravana Kannan]

On Mon, Jan 31, 2022 at 7:46 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Wed, 26 Jan 2022 at 04:46, Saravana Kannan <saravanak@google.com> wrote:
> >
> > I'm trying to understand the MMC suspend path a bit.
> >
> > I looked at the commit message of 6fa79651cc808f68db6f6f297be5a950ccd5dffb.
> >
> > IIUC, if MMC_CAP_WAIT_WHILE_BUSY is set then the mmc framework is
> > going to depend on the card_busy() op to ensure correctness instead of
> > using the S_A_TIMEOUT value from the card.
>
> MMC_CAP_WAIT_WHILE_BUSY indicates whether the mmc controller supports
> IRQ based busy detection completion. In other words, the mmc host
> driver can receive an IRQ when busy signaling is completed on DAT0 by
> the eMMC card.
>
> However, to avoid waiting for the IRQ forever, there is a maximum
> timeout that is specified by the mmc core, for the particular command
> in question. For eMMC sleep, the S_A_TIMEOUT.

Ah ok, thanks for the explanation.

>
> >
> > But I see a lot of mmc host drivers that implement card_busy() but
> > don't set the MMC_CAP_WAIT_WHILE_BUSY flag. That doesn't seem right to
> > me if my understanding is correct.
>
> That's perfectly okay. MMC_CAP_WAIT_WHILE_BUSY is IRQ based, while the
> ->card_busy() ops is used to poll for busy completion.

Yeah, it makes sense now.

One thing I noticed when playing with some hardware is that during
suspend, when MMC_CAP_WAIT_WHILE_BUSY isn't set and we have a
card_busy() implementation, we don't seem to be using card_busy() op
and just always using the timeout from S_A_TIMEOUT. To be more
specific, I'm talking about this code path:
_mmc_suspend() -> mmc_sleep() -> mmc_delay() -> msleep()

I'd think card_busy() could be used here if it's implemented. Is there
a reason for not using it in this path?

> >
> > If it's supposed to be "we'll use card_busy() if
> > MMC_CAP_WAIT_WHILE_BUSY isn't set", then why do we have some mmc host
> > drivers that have both?
> >
> > What am I misunderstanding?
>
> There are some additional complexity for the corresponding code. This
> has mostly ended up there because we also need to deal with mmc
> controller's HW limitations around this feature.
>
> For example, some mmc controllers have a HW limit for the length of
> the timeout that can be set. If the needed timeout is longer than what
> can be supported, we can't use IRQ based busy completion.
>
> Did this make it more clear?

Yes, it does. Much appreciated!

Thanks,
Saravana

Re: Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

[Ulf Hansson]

On Mon, 31 Jan 2022 at 21:15, Saravana Kannan <saravanak@google.com> wrote:
>
> On Mon, Jan 31, 2022 at 7:46 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Wed, 26 Jan 2022 at 04:46, Saravana Kannan <saravanak@google.com> wrote:
> > >
> > > I'm trying to understand the MMC suspend path a bit.
> > >
> > > I looked at the commit message of 6fa79651cc808f68db6f6f297be5a950ccd5dffb.
> > >
> > > IIUC, if MMC_CAP_WAIT_WHILE_BUSY is set then the mmc framework is
> > > going to depend on the card_busy() op to ensure correctness instead of
> > > using the S_A_TIMEOUT value from the card.
> >
> > MMC_CAP_WAIT_WHILE_BUSY indicates whether the mmc controller supports
> > IRQ based busy detection completion. In other words, the mmc host
> > driver can receive an IRQ when busy signaling is completed on DAT0 by
> > the eMMC card.
> >
> > However, to avoid waiting for the IRQ forever, there is a maximum
> > timeout that is specified by the mmc core, for the particular command
> > in question. For eMMC sleep, the S_A_TIMEOUT.
>
> Ah ok, thanks for the explanation.
>
> >
> > >
> > > But I see a lot of mmc host drivers that implement card_busy() but
> > > don't set the MMC_CAP_WAIT_WHILE_BUSY flag. That doesn't seem right to
> > > me if my understanding is correct.
> >
> > That's perfectly okay. MMC_CAP_WAIT_WHILE_BUSY is IRQ based, while the
> > ->card_busy() ops is used to poll for busy completion.
>
> Yeah, it makes sense now.
>
> One thing I noticed when playing with some hardware is that during
> suspend, when MMC_CAP_WAIT_WHILE_BUSY isn't set and we have a
> card_busy() implementation, we don't seem to be using card_busy() op
> and just always using the timeout from S_A_TIMEOUT. To be more
> specific, I'm talking about this code path:
> _mmc_suspend() -> mmc_sleep() -> mmc_delay() -> msleep()
>
> I'd think card_busy() could be used here if it's implemented. Is there
> a reason for not using it in this path?

That was exactly what commit 6fa79651cc80 ("mmc: core: Enable eMMC
sleep commands to use HW busy polling") implemented. The commit was
introduced in v5.14.

If it doesn't work, there is a bug somewhere.

[...]

Kind regards
Uffe

Re: Relation between MMC_CAP_WAIT_WHILE_BUSY and card_busy()

[Saravana Kannan]

On Tue, Feb 1, 2022 at 12:24 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 31 Jan 2022 at 21:15, Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Mon, Jan 31, 2022 at 7:46 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > On Wed, 26 Jan 2022 at 04:46, Saravana Kannan <saravanak@google.com> wrote:
> > > >
> > > > I'm trying to understand the MMC suspend path a bit.
> > > >
> > > > I looked at the commit message of 6fa79651cc808f68db6f6f297be5a950ccd5dffb.
> > > >
> > > > IIUC, if MMC_CAP_WAIT_WHILE_BUSY is set then the mmc framework is
> > > > going to depend on the card_busy() op to ensure correctness instead of
> > > > using the S_A_TIMEOUT value from the card.
> > >
> > > MMC_CAP_WAIT_WHILE_BUSY indicates whether the mmc controller supports
> > > IRQ based busy detection completion. In other words, the mmc host
> > > driver can receive an IRQ when busy signaling is completed on DAT0 by
> > > the eMMC card.
> > >
> > > However, to avoid waiting for the IRQ forever, there is a maximum
> > > timeout that is specified by the mmc core, for the particular command
> > > in question. For eMMC sleep, the S_A_TIMEOUT.
> >
> > Ah ok, thanks for the explanation.
> >
> > >
> > > >
> > > > But I see a lot of mmc host drivers that implement card_busy() but
> > > > don't set the MMC_CAP_WAIT_WHILE_BUSY flag. That doesn't seem right to
> > > > me if my understanding is correct.
> > >
> > > That's perfectly okay. MMC_CAP_WAIT_WHILE_BUSY is IRQ based, while the
> > > ->card_busy() ops is used to poll for busy completion.
> >
> > Yeah, it makes sense now.
> >
> > One thing I noticed when playing with some hardware is that during
> > suspend, when MMC_CAP_WAIT_WHILE_BUSY isn't set and we have a
> > card_busy() implementation, we don't seem to be using card_busy() op
> > and just always using the timeout from S_A_TIMEOUT. To be more
> > specific, I'm talking about this code path:
> > _mmc_suspend() -> mmc_sleep() -> mmc_delay() -> msleep()
> >
> > I'd think card_busy() could be used here if it's implemented. Is there
> > a reason for not using it in this path?
>
> That was exactly what commit 6fa79651cc80 ("mmc: core: Enable eMMC
> sleep commands to use HW busy polling") implemented. The commit was
> introduced in v5.14.
>
> If it doesn't work, there is a bug somewhere.

Ah I was checking an older kernel and meant to check the latest
mainline before I sent this email, but then forgot to do it. Thanks
for the pointer! Let me backport and see if it helps.

-Saravana