Hi,

> > > > +static int
> > > > +hwm_temp_read(struct hwm_drvdata *ddat, u32 attr, long *val) {
> > > > +       struct i915_hwmon *hwmon = ddat->hwmon;
> > > > +       intel_wakeref_t wakeref;
> > > > +       u32 reg_val;
> > > > +
> > > > +       switch (attr) {
> > > > +       case hwmon_temp_input:
> > > > +               with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> > > > +                       reg_val = intel_uncore_read(ddat->uncore, hwmon-
> > > > rg.pkg_temp);
> > > > +
> > > > +               /* HW register value is in degrees, convert to 
> > > > millidegrees. */
> > > > +               *val = REG_FIELD_GET(TEMP_MASK, reg_val) *
> > > MILLIDEGREE_PER_DEGREE;
> > > > +               return 0;
> > > > +       default:
> > > > +               return -EOPNOTSUPP;
> > > > +       }
> > > 
> > > I don't understand this love for single case switches.
> > IMHO this is kept to keep symmetry in this file to make it more readable.
> > Also it readable to return error using default case, which is followed in 
> > this entire file.
> I agree on this. Let’s stick to file-wide approach and ensure it is applied
> to the fan_input attribute as well.

Yes, that's why I'm giving the r-b. I don't like it, but that's
how you guys have decided to do it.

Thanks,
Andi

Reply via email to