Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751276AbdIEHHc (ORCPT ); Tue, 5 Sep 2017 03:07:32 -0400 Received: from mail-io0-f179.google.com ([209.85.223.179]:34817 "EHLO mail-io0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbdIEHH3 (ORCPT ); Tue, 5 Sep 2017 03:07:29 -0400 X-Google-Smtp-Source: ADKCNb6OcwESl4pV8tqCnAgxxQtRTO4c14w2YVPzu4pmFDL55iSI7g+7Hh6WLKY+akWAHiH9xR7b6QaF2iMSAp6upeY= MIME-Version: 1.0 In-Reply-To: References: <20170904082110.30925-1-corentin.chary@gmail.com> From: Corentin Chary Date: Tue, 5 Sep 2017 09:07:27 +0200 Message-ID: Subject: Re: [PATCH] drivers/x86: add thinkpad-wmi To: Andy Shevchenko Cc: Darren Hart , Andy Shevchenko , Platform Driver , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2894 Lines: 124 [re-send for the mailing list, I forgot that gmail was stupid] On Tue, Sep 5, 2017 at 9:05 AM, Corentin Chary wrote: > > > On Mon, Sep 4, 2017 at 7:15 PM, Andy Shevchenko > wrote: >> >> On Mon, Sep 4, 2017 at 11:21 AM, Corentin Chary >> 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 > > > Done > >> >> >> > + * Thinkpad WMI hotkey driver >> >> Solely for hot keys? >> > > /hotkey/configuration/ > >> >> > + * >> > + * Copyright(C) 2012 Corentin Chary >> >> 2012,2017? >> > > done > >> >> >> > + * 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. > > > done > >> >> >> >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> >> Alphabetical. >> > > done > >> >> > +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? >> > > No, I wasn't aware of it. I checked the dell-wmi conversion patch and it > should not be too hard. > I'll probably sent that as a patch on top the existing driver (in the same > series). > >> >> -- >> With Best Regards, >> Andy Shevchenko > > > > > -- > Corentin Chary > http://xf.iksaif.net -- Corentin Chary http://xf.iksaif.net