RE: [PATCH v3] mmc: core: Issue power off notification in mmc_remove()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux