[PATCH] power-domain: fix hang in endless loop on i.MX8
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@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@denx.de Reported-by: Oliver Graute oliver.graute@gmail.com Reported-by: Fabio Estevam festevam@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
On Sat, 15 Feb 2020 at 04:34, Anatolij Gustschin agust@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@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@denx.de Reported-by: Oliver Graute oliver.graute@gmail.com Reported-by: Fabio Estevam festevam@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@chromium.org
Regards, Simon
Hi Simon, Peng,
On Sun, 16 Feb 2020 12:02:55 -0700 Simon Glass sjg@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@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@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@denx.de Reported-by: Oliver Graute oliver.graute@gmail.com Reported-by: Fabio Estevam festevam@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@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@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@denx.de Reported-by: Oliver Graute oliver.graute@gmail.com Reported-by: Fabio Estevam festevam@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@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@denx.de Reported-by: Oliver Graute oliver.graute@gmail.com Reported-by: Fabio Estevam festevam@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
participants (6)
-
Anatolij Gustschin -
Lokesh Vutla -
Neil Armstrong -
Oliver Graute -
Peng Fan -
Simon Glass