Re: [PATCH 1/2] hwmon: Add Congatec Board Controller monitoring driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/6/24 05:46, Thomas Richard wrote:

+
+static struct cgbc_hwmon_sensor *cgbc_hwmon_find_sensor(struct
cgbc_hwmon_data *hwmon,
+                            enum hwmon_sensor_types type, int channel)
+{
+    struct cgbc_hwmon_sensor *sensor = NULL;
+    int i, cnt = 0;
+
+    for (i = 0; i < hwmon->nb_sensors; i++) {
+        if (hwmon->sensors[i].type == type && cnt++ == channel) {

Isn't that a bit fragile ? It assumes that the nth reported sensor of a
given type
reflects a specific named sensor. If that is indeed the case, why bother
with
all the code in cgbc_hwmon_probe_sensors() ? The index to sensor
association
should be well defined, and the sensor type plus the channel index
should always
be a constant.

I'm not sure to get your comment.

I cannot assume that the sensor list returned by the Board Controller
will be the same for all boards.
I know the MFD driver only supports one board, but I think it's better
if we can have a generic hwmon driver.

If I add some debug in cgbc_hwmon_probe_sensors() I can dump the sensor
list returned by the Board Controller:

cgbc_hwmon_probe_sensors: index=0 type=1 id=5 label=Chipset Temperature
cgbc_hwmon_probe_sensors: index=1 type=7 id=0 label=CPU Fan
cgbc_hwmon_probe_sensors: index=4 type=1 id=3 label=Board Temperature
cgbc_hwmon_probe_sensors: index=5 type=2 id=1 label=DC Runtime Voltage

It is the type and the id which give the name of the sensor.

I don't see how to do it in a different way if I cannot assume that the
list above is not the same for all boards.
If I assume that the list returned by the Board Controller is always the
same for a board (which I not even sure, if for example a fan is
plugged), I could have a static list for each different boards. But the
driver will not be generic.

If I miss something, please let me know.


My thought was to use the sensor ID as channel index. In general it
would be preferable to know that, say, in0 is always the CPU voltage
and that temp1 is always the CPU Temperature.

Guenter





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux