On 3 June 2014 13:07, Hans de Goede [off-list ref] wrote:

Hi,

On 06/03/2014 12:14 PM, Ulf Hansson wrote:

quoted

On 28 May 2014 11:42, Hans de Goede [off-list ref] wrote:

<mega snip>

quoted

quoted

quoted

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.

Why do we need to put the sdio functions devices in DT?

quoted

quoted

quoted

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.

quoted

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.

So, the consequence is that for CONFIG_PM_SLEEP systems not using
CONFIG_PM_RUNTIME - we don't have a good solution.

Is that acceptable?

quoted

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.

quoted

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.

quoted

4) The system suspend/resume sequence for the SDIO card, will be more
tricky to handle.

See below.

quoted

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).

Makes sense.

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.

That should already be handled through the flags MMC_PM_KEEP_POWER and
MMC_PM_WAKE_SDIO_IRQ.

Regards,

Hans

Kind regards
Ulf Hansson