linux-kernel - Re: [PATCH v2 2/3] platform/x86: asus_wmi: Support fan boost mode on FX505DY/FX705DY

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]

Message-ID: <CAHp75VfNu_G7zzaCV9JF+Ua9otSTftJgtgr9N4qhQNR0o+QHtA@mail.gmail.com>
Date:   Thu, 7 Nov 2019 19:15:28 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Leonid Maksymchuk <leonmaxx@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        acpi4asus-user <acpi4asus-user@...ts.sourceforge.net>,
        Chris Chiu <chiu@...lessm.com>,
        Yurii Pavlovskyi <yurii.pavlovskyi@...il.com>,
        Kristian Klausen <kristian@...usen.dk>,
        Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Corentin Chary <corentin.chary@...il.com>
Subject: Re: [PATCH v2 2/3] platform/x86: asus_wmi: Support fan boost mode on FX505DY/FX705DY

On Mon, Nov 4, 2019 at 11:38 PM Leonid Maksymchuk <leonmaxx@...il.com> wrote:
> On ASUS FX505DY/FX705DY laptops fan boost mode is same as in other
> TUF laptop models but have different ACPI device ID and different key
> code.

> +       if (err == 0 &&
> +                       (result & ASUS_WMI_DSTS_PRESENCE_BIT) &&
> +                       (result & ASUS_FAN_BOOST_MODES_MASK)) {
> +               asus->fan_boost_mode_available = 1;
> +               asus->fan_boost_mode_mask = result & ASUS_FAN_BOOST_MODES_MASK;
> +               return 0;
>         }
>
> -       if ((result & ASUS_WMI_DSTS_PRESENCE_BIT) &&
> +       err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_BOOST_MODE_2,
> +                                   &result);
> +
> +       if (err == 0 &&
> +                       (result & ASUS_WMI_DSTS_PRESENCE_BIT) &&
>                         (result & ASUS_FAN_BOOST_MODES_MASK)) {
> -               asus->fan_boost_mode_available = true;
> +               asus->fan_boost_mode_available = 2;
>                 asus->fan_boost_mode_mask = result & ASUS_FAN_BOOST_MODES_MASK;
>         }

The above differs only in one value to give and one value to set, I
suppose you may introduce an additional helper to it

> +       if (err == -ENODEV)
> +               return 0;

This should be explained or even separated to another patch. It
changes behaviour of the original code, why?

> +       u32 dev_id = asus->fan_boost_mode_available == 1 ?
> +                       ASUS_WMI_DEVID_FAN_BOOST_MODE :
> +                       ASUS_WMI_DEVID_FAN_BOOST_MODE_2;

I would prefer to see
 if (...)
  call with mode
else
  call with mode_2

below.

> -       err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_BOOST_MODE, value,
> -                                   &retval);
> +       err = asus_wmi_set_devstate(dev_id, value, &retval);

-- 
With Best Regards,
Andy Shevchenko