On 10-09-2024 10:07, Gupta, Anshuman wrote:
-----Original Message-----
From: Andi Shyti <andi.shyti@xxxxxxxxxx>
Sent: Tuesday, September 10, 2024 3:54 AM
To: Jadav, Raag <raag.jadav@xxxxxxxxx>
Cc: jani.nikula@xxxxxxxxxxxxxxx; joonas.lahtinen@xxxxxxxxxxxxxxx; Vivi,
Rodrigo <rodrigo.vivi@xxxxxxxxx>; tursulin@xxxxxxxxxxx; linux@xxxxxxxxxxxx;
andi.shyti@xxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; intel-
gfx@xxxxxxxxxxxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx; Gupta, Anshuman
<anshuman.gupta@xxxxxxxxx>; Nilawar, Badal <badal.nilawar@xxxxxxxxx>;
Tauro, Riana <riana.tauro@xxxxxxxxx>; Dixit, Ashutosh
<ashutosh.dixit@xxxxxxxxx>; Poosa, Karthik <karthik.poosa@xxxxxxxxx>
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.
I agree on this. Let’s stick to file-wide approach and ensure it is
applied to the fan_input attribute as well.
Regards,
Badal
Thanks,
Anshuman.
Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Thanks,
Andi
