[2/2] thermal: imx_scu_thermal: fix getting DT alert property value

Message ID 20200516203420.24409-2-agust@denx.de
State New
Headers show
Series
  • [1/2] cpu: imx8: fix type and rate detection
Related show

Commit Message

Anatolij Gustschin May 16, 2020, 8:34 p.m.
Fix boot hang with endless loop outputting:
CPU Temperature (47200C) has beyond alert (0C), close to critical (0C) waiting...

Signed-off-by: Anatolij Gustschin <agust at denx.de>
---
i.MX8QXP is broken again, this should be appied ASAP!

 drivers/thermal/imx_scu_thermal.c | 101 ++++++++++++------------------
 1 file changed, 41 insertions(+), 60 deletions(-)

Comments

Peng Fan May 17, 2020, 10:04 a.m. | #1
Hi Anatolij,

> Subject: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert property
> value
> 
> Fix boot hang with endless loop outputting:
> CPU Temperature (47200C) has beyond alert (0C), close to critical (0C)
> waiting...

Could you share more info which part was broken?

Thanks,
Peng.

> 
> Signed-off-by: Anatolij Gustschin <agust at denx.de>
> ---
> i.MX8QXP is broken again, this should be appied ASAP!
> 
>  drivers/thermal/imx_scu_thermal.c | 101 ++++++++++++------------------
>  1 file changed, 41 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/thermal/imx_scu_thermal.c
> b/drivers/thermal/imx_scu_thermal.c
> index da13121a09..679ce4e244 100644
> --- a/drivers/thermal/imx_scu_thermal.c
> +++ b/drivers/thermal/imx_scu_thermal.c
> @@ -57,7 +57,7 @@ int imx_sc_thermal_get_temp(struct udevice *dev, int
> *temp)
>  	if (ret)
>  		return ret;
> 
> -	while (cpu_temp >= pdata->alert) {
> +	while (cpu_temp >= pdata->alert && pdata->alert) {
>  		printf("CPU Temperature (%dC) has beyond alert (%dC), close to
> critical (%dC)",
>  		       cpu_temp, pdata->alert, pdata->critical);
>  		puts(" waiting...\n");
> @@ -78,7 +78,47 @@ static const struct dm_thermal_ops
> imx_sc_thermal_ops = {
> 
>  static int imx_sc_thermal_probe(struct udevice *dev)  {
> +	struct imx_sc_thermal_plat *pdata = dev_get_platdata(dev);
> +	struct fdtdec_phandle_args args;
> +	ofnode node, trips_np;
> +	int ret;
> +
>  	debug("%s dev name %s\n", __func__, dev->name);
> +
> +	trips_np = ofnode_path("/thermal-zones/cpu-thermal0/trips");
> +	node = ofnode_get_parent(trips_np);
> +	ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, node.of_offset,
> +					     "thermal-sensors",
> +					     "#thermal-sensor-cells",
> +					     0, 0, &args);
> +	if (ret)
> +		return ret;
> +
> +	if (args.args_count >= 1)
> +		pdata->id = args.args[0];
> +	else
> +		pdata->id = 0;
> +
> +	debug("args.args_count %d, id %d\n", args.args_count, pdata->id);
> +
> +	pdata->polling_delay = ofnode_read_u32_default(node, "polling-delay",
> +						       1000);
> +	ofnode_for_each_subnode(trips_np, trips_np) {
> +		const char *type;
> +
> +		type = ofnode_get_property(trips_np, "type", NULL);
> +		if (!type)
> +			continue;
> +		if (!strcmp(type, "critical"))
> +			pdata->critical =
> +			ofnode_read_u32_default(trips_np, "temperature", 85);
> +		else if (!strcmp(type, "passive"))
> +			pdata->alert =
> +			ofnode_read_u32_default(trips_np, "temperature", 80);
> +	}
> +
> +	debug("id %d polling_delay %d, critical %d, alert %d\n",
> +	      pdata->id, pdata->polling_delay, pdata->critical, pdata->alert);
>  	return 0;
>  }
> 
> @@ -121,64 +161,6 @@ static int imx_sc_thermal_bind(struct udevice *dev)
>  	return 0;
>  }
> 
> -static int imx_sc_thermal_ofdata_to_platdata(struct udevice *dev) -{
> -	struct imx_sc_thermal_plat *pdata = dev_get_platdata(dev);
> -	struct fdtdec_phandle_args args;
> -	const char *type;
> -	int ret;
> -	int trips_np;
> -
> -	debug("%s dev name %s\n", __func__, dev->name);
> -
> -	if (pdata->zone_node)
> -		return 0;
> -
> -	ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev_of_offset(dev),
> -					     "thermal-sensors",
> -					     "#thermal-sensor-cells",
> -					     0, 0, &args);
> -	if (ret)
> -		return ret;
> -
> -	if (args.node != dev_of_offset(dev->parent))
> -		return -EFAULT;
> -
> -	if (args.args_count >= 1)
> -		pdata->id = args.args[0];
> -	else
> -		pdata->id = 0;
> -
> -	debug("args.args_count %d, id %d\n", args.args_count, pdata->id);
> -
> -	pdata->polling_delay = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
> -					      "polling-delay", 1000);
> -
> -	trips_np = fdt_subnode_offset(gd->fdt_blob, dev_of_offset(dev),
> -				      "trips");
> -	fdt_for_each_subnode(trips_np, gd->fdt_blob, trips_np) {
> -		type = fdt_getprop(gd->fdt_blob, trips_np, "type", NULL);
> -		if (type) {
> -			if (strcmp(type, "critical") == 0) {
> -				pdata->critical = fdtdec_get_int(gd->fdt_blob,
> -								 trips_np,
> -								 "temperature",
> -								 85);
> -			} else if (strcmp(type, "passive") == 0) {
> -				pdata->alert = fdtdec_get_int(gd->fdt_blob,
> -							      trips_np,
> -							      "temperature",
> -							      80);
> -			}
> -		}
> -	}
> -
> -	debug("id %d polling_delay %d, critical %d, alert %d\n", pdata->id,
> -	      pdata->polling_delay, pdata->critical, pdata->alert);
> -
> -	return 0;
> -}
> -
>  static const sc_rsrc_t imx8qm_sensor_rsrc[] = {
>  	SC_R_A53, SC_R_A72, SC_R_GPU_0_PID0, SC_R_GPU_1_PID0,
>  	SC_R_DRC_0, SC_R_DRC_1, SC_R_VPU_PID0, SC_R_PMIC_0, @@
> -205,7 +187,6 @@ U_BOOT_DRIVER(imx_sc_thermal) = {
>  	.of_match = imx_sc_thermal_ids,
>  	.bind = imx_sc_thermal_bind,
>  	.probe	= imx_sc_thermal_probe,
> -	.ofdata_to_platdata = imx_sc_thermal_ofdata_to_platdata,
>  	.platdata_auto_alloc_size = sizeof(struct imx_sc_thermal_plat),
>  	.flags  = DM_FLAG_PRE_RELOC,
>  };
> --
> 2.17.1
Anatolij Gustschin May 17, 2020, 11:18 a.m. | #2
Hi Peng,

On Sun, 17 May 2020 10:04:58 +0000
Peng Fan peng.fan at nxp.com wrote:

> Hi Anatolij,
> 
> > Subject: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert property
> > value
> > 
> > Fix boot hang with endless loop outputting:
> > CPU Temperature (47200C) has beyond alert (0C), close to critical (0C)
> > waiting...  
> 
> Could you share more info which part was broken?

I've tested i.MX8QXP based capricorn/giedi board (giedi_defconfig, imx8-giedi.dts)
and it was not booting:

U-Boot SPL 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200)
Trying to boot from MMC1
Load image from MMC/SD 0x3ec00


U-Boot 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200) ##v01.07

sc_pm_get_clock_rate: resource:0 clk:2: res:3
Could not read CPU frequency: -22
CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
CPU Temperature (34600C) has beyond alert (0C), close to critical (0C) waiting...
CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
...

Other i.MX8QXP based boards might be broken as well.

--
Anatolij
Peng Fan May 17, 2020, 2:43 p.m. | #3
> Subject: Re: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert
> property value
> 
> Hi Peng,
> 
> On Sun, 17 May 2020 10:04:58 +0000
> Peng Fan peng.fan at nxp.com wrote:
> 
> > Hi Anatolij,
> >
> > > Subject: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert
> > > property value
> > >
> > > Fix boot hang with endless loop outputting:
> > > CPU Temperature (47200C) has beyond alert (0C), close to critical
> > > (0C) waiting...
> >
> > Could you share more info which part was broken?
> 
> I've tested i.MX8QXP based capricorn/giedi board (giedi_defconfig,
> imx8-giedi.dts) and it was not booting:
> 
> U-Boot SPL 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200)
> Trying to boot from MMC1 Load image from MMC/SD 0x3ec00
> 
> 
> U-Boot 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200)
> ##v01.07
> 
> sc_pm_get_clock_rate: resource:0 clk:2: res:3 Could not read CPU frequency:
> -22 CPU Temperature (34800C) has beyond alert (0C), close to critical (0C)
> waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C)
> waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C)
> waiting...
> CPU Temperature (34800C) has beyond alert (0C), close to critical (0C)
> waiting...
> CPU Temperature (34600C) has beyond alert (0C), close to critical (0C)
> waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C)
> waiting...

