All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device
@ 2018-10-03  7:25 Cao Van Dong
  2018-10-03  7:35 ` Geert Uytterhoeven
  2018-10-03  8:07 ` Marc Zyngier
  0 siblings, 2 replies; 5+ messages in thread
From: Cao Van Dong @ 2018-10-03  7:25 UTC (permalink / raw)
  To: broonie, geert+renesas, linux-pm
  Cc: marc.zyngier, kuninori.morimoto.gx, yoshihiro.shimoda.uh,
	linux-renesas-soc, h-inayoshi, nv-dung, na-hoan, cv-dong

Formerly, when registering the hwmon device, we pass NULL as the device. It's not a problem.
Recently, the developer has replaced the parameter NULL as the device by the actual device.
This causes the "No sensors found" error. Therefore, instead of using the device we will use pass
the parent of that device as parameter. This will sync with the processor on the hwmon core.
This patch is to fix this error.

This patch is based on the v4.19-rc3 tag.

---
 drivers/thermal/thermal_hwmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 40c69a5..a918ba9 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -143,7 +143,7 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
 	INIT_LIST_HEAD(&hwmon->tz_list);
 	strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
 	strreplace(hwmon->type, '-', '_');
-	hwmon->device = hwmon_device_register_with_info(&tz->device, hwmon->type,
+	hwmon->device = hwmon_device_register_with_info(tz->device.parent, hwmon->type,
 							hwmon, NULL, NULL);
 	if (IS_ERR(hwmon->device)) {
 		result = PTR_ERR(hwmon->device);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device
  2018-10-03  7:25 [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device Cao Van Dong
@ 2018-10-03  7:35 ` Geert Uytterhoeven
  2018-10-03  8:07 ` Marc Zyngier
  1 sibling, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2018-10-03  7:35 UTC (permalink / raw)
  To: カオ・ヴァン・ドン
  Cc: Mark Brown, Geert Uytterhoeven, Linux PM list, Marc Zyngier,
	Kuninori Morimoto, Yoshihiro Shimoda, Linux-Renesas,
	稲吉, Dung:人ソ,
	Hoan Nguyen An, Eduardo Valentin, Enric Balletbo i Serra,
	Guenter Roeck

CC Eduardo, Enric, Günter,

On Wed, Oct 3, 2018 at 9:27 AM Cao Van Dong <cv-dong@jinso.co.jp> wrote:
> Formerly, when registering the hwmon device, we pass NULL as the device. It's not a problem.
> Recently, the developer has replaced the parameter NULL as the device by the actual device.
> This causes the "No sensors found" error. Therefore, instead of using the device we will use pass
> the parent of that device as parameter. This will sync with the processor on the hwmon core.
> This patch is to fix this error.
>
> This patch is based on the v4.19-rc3 tag.

Fixes: f6b6b52ef7a54160 ("thermal_hwmon: Pass the originating device
down to hwmon_device_register_with_info")

> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -143,7 +143,7 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>         INIT_LIST_HEAD(&hwmon->tz_list);
>         strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
>         strreplace(hwmon->type, '-', '_');
> -       hwmon->device = hwmon_device_register_with_info(&tz->device, hwmon->type,
> +       hwmon->device = hwmon_device_register_with_info(tz->device.parent, hwmon->type,
>                                                         hwmon, NULL, NULL);
>         if (IS_ERR(hwmon->device)) {
>                 result = PTR_ERR(hwmon->device);

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] 5+ messages in thread

* Re: [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device
  2018-10-03  7:25 [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device Cao Van Dong
  2018-10-03  7:35 ` Geert Uytterhoeven
@ 2018-10-03  8:07 ` Marc Zyngier
  2018-10-04  1:29   ` Cao Van Dong
  1 sibling, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2018-10-03  8:07 UTC (permalink / raw)
  To: Cao Van Dong, broonie, geert+renesas, linux-pm
  Cc: kuninori.morimoto.gx, yoshihiro.shimoda.uh, linux-renesas-soc,
	h-inayoshi, nv-dung, na-hoan, Eduardo Valentin, Guenter Roeck,
	Enric Balletbo i Serra


On 03/10/18 08:25, Cao Van Dong wrote:
> Formerly, when registering the hwmon device, we pass NULL as the device. It's not a problem.
> Recently, the developer has replaced the parameter NULL as the device by the actual device.
> This causes the "No sensors found" error. Therefore, instead of using the device we will use pass

What does report this error? Is it a userspace application?

> the parent of that device as parameter. This will sync with the processor on the hwmon core.
> This patch is to fix this error.
> 
> This patch is based on the v4.19-rc3 tag.
> 
> ---

This patch has no SoB.

>   drivers/thermal/thermal_hwmon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index 40c69a5..a918ba9 100644
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -143,7 +143,7 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>   	INIT_LIST_HEAD(&hwmon->tz_list);
>   	strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
>   	strreplace(hwmon->type, '-', '_');
> -	hwmon->device = hwmon_device_register_with_info(&tz->device, hwmon->type,
> +	hwmon->device = hwmon_device_register_with_info(tz->device.parent, hwmon->type,
>   							hwmon, NULL, NULL);
>   	if (IS_ERR(hwmon->device)) {
>   		result = PTR_ERR(hwmon->device);
> 

It is not clear to me that this is any better. What is the parent device 
in this case? Can you give an example of what breaks in the hierarchy?

Given how close we are to to 4.19, I'd rather we revert f6b6b52ef7a54160 
if there are userspace visible regressions.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device
  2018-10-03  8:07 ` Marc Zyngier
@ 2018-10-04  1:29   ` Cao Van Dong
  2018-10-04 16:56     ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Cao Van Dong @ 2018-10-04  1:29 UTC (permalink / raw)
  To: Marc Zyngier, broonie, geert+renesas, linux-pm
  Cc: kuninori.morimoto.gx, yoshihiro.shimoda.uh, linux-renesas-soc,
	h-inayoshi, nv-dung, na-hoan, Eduardo Valentin, Guenter Roeck,
	Enric Balletbo i Serra

[-- Attachment #1: Type: text/plain, Size: 2990 bytes --]

Dear Marc-san,

Thanks for your comment!

>> Formerly, when registering the hwmon device, we pass NULL as the 
>> device. It's not a problem.
>> Recently, the developer has replaced the parameter NULL as the device 
>> by the actual device.
>> This causes the "No sensors found" error. Therefore, instead of using 
>> the device we will use pass
>
> What does report this error? Is it a userspace application?

Below are the comments of Guente-san:
  " The "No sensors found" message is from the "sensors" application.
    The device type associated with the passed device is unknown to
    libsensors (it is parsed from the parent device subsystem name -
    what is that, anyway ?). This could be addressed in libsensors,
    or the thermal subsystem could attach itself to a known subsystem,
    or the thermal subsystem must pass a pointer the parent device. "

I think the problem is the thermal subsystem must pass a pointer the 
parent device.

>> the parent of that device as parameter. This will sync with the 
>> processor on the hwmon core.
>> This patch is to fix this error.
>>
>> This patch is based on the v4.19-rc3 tag.
>>
>> ---
>
> This patch has no SoB.

I will add SoB later.

>>   drivers/thermal/thermal_hwmon.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/thermal_hwmon.c 
>> b/drivers/thermal/thermal_hwmon.c
>> index 40c69a5..a918ba9 100644
>> --- a/drivers/thermal/thermal_hwmon.c
>> +++ b/drivers/thermal/thermal_hwmon.c
>> @@ -143,7 +143,7 @@ int thermal_add_hwmon_sysfs(struct 
>> thermal_zone_device *tz)
>>       INIT_LIST_HEAD(&hwmon->tz_list);
>>       strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
>>       strreplace(hwmon->type, '-', '_');
>> -    hwmon->device = hwmon_device_register_with_info(&tz->device, 
>> hwmon->type,
>> +    hwmon->device = 
>> hwmon_device_register_with_info(tz->device.parent, hwmon->type,
>>                               hwmon, NULL, NULL);
>>       if (IS_ERR(hwmon->device)) {
>>           result = PTR_ERR(hwmon->device);
>>
>
> It is not clear to me that this is any better. What is the parent 
> device in this case? Can you give an example of what breaks in the 
> hierarchy?
>
> Given how close we are to to 4.19, I'd rather we revert 
> f6b6b52ef7a54160 if there are userspace visible regressions.
>

Initially, I considered your patch to be correct and went to look at the 
processor in the hwmon core. Then I created the v1 patch.
After patch v1, I noticed that it is necessary to check for cases where 
the device has parent and if there is no parent.
However, when looking at function hwmon_device_register_with_info() and 
its header, I see that the device parameter passed in
must be the parent device ("* @dev: the parent device"). Therefore, I 
think it's best to pass a parent device parameter to this function.
So, I decided to create this v2 patch.


Regards,
Dong/Jinso



[-- Attachment #2: Type: text/html, Size: 6006 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device
  2018-10-04  1:29   ` Cao Van Dong
@ 2018-10-04 16:56     ` Guenter Roeck
  0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2018-10-04 16:56 UTC (permalink / raw)
  To: Cao Van Dong
  Cc: Marc Zyngier, broonie, geert+renesas, linux-pm,
	kuninori.morimoto.gx, yoshihiro.shimoda.uh, linux-renesas-soc,
	h-inayoshi, nv-dung, na-hoan, Eduardo Valentin,
	Enric Balletbo i Serra

On Thu, Oct 04, 2018 at 10:29:19AM +0900, Cao Van Dong wrote:
> Dear Marc-san,
> 
> Thanks for your comment!
> 
> >>Formerly, when registering the hwmon device, we pass NULL as the device.
> >>It's not a problem.
> >>Recently, the developer has replaced the parameter NULL as the device by
> >>the actual device.
> >>This causes the "No sensors found" error. Therefore, instead of using
> >>the device we will use pass
> >
> >What does report this error? Is it a userspace application?
> 
> Below are the comments of Guente-san:
> �" The "No sensors found" message is from the "sensors" application.
> �� The device type associated with the passed device is unknown to
> �� libsensors (it is parsed from the parent device subsystem name -
> �� what is that, anyway ?). This could be addressed in libsensors,
> �� or the thermal subsystem could attach itself to a known subsystem,
> �� or the thermal subsystem must pass a pointer the parent device. "
> 
> I think the problem is the thermal subsystem must pass a pointer the parent
> device.
> 
Thinking about this more ...

We only know that passing the parent device works for your use case.
We do not know if parent device types are well defined and recognized
as supported by libsensors. As such, passing the parent device as
parameter may not always work as intended.

Unfortunately, the only means to test or evaluate this would be to
run "sensors" on all affected systems, and/or to inspect the thermal
device source code to determine all parent device types.

A better solution would have been to ensure that the device pointer
passed to hwmon is recognized by libsensors, but unfortunately it
is a bit late for that.

Guenter

> >>the parent of that device as parameter. This will sync with the
> >>processor on the hwmon core.
> >>This patch is to fix this error.
> >>
> >>This patch is based on the v4.19-rc3 tag.
> >>
> >>---
> >
> >This patch has no SoB.
> 
> I will add SoB later.
> 
> >>� drivers/thermal/thermal_hwmon.c | 2 +-
> >>� 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/thermal/thermal_hwmon.c
> >>b/drivers/thermal/thermal_hwmon.c
> >>index 40c69a5..a918ba9 100644
> >>--- a/drivers/thermal/thermal_hwmon.c
> >>+++ b/drivers/thermal/thermal_hwmon.c
> >>@@ -143,7 +143,7 @@ int thermal_add_hwmon_sysfs(struct
> >>thermal_zone_device *tz)
> >>����� INIT_LIST_HEAD(&hwmon->tz_list);
> >>����� strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
> >>����� strreplace(hwmon->type, '-', '_');
> >>-��� hwmon->device = hwmon_device_register_with_info(&tz->device,
> >>hwmon->type,
> >>+��� hwmon->device = hwmon_device_register_with_info(tz->device.parent,
> >>hwmon->type,
> >>����������������������������� hwmon, NULL, NULL);
> >>����� if (IS_ERR(hwmon->device)) {
> >>��������� result = PTR_ERR(hwmon->device);
> >>
> >
> >It is not clear to me that this is any better. What is the parent device
> >in this case? Can you give an example of what breaks in the hierarchy?
> >
> >Given how close we are to to 4.19, I'd rather we revert f6b6b52ef7a54160
> >if there are userspace visible regressions.
> >
> 
> Initially, I considered your patch to be correct and went to look at the
> processor in the hwmon core. Then I created the v1 patch.
> After patch v1, I noticed that it is necessary to check for cases where the
> device has parent and if there is no parent.
> However, when looking at function hwmon_device_register_with_info() and its
> header, I see that the device parameter passed in
> must be the parent device ("* @dev: the parent device"). Therefore, I think
> it's best to pass a parent device parameter to this function.
> So, I decided to create this v2 patch.
> 
> 
> Regards,
> Dong/Jinso
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-04 23:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03  7:25 [PATCH v2] thermal_hwmon: Fix the 'No sensors found' error after replacing the parameter NULL by the actual device Cao Van Dong
2018-10-03  7:35 ` Geert Uytterhoeven
2018-10-03  8:07 ` Marc Zyngier
2018-10-04  1:29   ` Cao Van Dong
2018-10-04 16:56     ` Guenter Roeck

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.