linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] hwmon: acpi_power_meter: fix style issue
@ 2022-05-02 12:42 Corentin Labbe
  2022-05-02 12:42 ` [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info Corentin Labbe
  0 siblings, 1 reply; 10+ messages in thread
From: Corentin Labbe @ 2022-05-02 12:42 UTC (permalink / raw)
  To: jdelvare, linux; +Cc: linux-hwmon, linux-kernel, Corentin Labbe

Fix style issues found by checkpatch.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/hwmon/acpi_power_meter.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index c405a5869581..d2545a1be9fc 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -481,7 +481,7 @@ static struct sensor_template meter_attrs[] = {
 	RO_SENSOR_TEMPLATE("power1_average_interval_max", show_val, 1),
 	RO_SENSOR_TEMPLATE("power1_is_battery", show_val, 5),
 	RW_SENSOR_TEMPLATE(POWER_AVG_INTERVAL_NAME, show_avg_interval,
-		set_avg_interval, 0),
+			   set_avg_interval, 0),
 	{},
 };
 
@@ -530,6 +530,7 @@ static void remove_domain_devices(struct acpi_power_meter_resource *resource)
 
 	for (i = 0; i < resource->num_domain_devices; i++) {
 		struct acpi_device *obj = resource->domain_devices[i];
+
 		if (!obj)
 			continue;
 
@@ -580,7 +581,7 @@ static int read_domain_devices(struct acpi_power_meter_resource *resource)
 	}
 
 	resource->holders_dir = kobject_create_and_add("measures",
-					&resource->acpi_dev->dev.kobj);
+						       &resource->acpi_dev->dev.kobj);
 	if (!resource->holders_dir) {
 		res = -ENOMEM;
 		goto exit_free;
@@ -590,7 +591,7 @@ static int read_domain_devices(struct acpi_power_meter_resource *resource)
 
 	for (i = 0; i < pss->package.count; i++) {
 		struct acpi_device *obj;
-		union acpi_object *element = &(pss->package.elements[i]);
+		union acpi_object *element = &pss->package.elements[i];
 
 		/* Refuse non-references */
 		if (element->type != ACPI_TYPE_LOCAL_REFERENCE)
@@ -603,7 +604,7 @@ static int read_domain_devices(struct acpi_power_meter_resource *resource)
 			continue;
 
 		res = sysfs_create_link(resource->holders_dir, &obj->dev.kobj,
-				      kobject_name(&obj->dev.kobj));
+					kobject_name(&obj->dev.kobj));
 		if (res) {
 			acpi_dev_put(obj);
 			resource->domain_devices[i] = NULL;
@@ -788,7 +789,7 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
 	str = &resource->model_number;
 
 	for (i = 11; i < 14; i++) {
-		union acpi_object *element = &(pss->package.elements[i]);
+		union acpi_object *element = &pss->package.elements[i];
 
 		if (element->type != ACPI_TYPE_STRING) {
 			res = -EINVAL;
@@ -868,8 +869,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	if (!device)
 		return -EINVAL;
 
-	resource = kzalloc(sizeof(struct acpi_power_meter_resource),
-			   GFP_KERNEL);
+	resource = kzalloc(sizeof(*resource), GFP_KERNEL);
 	if (!resource)
 		return -ENOMEM;
 
@@ -884,7 +884,8 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	if (res)
 		goto exit_free;
 
-	resource->trip[0] = resource->trip[1] = -1;
+	resource->trip[0] = -1;
+	resource->trip[1] = -1;
 
 	res = setup_attrs(resource);
 	if (res)
-- 
2.35.1


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

* [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 12:42 [PATCH 1/2] hwmon: acpi_power_meter: fix style issue Corentin Labbe
@ 2022-05-02 12:42 ` Corentin Labbe
  2022-05-02 13:34   ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Corentin Labbe @ 2022-05-02 12:42 UTC (permalink / raw)
  To: jdelvare, linux; +Cc: linux-hwmon, linux-kernel, Corentin Labbe

Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
So let's convert the driver to use hwmon_device_register_with_info().

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/hwmon/acpi_power_meter.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index d2545a1be9fc..98293727f980 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	if (res)
 		goto exit_free_capability;
 
-	resource->hwmon_dev = hwmon_device_register(&device->dev);
+	resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
+							      ACPI_POWER_METER_DEVICE_NAME,
+							      NULL, NULL,
+							      NULL);
 	if (IS_ERR(resource->hwmon_dev)) {
 		res = PTR_ERR(resource->hwmon_dev);
 		goto exit_remove;
-- 
2.35.1


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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 12:42 ` [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info Corentin Labbe
@ 2022-05-02 13:34   ` Guenter Roeck
  2022-05-02 18:59     ` LABBE Corentin
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2022-05-02 13:34 UTC (permalink / raw)
  To: Corentin Labbe, jdelvare; +Cc: linux-hwmon, linux-kernel

On 5/2/22 05:42, Corentin Labbe wrote:
> Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
> So let's convert the driver to use hwmon_device_register_with_info().
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>   drivers/hwmon/acpi_power_meter.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index d2545a1be9fc..98293727f980 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
>   	if (res)
>   		goto exit_free_capability;
>   
> -	resource->hwmon_dev = hwmon_device_register(&device->dev);
> +	resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
> +							      ACPI_POWER_METER_DEVICE_NAME,
> +							      NULL, NULL,
> +							      NULL);

NACK. That isn't a conversion to the new API, it just abuses the fact
that the new API has to accept a NULL info pointer for historic reasons.

Guenter

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 13:34   ` Guenter Roeck
@ 2022-05-02 18:59     ` LABBE Corentin
  2022-05-02 20:24       ` Guenter Roeck
  2022-05-02 20:31       ` Armin Wolf
  0 siblings, 2 replies; 10+ messages in thread
From: LABBE Corentin @ 2022-05-02 18:59 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: jdelvare, linux-hwmon, linux-kernel

Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
> On 5/2/22 05:42, Corentin Labbe wrote:
> > Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
> > So let's convert the driver to use hwmon_device_register_with_info().
> > 
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> >   drivers/hwmon/acpi_power_meter.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> > index d2545a1be9fc..98293727f980 100644
> > --- a/drivers/hwmon/acpi_power_meter.c
> > +++ b/drivers/hwmon/acpi_power_meter.c
> > @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
> >   	if (res)
> >   		goto exit_free_capability;
> >   
> > -	resource->hwmon_dev = hwmon_device_register(&device->dev);
> > +	resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
> > +							      ACPI_POWER_METER_DEVICE_NAME,
> > +							      NULL, NULL,
> > +							      NULL);
> 
> NACK. That isn't a conversion to the new API, it just abuses the fact
> that the new API has to accept a NULL info pointer for historic reasons.
> 

Hello

I am sorry, I found a driver doing it, so I believed it was okay.
Converting seems not to hard but, by using the new API, how can I convert power1_model_number/power1_is_battery attribute ?
There are the remaining attributes I dont find how to convert.

Regards

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 18:59     ` LABBE Corentin
@ 2022-05-02 20:24       ` Guenter Roeck
  2022-05-02 20:31       ` Armin Wolf
  1 sibling, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2022-05-02 20:24 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: jdelvare, linux-hwmon, linux-kernel

On 5/2/22 11:59, LABBE Corentin wrote:
> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>> On 5/2/22 05:42, Corentin Labbe wrote:
>>> Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
>>> So let's convert the driver to use hwmon_device_register_with_info().
>>>
>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>> ---
>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
>>> index d2545a1be9fc..98293727f980 100644
>>> --- a/drivers/hwmon/acpi_power_meter.c
>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
>>>    	if (res)
>>>    		goto exit_free_capability;
>>>    
>>> -	resource->hwmon_dev = hwmon_device_register(&device->dev);
>>> +	resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
>>> +							      ACPI_POWER_METER_DEVICE_NAME,
>>> +							      NULL, NULL,
>>> +							      NULL);
>>
>> NACK. That isn't a conversion to the new API, it just abuses the fact
>> that the new API has to accept a NULL info pointer for historic reasons.
>>
> 
> Hello
> 
> I am sorry, I found a driver doing it, so I believed it was okay.