Sorry for not be clear, I mean which code change breaks the boot?

Thanks,
Peng.

> ...
> 
> Other i.MX8QXP based boards might be broken as well.
> 
> --
> Anatolij
Anatolij Gustschin May 17, 2020, 2:53 p.m. | #4
On Sun, 17 May 2020 14:43:39 +0000
Peng Fan peng.fan at nxp.com wrote:
...
> > CPU Temperature (35000C) has beyond alert (0C), close to critical (0C)
> > waiting...  
> 
> Sorry for not be clear, I mean which code change breaks the boot?

I didn't bisect this, so I do not know exactly.
Last working revision installed on the board before was
d202f67db077 ("Merge branch '2020-04-25-master-imports'").
Will try to bisect later.

--
Anatolij
Fabio Estevam May 18, 2020, 11:32 p.m. | #5
Hi Anatolij and Heiko,

On Sun, May 17, 2020 at 8:19 AM Anatolij Gustschin <agust at denx.de> wrote:

> I've tested i.MX8QXP based capricorn/giedi board (giedi_defconfig, imx8-giedi.dts)
> and it was not booting:
>
> U-Boot SPL 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200)
> Trying to boot from MMC1
> Load image from MMC/SD 0x3ec00
>
>
> U-Boot 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200) ##v01.07
>
> sc_pm_get_clock_rate: resource:0 clk:2: res:3
> Could not read CPU frequency: -22
> CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
> CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
> CPU Temperature (34600C) has beyond alert (0C), close to critical (0C) waiting...
> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
> ...
>
> Other i.MX8QXP based boards might be broken as well.

Would it be possible to add this i.MX8QXP capricorn/giedi board into
Heiko's boot farm?

Thanks
Heiko Schocher May 19, 2020, 4:17 a.m. | #6
Hello Fabio, Anatolij,

Am 19.05.2020 um 01:32 schrieb Fabio Estevam:
> Hi Anatolij and Heiko,
> 
> On Sun, May 17, 2020 at 8:19 AM Anatolij Gustschin <agust at denx.de> wrote:
> 
>> I've tested i.MX8QXP based capricorn/giedi board (giedi_defconfig, imx8-giedi.dts)
>> and it was not booting:
>>
>> U-Boot SPL 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200)
>> Trying to boot from MMC1
>> Load image from MMC/SD 0x3ec00
>>
>>
>> U-Boot 2020.07-rc2-00124-g515f613253 (May 16 2020 - 09:22:49 +0200) ##v01.07
>>
>> sc_pm_get_clock_rate: resource:0 clk:2: res:3
>> Could not read CPU frequency: -22
>> CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
>> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
>> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
>> CPU Temperature (34800C) has beyond alert (0C), close to critical (0C) waiting...
>> CPU Temperature (34600C) has beyond alert (0C), close to critical (0C) waiting...
>> CPU Temperature (35000C) has beyond alert (0C), close to critical (0C) waiting...
>> ...
>>
>> Other i.MX8QXP based boards might be broken as well.
> 
> Would it be possible to add this i.MX8QXP capricorn/giedi board into
> Heiko's boot farm?

I am happy to add boards!

@Anatolij: Do we have this board in our vlab? Or somehow remote accessible?

@Fabio: I am sure you have some boards for testing, do you have a CI
   setup for them? With the approach to report testresults you can run
   tbot also just locally and report results... just an idea...

   If you need help to setup tbot, feel free to contact me.

You do not need to use tbot (I recommend it only, as it makes all for you
once setup, and you can use it for your daily developers work too)

You can test your boards however you want and report results without tbot.
Therefore I wrote an example:

https://github.com/EmbLux-Kft/uboot_results/blob/master/src/client.py

Of course, you have to fill real values for the report into:

https://github.com/EmbLux-Kft/uboot_results/blob/master/src/client.py#L88

I only have to add an useracount...

And be aware it is just a Proof of concept, so there are for sure bugs,
missing documentation, ugly code ...

Testers, ideas and patches welcome.

bye,
Heiko
Anatolij Gustschin May 19, 2020, 8:19 a.m. | #7
Hi Fabio,

On Mon, 18 May 2020 20:32:06 -0300
Fabio Estevam festevam at gmail.com wrote:
...
> Would it be possible to add this i.MX8QXP capricorn/giedi board into
> Heiko's boot farm?

this board requires a very special power supply and I have only one
power supply unit of that type and can't share it.

--
Anatolij
Anatolij Gustschin May 19, 2020, 8:47 a.m. | #8
Hi Heiko,

On Tue, 19 May 2020 06:17:58 +0200
Heiko Schocher hs at denx.de wrote:
...
> > Would it be possible to add this i.MX8QXP capricorn/giedi board into
> > Heiko's boot farm?  
> 
> I am happy to add boards!
> 
> @Anatolij: Do we have this board in our vlab? Or somehow remote accessible?

No, it is on my table and not remote accessible. I'd like to make it
remote accessible, but the problem currently is that I have only one
serial console adapter (special box with FFC cable required to get the
serial output, and I have only one such unit). I have another baseboard
with giedi SoM and here the special console adapter is not required,
but this baseboard has issues with its power supply circuit and requires
a very stable power source before turning it on. Otherwise the SoM can
be damaged and is not recoverable any more. I already have damaged two
SoMs here when remote powering and do not want to experiment with
remaining working SoMs.

--
Anatolij
Anatolij Gustschin May 19, 2020, 9:35 a.m. | #9
Hi Peng,

On Sun, 17 May 2020 16:53:17 +0200
Anatolij Gustschin agust at denx.de wrote:
...
> Will try to bisect later.

bisecting leads to:
# first bad commit: [3ee6ea443eb466399ab325a58b377326ac5c57b5]
cpu: imx_cpu: Print the CPU temperature for iMX8QM A72

--
Anatolij
Fabio Estevam May 19, 2020, 11:57 a.m. | #10
Hi Heiko,

On Tue, May 19, 2020 at 1:18 AM Heiko Schocher <hs at denx.de> wrote:

> I am happy to add boards!
>
> @Anatolij: Do we have this board in our vlab? Or somehow remote accessible?
>
> @Fabio: I am sure you have some boards for testing, do you have a CI
>    setup for them? With the approach to report testresults you can run

It is in my wish list for quite some time, but currently I don't have
a CI setup.

>    tbot also just locally and report results... just an idea...
>
>    If you need help to setup tbot, feel free to contact me.
>
> You do not need to use tbot (I recommend it only, as it makes all for you
> once setup, and you can use it for your daily developers work too)
>
> You can test your boards however you want and report results without tbot.
> Therefore I wrote an example:
>
> https://github.com/EmbLux-Kft/uboot_results/blob/master/src/client.py
>
> Of course, you have to fill real values for the report into:
>
> https://github.com/EmbLux-Kft/uboot_results/blob/master/src/client.py#L88
>
> I only have to add an useracount...
>
> And be aware it is just a Proof of concept, so there are for sure bugs,
> missing documentation, ugly code ...
>
> Testers, ideas and patches welcome.

Thanks for your help!

Patch

diff --git a/drivers/thermal/imx_scu_thermal.c b/drivers/thermal/imx_scu_thermal.c
index da13121a09..679ce4e244 100644
--- a/drivers/thermal/imx_scu_thermal.c
+++ b/drivers/thermal/imx_scu_thermal.c
@@ -57,7 +57,7 @@  int imx_sc_thermal_get_temp(struct udevice *dev, int *temp)
 	if (ret)
 		return ret;
 
