Hi Ulf, slowly coming back to this one. > > + card->state &= ~MMC_STATE_PRESENT; // TBD: mmc_card_clear_present() > > This is nice from a consistency point of view. Although, I don't want > us to use this bit as an indication to inform the bus_ops->suspend() > callback what to do. It seems prone to problems. Makes sense. We can use another bit like MMC_STATE_NEEDS_SUSPEND. That would also... > > + host->bus_ops->suspend(host); > > I think this is a step in the right direction. Somehow we need to be > able to call the bus_ops->suspend() before we call put_device() and > before we clear the host->card pointer. > > However, we don't want to call bus_ops->suspend() in all cases from > mmc_remove_card(), but *only* when it gets called from > mmc_remove_host()->mmc_stop_host(), via the > "host->bus_ops->remove(host)" callback. > > I am wondering whether we should just re-work/split-up the code a bit > to make this work. In principle, when mmc_remove_card() is called from > the path above, we should not let it call put_device(), but leave that > part to the caller (mmc_stop_host()) instead. Or something along those > lines. ... avoid this :) Refactoring every code which calls mmc_remove_card() to handle the additional put_device() on its own seems intrusive to me. And error prone, new users might forget to do it. And that for only one use case where we want to do extra stuff. Leaving out put_device() within mmc_remove_card() only for that one case is bad API design, of course. So, I envision more something along this: if (card->state & MMC_STATE_NEEDS_SUSPEND) mmc_suspend() Maybe even more generic? if (card->state & MMC_STATE_PREPARE_REMOVE) bus_ops->prepare_remove() Or am I missing something from your suggestion? > > + enum mmc_pm_reason reason = mmc_card_present(host->card) ? > > + MMC_PM_REASON_SUSPEND : MMC_PM_REASON_REMOVAL; > > I don't think it makes sense to differentiate between a regular > "suspend" and a "host-removal". The point is, we don't know what will > happen beyond the host-removal. The platform may shutdown or the host > driver probes again. > > Let's just use the same commands as we use for suspend. Sadly, I think it makes sense because of our HW. We cannot control the regulators directly. PSCI handles them. And for shutdown, it will do "something". For normal suspend, nothing will happen because PSCI will not be called. This is why Shimoda-san introduced "full-pwr-cycle-in-suspend" back then. The differentiation is needed a little above: > - ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend || > - (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND))) > + (can_pwr_cycle_now || reason == MMC_PM_REASON_SHUTDOWN)) > err = mmc_poweroff_notify(host->card, notify_type); Here. Poweroff notification is only valid in the POWEROFF case for us. Thanks for your time, Wolfram
Attachment:
signature.asc
Description: PGP signature
