[PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Adam Ford]

If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal
trip points for industrial rated parts.  This should apply to 8MQ,
8MM, 8MN, and 8MP.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Tim Harvey <tharvey@gateworks.com>
---
V2:  Switch the check from looking for industrial or checkoing for
     anything but commerical.  This expands the trip point updates
     other grades as well.

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 8e23e6da32..7175295c3c 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -30,6 +30,7 @@
 #include <fsl_wdog.h>
 #include <imx_sip.h>
 #include <linux/bitops.h>
+#include <imx_thermal.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -1207,10 +1208,10 @@ static int cleanup_nodes_for_efi(void *blob)
 
 int ft_system_setup(void *blob, struct bd_info *bd)
 {
+	__maybe_unused int nodeoff;
 #ifdef CONFIG_IMX8MQ
 	int i = 0;
 	int rc;
-	int nodeoff;
 
 	if (get_boot_device() == USB_BOOT) {
 		disable_dcss_nodes(blob);
@@ -1346,6 +1347,24 @@ usb_modify_speed:
 		disable_cpu_nodes(blob, 2);
 #endif
 
+#if defined(CONFIG_IMX_TMU)
+	int minc, maxc, prop;
+
+	nodeoff = fdt_path_offset(blob, "/thermal-zones/cpu-thermal/trips");
+
+	/* Only update non-Commerical grade parts */
+	if (get_cpu_temp_grade(&minc, &maxc) != TEMP_COMMERCIAL) {
+		fdt_for_each_subnode(prop, blob, nodeoff) {
+			const char *type = fdt_getprop(blob, prop, "type", NULL);
+
+			if (type && (!strcmp("critical", type)))
+				fdt_setprop_u32(blob, prop, "temperature", maxc * 1000);
+			else if (type && (!strcmp("passive", type)))
+				fdt_setprop_u32(blob, prop, "temperature", (maxc - 10) * 1000);
+		}
+	}
+#endif
+
 	cleanup_nodes_for_efi(blob);
 	return 0;
 }
-- 
2.34.1

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Marek Vasut]

On 5/7/22 14:04, Adam Ford wrote:
> If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal
> trip points for industrial rated parts.  This should apply to 8MQ,
> 8MM, 8MN, and 8MP.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Tim Harvey <tharvey@gateworks.com>
> ---
> V2:  Switch the check from looking for industrial or checkoing for
>       anything but commerical.  This expands the trip point updates
>       other grades as well.

Are the industrial and automotive trip points identical ?

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Adam Ford]

On Sat, May 7, 2022 at 5:00 PM Marek Vasut <marex@denx.de> wrote:
>
> On 5/7/22 14:04, Adam Ford wrote:
> > If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal
> > trip points for industrial rated parts.  This should apply to 8MQ,
> > 8MM, 8MN, and 8MP.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> > Reviewed-by: Tim Harvey <tharvey@gateworks.com>
> > ---
> > V2:  Switch the check from looking for industrial or checkoing for
> >       anything but commerical.  This expands the trip point updates
> >       other grades as well.
>
> Are the industrial and automotive trip points identical ?

I do not know nor do I have any to test, but Francisco asked for the
change.  I would expect the corresponding values would be fetched with
get_cpu_temp_grade()

I will be on holiday in Italy for the next two weeks, so if there are
changes needed, I can address them when I get back.

adam

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Marek Vasut]

On 5/8/22 04:59, Adam Ford wrote:
> On Sat, May 7, 2022 at 5:00 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 5/7/22 14:04, Adam Ford wrote:
>>> If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal
>>> trip points for industrial rated parts.  This should apply to 8MQ,
>>> 8MM, 8MN, and 8MP.
>>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>> Reviewed-by: Tim Harvey <tharvey@gateworks.com>
>>> ---
>>> V2:  Switch the check from looking for industrial or checkoing for
>>>        anything but commerical.  This expands the trip point updates
>>>        other grades as well.
>>
>> Are the industrial and automotive trip points identical ?
> 
> I do not know nor do I have any to test, but Francisco asked for the
> change.  I would expect the corresponding values would be fetched with
> get_cpu_temp_grade()

Ah, I see. But then I have to wonder, why are we updating only 
non-commercial trip points ? Why not update them always ?

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Francesco Dolcini]

On Sun, May 08, 2022 at 03:42:31PM +0200, Marek Vasut wrote:
> On 5/8/22 04:59, Adam Ford wrote:
> > On Sat, May 7, 2022 at 5:00 PM Marek Vasut <marex@denx.de> wrote:
> > > 
> > > On 5/7/22 14:04, Adam Ford wrote:
> > > > If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal
> > > > trip points for industrial rated parts.  This should apply to 8MQ,
> > > > 8MM, 8MN, and 8MP.
> > > > 
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > Reviewed-by: Tim Harvey <tharvey@gateworks.com>
> > > > ---
> > > > V2:  Switch the check from looking for industrial or checkoing for
> > > >        anything but commerical.  This expands the trip point updates
> > > >        other grades as well.
> > > 
> > > Are the industrial and automotive trip points identical ?
> > 
> > I do not know nor do I have any to test, but Francisco asked for the
> > change.  I would expect the corresponding values would be fetched with
> > get_cpu_temp_grade()
> 
> Ah, I see. But then I have to wonder, why are we updating only
> non-commercial trip points ? Why not update them always ?

I didn't think through it enough, I agree on updating the trip points
always.

Francesco

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Andrejs Cainikovs]

Hi Adam and all,

Thanks for your work. I was about to send a similar patch by myself, but 
I came up with slightly different code. I'm sending my version for 
downstream version of NXP (should apply with minor conflicts to 
upstream) at the end of this mail. Plus some code review.

On 07/05/2022 14:04, Adam Ford wrote:
> If the thermal sensor is enabled in U-Boot, adjust the cpu-thermal

Should be not only for `cpu-thermal`. See below.

> V2:  Switch the check from looking for industrial or checkoing for
>       anything but commerical.  This expands the trip point updates
>       other grades as well.

Typos: checkoing, commerical.

> +#include <imx_thermal.h>

Not needed if you drop temp grade check. See below.

> +#if defined(CONFIG_IMX_TMU)

I believe this is needed only if you do the temperature reading. 
get_cpu_temp_grade() does not require this. Please remove.

> +	nodeoff = fdt_path_offset(blob, "/thermal-zones/cpu-thermal/trips");

This is enough for all variants except 8MP, which has 2 temperature 
sensors: `cpu-thermal` and `soc-thermal`. Both needs to be updated with 
equal trips.

> +	/* Only update non-Commerical grade parts */
> +	if (get_cpu_temp_grade(&minc, &maxc) != TEMP_COMMERCIAL) {

As others said, I would remove this check, as I don't see any issues if 
code would set trips for a commercial grade as well. This implies 
removing the include I mentioned above.

My version which I was going to sent (after rebasing) is below. Feel 
free to modify/rework/adjust your patch with or without parts from my code.

 >> Are the industrial and automotive trip points identical ?
 >
 > I do not know nor do I have any to test...

After you're done with the new patchset I can test it for you. I have 
access to commercial and industrial variants.

Best regards,
Andrejs Cainikovs

---

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c

index 
e93ecd28467ccc33ab35f0073840963eb3a01800..d4438b78887b91c600aaf22db47ef8985462ccd8 
100644

--- a/arch/arm/mach-imx/imx8m/soc.c

+++ b/arch/arm/mach-imx/imx8m/soc.c

@@ -1001,8 +1001,52 @@ static int disable_cpu_nodes(void *blob, u32 
disabled_cores)

  	return 0;

  }



+int fixup_thermal_trips(void *blob, const char *name)

+{

+	int minc, maxc;

+	int node, trip;

+	const char *type;

+	int temp, ret;

+

+	node = fdt_path_offset(blob, "/thermal-zones");

+	if (node < 0)

+		return node;

+

+	node = fdt_subnode_offset(blob, node, name);

+	if (node < 0)

+		return node;

+

+	node = fdt_subnode_offset(blob, node, "trips");

+	if (node < 0)

+		return node;

+

+	get_cpu_temp_grade(&minc, &maxc);

+

+	fdt_for_each_subnode(trip, blob, node) {

+		type = fdt_getprop(blob, trip, "type", NULL);

+

+		temp = 0;

+		if (!strcmp(type, "critical")) {

+			temp = 1000 * maxc;

+		} else if (!strcmp(type, "passive")) {

+			temp = 1000 * (maxc - 10);

+		}

+		if (temp) {

+			ret = fdt_setprop_u32(blob, trip, "temperature", temp);

+			if (ret) {

+				printf("Could not update thermal trip\n");

+				return ret;

+			}

+		}

+	}

+

+	return 0;

+}

