linux-kernel - Re: [PATCH v5 1/2] hwmon: (asus_wmi_ec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAB95QASjUq4P3HhFJrCpBwtJLzwc0ig0q5YQg6FGTDaxkS3SPg@mail.gmail.com>
Date:   Mon, 11 Oct 2021 23:03:14 +0200
From:   Eugene Shalygin <eugene.shalygin@...il.com>
To:     Denis Pauk <pauk.denis@...il.com>
Cc:     andy.shevchenko@...il.com, platform-driver-x86@...r.kernel.org,
        Tor Vic <torvic9@...lbox.org>,
        kernel test robot <lkp@...el.com>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Jonathan Corbet <corbet@....net>, linux-hwmon@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/2] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI.

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