[PATCH v3 0/5] platform/surface: Create a platform subdirectory for Microsoft Surface devices

[PATCH v3 0/5] platform/surface: Create a platform subdirectory for Microsoft Surface devices

[Maximilian Luz]

As has come up in the discussion around

  [RFC PATCH] Add support for Microsoft Surface System Aggregator Module

it may make sense to add a Microsoft Surface specific platform
subdirectory. Andy has suggested drivers/platform/surface for that.
This series follows said suggestion and creates that subdirectory, as
well as moves Microsoft Surface related drivers over to it and updates
their MAINTAINERS entries (if available) accordingly.

This series does not modify any existing driver code, symbols, or help
text.

More details regarding maintainership can be found in

  [PATCH v3 1/5] platform: Add Surface platform directory

Thanks,
Max

Link to discussion:
  https://lore.kernel.org/lkml/CAHp75Vfp86h38Rd-VEgER7ASADdmz5ymAkuHvD0Q6WPDqZBqHw@mail.gmail.com/

Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>

Changes in v2:
 - Rebase onto linux-platform-drivers-x86/for-next to incorporate
   changes in Maintainer file.

Changes in v3:
 - Add separate MAINTAINERS entry for platform/surface
 - Add Maximilian Luz as reviewer for platform/surface

For more details regarding changes, refer to the individual patches.

Maximilian Luz (5):
  platform: Add Surface platform directory
  platform/surface: Move Surface 3 WMI driver to platform/surface
  platform/surface: Move Surface 3 Button driver to platform/surface
  platform/surface: Move Surface 3 Power OpRegion driver to
    platform/surface
  platform/surface: Move Surface Pro 3 Button driver to platform/surface

 MAINTAINERS                                   | 11 ++++-
 drivers/platform/Kconfig                      |  2 +
 drivers/platform/Makefile                     |  1 +
 drivers/platform/surface/Kconfig              | 49 +++++++++++++++++++
 drivers/platform/surface/Makefile             | 10 ++++
 .../platform/{x86 => surface}/surface3-wmi.c  |  0
 .../{x86 => surface}/surface3_button.c        |  0
 .../{x86 => surface}/surface3_power.c         |  0
 .../{x86 => surface}/surfacepro3_button.c     |  0
 drivers/platform/x86/Kconfig                  | 31 ------------
 drivers/platform/x86/Makefile                 |  6 ---
 11 files changed, 72 insertions(+), 38 deletions(-)
 create mode 100644 drivers/platform/surface/Kconfig
 create mode 100644 drivers/platform/surface/Makefile
 rename drivers/platform/{x86 => surface}/surface3-wmi.c (100%)
 rename drivers/platform/{x86 => surface}/surface3_button.c (100%)
 rename drivers/platform/{x86 => surface}/surface3_power.c (100%)
 rename drivers/platform/{x86 => surface}/surfacepro3_button.c (100%)

-- 
2.28.0

[PATCH v3 1/5] platform: Add Surface platform directory

[Maximilian Luz]

It may make sense to split the Microsoft Surface hardware platform
drivers out to a separate subdirectory, since some of it may be shared
between ARM and x86 in the future (regarding devices like the Surface
Pro X).

Further, newer Surface devices will require additional platform drivers
for fundamental support (mostly regarding their embedded controller),
which may also warrant this split from a size perspective.

This commit introduces a new platform/surface subdirectory for the
Surface device family, with subsequent commits moving existing Surface
drivers over from platform/x86.

A new MAINTAINERS entry is added for this directory. Patches to files in
this directory will be taken up by the platform-drivers-x86 team (i.e.
Hans de Goede and Mark Gross) after they have been reviewed by
Maximilian Luz.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---

Changes in v2:
 - Do not create a separate MAINTAINERS entry for platform/surface.
   Instead add drivers/platform/surface to x86 platform drivers entry.
   This incorporates recent changes in the x86 platform driver
   MAINTAINERS entry.
 - Fix typo in commit message.

Changes in v3:
 - Add separate MAINTAINERS entry for platform/surface
 - Add Maximilian Luz as reviewer for platform/surface

---
 MAINTAINERS                       |  9 +++++++++
 drivers/platform/Kconfig          |  2 ++
 drivers/platform/Makefile         |  1 +
 drivers/platform/surface/Kconfig  | 20 ++++++++++++++++++++
 drivers/platform/surface/Makefile |  7 +++++++
 5 files changed, 39 insertions(+)
 create mode 100644 drivers/platform/surface/Kconfig
 create mode 100644 drivers/platform/surface/Makefile

diff --git a/MAINTAINERS b/MAINTAINERS
index b1c97a16b6ce..1c5be5875769 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11532,6 +11532,15 @@ F:	drivers/scsi/smartpqi/smartpqi*.[ch]
 F:	include/linux/cciss*.h
 F:	include/uapi/linux/cciss*.h
 
+MICROSOFT SURFACE PLATFORM DRIVERS
+M:	Hans de Goede <hdegoede@redhat.com>
+M:	Mark Gross <mgross@linux.intel.com>
+M:	Maximilian Luz <luzmaximilian@gmail.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+T:	git git://git.infradead.org/linux-platform-drivers-x86.git
+F:	drivers/platform/surface/
+
 MICROSOFT SURFACE PRO 3 BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 971426bb4302..18fc6a08569e 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -13,3 +13,5 @@ source "drivers/platform/chrome/Kconfig"
 source "drivers/platform/mellanox/Kconfig"
 
 source "drivers/platform/olpc/Kconfig"
+
+source "drivers/platform/surface/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index 6fda58c021ca..af62d93f0e46 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_MIPS)		+= mips/
 obj-$(CONFIG_OLPC_EC)		+= olpc/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
 obj-$(CONFIG_CHROME_PLATFORMS)	+= chrome/
+obj-$(CONFIG_SURFACE_PLATFORM)	+= surface/
diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
new file mode 100644
index 000000000000..806b088ddae2
--- /dev/null
+++ b/drivers/platform/surface/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Microsoft Surface Platform-Specific Drivers
+#
+
+menuconfig SURFACE_PLATFORM
+	bool "Microsoft Surface Platform-Specific Device Drivers"
+	default y
+	help
+	  Say Y here to get to see options for platform-specific device drivers
+	  for Microsoft Surface devices. This option alone does not add any
+	  kernel code.
+
+	  If you say N, all options in this submenu will be skipped and disabled.
+
+if SURFACE_PLATFORM
+
+# place Microsoft Surface platform drivers here
+
+endif # SURFACE_PLATFORM
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
new file mode 100644
index 000000000000..11788a44713f
--- /dev/null
+++ b/drivers/platform/surface/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for linux/drivers/platform/surface
+# Microsoft Surface Platform-Specific Drivers
+#
+
+# place Microsoft Surface platform drivers here
-- 
2.28.0

[PATCH v3 2/5] platform/surface: Move Surface 3 WMI driver to platform/surface

[Maximilian Luz]

Move the Surface 3 WMI driver from platform/x86 to the newly created
platform/surface directory.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 drivers/platform/surface/Kconfig                 | 12 +++++++++++-
 drivers/platform/surface/Makefile                |  2 +-
 drivers/platform/{x86 => surface}/surface3-wmi.c |  0
 drivers/platform/x86/Kconfig                     | 12 ------------
 drivers/platform/x86/Makefile                    |  1 -
 5 files changed, 12 insertions(+), 15 deletions(-)
 rename drivers/platform/{x86 => surface}/surface3-wmi.c (100%)

diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 806b088ddae2..1566b32a1884 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -15,6 +15,16 @@ menuconfig SURFACE_PLATFORM
 
 if SURFACE_PLATFORM
 
-# place Microsoft Surface platform drivers here
+config SURFACE3_WMI
+	tristate "Surface 3 WMI Driver"
+	depends on ACPI_WMI
+	depends on DMI
+	depends on INPUT
+	depends on SPI
+	help
+	  Say Y here if you have a Surface 3.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called surface3-wmi.
 
 endif # SURFACE_PLATFORM
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 11788a44713f..f889d521420f 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -4,4 +4,4 @@
 # Microsoft Surface Platform-Specific Drivers
 #
 
-# place Microsoft Surface platform drivers here
+obj-$(CONFIG_SURFACE3_WMI)		+= surface3-wmi.o
diff --git a/drivers/platform/x86/surface3-wmi.c b/drivers/platform/surface/surface3-wmi.c
similarity index 100%
rename from drivers/platform/x86/surface3-wmi.c
rename to drivers/platform/surface/surface3-wmi.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0d91d136bc3b..0759913c9846 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -870,18 +870,6 @@ config INTEL_VBTN
 	  To compile this driver as a module, choose M here: the module will
 	  be called intel_vbtn.
 
-config SURFACE3_WMI
-	tristate "Surface 3 WMI Driver"
-	depends on ACPI_WMI
-	depends on DMI
-	depends on INPUT
-	depends on SPI
-	help
-	  Say Y here if you have a Surface 3.
-
-	  To compile this driver as a module, choose M here: the module will
-	  be called surface3-wmi.
-
 config SURFACE_3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface 3 tablet"
 	depends on ACPI && KEYBOARD_GPIO && I2C
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5f823f7eff45..29563a32b3e3 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -82,7 +82,6 @@ obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
 obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
 
 # Microsoft
-obj-$(CONFIG_SURFACE3_WMI)		+= surface3-wmi.o
 obj-$(CONFIG_SURFACE_3_BUTTON)		+= surface3_button.o
 obj-$(CONFIG_SURFACE_3_POWER_OPREGION)	+= surface3_power.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
-- 
2.28.0

[PATCH v3 3/5] platform/surface: Move Surface 3 Button driver to platform/surface

[Maximilian Luz]

Move the Surface 3 Button driver from platform/x86 to the newly created
platform/surface directory.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 drivers/platform/surface/Kconfig                    | 6 ++++++
 drivers/platform/surface/Makefile                   | 1 +
 drivers/platform/{x86 => surface}/surface3_button.c | 0
 drivers/platform/x86/Kconfig                        | 6 ------
 drivers/platform/x86/Makefile                       | 1 -
 5 files changed, 7 insertions(+), 7 deletions(-)
 rename drivers/platform/{x86 => surface}/surface3_button.c (100%)

diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 1566b32a1884..22075ca451b0 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -27,4 +27,10 @@ config SURFACE3_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called surface3-wmi.
 
+config SURFACE_3_BUTTON
+	tristate "Power/home/volume buttons driver for Microsoft Surface 3 tablet"
+	depends on ACPI && KEYBOARD_GPIO && I2C
+	help
+	  This driver handles the power/home/volume buttons on the Microsoft Surface 3 tablet.
+
 endif # SURFACE_PLATFORM
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index f889d521420f..8588dc178245 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -5,3 +5,4 @@
 #
 
 obj-$(CONFIG_SURFACE3_WMI)		+= surface3-wmi.o
+obj-$(CONFIG_SURFACE_3_BUTTON)		+= surface3_button.o
diff --git a/drivers/platform/x86/surface3_button.c b/drivers/platform/surface/surface3_button.c
similarity index 100%
rename from drivers/platform/x86/surface3_button.c
rename to drivers/platform/surface/surface3_button.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0759913c9846..5fba590a1a67 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -870,12 +870,6 @@ config INTEL_VBTN
 	  To compile this driver as a module, choose M here: the module will
 	  be called intel_vbtn.
 
-config SURFACE_3_BUTTON
-	tristate "Power/home/volume buttons driver for Microsoft Surface 3 tablet"
-	depends on ACPI && KEYBOARD_GPIO && I2C
-	help
-	  This driver handles the power/home/volume buttons on the Microsoft Surface 3 tablet.
-
 config SURFACE_3_POWER_OPREGION
 	tristate "Surface 3 battery platform operation region support"
 	depends on ACPI && I2C
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 29563a32b3e3..0fd70d5d2cf3 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -82,7 +82,6 @@ obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
 obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
 
 # Microsoft
-obj-$(CONFIG_SURFACE_3_BUTTON)		+= surface3_button.o
 obj-$(CONFIG_SURFACE_3_POWER_OPREGION)	+= surface3_power.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
 
-- 
2.28.0

[PATCH v3 4/5] platform/surface: Move Surface 3 Power OpRegion driver to platform/surface

[Maximilian Luz]

Move the Surface 3 Power operation region driver from platform/x86 to
the newly created platform/surface directory.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 drivers/platform/surface/Kconfig                   | 7 +++++++
 drivers/platform/surface/Makefile                  | 1 +
 drivers/platform/{x86 => surface}/surface3_power.c | 0
 drivers/platform/x86/Kconfig                       | 7 -------
 drivers/platform/x86/Makefile                      | 1 -
 5 files changed, 8 insertions(+), 8 deletions(-)
 rename drivers/platform/{x86 => surface}/surface3_power.c (100%)

diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 22075ca451b0..2af26379804c 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -33,4 +33,11 @@ config SURFACE_3_BUTTON
 	help
 	  This driver handles the power/home/volume buttons on the Microsoft Surface 3 tablet.
 
+config SURFACE_3_POWER_OPREGION
+	tristate "Surface 3 battery platform operation region support"
+	depends on ACPI && I2C
+	help
+	  This driver provides support for ACPI operation
+	  region of the Surface 3 battery platform driver.
+
 endif # SURFACE_PLATFORM
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 8588dc178245..4940d4db58b2 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -6,3 +6,4 @@
 
 obj-$(CONFIG_SURFACE3_WMI)		+= surface3-wmi.o
 obj-$(CONFIG_SURFACE_3_BUTTON)		+= surface3_button.o
+obj-$(CONFIG_SURFACE_3_POWER_OPREGION)	+= surface3_power.o
diff --git a/drivers/platform/x86/surface3_power.c b/drivers/platform/surface/surface3_power.c
similarity index 100%
rename from drivers/platform/x86/surface3_power.c
rename to drivers/platform/surface/surface3_power.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5fba590a1a67..8417ee0178d0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -870,13 +870,6 @@ config INTEL_VBTN
 	  To compile this driver as a module, choose M here: the module will
 	  be called intel_vbtn.
 
-config SURFACE_3_POWER_OPREGION
-	tristate "Surface 3 battery platform operation region support"
-	depends on ACPI && I2C
-	help
-	  This driver provides support for ACPI operation
-	  region of the Surface 3 battery platform driver.
-
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
 	depends on ACPI && INPUT
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 0fd70d5d2cf3..ffa31f57d9a2 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -82,7 +82,6 @@ obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
 obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
 
 # Microsoft
-obj-$(CONFIG_SURFACE_3_POWER_OPREGION)	+= surface3_power.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
 
 # MSI
-- 
2.28.0

[PATCH v3 5/5] platform/surface: Move Surface Pro 3 Button driver to platform/surface

[Maximilian Luz]

Move the Surface Pro 3 Button driver from platform/x86 to the newly
created platform/surface directory.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 MAINTAINERS                                            | 2 +-
 drivers/platform/surface/Kconfig                       | 6 ++++++
 drivers/platform/surface/Makefile                      | 1 +
 drivers/platform/{x86 => surface}/surfacepro3_button.c | 0
 drivers/platform/x86/Kconfig                           | 6 ------
 drivers/platform/x86/Makefile                          | 3 ---
 6 files changed, 8 insertions(+), 10 deletions(-)
 rename drivers/platform/{x86 => surface}/surfacepro3_button.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c5be5875769..2ea51dee885f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11545,7 +11545,7 @@ MICROSOFT SURFACE PRO 3 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/surface/surfacepro3_button.c
 
 MICROTEK X6 SCANNER
 M:	Oliver Neukum <oliver@neukum.org>
diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 2af26379804c..fcd80e860132 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -40,4 +40,10 @@ config SURFACE_3_POWER_OPREGION
 	  This driver provides support for ACPI operation
 	  region of the Surface 3 battery platform driver.
 
+config SURFACE_PRO3_BUTTON
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
+	depends on ACPI && INPUT
+	help
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
+
 endif # SURFACE_PLATFORM
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 4940d4db58b2..dcb1df06d57a 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -7,3 +7,4 @@
 obj-$(CONFIG_SURFACE3_WMI)		+= surface3-wmi.o
 obj-$(CONFIG_SURFACE_3_BUTTON)		+= surface3_button.o
 obj-$(CONFIG_SURFACE_3_POWER_OPREGION)	+= surface3_power.o
+obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
similarity index 100%
rename from drivers/platform/x86/surfacepro3_button.c
rename to drivers/platform/surface/surfacepro3_button.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 8417ee0178d0..6083f8241b7d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -870,12 +870,6 @@ config INTEL_VBTN
 	  To compile this driver as a module, choose M here: the module will
 	  be called intel_vbtn.
 
