[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75Vdr+9zoWG74d0ZfGEjj_b1xkX7gw1ka_4NkGtjmvKB73A@mail.gmail.com>
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
Powered by blists - more mailing lists