* [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
@ 2018-11-13 6:45 Nguyen An Hoan
2018-11-13 8:02 ` Geert Uytterhoeven
2018-12-11 7:42 ` Geert Uytterhoeven
0 siblings, 2 replies; 7+ messages in thread
From: Nguyen An Hoan @ 2018-11-13 6:45 UTC (permalink / raw)
To: linux-renesas-soc, geert+renesas
Cc: wsa, rui.zhang, niklas.soderlund, kuninori.morimoto.gx,
yoshihiro.shimoda.uh, h-inayoshi, nv-dung, na-hoan, cv-dong
From: Hoan Nguyen An <na-hoan@jinso.co.jp>
Gen3 thermal registered by devm_thermal_zone_of_sensor_register()
and this function does not enable hwmon sysfs extensions.
This patch enables it to keep compatibility to common systems
Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
drivers/thermal/rcar_gen3_thermal.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 75786cc..ae172db 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -19,6 +19,7 @@
#include <linux/thermal.h>
#include "thermal_core.h"
+#include "thermal_hwmon.h"
/* Register offsets */
#define REG_GEN3_IRQSTR 0x04
@@ -429,6 +430,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
if (ret < 0)
goto error_unregister;
+ /* Enable hwmon thermal sysfs */
+ tsc->zone->tzp->no_hwmon = false;
+ ret = thermal_add_hwmon_sysfs(tsc->zone);
+ if (ret)
+ dev_err(dev, "Can't register hwmon sysfs\n");
+
dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
2018-11-13 6:45 [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs Nguyen An Hoan
@ 2018-11-13 8:02 ` Geert Uytterhoeven
2018-11-13 11:57 ` Wolfram Sang
2018-12-11 7:42 ` Geert Uytterhoeven
1 sibling, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2018-11-13 8:02 UTC (permalink / raw)
To: Hoan Nguyen An
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
カオ・ヴァン・ドン,
Eduardo Valentin, Daniel Lezcano, Linux PM list
Hi Hoan,
Thanks for your patch!
Please run scripts/get_maintainer.pl on your patches, to get the list of
maintainers.
CC thermal
On Tue, Nov 13, 2018 at 7:46 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
>
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Gen3 thermal registered by devm_thermal_zone_of_sensor_register()
> and this function does not enable hwmon sysfs extensions.
> This patch enables it to keep compatibility to common systems
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
> drivers/thermal/rcar_gen3_thermal.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 75786cc..ae172db 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -19,6 +19,7 @@
> #include <linux/thermal.h>
>
> #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>
> /* Register offsets */
> #define REG_GEN3_IRQSTR 0x04
> @@ -429,6 +430,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_unregister;
>
> + /* Enable hwmon thermal sysfs */
> + tsc->zone->tzp->no_hwmon = false;
> + ret = thermal_add_hwmon_sysfs(tsc->zone);
> + if (ret)
> + dev_err(dev, "Can't register hwmon sysfs\n");
> +
> dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> }
>
You forgot to call thermal_remove_hwmon_sysfs() in the .remove()
callback?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
2018-11-13 8:02 ` Geert Uytterhoeven
@ 2018-11-13 11:57 ` Wolfram Sang
2018-12-04 9:43 ` Hoan
0 siblings, 1 reply; 7+ messages in thread
From: Wolfram Sang @ 2018-11-13 11:57 UTC (permalink / raw)
To: Geert Uytterhoeven, Hoan Nguyen An
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
カオ・ヴァン・ドン,
Eduardo Valentin, Daniel Lezcano, Linux PM list
[-- Attachment #1: Type: text/plain, Size: 889 bytes --]
> Please run scripts/get_maintainer.pl on your patches, to get the list of
> maintainers.
>
> CC thermal
Thanks, Geert!
Hi Hoan,
> + /* Enable hwmon thermal sysfs */
> + tsc->zone->tzp->no_hwmon = false;
A driver diving so deep into core structures always looks suspicious.
Questions I have:
1)
We have that code also in rcar_thermal.c, but the commit introducing
it[1] has a reason which does not apply for this driver, or?
2)
of_parse_thermal_zones() intentionally sets this flag with this
comment:
/* No hwmon because there might be hwmon drivers registering */
Why does it not apply for us?
3)
If it turns out, we really need it, there should be some thermal core
helper or flag for it, I'd think...
Regards,
Wolfram
[1] 64a411e8042e ("thermal: rcar-thermal: enable hwmon when thermal_zone_of_sensor_register is used")
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
2018-11-13 11:57 ` Wolfram Sang
@ 2018-12-04 9:43 ` Hoan
0 siblings, 0 replies; 7+ messages in thread
From: Hoan @ 2018-12-04 9:43 UTC (permalink / raw)
To: Wolfram Sang, Geert Uytterhoeven
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
カオ・ヴァン・ドン,
Eduardo Valentin, Daniel Lezcano, Linux PM list
Dear Geert-san, Wolfram-san
I'm sorry for the delay at the comments of this patch.
And I think it's not hurry!
> + /* Enable hwmon thermal sysfs */
> + tsc->zone->tzp->no_hwmon = false;
> + ret = thermal_add_hwmon_sysfs(tsc->zone);
> + if (ret)
> + dev_err(dev, "Can't register hwmon sysfs\n");
> +
> dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> }
>
> You forgot to call thermal_remove_hwmon_sysfs() in the .remove()
> callback?
I got it, Thanks!
On 2018/11/13 20:57, Wolfram Sang wrote:
> Hi Hoan,
>
>> + /* Enable hwmon thermal sysfs */
>> + tsc->zone->tzp->no_hwmon = false;
> A driver diving so deep into core structures always looks suspicious.
> Questions I have:
>
> 1)
>
> We have that code also in rcar_thermal.c, but the commit introducing
> it[1] has a reason which does not apply for this driver, or?
This is not the reason to apply this patch.
I would say here are the general Linux systems, or as if the systems
that thermal was registered by "thermal_zone_device_register()".
The Linux community has developed tools that use this, such as
"lm-sensors",
my Ubuntu and rcar-Gen2 are still compatible with lm-sensors but Gen3
does not.
Although I do not want to say paradoxical that the kernel drivers must
be compatible
with the user-space applications.
> 2)
>
> of_parse_thermal_zones() intentionally sets this flag with this
> comment:
>
> /* No hwmon because there might be hwmon drivers registering */
>
> Why does it not apply for us?
I think so it should be registered separately.
> 3)
>
> If it turns out, we really need it, there should be some thermal core
> helper or flag for it, I'd think...
Yes, It up to you!
> Regards,
>
> Wolfram
>
> [1] 64a411e8042e ("thermal: rcar-thermal: enable hwmon when thermal_zone_of_sensor_register is used")
>
Thank you very much for your comments :-)!
Hoan.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
@ 2018-12-11 7:42 ` Geert Uytterhoeven
0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2018-12-11 7:42 UTC (permalink / raw)
To: Hoan Nguyen An
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
Hoan Nguyen An,
カオ・ヴァン・ドン,
Marek Vasut
CCing Marek, since he was recently looking into the driver
On Tue, Nov 13, 2018 at 7:46 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
>
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Gen3 thermal registered by devm_thermal_zone_of_sensor_register()
> and this function does not enable hwmon sysfs extensions.
> This patch enables it to keep compatibility to common systems
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
> drivers/thermal/rcar_gen3_thermal.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 75786cc..ae172db 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -19,6 +19,7 @@
> #include <linux/thermal.h>
>
> #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>
> /* Register offsets */
> #define REG_GEN3_IRQSTR 0x04
> @@ -429,6 +430,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_unregister;
>
> + /* Enable hwmon thermal sysfs */
> + tsc->zone->tzp->no_hwmon = false;
> + ret = thermal_add_hwmon_sysfs(tsc->zone);
> + if (ret)
> + dev_err(dev, "Can't register hwmon sysfs\n");
> +
> dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
@ 2018-12-11 7:42 ` Geert Uytterhoeven
0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2018-12-11 7:42 UTC (permalink / raw)
To: Hoan Nguyen An
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
カオ・ヴァン・ドン,
Marek Vasut
CCing Marek, since he was recently looking into the driver
On Tue, Nov 13, 2018 at 7:46 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
>
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Gen3 thermal registered by devm_thermal_zone_of_sensor_register()
> and this function does not enable hwmon sysfs extensions.
> This patch enables it to keep compatibility to common systems
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
> drivers/thermal/rcar_gen3_thermal.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 75786cc..ae172db 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -19,6 +19,7 @@
> #include <linux/thermal.h>
>
> #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>
> /* Register offsets */
> #define REG_GEN3_IRQSTR 0x04
> @@ -429,6 +430,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_unregister;
>
> + /* Enable hwmon thermal sysfs */
> + tsc->zone->tzp->no_hwmon = false;
> + ret = thermal_add_hwmon_sysfs(tsc->zone);
> + if (ret)
> + dev_err(dev, "Can't register hwmon sysfs\n");
> +
> dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs
2018-12-11 7:42 ` Geert Uytterhoeven
(?)
@ 2018-12-12 1:52 ` Marek Vasut
-1 siblings, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2018-12-12 1:52 UTC (permalink / raw)
To: Geert Uytterhoeven, Hoan Nguyen An
Cc: Linux-Renesas, Geert Uytterhoeven, Wolfram Sang, Zhang Rui,
Niklas Söderlund, Kuninori Morimoto, Yoshihiro Shimoda,
稲吉, Dung:人ソ,
カオ・ヴァン・ドン
On 12/11/2018 08:42 AM, Geert Uytterhoeven wrote:
> CCing Marek, since he was recently looking into the driver
>
> On Tue, Nov 13, 2018 at 7:46 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
>>
>> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>>
>> Gen3 thermal registered by devm_thermal_zone_of_sensor_register()
>> and this function does not enable hwmon sysfs extensions.
>> This patch enables it to keep compatibility to common systems
>>
>> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
>> ---
>> drivers/thermal/rcar_gen3_thermal.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
>> index 75786cc..ae172db 100644
>> --- a/drivers/thermal/rcar_gen3_thermal.c
>> +++ b/drivers/thermal/rcar_gen3_thermal.c
>> @@ -19,6 +19,7 @@
>> #include <linux/thermal.h>
>>
>> #include "thermal_core.h"
>> +#include "thermal_hwmon.h"
>>
>> /* Register offsets */
>> #define REG_GEN3_IRQSTR 0x04
>> @@ -429,6 +430,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>> if (ret < 0)
>> goto error_unregister;
>>
>> + /* Enable hwmon thermal sysfs */
>> + tsc->zone->tzp->no_hwmon = false;
>> + ret = thermal_add_hwmon_sysfs(tsc->zone);
>> + if (ret)
>> + dev_err(dev, "Can't register hwmon sysfs\n");
>> +
>> dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
>> }
I submitted the following series [1], I think this might be a more
systematic approach to this topic than digging deep into tz structures.
[1] https://patchwork.kernel.org/cover/10725477/
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-12-12 2:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 6:45 [PATCH] thermal: rcar_gen3_thermal: Add supports the hwmon thermal sysfs Nguyen An Hoan
2018-11-13 8:02 ` Geert Uytterhoeven
2018-11-13 11:57 ` Wolfram Sang
2018-12-04 9:43 ` Hoan
2018-12-11 7:42 ` Geert Uytterhoeven
2018-12-11 7:42 ` Geert Uytterhoeven
2018-12-12 1:52 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.