Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1301063 > unrolled thread
| Started by | Darren Hart <dvhart@infradead.org> |
|---|---|
| First post | 2016-01-04 21:40 +0100 |
| Last post | 2016-01-05 01:10 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons Darren Hart <dvhart@infradead.org> - 2016-01-04 21:40 +0100
RE: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons "Chen, Yu C" <yu.c.chen@intel.com> - 2016-01-05 01:10 +0100
| From | Darren Hart <dvhart@infradead.org> |
|---|---|
| Date | 2016-01-04 21:40 +0100 |
| Subject | Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons |
| Message-ID | <qNk55-8oP-35@gated-at.bofh.it> |
On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wengxt@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 <wengxt@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+.
--
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 <yu.c.chen@intel.com>
> > L: platform-driver-x86@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 <oliver@neukum.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
>
--
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | "Chen, Yu C" <yu.c.chen@intel.com> |
|---|---|
| Date | 2016-01-05 01:10 +0100 |
| Message-ID | <qNnmh-2ix-3@gated-at.bofh.it> |
| In reply to | #1301063 |
Hi Darren, > -----Original Message----- > From: Darren Hart [mailto:dvhart@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-x86@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 <wengxt@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 <wengxt@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web