2022-08-17 06:17:51

by Farber, Eliav

Subject: [PATCH v2 01/16] hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not defined

Fix a bug that in case "intel,vm-map" is missing 'num' is set to 0,
and no voltage channel infos are allocated.

Signed-off-by: Eliav Farber <[email protected]>
---
drivers/hwmon/mr75203.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 046523d47c29..0e29877a1a9c 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -580,8 +580,6 @@ static int mr75203_probe(struct platform_device *pdev)
}

if (vm_num) {
- u32 num = vm_num;
-
ret = pvt_get_regmap(pdev, "vm", pvt);
if (ret)
return ret;
@@ -594,30 +592,28 @@ static int mr75203_probe(struct platform_device *pdev)
ret = device_property_read_u8_array(dev, "intel,vm-map",
pvt->vm_idx, vm_num);
if (ret) {
- num = 0;
+ /*
+ * Incase intel,vm-map property is not defined, we
+ * assume incremental channel numbers.
+ */
+ for (i = 0; i < vm_num; i++)
+ pvt->vm_idx[i] = i;
} else {
for (i = 0; i < vm_num; i++)
if (pvt->vm_idx[i] >= vm_num ||
- pvt->vm_idx[i] == 0xff) {
- num = i;
+ pvt->vm_idx[i] == 0xff)
break;
- }
- }

- /*
- * Incase intel,vm-map property is not defined, we assume
- * incremental channel numbers.
- */
- for (i = num; i < vm_num; i++)
- pvt->vm_idx[i] = i;
+ vm_num = i;
+ }

- in_config = devm_kcalloc(dev, num + 1,
+ in_config = devm_kcalloc(dev, vm_num + 1,
sizeof(*in_config), GFP_KERNEL);
if (!in_config)
return -ENOMEM;

- memset32(in_config, HWMON_I_INPUT, num);
- in_config[num] = 0;
+ memset32(in_config, HWMON_I_INPUT, vm_num);
+ in_config[vm_num] = 0;
pvt_in.config = in_config;

pvt_info[index++] = &pvt_in;
--
2.37.1

2022-08-18 19:53:57

by Guenter Roeck

Subject: Re: [PATCH v2 01/16] hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not defined

On Wed, Aug 17, 2022 at 05:43:06AM +0000, Eliav Farber wrote:
> Fix a bug that in case "intel,vm-map" is missing 'num' is set to 0,
> and no voltage channel infos are allocated.
>

"intel,vm-map" is listed as required property in moortec,mr75203.yaml.
If it is missing, the probe function should fail.

Guenter

> Signed-off-by: Eliav Farber <[email protected]>
> ---
> drivers/hwmon/mr75203.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
> index 046523d47c29..0e29877a1a9c 100644
> --- a/drivers/hwmon/mr75203.c
> +++ b/drivers/hwmon/mr75203.c
> @@ -580,8 +580,6 @@ static int mr75203_probe(struct platform_device *pdev)
> }
>
> if (vm_num) {
> - u32 num = vm_num;
> -
> ret = pvt_get_regmap(pdev, "vm", pvt);
> if (ret)
> return ret;
> @@ -594,30 +592,28 @@ static int mr75203_probe(struct platform_device *pdev)
> ret = device_property_read_u8_array(dev, "intel,vm-map",
> pvt->vm_idx, vm_num);
> if (ret) {
> - num = 0;
> + /*
> + * Incase intel,vm-map property is not defined, we
> + * assume incremental channel numbers.
> + */
> + for (i = 0; i < vm_num; i++)
> + pvt->vm_idx[i] = i;
> } else {
> for (i = 0; i < vm_num; i++)
> if (pvt->vm_idx[i] >= vm_num ||
> - pvt->vm_idx[i] == 0xff) {
> - num = i;
> + pvt->vm_idx[i] == 0xff)
> break;
> - }
> - }
>
> - /*
> - * Incase intel,vm-map property is not defined, we assume
> - * incremental channel numbers.
> - */
> - for (i = num; i < vm_num; i++)
> - pvt->vm_idx[i] = i;
> + vm_num = i;
> + }
>
> - in_config = devm_kcalloc(dev, num + 1,
> + in_config = devm_kcalloc(dev, vm_num + 1,
> sizeof(*in_config), GFP_KERNEL);
> if (!in_config)
> return -ENOMEM;
>
> - memset32(in_config, HWMON_I_INPUT, num);
> - in_config[num] = 0;
> + memset32(in_config, HWMON_I_INPUT, vm_num);
> + in_config[vm_num] = 0;
> pvt_in.config = in_config;
>
> pvt_info[index++] = &pvt_in;

2022-08-18 20:21:56

by Farber, Eliav

Subject: Re: [PATCH v2 01/16] hwmon: (mr75203) fix VM sensor allocation when "intel, vm-map" not defined

On 8/18/2022 10:40 PM, Guenter Roeck wrote:
> "intel,vm-map" is listed as required property in moortec,mr75203.yaml.
> If it is missing, the probe function should fail.

Indeed according to moortec,mr75203.yaml "intel,vm-map" is listed as
required
but the code indicates otherwise:

/*
 * Incase intel,vm-map property is not defined, we assume
 * incremental channel numbers.
 */

The probe function takes care in case it is missing and does not fail.
It also seems less reasonable that an Intel proprietary parameter will be
required and not optional.
So in patch 03 I fixed the moortec,mr75203.yaml documentation and
changed it to
be optional.

--
Regards, Eliav