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

Message ID 20200214110154.72e0eef0@crub
State New
Headers show
Series
  • Untitled series #44846
Related show

Commit Message

Anatolij Gustschin Feb. 14, 2020, 10:01 a.m.
On Fri, 14 Feb 2020 09:50:54 +0530
Lokesh Vutla lokeshvutla at ti.com 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).

On i.MX8QXP based SoM running U-Boot v2020.04-rc2 with below patch the
v4.14.98 kernel from NXP BSP boots:


The generated debug log:

Starting kernel ...

device_remove: OFF 'clk', uclass pd 0
device_remove: OFF 'gpioledsgrp', uclass pd 0
device_remove: OFF 'lpi2c0grp', uclass pd 0
device_remove: OFF 'lpi2c1grp', uclass pd 0
device_remove: OFF 'lpuart2grp', uclass pd 0
device_remove: OFF 'usdhc1grp', uclass pd 0
device_remove: OFF 'fec2grp', uclass pd 0
device_remove: OFF 'imx8qxp-som', uclass pd 0
device_remove: OFF 'iomuxc', uclass pd 0
device_remove: OFF 'mu at 5d1c0000', uclass pd 0
device_remove: 'lsio_gpio0', uclass pd 1
device_remove: 'lsio_gpio1', uclass pd 1
device_remove: 'lsio_gpio3', uclass pd 1
device_remove: 'lsio_gpio4', uclass pd 1
device_remove: 'lsio_gpio5', uclass pd 1
device_remove: 'lsio_power_domain', uclass pd 1
device_remove: 'conn_sdhc0', uclass pd 1
device_remove: 'conn_enet1', uclass pd 1
device_remove: 'connectivity_power_domain', uclass pd 1
device_remove: 'dma_lpi2c0', uclass pd 1
device_remove: 'dma_lpi2c1', uclass pd 1
device_remove: 'dma_lpuart2', uclass pd 1
device_remove: 'dma_power_domain', uclass pd 1
device_remove: OFF 'imx8qx-pm', uclass pd 0
device_remove: OFF 'generic_50', uclass pd 0
device_remove: OFF 'i2c at 5a800000', uclass pd 0
device_remove: 'dma_lpi2c0', uclass pd 1
device_remove: OFF 'i2c at 5a810000', uclass pd 0
device_remove: 'dma_lpi2c1', uclass pd 1
device_remove: OFF 'gpio at 5d080000', uclass pd 0
device_remove: 'lsio_gpio0', uclass pd 1
device_remove: OFF 'gpio at 5d090000', uclass pd 0
device_remove: 'lsio_gpio1', uclass pd 1
device_remove: OFF 'gpio at 5d0b0000', uclass pd 0
device_remove: 'lsio_gpio3', uclass pd 1
device_remove: OFF 'gpio at 5d0c0000', uclass pd 0
device_remove: 'lsio_gpio4', uclass pd 1
device_remove: OFF 'gpio at 5d0d0000', uclass pd 0
device_remove: 'lsio_gpio5', uclass pd 1
device_remove: 'serial at 5a080000', uclass pd 0
device_remove: OFF 'usdhc at 5b010000.blk', uclass pd 0
device_remove: OFF 'usdhc at 5b010000', uclass pd 0
device_remove: 'conn_sdhc0', uclass pd 1
device_remove: OFF 'ethernet at 5b050000', uclass pd 0
device_remove: 'conn_enet1', uclass pd 1
device_remove: OFF 'run', uclass pd 0
device_remove: OFF 'flt', uclass pd 0
device_remove: OFF 'svc', uclass pd 0
device_remove: OFF 'com1_tx', uclass pd 0
device_remove: OFF 'com1_rx', uclass pd 0
device_remove: OFF 'com2_tx', uclass pd 0
device_remove: OFF 'com2_rx', uclass pd 0
device_remove: OFF 'cloud', uclass pd 0
device_remove: OFF 'wlan', uclass pd 0
device_remove: OFF 'dbg1', uclass pd 0
device_remove: OFF 'dbg2', uclass pd 0
device_remove: OFF 'dbg3', uclass pd 0
device_remove: OFF 'dbg4', uclass pd 0
device_remove: OFF 'leds', uclass pd 0
device_remove: OFF 'root_driver', uclass pd 0

