Skip to contentSkip to conversation

platform: surface: Introduce Surface XBL Driver

PATCH Superseded Jarrett Schultz 7 participants 17 messages Dec 2, 2021 – Jan 14, 2022

Subsystems (6)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover platform: surface: Introduce Surface XBL Driver

Introduce the Surface Extensible Boot Loader driver for the Surface Duo. Exposes information about the driver to user space via sysfs.

---

Changes in v3:
- For the yaml documentation:
* Updated description
* Fixed examples
* Updated 'required' field
- Further propogated ACPI dependency in Kconfigs
- Updated sysfs several binding descriptions
- Renamed files to conform to naming conventions

---

Changes in v2:
- Per Maximilian, added patch 2: propagated ACPI dependency from the directory as a whole to each individual driver
- For the yaml documentation:
* Removed json-schema dependence
* Elaborated on description of driver
* Updated example
- Changed target KernelVersion in sysfs documentation
- Updated MAINTAINER changes to be properly applied across patches
- For the driver itself,
* Added types.h inclusion and removed unused inclusions
* Minor updates to code and acronym style
* Remove __packed attribute on driver struct
* Use .dev_groups for sysfs
- Added more in-depth description of driver in Kconfig
- Modified dts to reference a newly added section in sm8150.dtsi

---
Jarrett Schultz (5):
dt-bindings: platform: microsoft: Document surface xbl
platform: surface: Propagate ACPI Dependency
platform: surface: Add surface xbl
arm64: dts: qcom: sm8150: Add imem section
arm64: dts: qcom: surface-duo: Add surface xbl

 .../ABI/testing/sysfs-platform-surface-xbl    |  79 ++++++++
 .../platform/microsoft/surface-xbl.yaml       |  69 +++++++
 MAINTAINERS                                   |   9 +
 .../dts/qcom/sm8150-microsoft-surface-duo.dts |  10 +
 arch/arm64/boot/dts/qcom/sm8150.dtsi          |   8 +
 drivers/platform/surface/Kconfig              |  19 +-
 drivers/platform/surface/Makefile             |   1 +
 drivers/platform/surface/aggregator/Kconfig   |   1 +
 drivers/platform/surface/surface_xbl.c        | 186 ++++++++++++++++++
 9 files changed, 381 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl
 create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
 create mode 100644 drivers/platform/surface/surface_xbl.c
Signature · 1 line
2.25.1
#1 lore raw

PATCH v2 1/5 dt-bindings: platform: microsoft: Document surface xbl

Introduce yaml for surface xbl driver.

---

Changes in v3:
- Updated description to only pertain to the hardware
- Updated the required field to properly reflect the binding
- Removed the first example
- Fixed the size of the reg field in the second example

---

Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example

---
 .../platform/microsoft/surface-xbl.yaml       | 69 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +69 -0
new file mode 100644
index 000000000000..a661cfedd8fa
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/platform/microsoft/surface-xbl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Surface Extensible Bootloader for Microsoft Surface Duo
+
+maintainers:
+  - Jarrett Schultz <[email protected]>
+
+description: |
+  Defined to expose information that is used during production when
+  device is in manufacturing mode. Some of the information included
+  in this imem section is -
+    * board_id
+    * battery_present
+    * hw_init_retries
+    * is_customer_mode
+    * is_act_mode
+    * pmic_reset_reason
+    * touch_fw_version
+    * ocp_error_location
+  See sysfs documentation for more information.
+
+properties:
+  compatible:
+    const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  properties:
+    compatible:
+      const: microsoft,sm8150-surface-duo-xbl
+
+#    reg:
+#      maxItems: 1
+
+#  additionalProperties: false
+
+#  required:
+#    - compatible
+#    - reg
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - "#address-cells"
+  - "#size-cells"
+
+examples:
+  - |
+    imem@146bf000 {
+      compatible = "simple-mfd";
+      reg = <0x146bf000 0x1000>;
+      ranges = <0x0 0x146bf000 0x1000>;
+
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      xbl@a94 {
+        compatible = "microsoft,sm8150-surface-duo-xbl";
+        reg = <0xa94 0x100>;
+      };
+    };
MAINTAINERS +7 -0
index 913856599623..0daa416a2aba 100644
@@ -12650,6 +12650,13 @@ F:	Documentation/driver-api/surface_aggregator/clients/dtx.rst
 F:	drivers/platform/surface/surface_dtx.c
 F:	include/uapi/linux/surface_aggregator/dtx.h
 
