Hello Ulf,
> From: Ulf Hansson, Sent: Tuesday, November 17, 2020 1:47 AM
>
> On Tue, 10 Nov 2020 at 11:48, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >
> > User is possible to turn the power off after a host was removed.
> > So, call mmc_poweroff_notify(EXT_CSD_NO_POWER_NOTIFICATION)
> > in mmc_remove(). Note that the mmc and host driver will be
> > in the following modes when mmc_remove() is called:
> >
> > 1. mmc_card_suspended() == false &&
> > power_off_notification == EXT_CSD_POWER_ON
> > 2. mmc_card_suspended() == true &&
> > power_off_notification == EXT_CSD_POWER_OFF_{SHORT,LONG}
> > 3. mmc_card_suspended() == true && mmc_sleep() was called
> >
> > So, mmc_remove() calls _mmc_resume() anyway for the cases.
> > Otherwise:
> >
> > - _mmc_resume will be called via mmc_runtime_resume() and then
> > power_off_notification will be set to EXT_CSD_POWER_ON.
> > - timeout will happen in mmc_blk_part_switch() via mmc_blk_remove()
> > if "part_curr" of mmc block is not set to default.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
<snip>
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index ff3063c..18413f2 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1983,11 +1983,35 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
> > return err;
> > }
> >
> > +static int _mmc_resume(struct mmc_host *host);
> > /*
> > * Host is being removed. Free up the current card.
> > */
> > static void mmc_remove(struct mmc_host *host)
> > {
> > + /*
> > + * The mmc and host driver will be in the following modes here:
> > + * 1. mmc_card_suspended() == false &&
> > + * power_off_notification == EXT_CSD_POWER_ON
> > + * 2. mmc_card_suspended() == true &&
> > + * power_off_notification == EXT_CSD_POWER_OFF_{SHORT,LONG}
> > + * 3. mmc_card_suspended() == true && mmc_sleep() was called
> > + *
> > + * So, call _mmc_resume() here anyway for the cases. Otherwise:
> > + * - _mmc_resume will be called via mmc_runtime_resume() and then
> > + * power_off_notification will be set to EXT_CSD_POWER_ON.
> > + * - timeout will happen in mmc_blk_part_switch() via mmc_blk_remove()
> > + * if "part_curr" of mmc block is not set to default.
> > + */
> > + _mmc_resume(host);
> > +
> > + /* Disable power_off_notification byte in the ext_csd register */
> > + if (host->card->ext_csd.rev >= 6) {
> > + mmc_claim_host(host);
> > + mmc_poweroff_notify(host->card, EXT_CSD_NO_POWER_NOTIFICATION);
> > + mmc_release_host(host);
> > + }
>
> Unfortunate, I think there is even more complexity involved here. I
> don't think the above work for all cases.
>
> Let me try to elaborate - there are two scenarios of when mmc_remove()
> is called.
>
> 1)
> When the card becomes removed, likely not the case for eMMC but it may
> happen for a legacy MMC card, for example. In this case, there is not
> much we can do to fix the problem, as the card is already "dead".
>
> 2)
> The card is working properly (it may be suspended though) while
> mmc_remove_host() gets called because the host driver is being
> unbinded.
Thank you for your review and comments! I understood my patch was not
enough for these scenarios.
> For 1)
> We should only clean up and remove the card structs, which the current
> code already does.
>
> For 2)
> We want to support a graceful power off sequence or the card (to
> prevent data corruption for example). However, depending on the
> platform and host, it may not be possible to power off both VCC and
> VCCQ. For example, it's quite common that VCCQ remains powered on,
> while only VCC can be power gated. Just disabling the power off
> notification of the eMMC card (as you suggest above), doesn't really
> help. In fact, it could mean that we may violate the eMMC spec when
> power gating VCC through mmc_power_off().
>
> I am thinking of a few possible solutions. Perhaps easier if I post a
> patch that you try - unless you have ideas yourself of how to fix
> this.
Thank you! Unfortunately, I don't have any idea how to fix this now.
So, if you post a patch, I'll try.
Best regards,
Yoshihiro Shimoda
