diff mbox series

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

Message ID 20200516203420.24409-2-agust@denx.de
State Superseded
Delegated to: Stefano Babic
Headers show
Series [1/2] cpu: imx8: fix type and rate detection | expand

Commit Message

Anatolij Gustschin May 16, 2020, 8:34 p.m. UTC
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@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. UTC | #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@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. UTC | #2
Hi Peng,

On Sun, 17 May 2020 10:04:58 +0000
Peng Fan peng.fan@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. UTC | #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@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. UTC | #4
On Sun, 17 May 2020 14:43:39 +0000
Peng Fan peng.fan@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. UTC | #5
Hi Anatolij and Heiko,

On Sun, May 17, 2020 at 8:19 AM Anatolij Gustschin <agust@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. UTC | #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@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. UTC | #7
Hi Fabio,

On Mon, 18 May 2020 20:32:06 -0300
Fabio Estevam festevam@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. UTC | #8
Hi Heiko,

On Tue, 19 May 2020 06:17:58 +0200
Heiko Schocher hs@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. UTC | #9
Hi Peng,

On Sun, 17 May 2020 16:53:17 +0200
Anatolij Gustschin agust@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
Peng Fan May 19, 2020, 10:05 a.m. UTC | #10
> Subject: Re: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert
> property value
> 
> Hi Peng,
> 
> On Sun, 17 May 2020 16:53:17 +0200
> Anatolij Gustschin agust@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

Would this help?

diff --git a/drivers/cpu/imx8_cpu.c b/drivers/cpu/imx8_cpu.c
index 95c14c98d8..6c949ccbd4 100644
--- a/drivers/cpu/imx8_cpu.c
+++ b/drivers/cpu/imx8_cpu.c
@@ -68,10 +68,13 @@ static int cpu_imx_get_temp(struct cpu_imx_platdata *plat)
        struct udevice *thermal_dev;
        int cpu_tmp, ret;

-       if (!strcmp(plat->name, "A72"))
-               ret = uclass_get_device(UCLASS_THERMAL, 1, &thermal_dev);
-       else
-               ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
+       if (!strcmp(plat->name, "A72")) {
+               ret = uclass_get_device_by_name(UCLASS_THERMAL, "cpu-thermal1",
+                                               &thermal_dev);
+       } else {
+               ret = uclass_get_device_by_name(UCLASS_THERMAL, "cpu-thermal0",
+                                               &thermal_dev);
+       }

        if (!ret) {
                ret = thermal_get_temp(thermal_dev, &cpu_tmp);

Thanks,
Peng.
> 
> --
> Anatolij
Anatolij Gustschin May 19, 2020, 10:26 a.m. UTC | #11
On Tue, 19 May 2020 10:05:21 +0000
Peng Fan peng.fan@nxp.com wrote:
...
> Would this help?
> 
> diff --git a/drivers/cpu/imx8_cpu.c b/drivers/cpu/imx8_cpu.c
> index 95c14c98d8..6c949ccbd4 100644
> --- a/drivers/cpu/imx8_cpu.c
> +++ b/drivers/cpu/imx8_cpu.c
> @@ -68,10 +68,13 @@ static int cpu_imx_get_temp(struct cpu_imx_platdata *plat)
>         struct udevice *thermal_dev;
>         int cpu_tmp, ret;
> 
> -       if (!strcmp(plat->name, "A72"))
> -               ret = uclass_get_device(UCLASS_THERMAL, 1, &thermal_dev);
> -       else
> -               ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
> +       if (!strcmp(plat->name, "A72")) {
> +               ret = uclass_get_device_by_name(UCLASS_THERMAL, "cpu-thermal1",
> +                                               &thermal_dev);
> +       } else {
> +               ret = uclass_get_device_by_name(UCLASS_THERMAL, "cpu-thermal0",
> +                                               &thermal_dev);
> +       }
> 
>         if (!ret) {
>                 ret = thermal_get_temp(thermal_dev, &cpu_tmp);

Yes, this fixes the problem, thanks!

--
Anatolij
Peng Fan May 19, 2020, 11:45 a.m. UTC | #12
+Simon

> -----Original Message-----
> From: Anatolij Gustschin <agust@denx.de>
> Sent: 2020年5月19日 18:27
> To: Peng Fan <peng.fan@nxp.com>
> Cc: u-boot@lists.denx.de; Ye Li <ye.li@nxp.com>; Frank Li
> <frank.li@nxp.com>; sbabic@denx.de
> Subject: Re: [PATCH 2/2] thermal: imx_scu_thermal: fix getting DT alert
> property value
> 
> On Tue, 19 May 2020 10:05:21 +0000
> Peng Fan peng.fan@nxp.com wrote:
> ...
> > Would this help?
> >
> > diff --git a/drivers/cpu/imx8_cpu.c b/drivers/cpu/imx8_cpu.c index
> > 95c14c98d8..6c949ccbd4 100644
> > --- a/drivers/cpu/imx8_cpu.c
> > +++ b/drivers/cpu/imx8_cpu.c
> > @@ -68,10 +68,13 @@ static int cpu_imx_get_temp(struct
> cpu_imx_platdata *plat)
> >         struct udevice *thermal_dev;
> >         int cpu_tmp, ret;
> >
> > -       if (!strcmp(plat->name, "A72"))
> > -               ret = uclass_get_device(UCLASS_THERMAL, 1,
> &thermal_dev);
> > -       else
> > -               ret = uclass_get_device(UCLASS_THERMAL, 0,
> &thermal_dev);
> > +       if (!strcmp(plat->name, "A72")) {
> > +               ret = uclass_get_device_by_name(UCLASS_THERMAL,
> "cpu-thermal1",
> > +                                               &thermal_dev);
> > +       } else {
> > +               ret = uclass_get_device_by_name(UCLASS_THERMAL,
> "cpu-thermal0",
> > +                                               &thermal_dev);
> > +       }
> >
> >         if (!ret) {
> >                 ret = thermal_get_temp(thermal_dev, &cpu_tmp);
> 
> Yes, this fixes the problem, thanks!

Do you have more insights about uclass_get_device and uclass_get_device_byname?
uclass_get_device not work, but uclass_get_device_byname work.

Thanks,
Peng.

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

On Tue, May 19, 2020 at 1:18 AM Heiko Schocher <hs@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!
Anatolij Gustschin May 19, 2020, 10:05 p.m. UTC | #14
On Tue, 19 May 2020 11:45:55 +0000
Peng Fan peng.fan@nxp.com wrote:
...
> Do you have more insights about uclass_get_device and uclass_get_device_byname?
> uclass_get_device not work, but uclass_get_device_byname work.

well, we have three thermal uclass devices on i.MX8QXP:
 thermal       0  [   ]   imx_sc_thermal        |-- thermal-sensor
 thermal       1  [   ]   imx_sc_thermal        |   |-- cpu-thermal0
 thermal       2  [   ]   imx_sc_thermal        |   `-- drc-thermal0

when using uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev), the
first device ("thermal-sensor") is matching and for this device
imx_sc_thermal_ofdata_to_platdata() will be called, it then tries to
get the "thermal-sensors" list in the node of "thermal-sensor" device
(dev_of_offset(dev)), but this is wrong, since this list is a property
of the "cpu-thermal0" node according to bindings.

Therefore ofdata_to_platdata() can't find the "thermal-sensors" list
and does not initialize alert/critical pdata values.

When uclass_get_device_by_name() is used, then imx_sc_thermal_ofdata_to_platdata()
is called for "cpu-thermal0" device, here getting the list works
and alert/critical pdata values are initialized properly.

--
Anatolij
Anatolij Gustschin May 19, 2020, 11:37 p.m. UTC | #15
On Wed, 20 May 2020 00:05:01 +0200
Anatolij Gustschin agust@denx.de wrote:
...
> When uclass_get_device_by_name() is used, then imx_sc_thermal_ofdata_to_platdata()
> is called for "cpu-thermal0" device, here getting the list works
> and alert/critical pdata values are initialized properly.

This 2/2 patch can be superseded by patch:
 http://patchwork.ozlabs.org/project/uboot/patch/20200519233144.2426-1-agust@denx.de

--
Anatolij
diff mbox series

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,
 };