MIME-Version: 1.0
References: <20211122212850.321542-1-pauk.denis@gmail.com> <20211122212850.321542-4-pauk.denis@gmail.com>
 <CAHp75VeKosontsmFJSp-fbV9mPiSWJeLUCpx90=RHy1HFfBDnA@mail.gmail.com>
In-Reply-To: <CAHp75VeKosontsmFJSp-fbV9mPiSWJeLUCpx90=RHy1HFfBDnA@mail.gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Wed, 24 Nov 2021 18:24:51 +0200
Message-ID: <CAHp75Vdr+9zoWG74d0ZfGEjj_b1xkX7gw1ka_4NkGtjmvKB73A@mail.gmail.com>
Subject: Re: [PATCH 3/3] hwmon: (nct6775) add MAXIMUS VII HERO.
To:     Denis Pauk <pauk.denis@gmail.com>
Cc:     Eugene Shalygin <eugene.shalygin@gmail.com>,
        Platform Driver <platform-driver-x86@vger.kernel.org>,
        thomas@weissschuh.net, Olli Asikainen <olli.asikainen@gmail.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Jean Delvare <jdelvare@suse.com>, linux-hwmon@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Nov 24, 2021 at 6:21 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Nov 22, 2021 at 11:29 PM Denis Pauk <pauk.denis@gmail.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