[PATCH AUTOSEL 5.17 128/135] hwmon: Make chip parameter for with_info API mandatory

[PATCH AUTOSEL 5.17 128/135] hwmon: Make chip parameter for with_info API mandatory

[Sasha Levin]

From: Guenter Roeck <linux@roeck-us.net>

[ Upstream commit ddaefa209c4ac791c1262e97c9b2d0440c8ef1d5 ]

Various attempts were made recently to "convert" the old
hwmon_device_register() API to devm_hwmon_device_register_with_info()
by just changing the function name without actually converting the
driver. Prevent this from happening by making the 'chip' parameter of
devm_hwmon_device_register_with_info() mandatory.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 Documentation/hwmon/hwmon-kernel-api.rst |  2 +-
 drivers/hwmon/hwmon.c                    | 16 +++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index c41eb6108103..23f27fe78e37 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -72,7 +72,7 @@ hwmon_device_register_with_info is the most comprehensive and preferred means
 to register a hardware monitoring device. It creates the standard sysfs
 attributes in the hardware monitoring core, letting the driver focus on reading
 from and writing to the chip instead of having to bother with sysfs attributes.
-The parent device parameter cannot be NULL with non-NULL chip info. Its
+The parent device parameter as well as the chip parameter must not be NULL. Its
 parameters are described in more detail below.
 
 devm_hwmon_device_register_with_info is similar to
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 3ae961986fc3..55237a5fc49a 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -854,11 +854,12 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_groups);
 
 /**
  * hwmon_device_register_with_info - register w/ hwmon
- * @dev: the parent device
- * @name: hwmon name attribute
- * @drvdata: driver data to attach to created device
- * @chip: pointer to hwmon chip information
+ * @dev: the parent device (mandatory)
+ * @name: hwmon name attribute (mandatory)
+ * @drvdata: driver data to attach to created device (optional)
+ * @chip: pointer to hwmon chip information (mandatory)
  * @extra_groups: pointer to list of additional non-standard attribute groups
+ *	(optional)
  *
  * hwmon_device_unregister() must be called when the device is no
  * longer needed.
@@ -871,13 +872,10 @@ hwmon_device_register_with_info(struct device *dev, const char *name,
 				const struct hwmon_chip_info *chip,
 				const struct attribute_group **extra_groups)
 {
-	if (!name)
-		return ERR_PTR(-EINVAL);
-
-	if (chip && (!chip->ops || !chip->ops->is_visible || !chip->info))
+	if (!dev || !name || !chip)
 		return ERR_PTR(-EINVAL);
 
-	if (chip && !dev)
+	if (!chip->ops || !chip->ops->is_visible || !chip->info)
 		return ERR_PTR(-EINVAL);
 
 	return __hwmon_device_register(dev, name, drvdata, chip, extra_groups);
-- 
2.35.1

Re: [PATCH AUTOSEL 5.17 128/135] hwmon: Make chip parameter for with_info API mandatory

[Guenter Roeck]

On 5/30/22 06:31, Sasha Levin wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> [ Upstream commit ddaefa209c4ac791c1262e97c9b2d0440c8ef1d5 ]
> 
> Various attempts were made recently to "convert" the old
> hwmon_device_register() API to devm_hwmon_device_register_with_info()
> by just changing the function name without actually converting the
> driver. Prevent this from happening by making the 'chip' parameter of
> devm_hwmon_device_register_with_info() mandatory.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

This patch should not be backported. It is only relevant for new
kernel releases, and may have adverse affect if applied to older
kernels.

Guenter

> ---
>   Documentation/hwmon/hwmon-kernel-api.rst |  2 +-
>   drivers/hwmon/hwmon.c                    | 16 +++++++---------
>   2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
> index c41eb6108103..23f27fe78e37 100644
> --- a/Documentation/hwmon/hwmon-kernel-api.rst
> +++ b/Documentation/hwmon/hwmon-kernel-api.rst
> @@ -72,7 +72,7 @@ hwmon_device_register_with_info is the most comprehensive and preferred means
>   to register a hardware monitoring device. It creates the standard sysfs
>   attributes in the hardware monitoring core, letting the driver focus on reading
>   from and writing to the chip instead of having to bother with sysfs attributes.
> -The parent device parameter cannot be NULL with non-NULL chip info. Its
> +The parent device parameter as well as the chip parameter must not be NULL. Its
>   parameters are described in more detail below.
>   
>   devm_hwmon_device_register_with_info is similar to
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 3ae961986fc3..55237a5fc49a 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -854,11 +854,12 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_groups);
>   
>   /**
>    * hwmon_device_register_with_info - register w/ hwmon
> - * @dev: the parent device
> - * @name: hwmon name attribute
> - * @drvdata: driver data to attach to created device
> - * @chip: pointer to hwmon chip information
> + * @dev: the parent device (mandatory)
> + * @name: hwmon name attribute (mandatory)
> + * @drvdata: driver data to attach to created device (optional)
> + * @chip: pointer to hwmon chip information (mandatory)
>    * @extra_groups: pointer to list of additional non-standard attribute groups
> + *	(optional)
>    *
>    * hwmon_device_unregister() must be called when the device is no
>    * longer needed.
> @@ -871,13 +872,10 @@ hwmon_device_register_with_info(struct device *dev, const char *name,
>   				const struct hwmon_chip_info *chip,
>   				const struct attribute_group **extra_groups)
>   {
> -	if (!name)
> -		return ERR_PTR(-EINVAL);
> -
> -	if (chip && (!chip->ops || !chip->ops->is_visible || !chip->info))
> +	if (!dev || !name || !chip)
>   		return ERR_PTR(-EINVAL);
>   
> -	if (chip && !dev)
> +	if (!chip->ops || !chip->ops->is_visible || !chip->info)
>   		return ERR_PTR(-EINVAL);
>   
>   	return __hwmon_device_register(dev, name, drvdata, chip, extra_groups);

Re: [PATCH AUTOSEL 5.17 128/135] hwmon: Make chip parameter for with_info API mandatory

[Bagas Sanjaya]

On 5/30/22 21:27, Guenter Roeck wrote:
> On 5/30/22 06:31, Sasha Levin wrote:
>> From: Guenter Roeck <linux@roeck-us.net>
>>
>> [ Upstream commit ddaefa209c4ac791c1262e97c9b2d0440c8ef1d5 ]
>>
>> Various attempts were made recently to "convert" the old
>> hwmon_device_register() API to devm_hwmon_device_register_with_info()
>> by just changing the function name without actually converting the
>> driver. Prevent this from happening by making the 'chip' parameter of
>> devm_hwmon_device_register_with_info() mandatory.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
> 
> This patch should not be backported. It is only relevant for new
> kernel releases, and may have adverse affect if applied to older
> kernels.

So this patch is meant to be backported to 5.18 only, right?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH AUTOSEL 5.17 128/135] hwmon: Make chip parameter for with_info API mandatory

[Guenter Roeck]

On 5/30/22 19:38, Bagas Sanjaya wrote:
> On 5/30/22 21:27, Guenter Roeck wrote:
>> On 5/30/22 06:31, Sasha Levin wrote:
>>> From: Guenter Roeck <linux@roeck-us.net>
>>>
>>> [ Upstream commit ddaefa209c4ac791c1262e97c9b2d0440c8ef1d5 ]
>>>
>>> Various attempts were made recently to "convert" the old
>>> hwmon_device_register() API to devm_hwmon_device_register_with_info()
>>> by just changing the function name without actually converting the
>>> driver. Prevent this from happening by making the 'chip' parameter of
>>> devm_hwmon_device_register_with_info() mandatory.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>
>> This patch should not be backported. It is only relevant for new
>> kernel releases, and may have adverse affect if applied to older
>> kernels.
> 
> So this patch is meant to be backported to 5.18 only, right?
> 

I said "do not backport". I specifically asked not to backport
this patch. It does not include Cc: stable, and it does not
include a Fixes: tag. I even said "... may have adverse affect
if applied to older kernels".

I have no idea how that can be interpreted as "backport to 5.18".
Did I miss the explicit "do not backport to 5.18 ?" If so, my bad.

Ok, here it is a more explicit request:

Do not backport this patch to 5.18.y, 5.17.y, 5.15.y, 5.10.y, 5.4.y,
4.19.y, 4.14.y, 4.9.y, or any other stable release.

Guenter