Hi Andi,
On 09-08-2024 15:46, Andi Shyti wrote:
Hi Badal,
+static int
+hwm_fan_read(struct hwm_drvdata *ddat, u32 attr, long *val)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ struct hwm_fan_info *fi = &ddat->fi;
+ u32 reg_val, pulses, time, time_now;
+ intel_wakeref_t wakeref;
+ long rotations;
+ int ret = 0;
+
+ if (attr != hwmon_fan_input)
+ return -EOPNOTSUPP;
Using a switch case in rev3 is more logical here. It will also simplify
adding more fan attributes in the future. This is why switch cases are used
in other parts of the file.
it was my suggestion and to be honest I would rather prefer it
this way. I can understand it if we were expecting more cases in
the immediate, like it was in your case.
But I wouldn't have an ugly and unreadable one-case-switch in the
eventuality that something comes in the future. In that case, we
can always convert it.
My rationale for suggesting a switch case is that in the current
alignment hwm_XX_read() function is designed to handle all
possible/supported attributes of the XX sensor type.
With the proposed change, hwm_fan_read() would only manage the
hwmon_fan_input attribute.
If a single switch case isn’t preferred, I would recommend creating a
helper function dedicated to hwmon_fan_input.
hwm_fan_read()
{
if (attr == hwmon_fan_input)
return helper(); //hwmon_fan_input_read()
return -EOPNOTSUPP;
}
Regards,
Badal
Thanks,
Andi
