On 13 October 2014 04:48, Aaron Lu <aaron.lu@xxxxxxxxx> wrote:
> On 10/02/2014 08:27 AM, Dmitry Torokhov wrote:
>> Hi Ulf,
>>
>> On Fri, Sep 19, 2014 at 08:27:40PM +0200, Ulf Hansson wrote:
>>> Previously only the ACPI PM domain was supported by the sdio bus.
>>>
>>> Let's convert to the common attach/detach functions for PM domains,
>>> which currently means we are extending the support to include the
>>> generic PM domain as well.
>>>
>>> Cc: linux-mmc@xxxxxxxxxxxxxxx
>>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>>> Reviewed-by: Kevin Hilman <khilman@xxxxxxxxxx>
>>> ---
>>> drivers/mmc/core/sdio_bus.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
>>> index 4fa8fef9..1df0fc6 100644
>>> --- a/drivers/mmc/core/sdio_bus.c
>>> +++ b/drivers/mmc/core/sdio_bus.c
>>> @@ -315,7 +315,7 @@ int sdio_add_func(struct sdio_func *func)
>>> ret = device_add(&func->dev);
>>> if (ret == 0) {
>>> sdio_func_set_present(func);
>>> - acpi_dev_pm_attach(&func->dev, false);
>>> + dev_pm_domain_attach(&func->dev, false);
>>
>> Admittedly it is not brought in by your change, but I am a bit worried
>> about this code. In all other busses we attach power domain to a device
>> before probing it (and detach if probe fails). Why here we only attach
>> it to power domain after adding the device to the bus? If driver for the
>> function has already been loaded the probe() would run without device
>> being attached to a power domain...
>
> Sorry for replying late...
>
> I see your concern, but it should be safe here. The dev_pm_domain_attach
> does primarily two things(for ACPI based system): assign the pm_domian
> field so that later system and runtime PM transitions we have proper
> callbacks for this device; power on the device if needed.
>
> I was using Broadcom BCM43241 SDIO wifi card when developing the code,
> it doesn't need the pm_domain set in its probe function and since the
> SDIO wifi card is powered on after system boot, no power on is needed
> either. That's why I added the attach function after device_add to save
> a little code(detach if probe fails).
>
> Feel free to change the sequence if the current one is a bad one, I'm OK
> with either case.
Thanks for clarifying this. I plan to change the behaviour according
to how other buses handles it.
Though, the patch will be a part another patchset that handles a
related initialization problem for the generic PM domain. I will keep
you guys on cc when I post it.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
