| Message ID | 20230825205345.632792-4-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Enhancements for tmp51x driver | expand |
Hi Biju, On Fri, Aug 25, 2023 at 10:54 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Simplify tmp51x_read_properties() by replacing 'nfactor' ->'data->nfactor' > in device_property_read_u32_array() and drop the local variable as it is > unused. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/drivers/hwmon/tmp513.c > +++ b/drivers/hwmon/tmp513.c > @@ -680,10 +679,8 @@ static int tmp51x_read_properties(struct device *dev, struct tmp51x_data *data) > if (ret < 0) > return ret; > > - ret = device_property_read_u32_array(dev, "ti,nfactor", nfactor, > - data->max_channels - 1); > - if (ret >= 0) > - memcpy(data->nfactor, nfactor, data->max_channels - 1); > + device_property_read_u32_array(dev, "ti,nfactor", data->nfactor, > + data->max_channels - 1); My first thought was that this could cause a small change in behavior. However, the comments for of_property_read_u32_array() and of_property_read_variable_u32_array() state: * The out_values is modified only if a valid u32 value can be decoded. and the implementation behaves that way. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Mon, Aug 28, 2023 at 09:45:17AM +0200, Geert Uytterhoeven wrote: > On Fri, Aug 25, 2023 at 10:54 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Simplify tmp51x_read_properties() by replacing 'nfactor' ->'data->nfactor' > > in device_property_read_u32_array() and drop the local variable as it is > > unused. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> ... > > - ret = device_property_read_u32_array(dev, "ti,nfactor", nfactor, > > - data->max_channels - 1); > > - if (ret >= 0) > > - memcpy(data->nfactor, nfactor, data->max_channels - 1); > > + device_property_read_u32_array(dev, "ti,nfactor", data->nfactor, > > + data->max_channels - 1); > > My first thought was that this could cause a small change in behavior. > However, the comments for of_property_read_u32_array() and > of_property_read_variable_u32_array() state: I don't know how the latter is relevant here (or even anything from of_*() specific APIs). The device property APIs should give behaviour described in their own docs (although it must be compatible with the existing OF one). > * The out_values is modified only if a valid u32 value can be decoded. > > and the implementation behaves that way. > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c index 6bbae4735a4b..a1104833252d 100644 --- a/drivers/hwmon/tmp513.c +++ b/drivers/hwmon/tmp513.c @@ -662,7 +662,6 @@ static int tmp51x_pga_gain_to_reg(struct device *dev, struct tmp51x_data *data) static int tmp51x_read_properties(struct device *dev, struct tmp51x_data *data) { int ret; - u32 nfactor[3]; u32 val; ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &val); @@ -680,10 +679,8 @@ static int tmp51x_read_properties(struct device *dev, struct tmp51x_data *data) if (ret < 0) return ret; - ret = device_property_read_u32_array(dev, "ti,nfactor", nfactor, - data->max_channels - 1); - if (ret >= 0) - memcpy(data->nfactor, nfactor, data->max_channels - 1); + device_property_read_u32_array(dev, "ti,nfactor", data->nfactor, + data->max_channels - 1); // Check if shunt value is compatible with pga-gain if (data->shunt_uohms > data->pga_gain * 40 * 1000 * 1000) {
Simplify tmp51x_read_properties() by replacing 'nfactor' ->'data->nfactor' in device_property_read_u32_array() and drop the local variable as it is unused. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v3: * New patch. --- drivers/hwmon/tmp513.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)