-config SURFACE_PRO3_BUTTON
-	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
-	depends on ACPI && INPUT
-	help
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
-
 config MSI_LAPTOP
 	tristate "MSI Laptop Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index ffa31f57d9a2..aeff497e23a5 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -81,9 +81,6 @@ obj-$(CONFIG_INTEL_MENLOW)		+= intel_menlow.o
 obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
 obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
 
-# Microsoft
-obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
-
 # MSI
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_MSI_WMI)		+= msi-wmi.o
-- 
2.28.0

Re: [PATCH v3 1/5] platform: Add Surface platform directory

[Andy Shevchenko]

On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> It may make sense to split the Microsoft Surface hardware platform
> drivers out to a separate subdirectory, since some of it may be shared
> between ARM and x86 in the future (regarding devices like the Surface
> Pro X).
>
> Further, newer Surface devices will require additional platform drivers
> for fundamental support (mostly regarding their embedded controller),
> which may also warrant this split from a size perspective.
>
> This commit introduces a new platform/surface subdirectory for the
> Surface device family, with subsequent commits moving existing Surface
> drivers over from platform/x86.
>
> A new MAINTAINERS entry is added for this directory. Patches to files in
> this directory will be taken up by the platform-drivers-x86 team (i.e.
> Hans de Goede and Mark Gross) after they have been reviewed by
> Maximilian Luz.

Thanks for the patch, my minor comments below.

...

> +MICROSOFT SURFACE PLATFORM DRIVERS

(1)

> +M:     Hans de Goede <hdegoede@redhat.com>
> +M:     Mark Gross <mgross@linux.intel.com>
> +M:     Maximilian Luz <luzmaximilian@gmail.com>
> +L:     platform-driver-x86@vger.kernel.org
> +S:     Maintained

> +T:     git git://git.infradead.org/linux-platform-drivers-x86.git

It's now on kernel.org.
git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git

> +F:     drivers/platform/surface/

> @@ -9,3 +9,4 @@ obj-$(CONFIG_MIPS)              += mips/
>  obj-$(CONFIG_OLPC_EC)          += olpc/
>  obj-$(CONFIG_GOLDFISH)         += goldfish/
>  obj-$(CONFIG_CHROME_PLATFORMS) += chrome/
> +obj-$(CONFIG_SURFACE_PLATFORM) += surface/

(2)

> +menuconfig SURFACE_PLATFORM

(3a)

> +if SURFACE_PLATFORM

(3b)

> +endif # SURFACE_PLATFORM

(3c)

I think in (1), (2) and (3) it makes sense to mimic Chrome, i.e. use
plural: PLATFORMS.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] platform/surface: Move Surface 3 WMI driver to platform/surface

[Andy Shevchenko]

On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> Move the Surface 3 WMI driver from platform/x86 to the newly created
> platform/surface directory.

> -# place Microsoft Surface platform drivers here

> -# place Microsoft Surface platform drivers here

Not sure if we need these comments in the first place.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/5] platform/surface: Create a platform subdirectory for Microsoft Surface devices

[Andy Shevchenko]

On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> As has come up in the discussion around
>
>   [RFC PATCH] Add support for Microsoft Surface System Aggregator Module
>
> it may make sense to add a Microsoft Surface specific platform
> subdirectory. Andy has suggested drivers/platform/surface for that.
> This series follows said suggestion and creates that subdirectory, as
> well as moves Microsoft Surface related drivers over to it and updates
> their MAINTAINERS entries (if available) accordingly.
>
> This series does not modify any existing driver code, symbols, or help
> text.
>
> More details regarding maintainership can be found in
>
>   [PATCH v3 1/5] platform: Add Surface platform directory

For patches 3-5
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
1 and 2 have minor comments
You may add my Rb tag after addressing if you agree on them.

> Link to discussion:
>   https://lore.kernel.org/lkml/CAHp75Vfp86h38Rd-VEgER7ASADdmz5ymAkuHvD0Q6WPDqZBqHw@mail.gmail.com/
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Changes in v2:
>  - Rebase onto linux-platform-drivers-x86/for-next to incorporate
>    changes in Maintainer file.
>
> Changes in v3:
>  - Add separate MAINTAINERS entry for platform/surface
>  - Add Maximilian Luz as reviewer for platform/surface
>
> For more details regarding changes, refer to the individual patches.
>
> Maximilian Luz (5):
>   platform: Add Surface platform directory
>   platform/surface: Move Surface 3 WMI driver to platform/surface
>   platform/surface: Move Surface 3 Button driver to platform/surface
>   platform/surface: Move Surface 3 Power OpRegion driver to
>     platform/surface
>   platform/surface: Move Surface Pro 3 Button driver to platform/surface
>
>  MAINTAINERS                                   | 11 ++++-
>  drivers/platform/Kconfig                      |  2 +
>  drivers/platform/Makefile                     |  1 +
>  drivers/platform/surface/Kconfig              | 49 +++++++++++++++++++
>  drivers/platform/surface/Makefile             | 10 ++++
>  .../platform/{x86 => surface}/surface3-wmi.c  |  0
>  .../{x86 => surface}/surface3_button.c        |  0
>  .../{x86 => surface}/surface3_power.c         |  0
>  .../{x86 => surface}/surfacepro3_button.c     |  0
>  drivers/platform/x86/Kconfig                  | 31 ------------
>  drivers/platform/x86/Makefile                 |  6 ---
>  11 files changed, 72 insertions(+), 38 deletions(-)
>  create mode 100644 drivers/platform/surface/Kconfig
>  create mode 100644 drivers/platform/surface/Makefile
>  rename drivers/platform/{x86 => surface}/surface3-wmi.c (100%)
>  rename drivers/platform/{x86 => surface}/surface3_button.c (100%)
>  rename drivers/platform/{x86 => surface}/surface3_power.c (100%)
>  rename drivers/platform/{x86 => surface}/surfacepro3_button.c (100%)
>
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/5] platform: Add Surface platform directory

[Maximilian Luz]

On 10/8/20 4:52 PM, Andy Shevchenko wrote:
> On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> It may make sense to split the Microsoft Surface hardware platform
>> drivers out to a separate subdirectory, since some of it may be shared
>> between ARM and x86 in the future (regarding devices like the Surface
>> Pro X).
>>
>> Further, newer Surface devices will require additional platform drivers
>> for fundamental support (mostly regarding their embedded controller),
>> which may also warrant this split from a size perspective.
>>
>> This commit introduces a new platform/surface subdirectory for the
>> Surface device family, with subsequent commits moving existing Surface
>> drivers over from platform/x86.
>>
>> A new MAINTAINERS entry is added for this directory. Patches to files in
>> this directory will be taken up by the platform-drivers-x86 team (i.e.
>> Hans de Goede and Mark Gross) after they have been reviewed by
>> Maximilian Luz.
> 
> Thanks for the patch, my minor comments below.
> 
> ...
> 
>> +MICROSOFT SURFACE PLATFORM DRIVERS
> 
> (1)
> 
>> +M:     Hans de Goede <hdegoede@redhat.com>
>> +M:     Mark Gross <mgross@linux.intel.com>
>> +M:     Maximilian Luz <luzmaximilian@gmail.com>
>> +L:     platform-driver-x86@vger.kernel.org
>> +S:     Maintained
> 
>> +T:     git git://git.infradead.org/linux-platform-drivers-x86.git
> 
> It's now on kernel.org.
> git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git

Thank you, will update this. FYI: The entry was mostly copied from the
X86 PLATFORM DRIVERS entry, so it should probably be updated there, too.

>> +F:     drivers/platform/surface/
> 
>> @@ -9,3 +9,4 @@ obj-$(CONFIG_MIPS)              += mips/
>>   obj-$(CONFIG_OLPC_EC)          += olpc/
>>   obj-$(CONFIG_GOLDFISH)         += goldfish/
>>   obj-$(CONFIG_CHROME_PLATFORMS) += chrome/
>> +obj-$(CONFIG_SURFACE_PLATFORM) += surface/
> 
> (2)
> 
>> +menuconfig SURFACE_PLATFORM
> 
> (3a)
> 
>> +if SURFACE_PLATFORM
> 
> (3b)
> 
>> +endif # SURFACE_PLATFORM
> 
> (3c)
> 
> I think in (1), (2) and (3) it makes sense to mimic Chrome, i.e. use
> plural: PLATFORMS.

I agree with (2) and (3), but I'm not so sure about (1). For Chrome, the
entry is CHROME HARDWARE PLATFORM SUPPORT, so should I change it to
MICROSOFT SURFACE HARDWARE PLATFORM SUPPORT?

Thanks,
Max

