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