+

  int ft_system_setup(void *blob, bd_t *bd)

  {

+	int ret;

+

  #ifdef CONFIG_IMX8MQ

  	int i = 0;

  	int rc;

@@ -1128,6 +1172,16 @@ usb_modify_speed:

  		disable_cpu_nodes(blob, 2);

  #endif



+	ret = fixup_thermal_trips(blob, "cpu-thermal");

+#ifdef CONFIG_IMX8MP

+	if (!ret)

+		ret = fixup_thermal_trips(blob, "soc-thermal");

+#endif

+	if (ret) {

+		printf("Failed to update thermal trip\n");

+		return ret;

+	}

+

  	return ft_add_optee_node(blob, bd);

  }

  #endif

Re: [PATCH V2] ARM: imx: imx8m: Adjust thermal trip points for Industrial parts

[Andrejs Cainikovs]

Hi Adam,

Below is the final vesion of my patch after internal review.

Best regards,
Andrejs Cainikovs.

[PATCH] imx8m: fixup thermal trips

[Andrejs Cainikovs]

Fixup thermal trips in Linux device tree according to SoC thermal
grade.

Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
---
 arch/arm/mach-imx/imx8m/soc.c | 50 +++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index e93ecd2846..b451ece91f 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob, u32 disabled_cores)
 	return 0;
 }
 
