Hi, On 06/03/2014 12:14 PM, Ulf Hansson wrote: > On 28 May 2014 11:42, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: <mega snip> >>> If the mmc_of_parse() returns -EPROBE_DEFER, the mmc host driver will >>> return the same error code from it's ->probe(). This provides us with >>> the ability of waiting for the "powerup driver" to be probed. >> >> Ack. Note though that mmc_of_parse will likely not do the probe itself, >> the way I see it it will do a platform_device_register() and let the >> platform bus code do its thing. Downside of this is that >> platform_device_register() will not propagate probe errors such as >> -EPROBE_DEFER, so we need to check afterwards that a driver is actually >> bound, see above. > > Just to confirm your ideas, this is how I see the instantiation of the > device and probe of the "powerup driver" as well. Ok, so given that in another mail thread we've just decided to not use slot subnodes in the devicetree hierarchy, how are we going to represent the powerup-bits in devicetree? I suggest that we represent this with a separate subnode under the mmc host, with its own compatible string. Since reg == 0 is for the card device, and reg 1-7 is for the sdio function devices, I suggest that we use reg = <8> for the powerup subnode. Then the mmc-core can check for such a child subnode, and if it is there instantiate a platform device for it, and then handle the probe as described above. > >> >>> If the mmc_of_parse() returns another error code, due to that the >>> "powerup driver" failed to be probed, the mmc host driver's ->probe() >>> will return the same error code and consequentially no power up of the >>> card will be performed at all. >> >> Ack. >> >>> Powerup driver's ->probe(): >>> Typically the "powerup driver" will need to register a few callback >>> functions towards the mmc core. Typically at mmc_of_parse(), those >>> callbacks will have to be connected to a particular mmc host. >>> >>> I would like to see three different callbacks, mirroring each of the >>> mmc_ios power_mode states MMC_POWER_OFF|UP|ON. >> >> Hmm, can't we do something with runtime pm here instead? I would be >> nice if we could use the platform bus for this instead of inventing >> a new bus for this. > > We don't need another bus. The driver only have to register some mmc > specific callbacks, that's all I am saying. Of course these parts > can't be re-used for other subsystems, unless we find it useful to > have similar callbacks for all subsystems. > > Still, using runtime PM might work. > > I see these important things that follow if we decide to use runtime > PM to trigger the power up/off sequence. > 1) In cases of !CONFIG_PM_RUNTIME, it means the "powerup driver" once > probed, will keep it's resources enabled forever. Ack. > 2) If we want to use runtime PM to control fine grained power > management of the "powerup driver", now this can't be done. We can always add something more elaborate later if needed, the advantage of sticking with a platform-dev represented by its own dt subnode + runtime PM, is that powerup drivers can be used with other busses too, all the other busses will need is to specify the subnode location + address inside the tree, and add code to their subsys core to instantiate the platform device. > 3) The "powerup driver" must be able to cope with two states (on/off), > instead the three MMC_POWER_OFF|UP|ON states. Since we need to powerup before probing, I think this is fine, we will want to do the power-up before we do the OFF -> UP -> ON sequence in mmc_power_up(), and we will want to do the power-down after transitioning to OFF. > 4) The system suspend/resume sequence for the SDIO card, will be more > tricky to handle. See below. > In principle we need to decide what runtime PM should be used for in > this context. I think we should add a powerup_dev pdev pointer to the mmc-card struct, so that sdio drivers which want to shutdown the device to save power can do so (by making the relevant runtime pm calls on the pdev). The mmc core will never know if it is safe to actually power down the device again as even if the sdio driver indicates it is ok to shutdown the mmc-host, it may still need the sdio device to stay powered so as to not loose state. Or maybe even for system-wakeup through an oob irq. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html