On 10-09-2024 10:07, Gupta, Anshuman wrote:
-----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.
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 <[email protected]>
Thanks,
Andi
