| Message ID | 20220817054321.6519-2-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Variety of fixes and new features for mr75203 driver | expand |
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 <farbere@amazon.com> > --- > 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;
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
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;
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 <farbere@amazon.com> --- drivers/hwmon/mr75203.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)