linux-kernel - Re: [PATCH 3/3] hwmon: (nct6775) add MAXIMUS VII HERO.

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Nov 2021 18:24:51 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Denis Pauk <pauk.denis@...il.com>
Cc:     Eugene Shalygin <eugene.shalygin@...il.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        thomas@...ssschuh.net, Olli Asikainen <olli.asikainen@...il.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] hwmon: (nct6775) add MAXIMUS VII HERO.

On Wed, Nov 24, 2021 at 6:21 PM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
> On Mon, Nov 22, 2021 at 11:29 PM Denis Pauk <pauk.denis@...il.com> wrote:

...

> > +               if (access == access_asuswmi &&
> > +                   nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp)) {
> > +                       access = access_direct;
> > +                       pr_err("Can't read ChipID by Asus WMI.\n");
> > +               }
> > +
> > +               if (access == access_asuswmi) {
> > +                       if (tmp)
> > +                               pr_info("Using Asus WMI to access %#x chip.\n", tmp);
> > +                       else
> > +                               access = access_direct;
>
> Why not:

>         if (access == access_asuswmi) {
>                access = access_direct;

Oh, just noticed above... Looks not good due to possible confusion
which means this part needs to be thought through and refactored,
perhaps by intermediate variable that defines the access and then you
assign access= to it if it satisfies the conditions.

>                if (nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp))
>                        pr_err("Can't read ChipID by Asus WMI.\n");
>                if (tmp) {
>                        pr_info("Using Asus WMI to access %#x chip.\n", tmp);
>                       access = access_...; // do you have this?
>                }
>                ...
>         }
>
> ?


-- 
With Best Regards,
Andy Shevchenko