My original patch v3 had this additional condition for power domain off:

 (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN)

This was dropped in the merged patch for some reason, I don't know why.

--
Anatolij

Comments

Anatolij Gustschin Feb. 14, 2020, 10:14 a.m. | #1
On Fri, 14 Feb 2020 11:01:54 +0100
Anatolij Gustschin agust at denx.de wrote:
...
> My original patch v3 had this additional condition for power domain off:
> 
>  (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN)

The debug log generated without the above condition:

device_remove: OFF 'clk', uclass pd 0
device_remove: OFF 'gpioledsgrp', uclass pd 0
device_remove: OFF 'lpi2c0grp', uclass pd 0
device_remove: OFF 'lpi2c1grp', uclass pd 0
device_remove: OFF 'lpuart2grp', uclass pd 0
device_remove: OFF 'usdhc1grp', uclass pd 0
device_remove: OFF 'fec2grp', uclass pd 0
device_remove: OFF 'imx8qxp-som', uclass pd 0
device_remove: OFF 'iomuxc', uclass pd 0
device_remove: OFF 'mu at 5d1c0000', uclass pd 0
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
device_remove: OFF 'lsio_gpio0', uclass pd 1
...
Here it is stuck in an endless recursion for lsio_gpio0 device remove.

--
Anatolij
Lokesh Vutla Feb. 14, 2020, 10:22 a.m. | #2
On 14/02/20 3:31 PM, Anatolij Gustschin wrote:
> On Fri, 14 Feb 2020 09:50:54 +0530
> Lokesh Vutla lokeshvutla at ti.com 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).
> 
> On i.MX8QXP based SoM running U-Boot v2020.04-rc2 with below patch the
> v4.14.98 kernel from NXP BSP boots:
> 
> diff --git a/arch/arm/mach-imx/imx8/fdt.c b/arch/arm/mach-imx/imx8/fdt.c
> index 65c8ac1a7e..cf79a12f42 100644
> --- a/arch/arm/mach-imx/imx8/fdt.c
> +++ b/arch/arm/mach-imx/imx8/fdt.c
> @@ -280,7 +280,9 @@ int ft_system_setup(void *blob, bd_t *bd)
>  {
>  	int ret;
>  
> +	/*
>  	update_fdt_with_owned_resources(blob);
> +	*/
>  
>  	if (is_imx8qm()) {
>  		ret = config_smmu_fdt(blob);
> 
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 444e34b492..a218f481b5 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -194,9 +194,18 @@ int device_remove(struct udevice *dev, uint flags)
>  		}
>  	}
>  
> -	if (!(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
> -	    (dev != gd->cur_serial_dev))
> +	if (!(dev->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
> +	    (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) &&
> +	    (dev != gd->cur_serial_dev)) {
> +		dm_warn("%s: OFF '%s', uclass pd %d\n",
> +			__func__, dev->name,
> +			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
>  		dev_power_domain_off(dev);
> +	} else {
> +		dm_warn("%s: '%s', uclass pd %d\n",
> +			__func__, dev->name,
> +			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
> +	}
>  
>  	if (flags_remove(flags, drv->flags)) {
>  		device_free(dev);
> 
> The generated debug log:
> 
> Starting kernel ...
> 
> device_remove: OFF 'clk', uclass pd 0
> device_remove: OFF 'gpioledsgrp', uclass pd 0
> device_remove: OFF 'lpi2c0grp', uclass pd 0
> device_remove: OFF 'lpi2c1grp', uclass pd 0
> device_remove: OFF 'lpuart2grp', uclass pd 0
> device_remove: OFF 'usdhc1grp', uclass pd 0
> device_remove: OFF 'fec2grp', uclass pd 0
> device_remove: OFF 'imx8qxp-som', uclass pd 0
> device_remove: OFF 'iomuxc', uclass pd 0
> device_remove: OFF 'mu at 5d1c0000', uclass pd 0
> device_remove: 'lsio_gpio0', uclass pd 1
> device_remove: 'lsio_gpio1', uclass pd 1
> device_remove: 'lsio_gpio3', uclass pd 1
> device_remove: 'lsio_gpio4', uclass pd 1
> device_remove: 'lsio_gpio5', uclass pd 1
> device_remove: 'lsio_power_domain', uclass pd 1
> device_remove: 'conn_sdhc0', uclass pd 1
> device_remove: 'conn_enet1', uclass pd 1
> device_remove: 'connectivity_power_domain', uclass pd 1
> device_remove: 'dma_lpi2c0', uclass pd 1
> device_remove: 'dma_lpi2c1', uclass pd 1
> device_remove: 'dma_lpuart2', uclass pd 1
> device_remove: 'dma_power_domain', uclass pd 1
> device_remove: OFF 'imx8qx-pm', uclass pd 0
> device_remove: OFF 'generic_50', uclass pd 0
> device_remove: OFF 'i2c at 5a800000', uclass pd 0
> device_remove: 'dma_lpi2c0', uclass pd 1
> device_remove: OFF 'i2c at 5a810000', uclass pd 0
> device_remove: 'dma_lpi2c1', uclass pd 1
> device_remove: OFF 'gpio at 5d080000', uclass pd 0
> device_remove: 'lsio_gpio0', uclass pd 1
> device_remove: OFF 'gpio at 5d090000', uclass pd 0
> device_remove: 'lsio_gpio1', uclass pd 1
> device_remove: OFF 'gpio at 5d0b0000', uclass pd 0
> device_remove: 'lsio_gpio3', uclass pd 1
> device_remove: OFF 'gpio at 5d0c0000', uclass pd 0
> device_remove: 'lsio_gpio4', uclass pd 1
> device_remove: OFF 'gpio at 5d0d0000', uclass pd 0
> device_remove: 'lsio_gpio5', uclass pd 1
> device_remove: 'serial at 5a080000', uclass pd 0
> device_remove: OFF 'usdhc at 5b010000.blk', uclass pd 0
> device_remove: OFF 'usdhc at 5b010000', uclass pd 0
> device_remove: 'conn_sdhc0', uclass pd 1
> device_remove: OFF 'ethernet at 5b050000', uclass pd 0
> device_remove: 'conn_enet1', uclass pd 1
> device_remove: OFF 'run', uclass pd 0
> device_remove: OFF 'flt', uclass pd 0
> device_remove: OFF 'svc', uclass pd 0
> device_remove: OFF 'com1_tx', uclass pd 0
> device_remove: OFF 'com1_rx', uclass pd 0
> device_remove: OFF 'com2_tx', uclass pd 0
> device_remove: OFF 'com2_rx', uclass pd 0
> device_remove: OFF 'cloud', uclass pd 0
> device_remove: OFF 'wlan', uclass pd 0
> device_remove: OFF 'dbg1', uclass pd 0
> device_remove: OFF 'dbg2', uclass pd 0
> device_remove: OFF 'dbg3', uclass pd 0
> device_remove: OFF 'dbg4', uclass pd 0
> device_remove: OFF 'leds', uclass pd 0
> device_remove: OFF 'root_driver', uclass pd 0
> 
> My original patch v3 had this additional condition for power domain off:
> 
>  (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN)
> 
> This was dropped in the merged patch for some reason, I don't know why.

power_domain_get_by_index() handles this case. It tries to read power-domains in
the node, if not found it will fall back. I am not sure why it should be an
issue in this case.

I am not saying the above condition is wrong but it is already taken care the
power_domain_off.

Thanks and regards,
Lokesh
Oliver Graute Feb. 14, 2020, 10:50 a.m. | #3
On 14/02/20, Anatolij Gustschin wrote:
> On Fri, 14 Feb 2020 09:50:54 +0530
> Lokesh Vutla lokeshvutla at ti.com 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).
> 
> On i.MX8QXP based SoM running U-Boot v2020.04-rc2 with below patch the
> v4.14.98 kernel from NXP BSP boots:
> 
> diff --git a/arch/arm/mach-imx/imx8/fdt.c b/arch/arm/mach-imx/imx8/fdt.c
> index 65c8ac1a7e..cf79a12f42 100644
> --- a/arch/arm/mach-imx/imx8/fdt.c
> +++ b/arch/arm/mach-imx/imx8/fdt.c
> @@ -280,7 +280,9 @@ int ft_system_setup(void *blob, bd_t *bd)
>  {
>  	int ret;
>  
> +	/*
>  	update_fdt_with_owned_resources(blob);
> +	*/
>  
>  	if (is_imx8qm()) {
>  		ret = config_smmu_fdt(blob);
> 
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 444e34b492..a218f481b5 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -194,9 +194,18 @@ int device_remove(struct udevice *dev, uint flags)
>  		}
>  	}
>  
> -	if (!(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
> -	    (dev != gd->cur_serial_dev))
> +	if (!(dev->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
> +	    (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) &&
> +	    (dev != gd->cur_serial_dev)) {
> +		dm_warn("%s: OFF '%s', uclass pd %d\n",
> +			__func__, dev->name,
> +			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
>  		dev_power_domain_off(dev);
> +	} else {
> +		dm_warn("%s: '%s', uclass pd %d\n",
> +			__func__, dev->name,
> +			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
> +	}
>  
>  	if (flags_remove(flags, drv->flags)) {
>  		device_free(dev);
> 
> The generated debug log:

thx I did the same as Anatolij on my imx8qm congatec board:

done
Bytes transferred = 21224 (52e8 hex)
## Flattened Device Tree blob at 83000000
   Booting using the fdt blob at 0x83000000
   Using Device Tree in place at 0000000083000000, end 00000000830082e7

Starting kernel ...

device_remove: OFF 'clk', uclass pd 0
device_remove: OFF 'hoggrp', uclass pd 0
device_remove: OFF 'fec1grp', uclass pd 0
device_remove: OFF 'lpuart0grp', uclass pd 0
device_remove: OFF 'usdhc1grp', uclass pd 0
device_remove: OFF 'usdhc2grpgpio', uclass pd 0
device_remove: OFF 'usdhc2grp', uclass pd 0
device_remove: OFF 'usdhc3grpgpio', uclass pd 0
device_remove: OFF 'usdhc3grp', uclass pd 0
device_remove: OFF 'imx8qm-qmx8', uclass pd 0
device_remove: OFF 'iomuxc', uclass pd 0
device_remove: OFF 'mu at 5d1c0000', uclass pd 0
device_remove: 'lsio_gpio0', uclass pd 1
device_remove: 'lsio_gpio1', uclass pd 1
device_remove: 'lsio_gpio2', uclass pd 1
device_remove: 'lsio_gpio4', uclass pd 1
device_remove: 'lsio_gpio5', uclass pd 1
device_remove: 'lsio_power_domain', uclass pd 1
device_remove: 'conn_sdhc0', uclass pd 1
device_remove: 'conn_sdhc1', uclass pd 1
device_remove: 'conn_sdhc2', uclass pd 1
device_remove: 'conn_enet0', uclass pd 1
device_remove: 'connectivity_power_domain', uclass pd 1
device_remove: 'dma_lpuart0', uclass pd 1
device_remove: 'dma_power_domain', uclass pd 1
device_remove: OFF 'imx8qm-pm', uclass pd 0
device_remove: OFF 'gpio at 5d080000', uclass pd 0
power_domain_get_by_index() power_domain_get_by_index(dev=00000000fdaea0f0, power_domain=00000000fdae0118)
imx8_power_domain_of_xlate() imx8_power_domain_of_xlate(power_domain=00000000fdae0118)
imx8_power_domain_request() imx8_power_domain_request(power_domain=00000000fdae0118)
power_domain_off() power_domain_off(power_domain=00000000fdae0118)
imx8_power_domain_off() imx8_power_domain_off(power_domain=00000000fdae0118)
imx8_power_domain_off_node() imx8_power_domain_off_node, lsio_gpio0, state_on 1
imx8_power_domain_off_node() lsio_gpio0 is powered off
imx8_power_domain_off_parentnodes() imx8_power_domain_off_parentnodes, lsio_power_domain, state_on 1
imx8_power_domain_off_parentnodes() sibling lsio_gpio1, state_on 1
device_remove: 'lsio_gpio0', uclass pd 1
device_remove: OFF 'gpio at 5d090000', uclass pd 0
power_domain_get_by_index() power_domain_get_by_index(dev=00000000fdaea1d0, power_domain=00000000fdae0118)
imx8_power_domain_of_xlate() imx8_power_domain_of_xlate(power_domain=00000000fdae0118)
imx8_power_domain_request() imx8_power_domain_request(power_domain=00000000fdae0118)
power_domain_off() power_domain_off(power_domain=00000000fdae0118)
imx8_power_domain_off() imx8_power_domain_off(power_domain=00000000fdae0118)
imx8_power_domain_off_node() imx8_power_domain_off_node, lsio_gpio1, state_on 1
imx8_power_domain_off_node() lsio_gpio1 is powered off
imx8_power_domain_off_parentnodes() imx8_power_domain_off_parentnodes, lsio_power_domain, state_on 1
imx8_power_domain_off_parentnodes() sibling lsio_gpio2, state_on 1
device_remove: 'lsio_gpio1', uclass pd 1
device_remove: OFF 'gpio at 5d0a0000', uclass pd 0
power_domain_get_by_index() power_domain_get_by_index(dev=00000000fdaea2b0, power_domain=00000000fdae0118)

Best regards,

Oliver

Patch

diff --git a/arch/arm/mach-imx/imx8/fdt.c b/arch/arm/mach-imx/imx8/fdt.c
index 65c8ac1a7e..cf79a12f42 100644
--- a/arch/arm/mach-imx/imx8/fdt.c
+++ b/arch/arm/mach-imx/imx8/fdt.c
@@ -280,7 +280,9 @@  int ft_system_setup(void *blob, bd_t *bd)
 {
 	int ret;
 
+	/*
 	update_fdt_with_owned_resources(blob);
+	*/
 
 	if (is_imx8qm()) {
 		ret = config_smmu_fdt(blob);

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 444e34b492..a218f481b5 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -194,9 +194,18 @@  int device_remove(struct udevice *dev, uint flags)
 		}
 	}
 
-	if (!(drv->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
-	    (dev != gd->cur_serial_dev))
+	if (!(dev->flags & DM_FLAG_DEFAULT_PD_CTRL_OFF) &&
+	    (device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) &&
+	    (dev != gd->cur_serial_dev)) {
+		dm_warn("%s: OFF '%s', uclass pd %d\n",
+			__func__, dev->name,
+			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
 		dev_power_domain_off(dev);
+	} else {
+		dm_warn("%s: '%s', uclass pd %d\n",
+			__func__, dev->name,
+			device_get_uclass_id(dev) == UCLASS_POWER_DOMAIN);
+	}
 
 	if (flags_remove(flags, drv->flags)) {
 		device_free(dev);