+MICROSOFT SURFACE DUO XBL DRIVER
+M:	Jarrett Schultz <[email protected]>
+L:	[email protected]
+L:	[email protected]
+S:	Supported
+F:	Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
+
 MICROSOFT SURFACE GPE LID SUPPORT DRIVER
 M:	Maximilian Luz <[email protected]>
 L:	[email protected]
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH 1/5] dt-bindings: platform: microsoft: Document surface xbl

Quoted 5 lines · Jarrett Schultz

Introduce yaml for surface xbl driver.

Is it not possible to send patch email from @microsoft.com ?

---Trilok Soni

maintainer
#3 lore raw

Re: [PATCH 1/5] dt-bindings: platform: microsoft: Document surface xbl

In reply to Jarrett Schultz
Quoted 18 lines · Jarrett Schultz

Introduce yaml for surface xbl driver.

---

Changes in v3:
- Updated description to only pertain to the hardware
- Updated the required field to properly reflect the binding
- Removed the first example
- Fixed the size of the reg field in the second example

---

Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example

Quoted 6 lines
---
 .../platform/microsoft/surface-xbl.yaml       | 69 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml: properties:properties: 'compatible' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml: properties: 'properties' should not be valid under {'enum': ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'if', 'items', 'maxItems', 'maximum', 'minItems', 'minimum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'propertyNames', 'required', 'then', 'unevaluatedProperties']}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml: ignoring, error in schema: properties: properties
warning: no schema found in file: ./Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dt.yaml:0:0: /example-0/imem@146bf000/xbl@a94: failed to match any schema with compatible: ['microsoft,sm8150-surface-duo-xbl']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1562961

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#4 lore raw

PATCH v2 2/5 platform: surface: Propagate ACPI Dependency

In reply to Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

---

Changes in v3:
- Further propagated ACPI dependecy to SURFACE_AGGREGATOR

---

Changes in v2:
- Created to propagate ACPI dependency

---
 drivers/platform/surface/Kconfig            | 7 ++++++-
 drivers/platform/surface/aggregator/Kconfig | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)
drivers/platform/surface/Kconfig +6 -1
index 3105f651614f..5f0578e25f71 100644
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
 	bool "Microsoft Surface Platform-Specific Device Drivers"
-	depends on ACPI
 	default y
 	help
 	  Say Y here to get to see options for platform-specific device drivers
@@ -30,12 +29,14 @@ config SURFACE3_WMI
 
 config SURFACE_3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface 3 tablet"
+	depends on ACPI
 	depends on 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
 	depends on I2C
 	help
 	  This driver provides support for ACPI operation
@@ -126,6 +127,7 @@ config SURFACE_DTX
 
 config SURFACE_GPE
 	tristate "Surface GPE/Lid Support Driver"
+	depends on ACPI
 	depends on DMI
 	help
 	  This driver marks the GPEs related to the ACPI lid device found on
@@ -135,6 +137,7 @@ config SURFACE_GPE
 
 config SURFACE_HOTPLUG
 	tristate "Surface Hot-Plug Driver"
+	depends on ACPI
 	depends on GPIOLIB
 	help
 	  Driver for out-of-band hot-plug event signaling on Microsoft Surface
@@ -154,6 +157,7 @@ config SURFACE_HOTPLUG
 
 config SURFACE_PLATFORM_PROFILE
 	tristate "Surface Platform Profile Driver"
+	depends on ACPI
 	depends on SURFACE_AGGREGATOR_REGISTRY
 	select ACPI_PLATFORM_PROFILE
 	help
@@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE
 
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
+	depends on ACPI
 	depends on INPUT
 	help
 	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
drivers/platform/surface/aggregator/Kconfig +1 -0
index fd6dc452f3e8..cab020324256 100644
@@ -4,6 +4,7 @@
 menuconfig SURFACE_AGGREGATOR
 	tristate "Microsoft Surface System Aggregator Module Subsystem and Drivers"
 	depends on SERIAL_DEV_BUS
