RE: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Chen, Yu C 


Hi Darren,

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Tuesday, January 05, 2016 4:36 AM
> To: Andy Shevchenko
> Cc: Weng Xuetian; Chen, Yu C; linux-kernel@vger.kernel.org; platform-
> driver-...@vger.kernel.org
> Subject: Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> > On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian 
> wrote:
> > > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > > different from Surface Pro 3.
> > >
> > > This commit adds MSHW0040 to id list to support the Surface Pro 4,
> > > and renames the driver to surfacepro_button accordingly.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > > Signed-off-by: Weng Xuetian 
> >
> > Darren, this one looks fine for me, still couple of question up to you.
> >
> > First is do we really want to rename driver? (Renaming variables and
> > stuff like I said if fine to me)
> 
> Sorry, replied before seeing this.
> 
> I agree that renaming the file is probably not necessary (this works on v3+ as
> I understand it, so the name isn't really a problem, and dropping the 3
> suggests it works on v1 and v2).
> 
> I'd prefer to keep the name changing to a minimum, and add some
> information to the Kconfig help and driver comments making it clear that this
> supports 3+.
> 
[Yu] I agree, since I don't have a surface pro < 3 tested, there is no need to 
rename it.

thanks,
yu

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Darren Hart 
On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian  wrote:
> > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > different from Surface Pro 3.
> >
> > This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> > renames the driver to surfacepro_button accordingly.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > Signed-off-by: Weng Xuetian 
> 
> Darren, this one looks fine for me, still couple of question up to you.
> 
> First is do we really want to rename driver? (Renaming variables and
> stuff like I said if fine to me)

Sorry, replied before seeing this.

I agree that renaming the file is probably not necessary (this works on v3+ as I
understand it, so the name isn't really a problem, and dropping the 3 suggests
it works on v1 and v2).

I'd prefer to keep the name changing to a minimum, and add some information to
the Kconfig help and driver comments making it clear that this supports 3+.

--
Darren

> 
> > ---
> > v3:
> >  - Fix commit message grammar mistakes.
> > v2:
> >  - Reformat patch with -M -C
> > ---
> >  MAINTAINERS|  4 ++--
> >  drivers/platform/x86/Kconfig   |  6 +++---
> >  drivers/platform/x86/Makefile  |  2 +-
> >  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 
> > ++
> >  4 files changed, 16 insertions(+), 14 deletions(-)
> >  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} 
> > (93%)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 233f834..1c07436 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7019,11 +7019,11 @@ T:  git 
> > git://git.monstr.eu/linux-2.6-microblaze.git
> >  S: Supported
> >  F: arch/microblaze/
> >
> > -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> > +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
> >  M: Chen Yu 
> >  L: platform-driver-...@vger.kernel.org
> >  S: Supported
> > -F: drivers/platform/x86/surfacepro3_button.c
> > +F: drivers/platform/x86/surfacepro_button.c
> >
> >  MICROTEK X6 SCANNER
> >  M: Oliver Neukum 
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 1089eaa..3358fb0 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -939,9 +939,9 @@ config INTEL_PMC_IPC
> > The PMC is an ARC processor which defines IPC commands for 
> > communication
> > with other entities in the CPU.
> >
> > -config SURFACE_PRO3_BUTTON
> > -   tristate "Power/home/volume buttons driver for Microsoft Surface 
> > Pro 3 tablet"
> > +config SURFACE_PRO_BUTTON
> > +   tristate "Power/home/volume buttons driver for Microsoft Surface 
> > Pro Series tablet"
> > depends on ACPI && INPUT
> > ---help---
> > - This driver handles the power/home/volume buttons on the 
> > Microsoft Surface Pro 3 tablet.
> > + This driver handles the power/home/volume buttons on the 
> > Microsoft Surface Pro Series tablet.
> >  endif # X86_PLATFORM_DEVICES
> > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> > index 3ca78a3..b4ece33 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -61,4 +61,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += 
> > intel-smartconnect.o
> >  obj-$(CONFIG_PVPANIC)   += pvpanic.o
> >  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
> >  obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
> > -obj-$(CONFIG_SURFACE_PRO3_BUTTON)  += surfacepro3_button.o
> > +obj-$(CONFIG_SURFACE_PRO_BUTTON)   += surfacepro_button.o
> > diff --git a/drivers/platform/x86/surfacepro3_button.c 
> > b/drivers/platform/x86/surfacepro_button.c
> > similarity index 93%
> > rename from drivers/platform/x86/surfacepro3_button.c
> > rename to drivers/platform/x86/surfacepro_button.c
> > index f7dade3..cda52b8 100644
> > --- a/drivers/platform/x86/surfacepro3_button.c
> > +++ b/drivers/platform/x86/surfacepro_button.c
> > @@ -1,6 +1,6 @@
> >  /*
> >   * power/home/volume button support for
> > - * Microsoft Surface Pro 3 tablet.
> > + * Microsoft Surface Pro Series tablet.
> >   *
> >   * Copyright (c) 2015 Intel Corporation.
> >   * All rights reserved.
> > @@ -19,9 +19,10 @@
> >  #include 
> >  #include 
> >
> > -#define SURFACE_BUTTON_HID "MSHW0028"
> > +#define SURFACE_PRO3_BUTTON_HID"MSHW0028"
> > +#define SURFACE_PRO4_BUTTON_HID"MSHW0040"
> >  #define SURFACE_BUTTON_OBJ_NAME"VGBI"
> > -#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3 Buttons"
> > +#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro Series Buttons"
> >
> >  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER  0xc6
> >  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER0xc7
> > @@ -35,10 +36,10 @@
> >  #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Darren Hart 
On Sun, Dec 27, 2015 at 11:21:20AM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871

Is there a Surface Pro v1 and v2? If so, and they are not supported, then the 3
makes sense to keep. Or, at the very least, the Kconfig help and the comments in
the driver should make it clear that it supports v3 and later.

Chen, any concerns?

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Darren Hart 
On Sun, Dec 27, 2015 at 11:21:20AM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871

Is there a Surface Pro v1 and v2? If so, and they are not supported, then the 3
makes sense to keep. Or, at the very least, the Kconfig help and the comments in
the driver should make it clear that it supports v3 and later.

Chen, any concerns?

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Darren Hart 
On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian  wrote:
> > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > different from Surface Pro 3.
> >
> > This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> > renames the driver to surfacepro_button accordingly.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > Signed-off-by: Weng Xuetian 
> 
> Darren, this one looks fine for me, still couple of question up to you.
> 
> First is do we really want to rename driver? (Renaming variables and
> stuff like I said if fine to me)

Sorry, replied before seeing this.

I agree that renaming the file is probably not necessary (this works on v3+ as I
understand it, so the name isn't really a problem, and dropping the 3 suggests
it works on v1 and v2).

I'd prefer to keep the name changing to a minimum, and add some information to
the Kconfig help and driver comments making it clear that this supports 3+.

--
Darren

> 
> > ---
> > v3:
> >  - Fix commit message grammar mistakes.
> > v2:
> >  - Reformat patch with -M -C
> > ---
> >  MAINTAINERS|  4 ++--
> >  drivers/platform/x86/Kconfig   |  6 +++---
> >  drivers/platform/x86/Makefile  |  2 +-
> >  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 
> > ++
> >  4 files changed, 16 insertions(+), 14 deletions(-)
> >  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} 
> > (93%)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 233f834..1c07436 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7019,11 +7019,11 @@ T:  git 
> > git://git.monstr.eu/linux-2.6-microblaze.git
> >  S: Supported
> >  F: arch/microblaze/
> >
> > -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> > +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
> >  M: Chen Yu 
> >  L: platform-driver-...@vger.kernel.org
> >  S: Supported
> > -F: drivers/platform/x86/surfacepro3_button.c
> > +F: drivers/platform/x86/surfacepro_button.c
> >
> >  MICROTEK X6 SCANNER
> >  M: Oliver Neukum 
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 1089eaa..3358fb0 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -939,9 +939,9 @@ config INTEL_PMC_IPC
> > The PMC is an ARC processor which defines IPC commands for 
> > communication
> > with other entities in the CPU.
> >
> > -config SURFACE_PRO3_BUTTON
> > -   tristate "Power/home/volume buttons driver for Microsoft Surface 
> > Pro 3 tablet"
> > +config SURFACE_PRO_BUTTON
> > +   tristate "Power/home/volume buttons driver for Microsoft Surface 
> > Pro Series tablet"
> > depends on ACPI && INPUT
> > ---help---
> > - This driver handles the power/home/volume buttons on the 
> > Microsoft Surface Pro 3 tablet.
> > + This driver handles the power/home/volume buttons on the 
> > Microsoft Surface Pro Series tablet.
> >  endif # X86_PLATFORM_DEVICES
> > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> > index 3ca78a3..b4ece33 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -61,4 +61,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += 
> > intel-smartconnect.o
> >  obj-$(CONFIG_PVPANIC)   += pvpanic.o
> >  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
> >  obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
> > -obj-$(CONFIG_SURFACE_PRO3_BUTTON)  += surfacepro3_button.o
> > +obj-$(CONFIG_SURFACE_PRO_BUTTON)   += surfacepro_button.o
> > diff --git a/drivers/platform/x86/surfacepro3_button.c 
> > b/drivers/platform/x86/surfacepro_button.c
> > similarity index 93%
> > rename from drivers/platform/x86/surfacepro3_button.c
> > rename to drivers/platform/x86/surfacepro_button.c
> > index f7dade3..cda52b8 100644
> > --- a/drivers/platform/x86/surfacepro3_button.c
> > +++ b/drivers/platform/x86/surfacepro_button.c
> > @@ -1,6 +1,6 @@
> >  /*
> >   * power/home/volume button support for
> > - * Microsoft Surface Pro 3 tablet.
> > + * Microsoft Surface Pro Series tablet.
> >   *
> >   * Copyright (c) 2015 Intel Corporation.
> >   * All rights reserved.
> > @@ -19,9 +19,10 @@
> >  #include 
> >  #include 
> >
> > -#define SURFACE_BUTTON_HID "MSHW0028"
> > +#define SURFACE_PRO3_BUTTON_HID"MSHW0028"
> > +#define SURFACE_PRO4_BUTTON_HID"MSHW0040"
> >  #define SURFACE_BUTTON_OBJ_NAME"VGBI"
> > -#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3 Buttons"
> > +#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro Series Buttons"
> >
> >  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER  0xc6
> >  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER0xc7
> > @@

RE: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2016-01-04 Thread Chen, Yu C 


Hi Darren,

> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Tuesday, January 05, 2016 4:36 AM
> To: Andy Shevchenko
> Cc: Weng Xuetian; Chen, Yu C; linux-kernel@vger.kernel.org; platform-
> driver-...@vger.kernel.org
> Subject: Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> > On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wen...@gmail.com>
> wrote:
> > > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > > different from Surface Pro 3.
> > >
> > > This commit adds MSHW0040 to id list to support the Surface Pro 4,
> > > and renames the driver to surfacepro_button accordingly.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > > Signed-off-by: Weng Xuetian <wen...@gmail.com>
> >
> > Darren, this one looks fine for me, still couple of question up to you.
> >
> > First is do we really want to rename driver? (Renaming variables and
> > stuff like I said if fine to me)
> 
> Sorry, replied before seeing this.
> 
> I agree that renaming the file is probably not necessary (this works on v3+ as
> I understand it, so the name isn't really a problem, and dropping the 3
> suggests it works on v1 and v2).
> 
> I'd prefer to keep the name changing to a minimum, and add some
> information to the Kconfig help and driver comments making it clear that this
> supports 3+.
> 
[Yu] I agree, since I don't have a surface pro < 3 tested, there is no need to 
rename it.

thanks,
yu

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2015-12-27 Thread Andy Shevchenko 
On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian  wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
>
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian 

Darren, this one looks fine for me, still couple of question up to you.

First is do we really want to rename driver? (Renaming variables and
stuff like I said if fine to me)

> ---
> v3:
>  - Fix commit message grammar mistakes.
> v2:
>  - Reformat patch with -M -C
> ---
>  MAINTAINERS|  4 ++--
>  drivers/platform/x86/Kconfig   |  6 +++---
>  drivers/platform/x86/Makefile  |  2 +-
>  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 
> ++
>  4 files changed, 16 insertions(+), 14 deletions(-)
>  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} 
> (93%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 233f834..1c07436 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7019,11 +7019,11 @@ T:  git 
> git://git.monstr.eu/linux-2.6-microblaze.git
>  S: Supported
>  F: arch/microblaze/
>
> -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
>  M: Chen Yu 
>  L: platform-driver-...@vger.kernel.org
>  S: Supported
> -F: drivers/platform/x86/surfacepro3_button.c
> +F: drivers/platform/x86/surfacepro_button.c
>
>  MICROTEK X6 SCANNER
>  M: Oliver Neukum 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 1089eaa..3358fb0 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -939,9 +939,9 @@ config INTEL_PMC_IPC
> The PMC is an ARC processor which defines IPC commands for 
> communication
> with other entities in the CPU.
>
> -config SURFACE_PRO3_BUTTON
> -   tristate "Power/home/volume buttons driver for Microsoft Surface Pro 
> 3 tablet"
> +config SURFACE_PRO_BUTTON
> +   tristate "Power/home/volume buttons driver for Microsoft Surface Pro 
> Series tablet"
> depends on ACPI && INPUT
> ---help---
> - This driver handles the power/home/volume buttons on the Microsoft 
> Surface Pro 3 tablet.
> + This driver handles the power/home/volume buttons on the Microsoft 
> Surface Pro Series tablet.
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 3ca78a3..b4ece33 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -61,4 +61,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += 
> intel-smartconnect.o
>  obj-$(CONFIG_PVPANIC)   += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
>  obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
> -obj-$(CONFIG_SURFACE_PRO3_BUTTON)  += surfacepro3_button.o
> +obj-$(CONFIG_SURFACE_PRO_BUTTON)   += surfacepro_button.o
> diff --git a/drivers/platform/x86/surfacepro3_button.c 
> b/drivers/platform/x86/surfacepro_button.c
> similarity index 93%
> rename from drivers/platform/x86/surfacepro3_button.c
> rename to drivers/platform/x86/surfacepro_button.c
> index f7dade3..cda52b8 100644
> --- a/drivers/platform/x86/surfacepro3_button.c
> +++ b/drivers/platform/x86/surfacepro_button.c
> @@ -1,6 +1,6 @@
>  /*
>   * power/home/volume button support for
> - * Microsoft Surface Pro 3 tablet.
> + * Microsoft Surface Pro Series tablet.
>   *
>   * Copyright (c) 2015 Intel Corporation.
>   * All rights reserved.
> @@ -19,9 +19,10 @@
>  #include 
>  #include 
>
> -#define SURFACE_BUTTON_HID "MSHW0028"
> +#define SURFACE_PRO3_BUTTON_HID"MSHW0028"
> +#define SURFACE_PRO4_BUTTON_HID"MSHW0040"
>  #define SURFACE_BUTTON_OBJ_NAME"VGBI"
> -#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3 Buttons"
> +#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro Series Buttons"
>
>  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER  0xc6
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER0xc7
> @@ -35,10 +36,10 @@
>  #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN0xc2
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN  0xc3
>
> -ACPI_MODULE_NAME("surface pro 3 button");
> +ACPI_MODULE_NAME("surface pro series button");
>
>  MODULE_AUTHOR("Chen Yu");
> -MODULE_DESCRIPTION("Surface Pro3 Button Driver");
> +MODULE_DESCRIPTION("Surface Pro Series Button Driver");
>  MODULE_LICENSE("GPL v2");
>
>  /*
> @@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
>   * acpi_driver.
>   */
>  static const struct acpi_device_id surface_button_device_ids[] = {
> -   {SURFACE_BUTTON_HID,0},
> +   {SURFACE_PRO3_BUTTON_HID,0},
> +   {SURFACE_PRO4_BUTTON_HID,0},
> {"", 0},
>  };
>  MODULE_DEVICE_TABLE(acpi,

Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons

2015-12-27 Thread Andy Shevchenko 
On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian  wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
>
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian 

Darren, this one looks fine for me, still couple of question up to you.

First is do we really want to rename driver? (Renaming variables and
stuff like I said if fine to me)

> ---
> v3:
>  - Fix commit message grammar mistakes.
> v2:
>  - Reformat patch with -M -C
> ---
>  MAINTAINERS|  4 ++--
>  drivers/platform/x86/Kconfig   |  6 +++---
>  drivers/platform/x86/Makefile  |  2 +-
>  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 
> ++
>  4 files changed, 16 insertions(+), 14 deletions(-)
>  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} 
> (93%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 233f834..1c07436 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7019,11 +7019,11 @@ T:  git 
> git://git.monstr.eu/linux-2.6-microblaze.git
>  S: Supported
>  F: arch/microblaze/
>
> -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
>  M: Chen Yu 
>  L: platform-driver-...@vger.kernel.org
>  S: Supported
> -F: drivers/platform/x86/surfacepro3_button.c
> +F: drivers/platform/x86/surfacepro_button.c
>
>  MICROTEK X6 SCANNER
>  M: Oliver Neukum 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 1089eaa..3358fb0 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -939,9 +939,9 @@ config INTEL_PMC_IPC
> The PMC is an ARC processor which defines IPC commands for 
> communication
> with other entities in the CPU.
>
> -config SURFACE_PRO3_BUTTON
> -   tristate "Power/home/volume buttons driver for Microsoft Surface Pro 
> 3 tablet"
> +config SURFACE_PRO_BUTTON
> +   tristate "Power/home/volume buttons driver for Microsoft Surface Pro 
> Series tablet"
> depends on ACPI && INPUT
> ---help---
> - This driver handles the power/home/volume buttons on the Microsoft 
> Surface Pro 3 tablet.
> + This driver handles the power/home/volume buttons on the Microsoft 
> Surface Pro Series tablet.
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 3ca78a3..b4ece33 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -61,4 +61,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += 
> intel-smartconnect.o
>  obj-$(CONFIG_PVPANIC)   += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
>  obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
> -obj-$(CONFIG_SURFACE_PRO3_BUTTON)  += surfacepro3_button.o
> +obj-$(CONFIG_SURFACE_PRO_BUTTON)   += surfacepro_button.o
> diff --git a/drivers/platform/x86/surfacepro3_button.c 
> b/drivers/platform/x86/surfacepro_button.c
> similarity index 93%
> rename from drivers/platform/x86/surfacepro3_button.c
> rename to drivers/platform/x86/surfacepro_button.c
> index f7dade3..cda52b8 100644
> --- a/drivers/platform/x86/surfacepro3_button.c
> +++ b/drivers/platform/x86/surfacepro_button.c
> @@ -1,6 +1,6 @@
>  /*
>   * power/home/volume button support for
> - * Microsoft Surface Pro 3 tablet.
> + * Microsoft Surface Pro Series tablet.
>   *
>   * Copyright (c) 2015 Intel Corporation.
>   * All rights reserved.
> @@ -19,9 +19,10 @@
>  #include 
>  #include 
>
> -#define SURFACE_BUTTON_HID "MSHW0028"
> +#define SURFACE_PRO3_BUTTON_HID"MSHW0028"
> +#define SURFACE_PRO4_BUTTON_HID"MSHW0040"
>  #define SURFACE_BUTTON_OBJ_NAME"VGBI"
> -#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3 Buttons"
> +#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro Series Buttons"
>
>  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER  0xc6
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER0xc7
> @@ -35,10 +36,10 @@
>  #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN0xc2
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN  0xc3
>
> -ACPI_MODULE_NAME("surface pro 3 button");
> +ACPI_MODULE_NAME("surface pro series button");
>
>  MODULE_AUTHOR("Chen Yu");
> -MODULE_DESCRIPTION("Surface Pro3 Button Driver");
> +MODULE_DESCRIPTION("Surface Pro Series Button Driver");
>  MODULE_LICENSE("GPL v2");
>
>  /*
> @@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
>   * acpi_driver.
>   */
>  static const struct acpi_device_id surface_button_device_ids[] = {
> -   {SURFACE_BUTTON_HID,0},
> +   {SURFACE_PRO3_BUTTON_HID,0},
> +   {SURFACE_PRO4_BUTTON_HID,