[PATCH v2] drm/i915/hwmon: expose package temperature

[Gupta, Anshuman]

> -----Original Message-----
> From: Andi Shyti <andi.shyti at kernel.org>
> Sent: Tuesday, September 10, 2024 3:54 AM
> To: Jadav, Raag <raag.jadav at intel.com>
> Cc: jani.nikula at linux.intel.com; joonas.lahtinen at linux.intel.com; Vivi,
> Rodrigo <rodrigo.vivi at intel.com>; tursulin at ursulin.net; linux at roeck-us.net;
> andi.shyti at linux.intel.com; andriy.shevchenko at linux.intel.com; intel-
> gfx at lists.freedesktop.org; linux-hwmon at vger.kernel.org; Gupta, Anshuman
> <anshuman.gupta at intel.com>; Nilawar, Badal <badal.nilawar at intel.com>;
> Tauro, Riana <riana.tauro at intel.com>; Dixit, Ashutosh
> <ashutosh.dixit at intel.com>; Poosa, Karthik <karthik.poosa at intel.com>
> 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 <andi.shyti at linux.intel.com>
> 
> Thanks,
> Andi