From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Armstrong Date: Mon, 17 Feb 2020 09:55:30 +0100 Subject: [RFC 2/2] Revert "dm: core: device: switch off power domain after device removal" In-Reply-To: <914a0443-bdab-3b43-3534-9e5c5a6c65c9@ti.com> References: <20200203135619.10180-1-oliver.graute@kococonnector.com> <20200203135619.10180-3-oliver.graute@kococonnector.com> <914a0443-bdab-3b43-3534-9e5c5a6c65c9@ti.com> Message-ID: <30dbfda1-e062-b012-93a6-0172e5c84c69@baylibre.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 14/02/2020 05:20, Lokesh Vutla wrote: > > > On 13/02/20 10:42 PM, Neil Armstrong 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? > > Can you give more details on where the failure is happening?(logs would really > help). > >>> >>> 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 >>> #include >>> #include >>> -#include >>> >>> 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. > > This doesn't mean U-boot must leave the all power-domains un handled before > jumping to kernel. Please use DM_FLAG_DEFAULT_PD_CTRL_OFF in you driver and > enable power_domain in your probe. Your case is exactly the reason why > DM_FLAG_DEFAULT_PD_CTRL_OFF is introduced. This changed the behavior of the power domain core, and this is wrong. Reverting to manually handling the power domain in the probe defeats the initial goal of automatic handling of power domain at probe, and the current DM_FLAG_DEFAULT_PD_CTRL_OF doesn't mean anything, it's not explicit at all, and doesn't mean the power domain won't be enabled at probe. We must read the code to understand it. Please avoid breaking stuff and adding fuzzy flags without a proper behavior. The use-case of the flag would be to keep the power domain enabled at linux boot instead of never probing it. Neil > > Thanks and regards, > Lokesh >