On Tue, Apr 16, 2019 at 10:09 AM Daniel Lezcano <[email protected]> wrote: > > On 13/04/2019 10:27, Andrey Smirnov wrote: > > Tmu_get_temp will get called as a part of sensor registration via > > devm_thermal_zone_of_sensor_register(). To prevent it from retruning > > bogus data we need to enable sensor monitoring before that. Looking at > > the datasheet (i.MX8MQ RM) there doesn't seem to be any harm in > > enabling them all, so, for the sake of simplicity, change the code to > > do just that. > > > > Signed-off-by: Andrey Smirnov <[email protected]> > > Cc: Chris Healy <[email protected]> > > Cc: Lucas Stach <[email protected]> > > Cc: Eduardo Valentin <[email protected]> > > Cc: Daniel Lezcano <[email protected]> > > Cc: Angus Ainslie (Purism) <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > --- > > drivers/thermal/qoriq_thermal.c | 27 ++++++++++++++------------- > > 1 file changed, 14 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/thermal/qoriq_thermal.c > > b/drivers/thermal/qoriq_thermal.c > > index 9774323a17bf..abbbfe88422e 100644 > > --- a/drivers/thermal/qoriq_thermal.c > > +++ b/drivers/thermal/qoriq_thermal.c > > @@ -23,6 +23,7 @@ > > #define TMR_DISABLE 0x0 > > #define TMR_ME 0x80000000 > > #define TMR_ALPF 0x0c000000 > > +#define TMR_MSITE_ALL GENMASK(15, 0) > > > > #define REGS_TMTMIR 0x008 /* Temperature measurement interval Register > > */ > > #define TMTMIR_DEFAULT 0x0000000f > > @@ -61,28 +62,28 @@ static const struct thermal_zone_of_device_ops > > tmu_tz_ops = { > > static int qoriq_tmu_register_tmu_zone(struct device *dev, > > struct qoriq_tmu_data *qdata) > > { > > - int id, sites = 0; > > + int id, ret; > > + > > + regmap_write(qdata->regmap, REGS_TMR, > > + TMR_MSITE_ALL | TMR_ME | TMR_ALPF); > > > > for (id = 0; id < SITES_MAX; id++) { > > struct thermal_zone_device *tzd; > > > > tzd = devm_thermal_zone_of_sensor_register(dev, id, > > qdata, > > &tmu_tz_ops); > > - if (IS_ERR(tzd)) { > > - if (PTR_ERR(tzd) == -ENODEV) > > - continue; > > - else > > - return PTR_ERR(tzd); > > + ret = PTR_ERR_OR_ZERO(tzd); > > + switch (ret) { > > + case -ENODEV: > > + continue; > > + case 0: > > Why not simply: > > if (!IS_ERR(tzd) || PTR_ERR(tzd) == -ENODEV) > continue; > > regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE); > > return PTR_ERR(tzd); > > ?
Seemed like less typing and smaller diff for following patch that adds hwmon sensor registration. I can probable make "if" work kind of the same, so I'll change that. I do dislike that you had to use *_ERR macro three times though, so I'll stick with PTR_ERR_OR_ZERO. Thanks, Andrey Smirnov
