> -----Original Message-----
> From: Andi Shyti <[email protected]>
> Sent: Tuesday, September 10, 2024 3:54 AM
> To: Jadav, Raag <[email protected]>
> Cc: [email protected]; [email protected]; Vivi,
> Rodrigo <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; intel-
> [email protected]; [email protected]; Gupta, Anshuman
> <[email protected]>; Nilawar, Badal <[email protected]>;
> Tauro, Riana <[email protected]>; Dixit, Ashutosh
> <[email protected]>; Poosa, Karthik <[email protected]>
> Subject: Re: [PATCH v2] drm/i915/hwmon: expose package temperature
>
> Hi Raag,
>
> ...
>
> > +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.
Thanks,
Anshuman.
>
> Reviewed-by: Andi Shyti <[email protected]>
>
> Thanks,
> Andi