-	while (cpu_temp >= pdata->alert) {
+	while (cpu_temp >= pdata->alert && pdata->alert) {
 		printf("CPU Temperature (%dC) has beyond alert (%dC), close to critical (%dC)",
 		       cpu_temp, pdata->alert, pdata->critical);
 		puts(" waiting...\n");
@@ -78,7 +78,47 @@  static const struct dm_thermal_ops imx_sc_thermal_ops = {
 
 static int imx_sc_thermal_probe(struct udevice *dev)
 {
+	struct imx_sc_thermal_plat *pdata = dev_get_platdata(dev);
+	struct fdtdec_phandle_args args;
+	ofnode node, trips_np;
+	int ret;
+
 	debug("%s dev name %s\n", __func__, dev->name);
+
+	trips_np = ofnode_path("/thermal-zones/cpu-thermal0/trips");
+	node = ofnode_get_parent(trips_np);
+	ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, node.of_offset,
+					     "thermal-sensors",
+					     "#thermal-sensor-cells",
+					     0, 0, &args);
+	if (ret)
+		return ret;
+
+	if (args.args_count >= 1)
+		pdata->id = args.args[0];
+	else
+		pdata->id = 0;
+
+	debug("args.args_count %d, id %d\n", args.args_count, pdata->id);
+
+	pdata->polling_delay = ofnode_read_u32_default(node, "polling-delay",
+						       1000);
+	ofnode_for_each_subnode(trips_np, trips_np) {
+		const char *type;
+
+		type = ofnode_get_property(trips_np, "type", NULL);
+		if (!type)
+			continue;
+		if (!strcmp(type, "critical"))
+			pdata->critical =
+			ofnode_read_u32_default(trips_np, "temperature", 85);
+		else if (!strcmp(type, "passive"))
+			pdata->alert =
+			ofnode_read_u32_default(trips_np, "temperature", 80);
+	}
+
+	debug("id %d polling_delay %d, critical %d, alert %d\n",
+	      pdata->id, pdata->polling_delay, pdata->critical, pdata->alert);
 	return 0;
 }
 
@@ -121,64 +161,6 @@  static int imx_sc_thermal_bind(struct udevice *dev)
 	return 0;
 }
 
-static int imx_sc_thermal_ofdata_to_platdata(struct udevice *dev)
-{
-	struct imx_sc_thermal_plat *pdata = dev_get_platdata(dev);
-	struct fdtdec_phandle_args args;
-	const char *type;
-	int ret;
-	int trips_np;
-
-	debug("%s dev name %s\n", __func__, dev->name);
-
-	if (pdata->zone_node)
-		return 0;
-
-	ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev_of_offset(dev),
-					     "thermal-sensors",
-					     "#thermal-sensor-cells",
-					     0, 0, &args);
-	if (ret)
-		return ret;
-
-	if (args.node != dev_of_offset(dev->parent))
-		return -EFAULT;
-
-	if (args.args_count >= 1)
-		pdata->id = args.args[0];
-	else
-		pdata->id = 0;
-
-	debug("args.args_count %d, id %d\n", args.args_count, pdata->id);
-
-	pdata->polling_delay = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
-					      "polling-delay", 1000);
-
-	trips_np = fdt_subnode_offset(gd->fdt_blob, dev_of_offset(dev),
-				      "trips");
-	fdt_for_each_subnode(trips_np, gd->fdt_blob, trips_np) {
-		type = fdt_getprop(gd->fdt_blob, trips_np, "type", NULL);
-		if (type) {
-			if (strcmp(type, "critical") == 0) {
-				pdata->critical = fdtdec_get_int(gd->fdt_blob,
-								 trips_np,
-								 "temperature",
-								 85);
-			} else if (strcmp(type, "passive") == 0) {
-				pdata->alert = fdtdec_get_int(gd->fdt_blob,
-							      trips_np,
-							      "temperature",
-							      80);
-			}
-		}
-	}
-
-	debug("id %d polling_delay %d, critical %d, alert %d\n", pdata->id,
-	      pdata->polling_delay, pdata->critical, pdata->alert);
-
-	return 0;
-}
-
 static const sc_rsrc_t imx8qm_sensor_rsrc[] = {
 	SC_R_A53, SC_R_A72, SC_R_GPU_0_PID0, SC_R_GPU_1_PID0,
 	SC_R_DRC_0, SC_R_DRC_1, SC_R_VPU_PID0, SC_R_PMIC_0,
@@ -205,7 +187,6 @@  U_BOOT_DRIVER(imx_sc_thermal) = {
 	.of_match = imx_sc_thermal_ids,
 	.bind = imx_sc_thermal_bind,
 	.probe	= imx_sc_thermal_probe,
-	.ofdata_to_platdata = imx_sc_thermal_ofdata_to_platdata,
 	.platdata_auto_alloc_size = sizeof(struct imx_sc_thermal_plat),
 	.flags  = DM_FLAG_PRE_RELOC,
 };