linux-kernel - Re: [PATCH] drivers/x86: add thinkpad-wmi

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: <CAHp75VcWfzEiZAt+ESoF==q0LnCgWRzBuo6FDizmSJ7V-8Rw3w@mail.gmail.com>
Date:   Mon, 4 Sep 2017 20:15:32 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Corentin Chary <corentin.chary@...il.com>
Cc:     Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drivers/x86: add thinkpad-wmi

On Mon, Sep 4, 2017 at 11:21 AM, Corentin Chary
<corentin.chary@...il.com> wrote:
> This driver has been available on https://github.com/iksaif/thinkpad-wmi for
> a few year and is already deployed on large fleets of thinkpad laptops.
>
> The WMI interface is documented here: http://download.lenovo.com/ibmdl/pub/pc/pccbbs/thinkcentre_pdf/hrdeploy_en.pdf
> It mostly focused on changing BIOS/Firmware settings.

I will do full review later, few comments right now though.
Thanks for doing this btw.

> +Date:          Aug 2017
> +KernelVersion: 4.14

v4.15 apparently

> + * Thinkpad WMI hotkey driver

Solely for hot keys?

> + *
> + * Copyright(C) 2012 Corentin Chary <corentin.chary@...il.com>

2012,2017?


> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Remove this. It had been changed once, no guarantee it will not again.


> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/debugfs.h>
> +#include <linux/uaccess.h>
> +#include <linux/seq_file.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/acpi.h>

Alphabetical.

> +static int __init thinkpad_wmi_init(void)
> +{
> +       platform_device = platform_create_bundle(&platform_driver,
> +                                                thinkpad_wmi_probe,
> +                                                NULL, 0, NULL, 0);
> +       if (IS_ERR(platform_device))
> +               return PTR_ERR(platform_device);
> +       return 0;
> +}
> +
> +static void __exit thinkpad_wmi_exit(void)
> +{
> +       platform_device_unregister(platform_device);
> +       platform_driver_unregister(&platform_driver);
> +}
> +
> +module_init(thinkpad_wmi_init);
> +module_exit(thinkpad_wmi_exit);

I didn't read the code, does it use WMI bus which Andy L. introduced recently?

-- 
With Best Regards,
Andy Shevchenko