Re: [PATCH v3 2/5] platform/surface: Move Surface 3 WMI driver to platform/surface

[Maximilian Luz]

On 10/8/20 4:54 PM, Andy Shevchenko wrote:
> On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> Move the Surface 3 WMI driver from platform/x86 to the newly created
>> platform/surface directory.
> 
>> -# place Microsoft Surface platform drivers here
> 
>> -# place Microsoft Surface platform drivers here
> 
> Not sure if we need these comments in the first place.

I thought it might look a bit weird having the "if" directly followed by
"endif" without at least a comment in between. I'll remove them if you
prefer that.

Thanks,
Max

Re: [PATCH v3 1/5] platform: Add Surface platform directory

[Hans de Goede]

Hi,

On 10/8/20 5:18 PM, Maximilian Luz wrote:
> On 10/8/20 4:52 PM, Andy Shevchenko wrote:
>> On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>>
>>> It may make sense to split the Microsoft Surface hardware platform
>>> drivers out to a separate subdirectory, since some of it may be shared
>>> between ARM and x86 in the future (regarding devices like the Surface
>>> Pro X).
>>>
>>> Further, newer Surface devices will require additional platform drivers
>>> for fundamental support (mostly regarding their embedded controller),
>>> which may also warrant this split from a size perspective.
>>>
>>> This commit introduces a new platform/surface subdirectory for the
>>> Surface device family, with subsequent commits moving existing Surface
>>> drivers over from platform/x86.
>>>
>>> A new MAINTAINERS entry is added for this directory. Patches to files in
>>> this directory will be taken up by the platform-drivers-x86 team (i.e.
>>> Hans de Goede and Mark Gross) after they have been reviewed by
>>> Maximilian Luz.
>>
>> Thanks for the patch, my minor comments below.
>>
>> ...
>>
>>> +MICROSOFT SURFACE PLATFORM DRIVERS
>>
>> (1)
>>
>>> +M:     Hans de Goede <hdegoede@redhat.com>
>>> +M:     Mark Gross <mgross@linux.intel.com>
>>> +M:     Maximilian Luz <luzmaximilian@gmail.com>
>>> +L:     platform-driver-x86@vger.kernel.org
>>> +S:     Maintained
>>
>>> +T:     git git://git.infradead.org/linux-platform-drivers-x86.git
>>
>> It's now on kernel.org.
>> git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git
> 
> Thank you, will update this. FYI: The entry was mostly copied from the
> X86 PLATFORM DRIVERS entry, so it should probably be updated there, too.

That is a good point I've pushed a commit fixing this to the for-next
branch of: git://git.infradead.org/linux-platform-drivers-x86.git

Please base the next version of this patch-set on top of that.

Regards,

Hans



> 
>>> +F:     drivers/platform/surface/
>>
>>> @@ -9,3 +9,4 @@ obj-$(CONFIG_MIPS)              += mips/
>>>   obj-$(CONFIG_OLPC_EC)          += olpc/
>>>   obj-$(CONFIG_GOLDFISH)         += goldfish/
>>>   obj-$(CONFIG_CHROME_PLATFORMS) += chrome/
>>> +obj-$(CONFIG_SURFACE_PLATFORM) += surface/
>>
>> (2)
>>
>>> +menuconfig SURFACE_PLATFORM
>>
>> (3a)
>>
>>> +if SURFACE_PLATFORM
>>
>> (3b)
>>
>>> +endif # SURFACE_PLATFORM
>>
>> (3c)
>>
>> I think in (1), (2) and (3) it makes sense to mimic Chrome, i.e. use
>> plural: PLATFORMS.
> 
> I agree with (2) and (3), but I'm not so sure about (1). For Chrome, the
> entry is CHROME HARDWARE PLATFORM SUPPORT, so should I change it to
> MICROSOFT SURFACE HARDWARE PLATFORM SUPPORT?
> 
> Thanks,
> Max
> 

Re: [PATCH v3 5/5] platform/surface: Move Surface Pro 3 Button driver to platform/surface

[Chen Yu]

On Thu, Oct 08, 2020 at 04:34:55PM +0200, Maximilian Luz wrote:
> Move the Surface Pro 3 Button driver from platform/x86 to the newly
> created platform/surface directory.
> 
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
>
Acked-by: Chen Yu <yu.c.chen@intel.com>

Thanks!


best,
Chenyu