| Message ID | 20200215113443.6866-1-agust@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | power-domain: fix hang in endless loop on i.MX8 | expand |
On Sat, 15 Feb 2020 at 04:34, Anatolij Gustschin <agust at denx.de> wrote: > > Currently when booting the kernel on i.MX8 U-Boot hangs in an > endless loop when switching off dma, connectivity or lsio power > domains during device removal. It hapens first when removing > gpio0 (gpio at 5d080000) device, here its power domain device > 'lsio_gpio0' is obtained for switching off power. Since the > obtained 'lsio_gpio0' device is removed afterwards, its power > domain is also switched off and here the parent power domain > device 'lsio_power_domain' is optained for switching off the > power. Thereafter, when the obtained 'lsio_power_domain' is > removed, device_remove() removes its first child 'lsio_gpio0'. > During this child removal the 'lsio_power_domain' device is > obtained again for switching and when removing it later, > the same child removal is repeated, so we are stuck in an > endless loop. Below is a snippet from dm tree on i.MX8QXP > for better illustration of the DM devices relationship: > > Class Index Probed Driver Name > ----------------------------------------------------------- > root 0 [ + ] root_driver root_driver > ... > simple_bus 0 [ + ] generic_simple_bus |-- imx8qx-pm > power_doma 0 [ + ] imx8_power_domain | |-- lsio_power_domain > power_doma 1 [ + ] imx8_power_domain | | |-- lsio_gpio0 > power_doma 2 [ + ] imx8_power_domain | | |-- lsio_gpio1 > > Do not remove a power domain device if it is a parent of the > currently controlled device. > > Fixes: 52edfed65de9 ("dm: core: device: switch off power domain after device removal") > Signed-off-by: Anatolij Gustschin <agust at denx.de> > Reported-by: Oliver Graute <oliver.graute at gmail.com> > Reported-by: Fabio Estevam <festevam at gmail.com> > > --- > drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) Can we have a sandbox test for this case? Reviewed-by: Simon Glass <sjg at chromium.org> Regards, Simon
Hi Simon, Peng, On Sun, 16 Feb 2020 12:02:55 -0700 Simon Glass sjg at chromium.org wrote: ... > > drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > Can we have a sandbox test for this case? I didn't check if there are more platforms with such parent & child power domain devices in U-Boot. If there are some, then it makes sense to add a test. But for this i.MX8 case we will probably switch to another power domain bindings and driver like in Linux (we have to sync i.MX8 device trees with current Linux implementation which is different and it doesn't have such parent/child PD devices). The current i.MX8 PD U-Boot implementation seems wrong anyway, here the parent power domain devices for dma, connectivity or lsio domains actually do not control power switching, so there is no need to have such parent&child PD devices on i.MX8. @Peng: what do you think? -- Anatolij
> Subject: Re: [PATCH] power-domain: fix hang in endless loop on i.MX8 > > Hi Simon, Peng, > > On Sun, 16 Feb 2020 12:02:55 -0700 > Simon Glass sjg at chromium.org wrote: > ... > > > drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > Can we have a sandbox test for this case? > > I didn't check if there are more platforms with such parent & child power > domain devices in U-Boot. If there are some, then it makes sense to add a > test. But for this i.MX8 case we will probably switch to another power domain > bindings and driver like in Linux (we have to sync i.MX8 device trees with > current Linux implementation which is different and it doesn't have such > parent/child PD devices). > > The current i.MX8 PD U-Boot implementation seems wrong anyway, here the > parent power domain devices for dma, connectivity or lsio domains actually > do not control power switching, so there is no need to have such parent&child > PD devices on i.MX8. > > @Peng: what do you think? The current i.MX8 power domain driver needs finally be dropped. But I am still waiting the i.MX8 linux dts restructure from Aisheng ready, pending for long time. With Linux dts synced, there will no such parent/child in dts. Regards, Peng. > > -- > Anatolij
On 15/02/20 5:04 PM, Anatolij Gustschin wrote: > Currently when booting the kernel on i.MX8 U-Boot hangs in an > endless loop when switching off dma, connectivity or lsio power > domains during device removal. It hapens first when removing > gpio0 (gpio at 5d080000) device, here its power domain device > 'lsio_gpio0' is obtained for switching off power. Since the > obtained 'lsio_gpio0' device is removed afterwards, its power > domain is also switched off and here the parent power domain > device 'lsio_power_domain' is optained for switching off the > power. Thereafter, when the obtained 'lsio_power_domain' is > removed, device_remove() removes its first child 'lsio_gpio0'. > During this child removal the 'lsio_power_domain' device is > obtained again for switching and when removing it later, > the same child removal is repeated, so we are stuck in an > endless loop. Below is a snippet from dm tree on i.MX8QXP > for better illustration of the DM devices relationship: > > Class Index Probed Driver Name > ----------------------------------------------------------- > root 0 [ + ] root_driver root_driver > ... > simple_bus 0 [ + ] generic_simple_bus |-- imx8qx-pm > power_doma 0 [ + ] imx8_power_domain | |-- lsio_power_domain > power_doma 1 [ + ] imx8_power_domain | | |-- lsio_gpio0 > power_doma 2 [ + ] imx8_power_domain | | |-- lsio_gpio1 > > Do not remove a power domain device if it is a parent of the > currently controlled device. > > Fixes: 52edfed65de9 ("dm: core: device: switch off power domain after device removal") > Signed-off-by: Anatolij Gustschin <agust at denx.de> > Reported-by: Oliver Graute <oliver.graute at gmail.com> > Reported-by: Fabio Estevam <festevam at gmail.com> > > --- > drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c > index d9c623b56e..d8fe4d4877 100644 > --- a/drivers/power/domain/power-domain-uclass.c > +++ b/drivers/power/domain/power-domain-uclass.c > @@ -127,6 +127,18 @@ static int dev_power_domain_ctrl(struct udevice *dev, bool on) > ret = power_domain_off(&pd); > } > > + /* > + * For platforms with parent and child power-domain devices > + * we may not run device_remove() on the power-domain parent > + * because it will result in removing its children and switching > + * off their power-domain parent. So we will get here again and > + * again and will be stuck in an endless loop. > + */ > + if (!on && dev_get_parent(dev) == pd.dev && > + device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN) { > + return ret; > + } Please drop the braces here. Otherwise: Reviewed-by: Lokesh Vutla <lokeshvutla at ti.com> Thanks and regards, Lokesh
On 15/02/20, Anatolij Gustschin wrote: > Currently when booting the kernel on i.MX8 U-Boot hangs in an > endless loop when switching off dma, connectivity or lsio power > domains during device removal. It hapens first when removing > gpio0 (gpio at 5d080000) device, here its power domain device > 'lsio_gpio0' is obtained for switching off power. Since the > obtained 'lsio_gpio0' device is removed afterwards, its power > domain is also switched off and here the parent power domain > device 'lsio_power_domain' is optained for switching off the > power. Thereafter, when the obtained 'lsio_power_domain' is > removed, device_remove() removes its first child 'lsio_gpio0'. > During this child removal the 'lsio_power_domain' device is > obtained again for switching and when removing it later, > the same child removal is repeated, so we are stuck in an > endless loop. Below is a snippet from dm tree on i.MX8QXP > for better illustration of the DM devices relationship: > > Class Index Probed Driver Name > ----------------------------------------------------------- > root 0 [ + ] root_driver root_driver > ... > simple_bus 0 [ + ] generic_simple_bus |-- imx8qx-pm > power_doma 0 [ + ] imx8_power_domain | |-- lsio_power_domain > power_doma 1 [ + ] imx8_power_domain | | |-- lsio_gpio0 > power_doma 2 [ + ] imx8_power_domain | | |-- lsio_gpio1 > > Do not remove a power domain device if it is a parent of the > currently controlled device. > > Fixes: 52edfed65de9 ("dm: core: device: switch off power domain after device removal") > Signed-off-by: Anatolij Gustschin <agust at denx.de> > Reported-by: Oliver Graute <oliver.graute at gmail.com> > Reported-by: Fabio Estevam <festevam at gmail.com> > > --- > drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c > index d9c623b56e..d8fe4d4877 100644 > --- a/drivers/power/domain/power-domain-uclass.c > +++ b/drivers/power/domain/power-domain-uclass.c > @@ -127,6 +127,18 @@ static int dev_power_domain_ctrl(struct udevice *dev, bool on) > ret = power_domain_off(&pd); > } > > + /* > + * For platforms with parent and child power-domain devices > + * we may not run device_remove() on the power-domain parent > + * because it will result in removing its children and switching > + * off their power-domain parent. So we will get here again and > + * again and will be stuck in an endless loop. > + */ > + if (!on && dev_get_parent(dev) == pd.dev && > + device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN) { > + return ret; > + } > + > /* > * power_domain_get() bound the device, thus > * we must remove it again to prevent unbinding > -- > 2.17.1 thx, just tested this and now I can drop my revert mentioned here: https://lists.denx.de/pipermail/u-boot/2020-February/398910.html Best regards, Oliver
Hi, On 17/02/2020 10:07, Oliver Graute wrote: > On 15/02/20, Anatolij Gustschin wrote: >> Currently when booting the kernel on i.MX8 U-Boot hangs in an >> endless loop when switching off dma, connectivity or lsio power >> domains during device removal. It hapens first when removing >> gpio0 (gpio at 5d080000) device, here its power domain device >> 'lsio_gpio0' is obtained for switching off power. Since the >> obtained 'lsio_gpio0' device is removed afterwards, its power >> domain is also switched off and here the parent power domain >> device 'lsio_power_domain' is optained for switching off the >> power. Thereafter, when the obtained 'lsio_power_domain' is >> removed, device_remove() removes its first child 'lsio_gpio0'. >> During this child removal the 'lsio_power_domain' device is >> obtained again for switching and when removing it later, >> the same child removal is repeated, so we are stuck in an >> endless loop. Below is a snippet from dm tree on i.MX8QXP >> for better illustration of the DM devices relationship: >> >> Class Index Probed Driver Name >> ----------------------------------------------------------- >> root 0 [ + ] root_driver root_driver >> ... >> simple_bus 0 [ + ] generic_simple_bus |-- imx8qx-pm >> power_doma 0 [ + ] imx8_power_domain | |-- lsio_power_domain >> power_doma 1 [ + ] imx8_power_domain | | |-- lsio_gpio0 >> power_doma 2 [ + ] imx8_power_domain | | |-- lsio_gpio1 >> >> Do not remove a power domain device if it is a parent of the >> currently controlled device. >> >> Fixes: 52edfed65de9 ("dm: core: device: switch off power domain after device removal") >> Signed-off-by: Anatolij Gustschin <agust at denx.de> >> Reported-by: Oliver Graute <oliver.graute at gmail.com> >> Reported-by: Fabio Estevam <festevam at gmail.com> >> >> --- >> drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c >> index d9c623b56e..d8fe4d4877 100644 >> --- a/drivers/power/domain/power-domain-uclass.c >> +++ b/drivers/power/domain/power-domain-uclass.c >> @@ -127,6 +127,18 @@ static int dev_power_domain_ctrl(struct udevice *dev, bool on) >> ret = power_domain_off(&pd); >> } >> >> + /* >> + * For platforms with parent and child power-domain devices >> + * we may not run device_remove() on the power-domain parent >> + * because it will result in removing its children and switching >> + * off their power-domain parent. So we will get here again and >> + * again and will be stuck in an endless loop. >> + */ >> + if (!on && dev_get_parent(dev) == pd.dev && >> + device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN) { >> + return ret; >> + } >> + >> /* >> * power_domain_get() bound the device, thus >> * we must remove it again to prevent unbinding >> -- >> 2.17.1 > > thx, just tested this and now I can drop my revert mentioned here: > > https://lists.denx.de/pipermail/u-boot/2020-February/398910.html The revert is still needed... it breaks multiple amlogic boards and the fix is not as simple as this one. Neil > > Best regards, > > Oliver >
diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c index d9c623b56e..d8fe4d4877 100644 --- a/drivers/power/domain/power-domain-uclass.c +++ b/drivers/power/domain/power-domain-uclass.c @@ -127,6 +127,18 @@ static int dev_power_domain_ctrl(struct udevice *dev, bool on) ret = power_domain_off(&pd); } + /* + * For platforms with parent and child power-domain devices + * we may not run device_remove() on the power-domain parent + * because it will result in removing its children and switching + * off their power-domain parent. So we will get here again and + * again and will be stuck in an endless loop. + */ + if (!on && dev_get_parent(dev) == pd.dev && + device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN) { + return ret; + } + /* * power_domain_get() bound the device, thus * we must remove it again to prevent unbinding
Currently when booting the kernel on i.MX8 U-Boot hangs in an endless loop when switching off dma, connectivity or lsio power domains during device removal. It hapens first when removing gpio0 (gpio at 5d080000) device, here its power domain device 'lsio_gpio0' is obtained for switching off power. Since the obtained 'lsio_gpio0' device is removed afterwards, its power domain is also switched off and here the parent power domain device 'lsio_power_domain' is optained for switching off the power. Thereafter, when the obtained 'lsio_power_domain' is removed, device_remove() removes its first child 'lsio_gpio0'. During this child removal the 'lsio_power_domain' device is obtained again for switching and when removing it later, the same child removal is repeated, so we are stuck in an endless loop. Below is a snippet from dm tree on i.MX8QXP for better illustration of the DM devices relationship: Class Index Probed Driver Name ----------------------------------------------------------- root 0 [ + ] root_driver root_driver ... simple_bus 0 [ + ] generic_simple_bus |-- imx8qx-pm power_doma 0 [ + ] imx8_power_domain | |-- lsio_power_domain power_doma 1 [ + ] imx8_power_domain | | |-- lsio_gpio0 power_doma 2 [ + ] imx8_power_domain | | |-- lsio_gpio1 Do not remove a power domain device if it is a parent of the currently controlled device. Fixes: 52edfed65de9 ("dm: core: device: switch off power domain after device removal") Signed-off-by: Anatolij Gustschin <agust at denx.de> Reported-by: Oliver Graute <oliver.graute at gmail.com> Reported-by: Fabio Estevam <festevam at gmail.com> --- drivers/power/domain/power-domain-uclass.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)