Yeah, that is what happens if an API is flexible: It is prone to
abuse, and once the abuse starts others start to abuse it as well
"because that other driver did it".

> Converting seems not to hard but, by using the new API, how can I convert power1_model_number/power1_is_battery attribute ?
> There are the remaining attributes I dont find how to convert.
> 

It is ok to use the pointer to sysfs attribute groups
for non-standard attributes.

Guenter

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 18:59     ` LABBE Corentin
  2022-05-02 20:24       ` Guenter Roeck
@ 2022-05-02 20:31       ` Armin Wolf
  2022-05-02 20:42         ` Guenter Roeck
  1 sibling, 1 reply; 10+ messages in thread
From: Armin Wolf @ 2022-05-02 20:31 UTC (permalink / raw)
  To: LABBE Corentin, Guenter Roeck; +Cc: jdelvare, linux-hwmon, linux-kernel

Am 02.05.22 um 20:59 schrieb LABBE Corentin:

> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>> On 5/2/22 05:42, Corentin Labbe wrote:
>>> Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
>>> So let's convert the driver to use hwmon_device_register_with_info().
>>>
>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>> ---
>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
>>> index d2545a1be9fc..98293727f980 100644
>>> --- a/drivers/hwmon/acpi_power_meter.c
>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
>>>    	if (res)
>>>    		goto exit_free_capability;
>>>
>>> -	resource->hwmon_dev = hwmon_device_register(&device->dev);
>>> +	resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
>>> +							      ACPI_POWER_METER_DEVICE_NAME,
>>> +							      NULL, NULL,
>>> +							      NULL);
>> NACK. That isn't a conversion to the new API, it just abuses the fact
>> that the new API has to accept a NULL info pointer for historic reasons.
>>
> Hello
>
> I am sorry, I found a driver doing it, so I believed it was okay.
> Converting seems not to hard but, by using the new API, how can I convert power1_model_number/power1_is_battery attribute ?
> There are the remaining attributes I dont find how to convert.
>
> Regards

Hi,

for allowing the driver to provide nonstandard attributes, hwmon_device_register_with_info()
has the argument "extra_groups" which is an pointer to a list of sysfs attribute groups.
There are some drivers which are using this functionality, maybe you can use them as an
inspiration.

Just a question: what is the name of the driver you originally used as an inspiration?

Armin Wolf


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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 20:31       ` Armin Wolf
@ 2022-05-02 20:42         ` Guenter Roeck
  2022-05-02 21:00           ` Armin Wolf
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2022-05-02 20:42 UTC (permalink / raw)
  To: Armin Wolf, LABBE Corentin; +Cc: jdelvare, linux-hwmon, linux-kernel

On 5/2/22 13:31, Armin Wolf wrote:
> Am 02.05.22 um 20:59 schrieb LABBE Corentin:
> 
>> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>>> On 5/2/22 05:42, Corentin Labbe wrote:
>>>> Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
>>>> So let's convert the driver to use hwmon_device_register_with_info().
>>>>
>>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>>> ---
>>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
>>>> index d2545a1be9fc..98293727f980 100644
>>>> --- a/drivers/hwmon/acpi_power_meter.c
>>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
>>>>        if (res)
>>>>            goto exit_free_capability;
>>>>
>>>> -    resource->hwmon_dev = hwmon_device_register(&device->dev);
>>>> +    resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
>>>> +                                  ACPI_POWER_METER_DEVICE_NAME,
>>>> +                                  NULL, NULL,
>>>> +                                  NULL);
>>> NACK. That isn't a conversion to the new API, it just abuses the fact
>>> that the new API has to accept a NULL info pointer for historic reasons.
>>>
>> Hello
>>
>> I am sorry, I found a driver doing it, so I believed it was okay.
>> Converting seems not to hard but, by using the new API, how can I convert power1_model_number/power1_is_battery attribute ?
>> There are the remaining attributes I dont find how to convert.
>>
>> Regards
> 
> Hi,
> 
> for allowing the driver to provide nonstandard attributes, hwmon_device_register_with_info()
> has the argument "extra_groups" which is an pointer to a list of sysfs attribute groups.
> There are some drivers which are using this functionality, maybe you can use them as an
> inspiration.
> 
> Just a question: what is the name of the driver you originally used as an inspiration?
> 

Originally it was for drivers/thermal/thermal_hwmon.c. Now there is also
drivers/platform/mips/cpu_hwmon.c which is clearly an abuse. I may have
missed others.

Guenter

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 20:42         ` Guenter Roeck
@ 2022-05-02 21:00           ` Armin Wolf
  2022-05-02 23:43             ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Armin Wolf @ 2022-05-02 21:00 UTC (permalink / raw)
  To: Guenter Roeck, LABBE Corentin; +Cc: jdelvare, linux-hwmon, linux-kernel

Am 02.05.22 um 22:42 schrieb Guenter Roeck:
> On 5/2/22 13:31, Armin Wolf wrote:
>> Am 02.05.22 um 20:59 schrieb LABBE Corentin:
>>
>>> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>>>> On 5/2/22 05:42, Corentin Labbe wrote:
>>>>> Booting lead to a hwmon_device_register() is deprecated. Please
>>>>> convert the driver to use hwmon_device_register_with_info().
>>>>> So let's convert the driver to use hwmon_device_register_with_info().
>>>>>
>>>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>>>> ---
>>>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/hwmon/acpi_power_meter.c
>>>>> b/drivers/hwmon/acpi_power_meter.c
>>>>> index d2545a1be9fc..98293727f980 100644
>>>>> --- a/drivers/hwmon/acpi_power_meter.c
>>>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct
>>>>> acpi_device *device)
>>>>>        if (res)
>>>>>            goto exit_free_capability;
>>>>>
>>>>> -    resource->hwmon_dev = hwmon_device_register(&device->dev);
>>>>> +    resource->hwmon_dev =
>>>>> hwmon_device_register_with_info(&device->dev,
>>>>> + ACPI_POWER_METER_DEVICE_NAME,
>>>>> +                                  NULL, NULL,
>>>>> +                                  NULL);
>>>> NACK. That isn't a conversion to the new API, it just abuses the fact
>>>> that the new API has to accept a NULL info pointer for historic
>>>> reasons.
>>>>
>>> Hello
>>>
>>> I am sorry, I found a driver doing it, so I believed it was okay.
>>> Converting seems not to hard but, by using the new API, how can I
>>> convert power1_model_number/power1_is_battery attribute ?
>>> There are the remaining attributes I dont find how to convert.
>>>
>>> Regards
>>
>> Hi,
>>
>> for allowing the driver to provide nonstandard attributes,
>> hwmon_device_register_with_info()
>> has the argument "extra_groups" which is an pointer to a list of
>> sysfs attribute groups.
>> There are some drivers which are using this functionality, maybe you
>> can use them as an
>> inspiration.
>>
>> Just a question: what is the name of the driver you originally used
>> as an inspiration?
>>
>
> Originally it was for drivers/thermal/thermal_hwmon.c. Now there is also
> drivers/platform/mips/cpu_hwmon.c which is clearly an abuse. I may have
> missed others.
>
> Guenter
Should we notify the maintainers of cpu_hwmon.c about that?
This could potentially prevent such incidents from happening again.

Armin Wolf

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 21:00           ` Armin Wolf
@ 2022-05-02 23:43             ` Guenter Roeck
  2022-05-04  7:05               ` Armin Wolf
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2022-05-02 23:43 UTC (permalink / raw)
  To: Armin Wolf, LABBE Corentin; +Cc: jdelvare, linux-hwmon, linux-kernel