+	depends on ACPI
 	select CRC_CCITT
 	help
 	  The Surface System Aggregator Module (Surface SAM or SSAM) is an
Signature · 1 line
2.25.1
#5 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi Jarett,

Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

I think I will already merge this patch into the pdx86 tree:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/

While we are waiting for the rest of the series to get hashed out.

But as already pointed out by Trilok Soni your From: and Signed-off-by email addresses don't match.

I can fix up the From to match the Signed-off-by while I apply this, but before I do that I wanted to check with you that setting both to "Jarrett Schultz <[email protected]>" is the right thing to do ?

Regards,

Hans

Quoted 87 lines

---

Changes in v3:
- Further propagated ACPI dependecy to SURFACE_AGGREGATOR

---

Changes in v2:
- Created to propagate ACPI dependency

---
 drivers/platform/surface/Kconfig            | 7 ++++++-
 drivers/platform/surface/aggregator/Kconfig | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)
drivers/platform/surface/Kconfig +6 -1
index 3105f651614f..5f0578e25f71 100644
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
 	bool "Microsoft Surface Platform-Specific Device Drivers"
-	depends on ACPI
 	default y
 	help
 	  Say Y here to get to see options for platform-specific device drivers
@@ -30,12 +29,14 @@ config SURFACE3_WMI
 
 config SURFACE_3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface 3 tablet"
+	depends on ACPI
 	depends on 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
 	depends on I2C
 	help
 	  This driver provides support for ACPI operation
@@ -126,6 +127,7 @@ config SURFACE_DTX
 
 config SURFACE_GPE
 	tristate "Surface GPE/Lid Support Driver"
+	depends on ACPI
 	depends on DMI
 	help
 	  This driver marks the GPEs related to the ACPI lid device found on
@@ -135,6 +137,7 @@ config SURFACE_GPE
 
 config SURFACE_HOTPLUG
 	tristate "Surface Hot-Plug Driver"
+	depends on ACPI
 	depends on GPIOLIB
 	help
 	  Driver for out-of-band hot-plug event signaling on Microsoft Surface
@@ -154,6 +157,7 @@ config SURFACE_HOTPLUG
 
 config SURFACE_PLATFORM_PROFILE
 	tristate "Surface Platform Profile Driver"
+	depends on ACPI
 	depends on SURFACE_AGGREGATOR_REGISTRY
 	select ACPI_PLATFORM_PROFILE
 	help
@@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE
 
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
+	depends on ACPI
 	depends on INPUT
 	help
 	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
drivers/platform/surface/aggregator/Kconfig +1 -0
index fd6dc452f3e8..cab020324256 100644
@@ -4,6 +4,7 @@
 menuconfig SURFACE_AGGREGATOR
 	tristate "Microsoft Surface System Aggregator Module Subsystem and Drivers"
 	depends on SERIAL_DEV_BUS
+	depends on ACPI
 	select CRC_CCITT
 	help
 	  The Surface System Aggregator Module (Surface SAM or SSAM) is an
#6 lore raw

RE: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Quoted 42 lines

-----Original Message-----
From: Hans de Goede <[email protected]>
Sent: Friday, December 3, 2021 1:59 AM
To: Jarrett Schultz <[email protected]>; Rob Herring <[email protected]>; Andy Gross <[email protected]>; Bjorn Andersson <[email protected]>; Mark Gross <[email protected]>; Maximilian Luz <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Felipe Balbi <[email protected]>; Jarrett Schultz <[email protected]>
Subject: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi Jarett,

Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

I think I will already merge this patch into the pdx86 tree:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpdx86%2Fplatform- drivers- x86.git%2F&amp;data=04%7C01%7Cjaschultz%40microsoft.com%7C0ab6fcc6 4a5c4fd8657308d9b64391dd%7C72f988bf86f141af91ab2d7cd011db47%7C0 %7C0%7C637741223627024908%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000& amp;sdata=fEszuAgBLL3g2Z9Lh3DPQ%2BlzrWZR3o6aUst6fDmLOrE%3D&amp ;reserved=0

While we are waiting for the rest of the series to get hashed out.

But as already pointed out by Trilok Soni your From: and Signed-off-by email addresses don't match.

Quoted 9 lines

