[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VepPax_eqc4PLgDMvzM4jpQNRXdUe5q3rXPOj81MFcwHQ@mail.gmail.com>
Date: Sun, 27 Dec 2015 22:58:06 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Weng Xuetian <wengxt@...il.com>
Cc: Chen Yu <yu.c.chen@...el.com>, Darren Hart <dvhart@...radead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wengxt@...il.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 <wengxt@...il.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)
> ---
> 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 <yu.c.chen@...el.com>
> L: platform-driver-x86@...r.kernel.org
> S: Supported
> -F: drivers/platform/x86/surfacepro3_button.c
> +F: drivers/platform/x86/surfacepro_button.c
>
> MICROTEK X6 SCANNER
> M: Oliver Neukum <oliver@...kum.org>
> 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 <linux/acpi.h>
> #include <acpi/button.h>
>
> -#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_POWER 0xc7
> @@ -35,10 +36,10 @@
> #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN 0xc2
> #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, surface_button_device_ids);
> @@ -202,8 +204,8 @@ static SIMPLE_DEV_PM_OPS(surface_button_pm,
> surface_button_suspend, surface_button_resume);
>
> static struct acpi_driver surface_button_driver = {
> - .name = "surface_pro3_button",
> - .class = "SurfacePro3",
> + .name = "surface_pro_button",
> + .class = "SurfacePro",
So, beside the driver renaming I don't know the side effect of
renaming .class field here.
> .ids = surface_button_device_ids,
> .ops = {
> .add = surface_button_add,
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists