linux-kernel - Re: [PATCH v5 1/2] platform/x86 asus_wmi: Support throttle thermal policy

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

Message-ID: <CAHp75VcVj5=DQ8faTtC2CWk86-VMPGroQ+aCK+tLpkFEyPe-MQ@mail.gmail.com>
Date:   Mon, 25 Nov 2019 13:12:43 +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>,
        Kristian Klausen <kristian@...usen.dk>,
        Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Corentin Chary <corentin.chary@...il.com>
Subject: Re: [PATCH v5 1/2] platform/x86 asus_wmi: Support throttle thermal policy

On Sun, Nov 24, 2019 at 4:07 PM Leonid Maksymchuk <leonmaxx@...il.com> wrote:
>
> Throttle thermal policy ACPI device is used to control CPU cooling and

> throttling. This patch adds sysfs entry for setting current mode and
> Fn+F5 hotkey that switches to next.

sysfs means ABI. ABI must be documented.

> Policy modes:
> * 0x00 - default
> * 0x01 - overboost
> * 0x02 - silent

> +static int throttle_thermal_policy_check_present(struct asus_wmi *asus)
> +{
> +       u32 result;
> +       int err;
> +
> +       asus->throttle_thermal_policy_available = false;
> +
> +       err = asus_wmi_get_devstate(asus,
> +                                   ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY,
> +                                   &result);
> +       if (err) {
> +               if (err == -ENODEV)
> +                       return 0;

> +               else

Redundant.

> +                       return err;
> +       }
> +

> +       if ((result & ASUS_WMI_DSTS_PRESENCE_BIT))

Too many parentheses.

> +               asus->throttle_thermal_policy_available = true;
> +
> +       return 0;
> +}

> +       pr_info("Set throttle thermal policy: %u\n", value);

Do we really need this message?

> +       return scnprintf(buf, PAGE_SIZE, "%d\n",
> +                        asus->throttle_thermal_policy_mode);

Can it be one line?

> +       result = kstrtou8(buf, 10, &new_mode);
> +       if (result < 0) {

> +               pr_warn("Trying to store invalid value\n");

Redundant. By error code user space will get a message.

> +               return result;
> +       }



-- 
With Best Regards,
Andy Shevchenko