I can fix up the From to match the Signed-off-by while I apply this, but before I do that I wanted to check with you that setting both to "Jarrett Schultz <[email protected]>" is the right thing to do ?

Regards,

Hans

Hans,

Yes, that is the correct email. Still trying to get all the kinks worked out, I appreciate your patience.

Thank you,
Jarrett

Quoted 98 lines
Quoted 34 lines

---

Changes in v3:
- Further propagated ACPI dependecy to SURFACE_AGGREGATOR

---

Changes in v2:
- Created to propagate ACPI dependency

---
 drivers/platform/surface/Kconfig            | 7 ++++++-
 drivers/platform/surface/aggregator/Kconfig | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)
drivers/platform/surface/Kconfig +0 -1
index 3105f651614f..5f0578e25f71 100644
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
 	bool "Microsoft Surface Platform-Specific Device Drivers"
-	depends on ACPI
 	default y
 	help
 	  Say Y here to get to see options for platform-specific device

drivers @@ -30,12 +29,14 @@ config SURFACE3_WMI

Quoted 2 lines

config SURFACE_3_BUTTON
tristate "Power/home/volume buttons driver for Microsoft Surface 3

tablet"

drivers/platform/surface/Kconfig config SURFACE3_WMI

Quoted 4 lines
drivers/platform/surface/Kconfig +1 -0
+	depends on ACPI
 	depends on KEYBOARD_GPIO && I2C
 	help
 	  This driver handles the power/home/volume buttons on the

Microsoft Surface 3 tablet.

Quoted 32 lines

config SURFACE_3_POWER_OPREGION

drivers/platform/surface/Kconfig +1 -0
 	tristate "Surface 3 battery platform operation region support"
+	depends on ACPI
 	depends on I2C
 	help
 	  This driver provides support for ACPI operation @@ -126,6 +127,7

@@ config SURFACE_DTX

config SURFACE_GPE

drivers/platform/surface/Kconfig +2 -0
 	tristate "Surface GPE/Lid Support Driver"
+	depends on ACPI
 	depends on DMI
 	help
 	  This driver marks the GPEs related to the ACPI lid device found on
@@ -135,6 +137,7 @@ config SURFACE_GPE
 
 config SURFACE_HOTPLUG
 	tristate "Surface Hot-Plug Driver"
+	depends on ACPI
 	depends on GPIOLIB
 	help
 	  Driver for out-of-band hot-plug event signaling on Microsoft

Surface @@ -154,6 +157,7 @@ config SURFACE_HOTPLUG

config SURFACE_PLATFORM_PROFILE

drivers/platform/surface/Kconfig +1 -0
 	tristate "Surface Platform Profile Driver"
+	depends on ACPI
 	depends on SURFACE_AGGREGATOR_REGISTRY
 	select ACPI_PLATFORM_PROFILE
 	help

drivers/platform/surface/Kconfig config SURFACE_PLATFORM_PROFILE · line 182

Quoted 3 lines
drivers/platform/surface/Kconfig +0 -0
@@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE
 
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface

Pro 3/4 tablet"

drivers/platform/surface/Kconfig config SURFACE_PLATFORM_PROFILE

Quoted 4 lines
drivers/platform/surface/Kconfig +1 -0
+	depends on ACPI
 	depends on INPUT
 	help
 	  This driver handles the power/home/volume buttons on the

Microsoft Surface Pro 3/4 tablet.

drivers/platform/surface/aggregator/Kconfig line 5

Quoted 8 lines
drivers/platform/surface/aggregator/Kconfig +0 -0
index fd6dc452f3e8..cab020324256 100644
@@ -4,6 +4,7 @@
 menuconfig SURFACE_AGGREGATOR
 	tristate "Microsoft Surface System Aggregator Module Subsystem

and Drivers"

Quoted 6 lines
drivers/platform/surface/aggregator/Kconfig +1 -0
 	depends on SERIAL_DEV_BUS
+	depends on ACPI
 	select CRC_CCITT
 	help
 	  The Surface System Aggregator Module (Surface SAM or SSAM) is an
#7 lore raw

Re: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi,

Quoted 48 lines · Jarrett Schultz
Quoted 48 lines