+int fixup_thermal_trips(void *blob, const char *name)
+{
+	int minc, maxc;
+	int node, trip;
+
+	node = fdt_path_offset(blob, "/thermal-zones");
+	if (node < 0)
+		return node;
+
+	node = fdt_subnode_offset(blob, node, name);
+	if (node < 0)
+		return node;
+
+	node = fdt_subnode_offset(blob, node, "trips");
+	if (node < 0)
+		return node;
+
+	get_cpu_temp_grade(&minc, &maxc);
+
+	fdt_for_each_subnode(trip, blob, node) {
+		const char *type;
+		int temp, ret;
+
+		type = fdt_getprop(blob, trip, "type", NULL);
+		if (!type)
+			continue;
+
+		temp = 0;
+		if (!strcmp(type, "critical")) {
+			temp = 1000 * maxc;
+		} else if (!strcmp(type, "passive")) {
+			temp = 1000 * (maxc - 10);
+		}
+		if (temp) {
+			ret = fdt_setprop_u32(blob, trip, "temperature", temp);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 int ft_system_setup(void *blob, bd_t *bd)
 {
 #ifdef CONFIG_IMX8MQ
@@ -1128,6 +1171,13 @@ usb_modify_speed:
 		disable_cpu_nodes(blob, 2);
 #endif
 
+	if (fixup_thermal_trips(blob, "cpu-thermal"))
+		printf("Failed to update cpu-thermal trip(s)");
+#ifdef CONFIG_IMX8MP
+	if (fixup_thermal_trips(blob, "soc-thermal"))
+		printf("Failed to update soc-thermal trip(s)");
+#endif
+
 	return ft_add_optee_node(blob, bd);
 }
 #endif
-- 
2.34.1

Re: [PATCH] imx8m: fixup thermal trips

[Adam Ford]

On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
<andrejs.cainikovs@toradex.com> wrote:
>
> Fixup thermal trips in Linux device tree according to SoC thermal
> grade.
>
> Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
> ---
>  arch/arm/mach-imx/imx8m/soc.c | 50 +++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index e93ecd2846..b451ece91f 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob, u32 disabled_cores)
>         return 0;
>  }
>
> +int fixup_thermal_trips(void *blob, const char *name)

Is there any reason this function cannot be static?

> +{
> +       int minc, maxc;
> +       int node, trip;
> +
> +       node = fdt_path_offset(blob, "/thermal-zones");
> +       if (node < 0)
> +               return node;
> +
> +       node = fdt_subnode_offset(blob, node, name);
> +       if (node < 0)
> +               return node;
> +
> +       node = fdt_subnode_offset(blob, node, "trips");
> +       if (node < 0)
> +               return node;
> +
> +       get_cpu_temp_grade(&minc, &maxc);
> +
> +       fdt_for_each_subnode(trip, blob, node) {
> +               const char *type;
> +               int temp, ret;
> +
> +               type = fdt_getprop(blob, trip, "type", NULL);
> +               if (!type)
> +                       continue;
> +
> +               temp = 0;
> +               if (!strcmp(type, "critical")) {
> +                       temp = 1000 * maxc;
> +               } else if (!strcmp(type, "passive")) {
> +                       temp = 1000 * (maxc - 10);
> +               }
> +               if (temp) {
> +                       ret = fdt_setprop_u32(blob, trip, "temperature", temp);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  int ft_system_setup(void *blob, bd_t *bd)
>  {
>  #ifdef CONFIG_IMX8MQ
> @@ -1128,6 +1171,13 @@ usb_modify_speed:
>                 disable_cpu_nodes(blob, 2);
>  #endif
>
> +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> +               printf("Failed to update cpu-thermal trip(s)");
> +#ifdef CONFIG_IMX8MP
> +       if (fixup_thermal_trips(blob, "soc-thermal"))
> +               printf("Failed to update soc-thermal trip(s)");
> +#endif
> +
>         return ft_add_optee_node(blob, bd);
>  }
>  #endif
> --
> 2.34.1
>

Re: [PATCH] imx8m: fixup thermal trips

[Andrejs Cainikovs]

Hi Adam,

On Wed, 2022-05-25 at 06:41 -0500, Adam Ford wrote:
> On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
> <andrejs.cainikovs@toradex.com> wrote:
> > 
> > Fixup thermal trips in Linux device tree according to SoC thermal
> > grade.
> > 
> > Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
> > ---
> >  arch/arm/mach-imx/imx8m/soc.c | 50
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > imx/imx8m/soc.c
> > index e93ecd2846..b451ece91f 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob, u32
> > disabled_cores)
> >         return 0;
> >  }
> > 
> > +int fixup_thermal_trips(void *blob, const char *name)
> 
> Is there any reason this function cannot be static?

No, this function should be static indeed.

> 
> > +{
> > +       int minc, maxc;
> > +       int node, trip;
> > +
> > +       node = fdt_path_offset(blob, "/thermal-zones");
> > +       if (node < 0)
> > +               return node;
> > +
> > +       node = fdt_subnode_offset(blob, node, name);
> > +       if (node < 0)
> > +               return node;
> > +
> > +       node = fdt_subnode_offset(blob, node, "trips");
> > +       if (node < 0)
> > +               return node;
> > +
> > +       get_cpu_temp_grade(&minc, &maxc);
> > +
> > +       fdt_for_each_subnode(trip, blob, node) {
> > +               const char *type;
> > +               int temp, ret;
> > +
> > +               type = fdt_getprop(blob, trip, "type", NULL);
> > +               if (!type)
> > +                       continue;
> > +
> > +               temp = 0;
> > +               if (!strcmp(type, "critical")) {
> > +                       temp = 1000 * maxc;
> > +               } else if (!strcmp(type, "passive")) {
> > +                       temp = 1000 * (maxc - 10);
> > +               }
> > +               if (temp) {
> > +                       ret = fdt_setprop_u32(blob, trip,
> > "temperature", temp);
> > +                       if (ret)
> > +                               return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  int ft_system_setup(void *blob, bd_t *bd)
> >  {
> >  #ifdef CONFIG_IMX8MQ
> > @@ -1128,6 +1171,13 @@ usb_modify_speed:
> >                 disable_cpu_nodes(blob, 2);
> >  #endif
> > 
> > +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> > +               printf("Failed to update cpu-thermal trip(s)");
> > +#ifdef CONFIG_IMX8MP
> > +       if (fixup_thermal_trips(blob, "soc-thermal"))
> > +               printf("Failed to update soc-thermal trip(s)");
> > +#endif
> > +
> >         return ft_add_optee_node(blob, bd);
> >  }
> >  #endif
> > --
> > 2.34.1
> > 
> 

Re: [PATCH] imx8m: fixup thermal trips

[Adam Ford]

On Wed, May 25, 2022 at 7:19 AM Andrejs Cainikovs
<andrejs.cainikovs@toradex.com> wrote:
>
> Hi Adam,
>
> On Wed, 2022-05-25 at 06:41 -0500, Adam Ford wrote:
> > On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
> > <andrejs.cainikovs@toradex.com> wrote:
> > >
> > > Fixup thermal trips in Linux device tree according to SoC thermal
> > > grade.
> > >
> > > Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
> > > ---
> > >  arch/arm/mach-imx/imx8m/soc.c | 50
> > > +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >
> > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > > imx/imx8m/soc.c
> > > index e93ecd2846..b451ece91f 100644
> > > --- a/arch/arm/mach-imx/imx8m/soc.c
> > > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > > @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob, u32
> > > disabled_cores)
> > >         return 0;
> > >  }
> > >
> > > +int fixup_thermal_trips(void *blob, const char *name)
> >
> > Is there any reason this function cannot be static?
>
> No, this function should be static indeed.

With that change:

Tested-by: Adam Ford <aford173@gmail.com>

>
> >
> > > +{
> > > +       int minc, maxc;
> > > +       int node, trip;
> > > +
> > > +       node = fdt_path_offset(blob, "/thermal-zones");
> > > +       if (node < 0)
> > > +               return node;
> > > +
> > > +       node = fdt_subnode_offset(blob, node, name);
> > > +       if (node < 0)
> > > +               return node;
> > > +
> > > +       node = fdt_subnode_offset(blob, node, "trips");
> > > +       if (node < 0)
> > > +               return node;
> > > +
> > > +       get_cpu_temp_grade(&minc, &maxc);
> > > +
> > > +       fdt_for_each_subnode(trip, blob, node) {
> > > +               const char *type;
> > > +               int temp, ret;
> > > +
> > > +               type = fdt_getprop(blob, trip, "type", NULL);
> > > +               if (!type)
> > > +                       continue;
> > > +
> > > +               temp = 0;
> > > +               if (!strcmp(type, "critical")) {
> > > +                       temp = 1000 * maxc;
> > > +               } else if (!strcmp(type, "passive")) {
> > > +                       temp = 1000 * (maxc - 10);
> > > +               }
> > > +               if (temp) {
> > > +                       ret = fdt_setprop_u32(blob, trip,
> > > "temperature", temp);
> > > +                       if (ret)
> > > +                               return ret;
> > > +               }
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  int ft_system_setup(void *blob, bd_t *bd)
> > >  {
> > >  #ifdef CONFIG_IMX8MQ
> > > @@ -1128,6 +1171,13 @@ usb_modify_speed:
> > >                 disable_cpu_nodes(blob, 2);
> > >  #endif
> > >
> > > +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> > > +               printf("Failed to update cpu-thermal trip(s)");
> > > +#ifdef CONFIG_IMX8MP
> > > +       if (fixup_thermal_trips(blob, "soc-thermal"))
> > > +               printf("Failed to update soc-thermal trip(s)");
> > > +#endif
> > > +
> > >         return ft_add_optee_node(blob, bd);
> > >  }
> > >  #endif
> > > --
> > > 2.34.1
> > >
> >
>

Re: [PATCH] imx8m: fixup thermal trips

[Andrejs Cainikovs]

Adam,

So you want me to rebase this patch to upstream, update according to
your review, and send this one instead of yours?

Best regards,
Andrejs Cainikovs.

On Wed, 2022-05-25 at 07:21 -0500, Adam Ford wrote:
> On Wed, May 25, 2022 at 7:19 AM Andrejs Cainikovs
> <andrejs.cainikovs@toradex.com> wrote:
> > 
> > Hi Adam,
> > 
> > On Wed, 2022-05-25 at 06:41 -0500, Adam Ford wrote:
> > > On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
> > > <andrejs.cainikovs@toradex.com> wrote:
> > > > 
> > > > Fixup thermal trips in Linux device tree according to SoC
> > > > thermal
> > > > grade.
> > > > 
> > > > Signed-off-by: Andrejs Cainikovs
> > > > <andrejs.cainikovs@toradex.com>
> > > > ---
> > > >  arch/arm/mach-imx/imx8m/soc.c | 50
> > > > +++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 50 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > > > imx/imx8m/soc.c
> > > > index e93ecd2846..b451ece91f 100644
> > > > --- a/arch/arm/mach-imx/imx8m/soc.c
> > > > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > > > @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob,
> > > > u32
> > > > disabled_cores)
> > > >         return 0;
> > > >  }
> > > > 
> > > > +int fixup_thermal_trips(void *blob, const char *name)
> > > 
> > > Is there any reason this function cannot be static?
> > 
> > No, this function should be static indeed.
> 
> With that change:
> 
> Tested-by: Adam Ford <aford173@gmail.com>
> 
> > 
> > > 
> > > > +{
> > > > +       int minc, maxc;
> > > > +       int node, trip;
> > > > +
> > > > +       node = fdt_path_offset(blob, "/thermal-zones");
> > > > +       if (node < 0)
> > > > +               return node;
> > > > +
> > > > +       node = fdt_subnode_offset(blob, node, name);
> > > > +       if (node < 0)
> > > > +               return node;
> > > > +
> > > > +       node = fdt_subnode_offset(blob, node, "trips");
> > > > +       if (node < 0)
> > > > +               return node;
> > > > +
> > > > +       get_cpu_temp_grade(&minc, &maxc);
> > > > +
> > > > +       fdt_for_each_subnode(trip, blob, node) {
> > > > +               const char *type;
> > > > +               int temp, ret;
> > > > +
> > > > +               type = fdt_getprop(blob, trip, "type", NULL);
> > > > +               if (!type)
> > > > +                       continue;
> > > > +
> > > > +               temp = 0;
> > > > +               if (!strcmp(type, "critical")) {
> > > > +                       temp = 1000 * maxc;
> > > > +               } else if (!strcmp(type, "passive")) {
> > > > +                       temp = 1000 * (maxc - 10);
> > > > +               }
> > > > +               if (temp) {
> > > > +                       ret = fdt_setprop_u32(blob, trip,
> > > > "temperature", temp);
> > > > +                       if (ret)
> > > > +                               return ret;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  int ft_system_setup(void *blob, bd_t *bd)
> > > >  {
> > > >  #ifdef CONFIG_IMX8MQ
> > > > @@ -1128,6 +1171,13 @@ usb_modify_speed:
> > > >                 disable_cpu_nodes(blob, 2);
> > > >  #endif
> > > > 
> > > > +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> > > > +               printf("Failed to update cpu-thermal trip(s)");
> > > > +#ifdef CONFIG_IMX8MP
> > > > +       if (fixup_thermal_trips(blob, "soc-thermal"))
> > > > +               printf("Failed to update soc-thermal trip(s)");
> > > > +#endif
> > > > +
> > > >         return ft_add_optee_node(blob, bd);
> > > >  }
> > > >  #endif
> > > > --
> > > > 2.34.1
> > > > 
> > > 
> > 
> 

Re: [PATCH] imx8m: fixup thermal trips

[Adam Ford]

On Wed, May 25, 2022 at 7:24 AM Andrejs Cainikovs
<andrejs.cainikovs@toradex.com> wrote:
>
> Adam,
>
> So you want me to rebase this patch to upstream, update according to
> your review, and send this one instead of yours?

Go ahead.  Yours has all the requested improvements already applied.
I'm still playing catch up from being away for 2 weeks.

thanks

adam
>
> Best regards,
> Andrejs Cainikovs.
>
> On Wed, 2022-05-25 at 07:21 -0500, Adam Ford wrote:
> > On Wed, May 25, 2022 at 7:19 AM Andrejs Cainikovs
> > <andrejs.cainikovs@toradex.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > On Wed, 2022-05-25 at 06:41 -0500, Adam Ford wrote:
> > > > On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
> > > > <andrejs.cainikovs@toradex.com> wrote:
> > > > >
> > > > > Fixup thermal trips in Linux device tree according to SoC
> > > > > thermal
> > > > > grade.
> > > > >
> > > > > Signed-off-by: Andrejs Cainikovs
> > > > > <andrejs.cainikovs@toradex.com>
> > > > > ---
> > > > >  arch/arm/mach-imx/imx8m/soc.c | 50
> > > > > +++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 50 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > > > > imx/imx8m/soc.c
> > > > > index e93ecd2846..b451ece91f 100644
> > > > > --- a/arch/arm/mach-imx/imx8m/soc.c
> > > > > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > > > > @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void *blob,
> > > > > u32
> > > > > disabled_cores)
> > > > >         return 0;
> > > > >  }
> > > > >
> > > > > +int fixup_thermal_trips(void *blob, const char *name)
> > > >
> > > > Is there any reason this function cannot be static?
> > >
> > > No, this function should be static indeed.
> >
> > With that change:
> >
> > Tested-by: Adam Ford <aford173@gmail.com>
> >
> > >
> > > >
> > > > > +{
> > > > > +       int minc, maxc;
> > > > > +       int node, trip;
> > > > > +
> > > > > +       node = fdt_path_offset(blob, "/thermal-zones");
> > > > > +       if (node < 0)
> > > > > +               return node;
> > > > > +
> > > > > +       node = fdt_subnode_offset(blob, node, name);
> > > > > +       if (node < 0)
> > > > > +               return node;
> > > > > +
> > > > > +       node = fdt_subnode_offset(blob, node, "trips");
> > > > > +       if (node < 0)
> > > > > +               return node;
> > > > > +
> > > > > +       get_cpu_temp_grade(&minc, &maxc);
> > > > > +
> > > > > +       fdt_for_each_subnode(trip, blob, node) {
> > > > > +               const char *type;
> > > > > +               int temp, ret;
> > > > > +
> > > > > +               type = fdt_getprop(blob, trip, "type", NULL);
> > > > > +               if (!type)
> > > > > +                       continue;
> > > > > +
> > > > > +               temp = 0;
> > > > > +               if (!strcmp(type, "critical")) {
> > > > > +                       temp = 1000 * maxc;
> > > > > +               } else if (!strcmp(type, "passive")) {
> > > > > +                       temp = 1000 * (maxc - 10);
> > > > > +               }
> > > > > +               if (temp) {
> > > > > +                       ret = fdt_setprop_u32(blob, trip,
> > > > > "temperature", temp);
> > > > > +                       if (ret)
> > > > > +                               return ret;
> > > > > +               }
> > > > > +       }
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > +
> > > > >  int ft_system_setup(void *blob, bd_t *bd)
> > > > >  {
> > > > >  #ifdef CONFIG_IMX8MQ
> > > > > @@ -1128,6 +1171,13 @@ usb_modify_speed:
> > > > >                 disable_cpu_nodes(blob, 2);
> > > > >  #endif
> > > > >
> > > > > +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> > > > > +               printf("Failed to update cpu-thermal trip(s)");
> > > > > +#ifdef CONFIG_IMX8MP
> > > > > +       if (fixup_thermal_trips(blob, "soc-thermal"))
> > > > > +               printf("Failed to update soc-thermal trip(s)");
> > > > > +#endif
> > > > > +
> > > > >         return ft_add_optee_node(blob, bd);
> > > > >  }
> > > > >  #endif
> > > > > --
> > > > > 2.34.1
> > > > >
> > > >
> > >
> >
>

Re: [PATCH] imx8m: fixup thermal trips

[Andrejs Cainikovs]

On Wed, 2022-05-25 at 07:26 -0500, Adam Ford wrote:
> On Wed, May 25, 2022 at 7:24 AM Andrejs Cainikovs
> <andrejs.cainikovs@toradex.com> wrote:
> > 
> > Adam,
> > 
> > So you want me to rebase this patch to upstream, update according
> > to
> > your review, and send this one instead of yours?
> 
> Go ahead.  Yours has all the requested improvements already applied.
> I'm still playing catch up from being away for 2 weeks.

Ok, will do. Thanks.

> 
> thanks
> 
> adam
> > 
> > Best regards,
> > Andrejs Cainikovs.
> > 
> > On Wed, 2022-05-25 at 07:21 -0500, Adam Ford wrote:
> > > On Wed, May 25, 2022 at 7:19 AM Andrejs Cainikovs
> > > <andrejs.cainikovs@toradex.com> wrote:
> > > > 
> > > > Hi Adam,
> > > > 
> > > > On Wed, 2022-05-25 at 06:41 -0500, Adam Ford wrote:
> > > > > On Thu, May 12, 2022 at 5:13 AM Andrejs Cainikovs
> > > > > <andrejs.cainikovs@toradex.com> wrote:
> > > > > > 
> > > > > > Fixup thermal trips in Linux device tree according to SoC
> > > > > > thermal
> > > > > > grade.
> > > > > > 
> > > > > > Signed-off-by: Andrejs Cainikovs
> > > > > > <andrejs.cainikovs@toradex.com>
> > > > > > ---
> > > > > >  arch/arm/mach-imx/imx8m/soc.c | 50
> > > > > > +++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 50 insertions(+)
> > > > > > 
> > > > > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > > > > > imx/imx8m/soc.c
> > > > > > index e93ecd2846..b451ece91f 100644
> > > > > > --- a/arch/arm/mach-imx/imx8m/soc.c
> > > > > > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > > > > > @@ -1001,6 +1001,49 @@ static int disable_cpu_nodes(void
> > > > > > *blob,
> > > > > > u32
> > > > > > disabled_cores)
> > > > > >         return 0;
> > > > > >  }
> > > > > > 
> > > > > > +int fixup_thermal_trips(void *blob, const char *name)
> > > > > 
> > > > > Is there any reason this function cannot be static?
> > > > 
> > > > No, this function should be static indeed.
> > > 
> > > With that change:
> > > 
> > > Tested-by: Adam Ford <aford173@gmail.com>
> > > 
> > > > 
> > > > > 
> > > > > > +{
> > > > > > +       int minc, maxc;
> > > > > > +       int node, trip;
> > > > > > +
> > > > > > +       node = fdt_path_offset(blob, "/thermal-zones");
> > > > > > +       if (node < 0)
> > > > > > +               return node;
> > > > > > +
> > > > > > +       node = fdt_subnode_offset(blob, node, name);
> > > > > > +       if (node < 0)
> > > > > > +               return node;
> > > > > > +
> > > > > > +       node = fdt_subnode_offset(blob, node, "trips");
> > > > > > +       if (node < 0)
> > > > > > +               return node;
> > > > > > +
> > > > > > +       get_cpu_temp_grade(&minc, &maxc);
> > > > > > +
> > > > > > +       fdt_for_each_subnode(trip, blob, node) {
> > > > > > +               const char *type;
> > > > > > +               int temp, ret;
> > > > > > +
> > > > > > +               type = fdt_getprop(blob, trip, "type",
> > > > > > NULL);
> > > > > > +               if (!type)
> > > > > > +                       continue;
> > > > > > +
> > > > > > +               temp = 0;
> > > > > > +               if (!strcmp(type, "critical")) {
> > > > > > +                       temp = 1000 * maxc;
> > > > > > +               } else if (!strcmp(type, "passive")) {
> > > > > > +                       temp = 1000 * (maxc - 10);
> > > > > > +               }
> > > > > > +               if (temp) {
> > > > > > +                       ret = fdt_setprop_u32(blob, trip,
> > > > > > "temperature", temp);
> > > > > > +                       if (ret)
> > > > > > +                               return ret;
> > > > > > +               }
> > > > > > +       }
> > > > > > +
> > > > > > +       return 0;
> > > > > > +}
> > > > > > +
> > > > > >  int ft_system_setup(void *blob, bd_t *bd)
> > > > > >  {
> > > > > >  #ifdef CONFIG_IMX8MQ
> > > > > > @@ -1128,6 +1171,13 @@ usb_modify_speed:
> > > > > >                 disable_cpu_nodes(blob, 2);
> > > > > >  #endif
> > > > > > 
> > > > > > +       if (fixup_thermal_trips(blob, "cpu-thermal"))
> > > > > > +               printf("Failed to update cpu-thermal
> > > > > > trip(s)");
> > > > > > +#ifdef CONFIG_IMX8MP
> > > > > > +       if (fixup_thermal_trips(blob, "soc-thermal"))
> > > > > > +               printf("Failed to update soc-thermal
> > > > > > trip(s)");
> > > > > > +#endif
> > > > > > +
> > > > > >         return ft_add_optee_node(blob, bd);
> > > > > >  }
> > > > > >  #endif
> > > > > > --
> > > > > > 2.34.1
> > > > > > 
> > > > > 
> > > > 
> > > 
> > 
>