| Message ID | 20191217121431.28819-1-igor.opaniuk@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v1] Revert "core: device: use dev_power_domain_on" | expand |
On Tue, Dec 17, 2019 at 2:14 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote: > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > This was previously reported that f0cc4eae9a ("core: device: > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > --- > > drivers/core/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index 4e037083a6..c5b232c259 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) > > int device_probe(struct udevice *dev) > { > + struct power_domain pd; > const struct driver *drv; > int size = 0; > int ret; > @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && > !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { > - ret = dev_power_domain_on(dev); > - if (ret) > - goto fail; > + if (!power_domain_get(dev, &pd)) > + power_domain_on(&pd); > } > > ret = uclass_pre_probe_device(dev); > -- > 2.17.1 > -- Best regards Oleksandr Suvorov Toradex AG Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800 (main line)
On Tue, Dec 17, 2019 at 9:14 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote: > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > This was previously reported that f0cc4eae9a ("core: device: > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> Being so close from the 2020.01, I think this is the safest approach for now: Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Subject: [PATCH v1] Revert "core: device: use dev_power_domain_on" > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > This was previously reported that f0cc4eae9a ("core: device: > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. This is not the root cause. Please check your driver power domain. Thanks Peng. > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > --- > > drivers/core/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c index > 4e037083a6..c5b232c259 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) > > int device_probe(struct udevice *dev) > { > + struct power_domain pd; > const struct driver *drv; > int size = 0; > int ret; > @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && > !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { > - ret = dev_power_domain_on(dev); > - if (ret) > - goto fail; > + if (!power_domain_get(dev, &pd)) > + power_domain_on(&pd); > } > > ret = uclass_pre_probe_device(dev); > -- > 2.17.1
Hi Peng, On Thu, Dec 19, 2019 at 10:14 PM Peng Fan <peng.fan@nxp.com> wrote: > > > Subject: [PATCH v1] Revert "core: device: use dev_power_domain_on" > > > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > > > This was previously reported that f0cc4eae9a ("core: device: > > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > This is not the root cause. Please check your driver power domain. Could this error be dependant on the SCFW version? BTW, are you able to boot mx8qxp mek with mainline? It is hanging for me as posted at https://lists.denx.de/pipermail/u-boot/2019-December/394054.html
On 17/12/19 5:44 PM, Igor Opaniuk wrote: > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > This was previously reported that f0cc4eae9a ("core: device: > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> NAK. This breaks other drivers with multiple power domains that rely on core framework to enable. > --- > > drivers/core/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index 4e037083a6..c5b232c259 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) > > int device_probe(struct udevice *dev) > { > + struct power_domain pd; > const struct driver *drv; > int size = 0; > int ret; > @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && > !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { > - ret = dev_power_domain_on(dev); > - if (ret) > - goto fail; Can you check by not returning on failure here? If yes then check the power-domain/driver that is failing. If any driver doesn't expect core to enable power-domain then enable DM_FLAG_DEFAULT_PD_CTRL_OFF in the respective driver. Thanks and regards, Lokesh > + if (!power_domain_get(dev, &pd)) > + power_domain_on(&pd); > } > > ret = uclass_pre_probe_device(dev); >
Hi Peng, On Fri, Dec 20, 2019 at 3:14 AM Peng Fan <peng.fan@nxp.com> wrote: > > > Subject: [PATCH v1] Revert "core: device: use dev_power_domain_on" > > > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > > > This was previously reported that f0cc4eae9a ("core: device: > > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > This is not the root cause. Please check your driver power domain. There is no any statement anywhere that this is the "root cause". Me and Oliver pointed to this commit as it was the first one when two MX8-based boards from different vendors just stoped booting. That is pretty obvious that there were updates also on SCFW/TF-A side, but unfortunately I haven't found any related information (nor in the commit message, neither any updates in board/freescale/imx8*/README), just some notice in one of ML threads, where Oliver actually reported this issue. Please provide more info about what version of SECO, SCFW and TF-A using. Thanks > > Thanks > Peng. > > > > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > > --- > > > > drivers/core/device.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/core/device.c b/drivers/core/device.c index > > 4e037083a6..c5b232c259 100644 > > --- a/drivers/core/device.c > > +++ b/drivers/core/device.c > > @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) > > > > int device_probe(struct udevice *dev) > > { > > + struct power_domain pd; > > const struct driver *drv; > > int size = 0; > > int ret; > > @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) > > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > > (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && > > !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { > > - ret = dev_power_domain_on(dev); > > - if (ret) > > - goto fail; > > + if (!power_domain_get(dev, &pd)) > > + power_domain_on(&pd); > > } > > > > ret = uclass_pre_probe_device(dev); > > -- > > 2.17.1 >
On Fri, Dec 20, 2019 at 11:16:38AM +0530, Lokesh Vutla wrote: > > > On 17/12/19 5:44 PM, Igor Opaniuk wrote: > > From: Igor Opaniuk <igor.opaniuk@toradex.com> > > > > This reverts commit f0cc4eae9a1702a768817ea25d9f23cece69d021 > > > > This was previously reported that f0cc4eae9a ("core: device: > > use dev_power_domain_on") breaks initial boot on Colibri iMX8X and > > IMX8 QM ROM 7720a1 board. Revert it until the problem is properly fixed. > > > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > > NAK. This breaks other drivers with multiple power domains that rely on core > framework to enable. > > > --- > > > > drivers/core/device.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/core/device.c b/drivers/core/device.c > > index 4e037083a6..c5b232c259 100644 > > --- a/drivers/core/device.c > > +++ b/drivers/core/device.c > > @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) > > > > int device_probe(struct udevice *dev) > > { > > + struct power_domain pd; > > const struct driver *drv; > > int size = 0; > > int ret; > > @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) > > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > > (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && > > !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { > > - ret = dev_power_domain_on(dev); > > - if (ret) > > - goto fail; > > Can you check by not returning on failure here? If yes then check the > power-domain/driver that is failing. If any driver doesn't expect core to enable > power-domain then enable DM_FLAG_DEFAULT_PD_CTRL_OFF in the respective driver. I'm adding a few other people to this part of the thread as I don't think breaking some boards to fix other boards is a viable choice, even at this stage in the release. Thanks all.
diff --git a/drivers/core/device.c b/drivers/core/device.c index 4e037083a6..c5b232c259 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -313,6 +313,7 @@ static void *alloc_priv(int size, uint flags) int device_probe(struct udevice *dev) { + struct power_domain pd; const struct driver *drv; int size = 0; int ret; @@ -396,9 +397,8 @@ int device_probe(struct udevice *dev) if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) && !(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF)) { - ret = dev_power_domain_on(dev); - if (ret) - goto fail; + if (!power_domain_get(dev, &pd)) + power_domain_on(&pd); } ret = uclass_pre_probe_device(dev);