-----Original Message-----
From: Hans de Goede <[email protected]>
Sent: Friday, December 3, 2021 1:59 AM
To: Jarrett Schultz <[email protected]>; Rob Herring <[email protected]>; Andy Gross <[email protected]>; Bjorn Andersson <[email protected]>; Mark Gross <[email protected]>; Maximilian Luz <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Felipe Balbi <[email protected]>; Jarrett Schultz <[email protected]>
Subject: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi Jarett,

Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

I think I will already merge this patch into the pdx86 tree:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpdx86%2Fplatform- drivers- x86.git%2F&amp;data=04%7C01%7Cjaschultz%40microsoft.com%7C0ab6fcc6 4a5c4fd8657308d9b64391dd%7C72f988bf86f141af91ab2d7cd011db47%7C0 %7C0%7C637741223627024908%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000& amp;sdata=fEszuAgBLL3g2Z9Lh3DPQ%2BlzrWZR3o6aUst6fDmLOrE%3D&amp ;reserved=0

While we are waiting for the rest of the series to get hashed out.

But as already pointed out by Trilok Soni your From: and Signed-off-by email addresses don't match.

I can fix up the From to match the Signed-off-by while I apply this, but before I do that I wanted to check with you that setting both to "Jarrett Schultz <[email protected]>" is the right thing to do ?

Regards,

Quoted 7 lines
Quoted 3 lines

Hans

Hans,

Yes, that is the correct email. Still trying to get all the kinks worked out, I appreciate your patience.

Ok, I've merged this patch now, so you can drop it from the next version of the series.

Thank you for your patch, I've applied this patch to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window.

Regards,

Hans

Quoted 94 lines
Quoted 93 lines
Quoted 29 lines

---

Changes in v3:
- Further propagated ACPI dependecy to SURFACE_AGGREGATOR

---

Changes in v2:
- Created to propagate ACPI dependency

---
 drivers/platform/surface/Kconfig            | 7 ++++++-
 drivers/platform/surface/aggregator/Kconfig | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)
drivers/platform/surface/Kconfig +0 -1
index 3105f651614f..5f0578e25f71 100644
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
 	bool "Microsoft Surface Platform-Specific Device Drivers"
-	depends on ACPI
 	default y
 	help
 	  Say Y here to get to see options for platform-specific device

drivers @@ -30,12 +29,14 @@ config SURFACE3_WMI

Quoted 2 lines

config SURFACE_3_BUTTON
tristate "Power/home/volume buttons driver for Microsoft Surface 3

tablet"

drivers/platform/surface/Kconfig config SURFACE3_WMI

Quoted 4 lines
drivers/platform/surface/Kconfig +1 -0
+	depends on ACPI
 	depends on KEYBOARD_GPIO && I2C
 	help
 	  This driver handles the power/home/volume buttons on the

Microsoft Surface 3 tablet.

Quoted 32 lines

config SURFACE_3_POWER_OPREGION

drivers/platform/surface/Kconfig +1 -0
 	tristate "Surface 3 battery platform operation region support"
+	depends on ACPI
 	depends on I2C
 	help
 	  This driver provides support for ACPI operation @@ -126,6 +127,7

@@ config SURFACE_DTX

config SURFACE_GPE

drivers/platform/surface/Kconfig +2 -0
 	tristate "Surface GPE/Lid Support Driver"
+	depends on ACPI
 	depends on DMI
 	help
 	  This driver marks the GPEs related to the ACPI lid device found on
@@ -135,6 +137,7 @@ config SURFACE_GPE
 
 config SURFACE_HOTPLUG
 	tristate "Surface Hot-Plug Driver"
+	depends on ACPI
 	depends on GPIOLIB
 	help
 	  Driver for out-of-band hot-plug event signaling on Microsoft

Surface @@ -154,6 +157,7 @@ config SURFACE_HOTPLUG

config SURFACE_PLATFORM_PROFILE

drivers/platform/surface/Kconfig +1 -0
 	tristate "Surface Platform Profile Driver"
+	depends on ACPI
 	depends on SURFACE_AGGREGATOR_REGISTRY
 	select ACPI_PLATFORM_PROFILE
 	help

drivers/platform/surface/Kconfig config SURFACE_PLATFORM_PROFILE · line 182

Quoted 3 lines
drivers/platform/surface/Kconfig +0 -0
@@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE
 
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface

Pro 3/4 tablet"

drivers/platform/surface/Kconfig config SURFACE_PLATFORM_PROFILE

Quoted 4 lines
drivers/platform/surface/Kconfig +1 -0
+	depends on ACPI
 	depends on INPUT
 	help
 	  This driver handles the power/home/volume buttons on the

Microsoft Surface Pro 3/4 tablet.

drivers/platform/surface/aggregator/Kconfig line 5

Quoted 8 lines
drivers/platform/surface/aggregator/Kconfig +0 -0
index fd6dc452f3e8..cab020324256 100644
@@ -4,6 +4,7 @@
 menuconfig SURFACE_AGGREGATOR
 	tristate "Microsoft Surface System Aggregator Module Subsystem

and Drivers"

Quoted 6 lines
drivers/platform/surface/aggregator/Kconfig +1 -0
 	depends on SERIAL_DEV_BUS
+	depends on ACPI
 	select CRC_CCITT
 	help
 	  The Surface System Aggregator Module (Surface SAM or SSAM) is an
#8 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

In reply to Jarrett Schultz

Hi Jarrett,

Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0 ("platform:
surface: Propagate ACPI Dependency").

drivers/platform/surface/Kconfig line 10

Quoted 10 lines
drivers/platform/surface/Kconfig +0 -1
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#9 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi,

Quoted 17 lines · Geert Uytterhoeven

Hi Jarrett,

Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0 ("platform:
surface: Propagate ACPI Dependency").

Quoted 4 lines
drivers/platform/surface/Kconfig +0 -0
@@ -5,7 +5,6 @@
 
Quoted 10 lines

drivers/platform/surface/Kconfig line 10

Quoted 6 lines
drivers/platform/surface/Kconfig +0 -1
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas?

In any case, what's the problem of being asked about a new symbol? That happens all the time whenever new drivers are merged, right?

Signature · 1 line
balbi
#10 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi Felipe,

Quoted 27 lines · Felipe Balbi
Quoted 15 lines · Geert Uytterhoeven
Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0 ("platform:
surface: Propagate ACPI Dependency").

Quoted 4 lines
drivers/platform/surface/Kconfig +0 -0
@@ -5,7 +5,6 @@
 
Quoted 10 lines

drivers/platform/surface/Kconfig line 10

Quoted 6 lines
drivers/platform/surface/Kconfig +0 -1
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

Quoted 2 lines

there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas?

depends on ARM64 || X86 || COMPILE_TEST?

Quoted 2 lines

In any case, what's the problem of being asked about a new symbol? That happens all the time whenever new drivers are merged, right?

In se there is no problem with being asked about a new symbol. The problem is the sheer number of symbols, and irrelevant questions (e.g. this one, on !x86 and !arm64).
Time spent on adding proper dependencies is IMHO well-spent, as it will save time for everyone who configures his own kernel.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#11 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi,

Quoted 30 lines · Geert Uytterhoeven

Hi Felipe,

Quoted 27 lines · Felipe Balbi
Quoted 25 lines · Geert Uytterhoeven
Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0 ("platform:
surface: Propagate ACPI Dependency").

drivers/platform/surface/Kconfig line 10

Quoted 10 lines
drivers/platform/surface/Kconfig +0 -1
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

Quoted 4 lines
Quoted 2 lines

there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas?

depends on ARM64 || X86 || COMPILE_TEST?

That sounds reasonable to me, I would be happy to take a patch for that.

Regards,

Hans

#12 lore raw

Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi,

Quoted 33 lines · Hans de Goede

Hi,

Quoted 30 lines · Geert Uytterhoeven

Hi Felipe,

Quoted 27 lines · Felipe Balbi
Quoted 25 lines · Geert Uytterhoeven
Quoted 6 lines · Jarrett Schultz

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0 ("platform:
surface: Propagate ACPI Dependency").

drivers/platform/surface/Kconfig line 10

Quoted 10 lines
drivers/platform/surface/Kconfig +0 -1
@@ -5,7 +5,6 @@
 
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas?

depends on ARM64 || X86 || COMPILE_TEST?

That sounds reasonable to me, I would be happy to take a patch for that.

fair enough, let's see what Jarrett replies

Signature · 1 line
balbi
#13 lore raw

RE: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI Dependency

Hi,

Quoted 44 lines

Felipe Balbi <[email protected]> writes:

Hi,

Quoted 8 lines · Hans de Goede

Hi,

Quoted 5 lines · Geert Uytterhoeven

Hi Felipe,

Quoted 2 lines · Felipe Balbi
Quoted 1 line · Geert Uytterhoeven

On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz

Quoted 8 lines · <[email protected]> wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines

Since the Surface XBL Driver does not depend on ACPI, the platform/surface directory as a whole no longer depends on ACPI. With respect to this, the ACPI dependency is moved into each config that depends on ACPI individually.

Thanks for your patch, which is now commit 272479928172edf0

("platform:

Quoted 21 lines
Quoted 21 lines
Quoted 20 lines
Quoted 6 lines

surface: Propagate ACPI Dependency").

Quoted 4 lines
drivers/platform/surface/Kconfig +0 -0
@@ -5,7 +5,6 @@
 
Quoted 11 lines
Quoted 7 lines
drivers/platform/surface/Kconfig +0 -1
 menuconfig SURFACE_PLATFORMS
        bool "Microsoft Surface Platform-Specific Device Drivers"
-       depends on ACPI
        default y
        help
          Say Y here to get to see options for platform-specific

device drivers

Without any dependency, all users configuring a kernel are now asked about this. Is there any other platform dependency that can be used instead?

there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas?

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

depends on ARM64 || X86 || COMPILE_TEST?

That sounds reasonable to me, I would be happy to take a patch for that.

fair enough, let's see what Jarrett replies

--
Balbi

Sounds good to me, I'll include this in the next patch version. Thank you for pointing this out.

-Jarrett

#14 lore raw

PATCH v2 3/5 platform: surface: Add surface xbl

In reply to Jarrett Schultz

Introduce support for the Extensible Boot Loader driver found on the Surface Duo. Makes device information available to users via sysfs.

---

Changed in v3:
- Updated several descriptions in the yaml to be more descriptive
- Renamed files to use an underscore instead of a dash

---

Changes in v2:
- Added types.h inclusion and removed unused inclusions
- Minor updates to code and acronym style
- Remove __packed attribute on driver struct
- Use .dev_groups for sysfs
- Added more in-depth description of driver in Kconfig
- Changed target KernelVersion in sysfs documentation

---
 .../ABI/testing/sysfs-platform-surface-xbl    |  79 ++++++++
 MAINTAINERS                                   |   2 +
 drivers/platform/surface/Kconfig              |  12 ++
 drivers/platform/surface/Makefile             |   1 +
 drivers/platform/surface/surface_xbl.c        | 186 ++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl
 create mode 100644 drivers/platform/surface/surface_xbl.c
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +79 -0
new file mode 100644
index 000000000000..505a662d8511
@@ -0,0 +1,79 @@
+What:		/sys/devices/platform/146bfa94.xbl/battery_present
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns whether the battery is present. Valid
+		values are:
+			0 - battery absent
+			1 - battery present
+
+What:		/sys/devices/platform/146bfa94.xbl/board_id
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns a Microsoft specific board id.
+
+What:		/sys/devices/platform/146bfa94.xbl/hw_init_retries
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns a Microsoft specific value related to the
+		retries attempted to properly initialize the battery charging
+		subsystem.
+
+What:		/sys/devices/platform/146bfa94.xbl/is_act_mode
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns whether ACT mode is enabled. Valid values
+		are:
+			0 - ACT disabled
+			1 - ACT enabled
+
+		ACT mode is used to run checks and put the device to shipmode
+		at factory.
+
+What:		/sys/devices/platform/146bfa94.xbl/is_customer_mode
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+
+		mode. Valid values are:
+			0 - Not in manufacturing mode
+			1 - In manufacturing mode
+
+What:		/sys/devices/platform/146bfa94.xbl/ocp_error_location
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns 0 or the id of the power rail which has the
+		OCP error. Valid values are:
+			Bit(s)		Meaning
+			15		More than one OCP error occurred
+			14-12		PMIC
+			11-7		SMPS
+			6-2		LDO
+			1-0		BOB
+
+What:		/sys/devices/platform/146bfa94.xbl/pmic_reset_reason
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns the reason for the reset. Valid values
+		are:
+			0 - no reason lol
+			9 - Battery driver triggered
+
+What:		/sys/devices/platform/146bfa94.xbl/touch_fw_version
+Date:		October 2021
+KernelVersion:	5.16
+Contact:	[email protected]
+Description:
+		Read only. It returns the version of the firmware.
MAINTAINERS +2 -0
index 0daa416a2aba..4f04f7afa3e1 100644
@@ -12655,7 +12655,9 @@ M:	Jarrett Schultz <[email protected]>
 L:	[email protected]
 L:	[email protected]
 S:	Supported
+F:	Documentation/ABI/testing/sysfs-platform-surface-xbl
 F:	Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
+F:	drivers/platform/surface/surface-xbl.c
 
 MICROSOFT SURFACE GPE LID SUPPORT DRIVER
 M:	Maximilian Luz <[email protected]>
drivers/platform/surface/Kconfig +12 -0
index 5f0578e25f71..96d6030d8d49 100644
@@ -185,6 +185,18 @@ config SURFACE_PRO3_BUTTON
 	help
 	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
 
+config SURFACE_XBL
+        tristate "Surface XBL Driver"
+        depends on ARM64 || COMPILE_TEST
+        depends on OF
+        help
+          If you say 'Y' to this option, support will be included for the
+          Surface Extensible Boot Loader (XBL) Driver. This driver exposes
+          information about the device through sysfs.
+
+          This driver can also be built as a module.  If so, the module
+          will be called surface-xbl.
+
 source "drivers/platform/surface/aggregator/Kconfig"
 
 endif # SURFACE_PLATFORMS
drivers/platform/surface/Makefile +1 -0
index 32889482de55..eef05d247f75 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_SURFACE_GPE)		+= surface_gpe.o
 obj-$(CONFIG_SURFACE_HOTPLUG)		+= surface_hotplug.o
 obj-$(CONFIG_SURFACE_PLATFORM_PROFILE)	+= surface_platform_profile.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
+obj-$(CONFIG_SURFACE_XBL)               += surface_xbl.o
drivers/platform/surface/surface_xbl.c new file +186 -0

186 lines not on this page · raw message

Signature · 1 line
2.25.1
#15 lore raw

PATCH v2 4/5 arm64: dts: qcom: sm8150: Add imem section

In reply to Jarrett Schultz

Introduce the imem section in preparation for the surface xbl driver.

---
Changes in v3:
- N/A

---

Changes in v2:
- Created to properly reference the xbl section inside of imem

---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)
arch/arm64/boot/dts/qcom/sm8150.dtsi +8 -0
index 81b4ff2cc4cd..bcfcb7306cdc 100644
@@ -1175,6 +1175,14 @@ gpi_dma1: dma-controller@a00000 {
 			status = "disabled";
 		};
 
+		imem: imem@146bf000 {
+			compatible = "simple-mfd";
+			reg = <0x0 0x146bf000 0x0 0x1000>;
+			ranges = <0x0 0x0 0x146bf000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		qupv3_id_1: geniqup@ac0000 {
 			compatible = "qcom,geni-se-qup";
 			reg = <0x0 0x00ac0000 0x0 0x6000>;
Signature · 1 line
2.25.1
#16 lore raw

PATCH v2 5/5 arm64: dts: qcom: surface-duo: Add surface xbl

In reply to Jarrett Schultz

Introduce device tree source for the surface xbl driver.

---

Changes in v3:
- N/A

---

Changes in v2:
- Updated to reference an offset inside of imem

---
 .../boot/dts/qcom/sm8150-microsoft-surface-duo.dts     | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts +10 -0
index 5901c28e6696..abb7964daac4 100644
@@ -430,6 +430,16 @@ &i2c19 {
 	/* MAX34417 @ 0x1e */
 };
 
+&imem {
+	status = "okay";
+
+	xbl@a94 {
+		compatible = "microsoft,sm8150-surface-duo-xbl";
+		reg = <0xa94 0x100>;
+		status = "okay";
+	};
+};
+
 &pon {
 	pwrkey {
 		status = "okay";
Signature · 1 line
2.25.1