[RFC 2/2] Revert "dm: core: device: switch off power domain after device removal"

[Lokesh Vutla]

On 17/02/20 6:35 PM, Anatolij Gustschin wrote:
> On Thu, 13 Feb 2020 18:12:12 +0100
> Neil Armstrong narmstrong at baylibre.com wrote:
> 
>> Hi,
>>
>> On 03/02/2020 14:59, Oliver Graute wrote:
>>> The conga-imx8 board isn't booting without this revert. Can someone tell me
>>> what I need to Do so that this revert is not necessary?
>>>
>>> This reverts commit 52edfed65de967a86983a55c51ba0727090efc43.
>>> ---
>>>  drivers/core/device-remove.c | 5 -----
>>>  1 file changed, 5 deletions(-)
>>>
>>> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
>>> index 5c8dc4ad70..586fadee0a 100644
>>> --- a/drivers/core/device-remove.c
>>> +++ b/drivers/core/device-remove.c
>>> @@ -16,7 +16,6 @@
>>>  #include <dm/uclass.h>
>>>  #include <dm/uclass-internal.h>
>>>  #include <dm/util.h>
>>> -#include <power-domain.h>
>>>  
>>>  int device_chld_unbind(struct udevice *dev, struct driver *drv)
>>>  {
>>> @@ -193,10 +192,6 @@ int device_remove(struct udevice *dev, uint flags)
>>>  		}
>>>  	}
>>>  
>>> -	if (!(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
>>> -	    (dev != gd->cur_serial_dev))
>>> -		dev_power_domain_off(dev);
>>> -
>>>  	if (flags_remove(flags, drv->flags)) {
>>>  		device_free(dev);
>>>  
>>>   
>>
>> It also breaks amlogic boards with video, since we can't tell we want
>> to keep the power domains up after boot, it cuts the power domains and
>> thus defeats all the goal of u-boot video...
>>
>> The problem is DM_FLAG_DEFAULT_PD_CTRL_OFF disables auto power domain on
>> aswell, reverting to manual power domain enable.
> 
> Yes, in some special cases manual power domain enable is required,
> this is the reason why this flag has been introduced. This flag is
> not for controlling power off for auto-enabled domains.
> 
> My original patch v3 [1] suggested another flag DM_FLAG_REMOVE_WITH_PD_ON,
> but in the subsequent patch (which was then merged) Lokesh replaced this
> with another flag DM_FLAG_DEFAULT_PD_CTRL_OFF for different purpose and
> dropped DM_FLAG_REMOVE_WITH_PD_ON.

May be you should see the patch that introduces the specific flag[0] :). Flag
DM_FLAG_REMOVE_WITH_PD_ON[1] does not have any users at that point. Why would we
introduce it without any users?. Even with the patch it would be breaking the
above driver unless explicitly enabled.

IMHO, it is dm core controlling the PD or not. Why will it control just the
enabling part. May be I am wrong, but anyways Ill let Simon and Tom to have a
decision on it.

[0] https://patchwork.ozlabs.org/patch/1168343/
[1] https://patchwork.ozlabs.org/patch/1140138/

Thanks and regards,
Lokesh