Re: [PATCH v5 1/2] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI.

[Eugene Shalygin]

Hi, Denis,

> +       for (i_sensor = 0; i_sensor < ec->nr_sensors; i_sensor++) {
> +               s = &ec->sensors[i_sensor];
> +               si = &known_ec_sensors[s->info_index];
> +
> +               switch (si->addr.size) {
> +               case 1:
> +                       s->cached_value = ec->read_buffer[read_reg_ct];
> +                       break;
> +               case 2:
> +                       s->cached_value = get_unaligned_be16(&ec->read_buffer[read_reg_ct]);
> +                       break;
> +               case 4:
> +                       s->cached_value = get_unaligned_be32(&ec->read_buffer[read_reg_ct]);
> +                       break;
> +               default:
> +                       s->cached_value =  0;
> +               }
> +               read_reg_ct += si->addr.size;

There is at least one more sensor hiding in the EC address space: the
south bridge voltage. And it seems its value is not an integer, so the
conversion to mV will not be a simple get_unaligned_xx() call when we
locate and add it. Thus, I would suggest extracting this switch in a
separate function to make the future modification simpler. Something
like the following:

static inline u32 get_sensor_value(const struct ec_sensor_info *si, u8
*data) // si for the data encoding scheme
{
    switch (si->addr.components.size) {
    case 1:
        return *data;
    case 2:
        return get_unaligned_be16(data);
    case 4:
        return get_unaligned_be32(data);
    }
}

static void update_sensor_values(struct ec_sensors_data *ec, u8 *data)
{
    const struct ec_sensor_info *si;
    struct ec_sensor *s;

    for (s = ec->sensors; s != ec->sensors + ec->nr_sensors; s++) {
        si = &known_ec_sensors[s->info_index];
        s->cached_value = get_sensor_value(si, data);
        data += si->addr.components.size;
    }
}

Additionally, this would simplify update_ec_sensors() body:

mutex_lock(&ec->lock);
make_asus_wmi_block_read_query(ec);
status = asus_ec_block_read(dev, METHODID_BLOCK_READ_EC, ec->read_arg,
    buffer);

if (!status) {
    update_sensor_values(ec, buffer);
}
mutex_unlock(&ec->lock);


Eugene