On 5/2/22 14:00, Armin Wolf wrote:
> Am 02.05.22 um 22:42 schrieb Guenter Roeck:
>> On 5/2/22 13:31, Armin Wolf wrote:
>>> Am 02.05.22 um 20:59 schrieb LABBE Corentin:
>>>
>>>> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>>>>> On 5/2/22 05:42, Corentin Labbe wrote:
>>>>>> Booting lead to a hwmon_device_register() is deprecated. Please
>>>>>> convert the driver to use hwmon_device_register_with_info().
>>>>>> So let's convert the driver to use hwmon_device_register_with_info().
>>>>>>
>>>>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>>>>> ---
>>>>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/hwmon/acpi_power_meter.c
>>>>>> b/drivers/hwmon/acpi_power_meter.c
>>>>>> index d2545a1be9fc..98293727f980 100644
>>>>>> --- a/drivers/hwmon/acpi_power_meter.c
>>>>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>>>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct
>>>>>> acpi_device *device)
>>>>>>        if (res)
>>>>>>            goto exit_free_capability;
>>>>>>
>>>>>> -    resource->hwmon_dev = hwmon_device_register(&device->dev);
>>>>>> +    resource->hwmon_dev =
>>>>>> hwmon_device_register_with_info(&device->dev,
>>>>>> + ACPI_POWER_METER_DEVICE_NAME,
>>>>>> +                                  NULL, NULL,
>>>>>> +                                  NULL);
>>>>> NACK. That isn't a conversion to the new API, it just abuses the fact
>>>>> that the new API has to accept a NULL info pointer for historic
>>>>> reasons.
>>>>>
>>>> Hello
>>>>
>>>> I am sorry, I found a driver doing it, so I believed it was okay.
>>>> Converting seems not to hard but, by using the new API, how can I
>>>> convert power1_model_number/power1_is_battery attribute ?
>>>> There are the remaining attributes I dont find how to convert.
>>>>
>>>> Regards
>>>
>>> Hi,
>>>
>>> for allowing the driver to provide nonstandard attributes,
>>> hwmon_device_register_with_info()
>>> has the argument "extra_groups" which is an pointer to a list of
>>> sysfs attribute groups.
>>> There are some drivers which are using this functionality, maybe you
>>> can use them as an
>>> inspiration.
>>>
>>> Just a question: what is the name of the driver you originally used
>>> as an inspiration?
>>>
>>
>> Originally it was for drivers/thermal/thermal_hwmon.c. Now there is also
>> drivers/platform/mips/cpu_hwmon.c which is clearly an abuse. I may have
>> missed others.
>>
>> Guenter
> Should we notify the maintainers of cpu_hwmon.c about that?
> This could potentially prevent such incidents from happening again.
> 

I am more inclined to create a special API for thermal, call it from
the thermal code, and issue a warning with backtrace if anyone else
calls hwmon_device_register_with_info() with NULL info pointer.
And explain all that in detail in the API documentation.

Guenter

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

* Re: [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info
  2022-05-02 23:43             ` Guenter Roeck
@ 2022-05-04  7:05               ` Armin Wolf
  0 siblings, 0 replies; 10+ messages in thread
From: Armin Wolf @ 2022-05-04  7:05 UTC (permalink / raw)
  To: Guenter Roeck, LABBE Corentin; +Cc: jdelvare, linux-hwmon, linux-kernel

Am 03.05.22 um 01:43 schrieb Guenter Roeck:

> On 5/2/22 14:00, Armin Wolf wrote:
>> Am 02.05.22 um 22:42 schrieb Guenter Roeck:
>>> On 5/2/22 13:31, Armin Wolf wrote:
>>>> Am 02.05.22 um 20:59 schrieb LABBE Corentin:
>>>>
>>>>> Le Mon, May 02, 2022 at 06:34:44AM -0700, Guenter Roeck a écrit :
>>>>>> On 5/2/22 05:42, Corentin Labbe wrote:
>>>>>>> Booting lead to a hwmon_device_register() is deprecated. Please
>>>>>>> convert the driver to use hwmon_device_register_with_info().
>>>>>>> So let's convert the driver to use
>>>>>>> hwmon_device_register_with_info().
>>>>>>>
>>>>>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>>>>>> ---
>>>>>>>    drivers/hwmon/acpi_power_meter.c | 5 ++++-
>>>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hwmon/acpi_power_meter.c
>>>>>>> b/drivers/hwmon/acpi_power_meter.c
>>>>>>> index d2545a1be9fc..98293727f980 100644
>>>>>>> --- a/drivers/hwmon/acpi_power_meter.c
>>>>>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>>>>>> @@ -891,7 +891,10 @@ static int acpi_power_meter_add(struct
>>>>>>> acpi_device *device)
>>>>>>>        if (res)
>>>>>>>            goto exit_free_capability;
>>>>>>>
>>>>>>> -    resource->hwmon_dev = hwmon_device_register(&device->dev);
>>>>>>> +    resource->hwmon_dev =
>>>>>>> hwmon_device_register_with_info(&device->dev,
>>>>>>> + ACPI_POWER_METER_DEVICE_NAME,
>>>>>>> +                                  NULL, NULL,
>>>>>>> +                                  NULL);
>>>>>> NACK. That isn't a conversion to the new API, it just abuses the
>>>>>> fact
>>>>>> that the new API has to accept a NULL info pointer for historic
>>>>>> reasons.
>>>>>>
>>>>> Hello
>>>>>
>>>>> I am sorry, I found a driver doing it, so I believed it was okay.
>>>>> Converting seems not to hard but, by using the new API, how can I
>>>>> convert power1_model_number/power1_is_battery attribute ?
>>>>> There are the remaining attributes I dont find how to convert.
>>>>>
>>>>> Regards
>>>>
>>>> Hi,
>>>>
>>>> for allowing the driver to provide nonstandard attributes,
>>>> hwmon_device_register_with_info()
>>>> has the argument "extra_groups" which is an pointer to a list of
>>>> sysfs attribute groups.
>>>> There are some drivers which are using this functionality, maybe you
>>>> can use them as an
>>>> inspiration.
>>>>
>>>> Just a question: what is the name of the driver you originally used
>>>> as an inspiration?
>>>>
>>>
>>> Originally it was for drivers/thermal/thermal_hwmon.c. Now there is
>>> also
>>> drivers/platform/mips/cpu_hwmon.c which is clearly an abuse. I may have
>>> missed others.
>>>
>>> Guenter
>> Should we notify the maintainers of cpu_hwmon.c about that?
>> This could potentially prevent such incidents from happening again.
>>
>
> I am more inclined to create a special API for thermal, call it from
> the thermal code, and issue a warning with backtrace if anyone else
> calls hwmon_device_register_with_info() with NULL info pointer.
> And explain all that in detail in the API documentation.
>
> Guenter
Sounds good.

Armin Wolf

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

end of thread, other threads:[~2022-05-04  7:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-02 12:42 [PATCH 1/2] hwmon: acpi_power_meter: fix style issue Corentin Labbe
2022-05-02 12:42 ` [PATCH 2/2] hwmon: acpi_power_meter: convert to hwmon_device_register_with_info Corentin Labbe
2022-05-02 13:34   ` Guenter Roeck
2022-05-02 18:59     ` LABBE Corentin
2022-05-02 20:24       ` Guenter Roeck
2022-05-02 20:31       ` Armin Wolf
2022-05-02 20:42         ` Guenter Roeck
2022-05-02 21:00           ` Armin Wolf
2022-05-02 23:43             ` Guenter Roeck
2022-05-04  7:05               ` Armin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).