platform: surface: Introduce Surface XBL Driver
Subsystems (5)
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 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: Propogate 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 | 78 +++++++ .../platform/microsoft/surface-xbl.yaml | 57 +++++ MAINTAINERS | 9 + .../dts/qcom/sm8150-microsoft-surface-duo.dts | 10 + arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 + drivers/platform/surface/Kconfig | 24 +- drivers/platform/surface/Makefile | 1 + drivers/platform/surface/surface-xbl.c | 215 ++++++++++++++++++ 8 files changed, 401 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
PATCH v2 1/5 dt-bindings: platform: microsoft: Document surface xbl
Introduce yaml for surface xbl driver.
---
Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example
---
.../platform/microsoft/surface-xbl.yaml | 57 +++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +57 -0
@@ -0,0 +1,57 @@ +# 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: | + Exposes the following device information to user space via sysfs - + * 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: microsoft,sm8150-surface-duo-xbl + + reg: + maxItems: 1 + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts + +examples: + - | + xbl@146bfa94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0x00 0x146bfa94 0x00 0x100>; + }; + - | + imem@146bf000 { + compatible = "simple-mfd"; + reg = <0x0 0x146bf000 0x0 0x1000>; + ranges = <0x0 0x0 0x146bf000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + + xbl@a94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0xa94 0x100>; + }; + };
MAINTAINERS +7 -0
@@ -12423,6 +12423,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
Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl
Quoted 27 lines · Jarrett Schultz
Introduce yaml for surface xbl driver.
Signed-off-by Jarrett Schultz <[email protected]>---
Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example---
.../platform/microsoft/surface-xbl.yaml | 57 +++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yamlDocumentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +3
-0@@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10
-0+$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: | + Exposes the following device information to user space via sysfs -
The devicetree should describe the hardware, or in this case the imem region. User space, sysfs etc are concepts of one possible consumer of this information and should not be part of the binding.
It might make sense to update this description to still document what's to be found in the memory region though.
Quoted 13 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +13
-0+ * 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: microsoft,sm8150-surface-duo-xbl
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 36
Quoted 10 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10
-0+ + reg: + maxItems: 1 + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts
I believe interrupts is a leftover...
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 42
Quoted 6 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +6
-0+ +examples: + - | + xbl@146bfa94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0x00 0x146bfa94 0x00 0x100>;
The example is compiled with #address-cells == #size-cells = <1>, so you should omit the extra 0 in both address and size, in both examples.
Regards,
Bjorn
Quoted 36 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +15
-0+ }; + - | + imem@146bf000 { + compatible = "simple-mfd"; + reg = <0x0 0x146bf000 0x0 0x1000>; + ranges = <0x0 0x0 0x146bf000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + + xbl@a94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0xa94 0x100>; + }; + };MAINTAINERS +7
-0@@ -12423,6 +12423,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
Rob Herring
maintainerRe: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl
In reply to Jarrett SchultzQuoted 11 lines · Jarrett Schultz
Introduce yaml for surface xbl driver.
Signed-off-by Jarrett Schultz <[email protected]>---
Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example
Quoted 7 lines
---
.../platform/microsoft/surface-xbl.yaml | 57 +++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 64 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: 'additionalProperties' is a required property
hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dts:44.11-48: Warning (ranges_format): /example-1/imem@146bf000:ranges: "ranges" property has invalid length (16 bytes) (parent #address-cells == 1, child #address-cells == 1, #size-cells == 1) /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dt.yaml: example-0: xbl@146bfa94:reg:0: [0, 342620820, 0, 256] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yamlDocumentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dt.yaml:0:0: /example-0/xbl@146bfa94: failed to match any schema with compatible: ['microsoft,sm8150-surface-duo-xbl'] /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dt.yaml: example-1: imem@146bf000:reg:0: [0, 342618112, 0, 4096] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yamlDocumentation/devicetree/bindings/platform/microsoft/surface-xbl.example.dt.yaml:0:0: /example-1/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/1552442
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.
Rob Herring
maintainerRe: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl
In reply to Jarrett SchultzQuoted 3 lines · Jarrett Schultz
Introduce yaml for surface xbl driver.
Signed-off-by Jarrett Schultz <[email protected]>
Author and Sob emails need to match.
Quoted 24 lines
---
Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example---
.../platform/microsoft/surface-xbl.yaml | 57 +++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yamlDocumentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +3
-0@@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10
-0+$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: | + Exposes the following device information to user space via sysfs -
What's sysfs? :) Linux details don't go in bindings.
Quoted 23 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +23
-0+ * 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: microsoft,sm8150-surface-duo-xbl + + reg: + maxItems: 1 + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 46
Quoted 10 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10
-0+ +examples: + - | + xbl@146bfa94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0x00 0x146bfa94 0x00 0x100>; + }; + - | + imem@146bf000 { + compatible = "simple-mfd";
'simple-mfd' needs a specific compatible for the block.
Quoted 33 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +11
-0+ reg = <0x0 0x146bf000 0x0 0x1000>; + ranges = <0x0 0x0 0x146bf000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + + xbl@a94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0xa94 0x100>; + }; + };MAINTAINERS +7
-0@@ -12423,6 +12423,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
Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl
Quoted 71 lines · Rob Herring
Quoted 3 lines · Jarrett Schultz
Introduce yaml for surface xbl driver.
Signed-off-by Jarrett Schultz <[email protected]>Author and Sob emails need to match.
Quoted 24 lines
---
Changes in v2:
- Removed json-schema dependence
- Elaborated on description of driver
- Updated example---
.../platform/microsoft/surface-xbl.yaml | 57 +++++++++++++++++++ MAINTAINERS | 7 +++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yamlDocumentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +3
-0@@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10
-0+$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: | + Exposes the following device information to user space via sysfs -What's sysfs? :) Linux details don't go in bindings.
Quoted 27 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +27
-0+ * 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: microsoft,sm8150-surface-duo-xbl + + reg: + maxItems: 1 + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts + +examples: + - | + xbl@146bfa94 {
Quoted 9 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml line 46
Quoted 6 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +6
-0+ compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0x00 0x146bfa94 0x00 0x100>; + }; + - | + imem@146bf000 { + compatible = "simple-mfd";'simple-mfd' needs a specific compatible for the block.
Is there any need to describe the inner "xbl@a94" binding? If so, could you point me in the right direction?
Quoted 33 lines
Quoted 33 lines
Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +11
-0+ reg = <0x0 0x146bf000 0x0 0x1000>; + ranges = <0x0 0x0 0x146bf000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + + xbl@a94 { + compatible = "microsoft,sm8150-surface-duo-xbl"; + reg = <0xa94 0x100>; + }; + };MAINTAINERS +7
-0@@ -12423,6 +12423,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
PATCH v2 2/5 platform: surface: Propagate ACPI Dependency
In reply to Jarrett SchultzFrom: Jarrett Schultz <[email protected]>
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 v2:
- Created to propagate ACPI dependency
---
drivers/platform/surface/Kconfig | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/platform/surface/Kconfig +11 -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
@@ -18,6 +17,7 @@ if SURFACE_PLATFORMS config SURFACE3_WMI tristate "Surface 3 WMI Driver" + depends on ACPI depends on ACPI_WMI depends on DMI depends on INPUT
@@ -30,12 +30,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
@@ -43,6 +45,7 @@ config SURFACE_3_POWER_OPREGION config SURFACE_ACPI_NOTIFY tristate "Surface ACPI Notify Driver" + depends on ACPI depends on SURFACE_AGGREGATOR help Surface ACPI Notify (SAN) driver for Microsoft Surface devices.
@@ -62,6 +65,7 @@ config SURFACE_ACPI_NOTIFY config SURFACE_AGGREGATOR_CDEV tristate "Surface System Aggregator Module User-Space Interface" + depends on ACPI depends on SURFACE_AGGREGATOR help Provides a misc-device interface to the Surface System Aggregator
@@ -79,6 +83,7 @@ config SURFACE_AGGREGATOR_CDEV config SURFACE_AGGREGATOR_REGISTRY tristate "Surface System Aggregator Module Device Registry" + depends on ACPI depends on SURFACE_AGGREGATOR depends on SURFACE_AGGREGATOR_BUS help
@@ -106,6 +111,7 @@ config SURFACE_AGGREGATOR_REGISTRY config SURFACE_DTX tristate "Surface DTX (Detachment System) Driver" + depends on ACPI depends on SURFACE_AGGREGATOR depends on INPUT help
@@ -126,6 +132,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 +142,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 +162,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 +185,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.
Signature · 1 line
2.25.1
Maximilian Luz
maintainerRe: [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency
On 11/8/21 17:44, Jarrett Schultz wrote:> From: Jarrett Schultz <[email protected]>
Quoted 7 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.
Signed-off-by Jarrett Schultz <[email protected]>
Some remarks inline:
Quoted 8 lines
---
Changes in v2:
- Created to propagate ACPI dependency---
drivers/platform/surface/Kconfig | 12 +++++++++++-
You also need to account for included Kconfigs, specifically:
drivers/platform/surface/aggregator/Kconfig.
Quoted 15 lines
1 file changed, 11 insertions(+), 1 deletion(-)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
drivers/platform/surface/Kconfig if SURFACE_PLATFORMS · line 20
Quoted 4 lines
drivers/platform/surface/Kconfig +1
-0@@ -18,6 +17,7 @@ if SURFACE_PLATFORMS config SURFACE3_WMI tristate "Surface 3 WMI Driver" + depends on ACPI
This is redundant, you can drop that. ACPI_WMI already depends on ACPI.
Quoted 19 lines
drivers/platform/surface/Kconfig +2
-0depends on ACPI_WMI depends on DMI depends on INPUT@@ -30,12 +30,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
drivers/platform/surface/Kconfig config SURFACE_3_POWER_OPREGION · line 48
Quoted 4 lines
drivers/platform/surface/Kconfig +1
-0@@ -43,6 +45,7 @@ config SURFACE_3_POWER_OPREGION config SURFACE_ACPI_NOTIFY tristate "Surface ACPI Notify Driver" + depends on ACPI
As mentioned above, you're missing aggregator/Kconfig. All you need to do is add "depends on ACPI" to SURFACE_AGGREGATOR in that file. Then you can drop the "depends on ACPI" for anything that depends on that.
Same holds for the couple of options depending on SURFACE_AGGREGATOR below.
Quoted 60 lines
drivers/platform/surface/Kconfig +7
-0depends on SURFACE_AGGREGATOR help Surface ACPI Notify (SAN) driver for Microsoft Surface devices.@@ -62,6 +65,7 @@ config SURFACE_ACPI_NOTIFY config SURFACE_AGGREGATOR_CDEV tristate "Surface System Aggregator Module User-Space Interface" + depends on ACPI depends on SURFACE_AGGREGATOR help Provides a misc-device interface to the Surface System Aggregator@@ -79,6 +83,7 @@ config SURFACE_AGGREGATOR_CDEV config SURFACE_AGGREGATOR_REGISTRY tristate "Surface System Aggregator Module Device Registry" + depends on ACPI depends on SURFACE_AGGREGATOR depends on SURFACE_AGGREGATOR_BUS help@@ -106,6 +111,7 @@ config SURFACE_AGGREGATOR_REGISTRY config SURFACE_DTX tristate "Surface DTX (Detachment System) Driver" + depends on ACPI depends on SURFACE_AGGREGATOR depends on INPUT help@@ -126,6 +132,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 +142,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 +162,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 +185,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.
kernel test robot
build botRe: [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency
In reply to Jarrett SchultzI love your patch! Yet something to improve:
Hi Jarrett,
I love your patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on platform-drivers-x86/for-next linus/master v5.15 next-20211111] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jarrett-Schultz/platform-surface-Introduce-Surface-XBL-Driver/20211109-004605 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: arc-allyesconfig (attached as .config) compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/6cc91cd949ff1d32a3f6b323d055b1925627be02
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jarrett-Schultz/platform-surface-Introduce-Surface-XBL-Driver/20211109-004605 git checkout 6cc91cd949ff1d32a3f6b323d055b1925627be02
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
drivers/platform/surface/aggregator/core.c: In function 'ssam_serial_hub_probe':
>> drivers/platform/surface/aggregator/core.c:648:49: error: invalid use of undefined type 'struct acpi_device'
648 | astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev);
| ^~
>> drivers/platform/surface/aggregator/core.c:702:9: error: implicit declaration of function 'acpi_dev_clear_dependencies' [-Werror=implicit-function-declaration]
702 | acpi_dev_clear_dependencies(ssh);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/trace_events.h:21,
from include/trace/define_trace.h:102,
from drivers/platform/surface/aggregator/trace.h:632,
from drivers/platform/surface/aggregator/core.c:30:
At top level:
arch/arc/include/asm/perf_event.h:126:27: error: 'arc_pmu_cache_map' defined but not used [-Werror=unused-const-variable=]
126 | static const unsigned int arc_pmu_cache_map[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
| ^~~~~~~~~~~~~~~~~
arch/arc/include/asm/perf_event.h:91:27: error: 'arc_pmu_ev_hw_map' defined but not used [-Werror=unused-const-variable=]
91 | static const char * const arc_pmu_ev_hw_map[] = {
| ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--
drivers/platform/surface/aggregator/controller.c: In function 'ssam_dsm_get_functions':
>> drivers/platform/surface/aggregator/controller.c:1044:14: error: implicit declaration of function 'acpi_has_method'; did you mean 'acpi_has_watchdog'? [-Werror=implicit-function-declaration]
1044 | if (!acpi_has_method(handle, "_DSM"))
| ^~~~~~~~~~~~~~~
| acpi_has_watchdog
>> drivers/platform/surface/aggregator/controller.c:1047:15: error: implicit declaration of function 'acpi_evaluate_dsm_typed'; did you mean 'acpi_evaluate_dsm'? [-Werror=implicit-function-declaration]
1047 | obj = acpi_evaluate_dsm_typed(handle, &SSAM_SSH_DSM_GUID,
| ^~~~~~~~~~~~~~~~~~~~~~~
| acpi_evaluate_dsm
>> drivers/platform/surface/aggregator/controller.c:1047:13: error: assignment to 'union acpi_object *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion] 1047 | obj = acpi_evaluate_dsm_typed(handle, &SSAM_SSH_DSM_GUID,
| ^
drivers/platform/surface/aggregator/controller.c: In function 'ssam_dsm_load_u32':
drivers/platform/surface/aggregator/controller.c:1071:13: error: assignment to 'union acpi_object *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
1071 | obj = acpi_evaluate_dsm_typed(handle, &SSAM_SSH_DSM_GUID,
| ^
cc1: all warnings being treated as errors
vim +648 drivers/platform/surface/aggregator/core.c
c167b9c7e3d613 Maximilian Luz 2020-12-21 614
c167b9c7e3d613 Maximilian Luz 2020-12-21 615 static int ssam_serial_hub_probe(struct serdev_device *serdev)
c167b9c7e3d613 Maximilian Luz 2020-12-21 616 {
a9e10e58730432 Daniel Scally 2021-06-03 617 struct acpi_device *ssh = ACPI_COMPANION(&serdev->dev);
c167b9c7e3d613 Maximilian Luz 2020-12-21 618 struct ssam_controller *ctrl;
c167b9c7e3d613 Maximilian Luz 2020-12-21 619 acpi_status astatus;
c167b9c7e3d613 Maximilian Luz 2020-12-21 620 int status;
c167b9c7e3d613 Maximilian Luz 2020-12-21 621
c167b9c7e3d613 Maximilian Luz 2020-12-21 622 if (gpiod_count(&serdev->dev, NULL) < 0)
c167b9c7e3d613 Maximilian Luz 2020-12-21 623 return -ENODEV;
c167b9c7e3d613 Maximilian Luz 2020-12-21 624
c167b9c7e3d613 Maximilian Luz 2020-12-21 625 status = devm_acpi_dev_add_driver_gpios(&serdev->dev, ssam_acpi_gpios);
c167b9c7e3d613 Maximilian Luz 2020-12-21 626 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 627 return status;
c167b9c7e3d613 Maximilian Luz 2020-12-21 628
c167b9c7e3d613 Maximilian Luz 2020-12-21 629 /* Allocate controller. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 630 ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
c167b9c7e3d613 Maximilian Luz 2020-12-21 631 if (!ctrl)
c167b9c7e3d613 Maximilian Luz 2020-12-21 632 return -ENOMEM;
c167b9c7e3d613 Maximilian Luz 2020-12-21 633
c167b9c7e3d613 Maximilian Luz 2020-12-21 634 /* Initialize controller. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 635 status = ssam_controller_init(ctrl, serdev);
c167b9c7e3d613 Maximilian Luz 2020-12-21 636 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 637 goto err_ctrl_init;
c167b9c7e3d613 Maximilian Luz 2020-12-21 638
c167b9c7e3d613 Maximilian Luz 2020-12-21 639 ssam_controller_lock(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 640
c167b9c7e3d613 Maximilian Luz 2020-12-21 641 /* Set up serdev device. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 642 serdev_device_set_drvdata(serdev, ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 643 serdev_device_set_client_ops(serdev, &ssam_serdev_ops);
c167b9c7e3d613 Maximilian Luz 2020-12-21 644 status = serdev_device_open(serdev);
c167b9c7e3d613 Maximilian Luz 2020-12-21 645 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 646 goto err_devopen;
c167b9c7e3d613 Maximilian Luz 2020-12-21 647
a9e10e58730432 Daniel Scally 2021-06-03 @648 astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev);
c167b9c7e3d613 Maximilian Luz 2020-12-21 649 if (ACPI_FAILURE(astatus)) {
c167b9c7e3d613 Maximilian Luz 2020-12-21 650 status = -ENXIO;
c167b9c7e3d613 Maximilian Luz 2020-12-21 651 goto err_devinit;
c167b9c7e3d613 Maximilian Luz 2020-12-21 652 }
c167b9c7e3d613 Maximilian Luz 2020-12-21 653
c167b9c7e3d613 Maximilian Luz 2020-12-21 654 /* Start controller. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 655 status = ssam_controller_start(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 656 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 657 goto err_devinit;
c167b9c7e3d613 Maximilian Luz 2020-12-21 658
c167b9c7e3d613 Maximilian Luz 2020-12-21 659 ssam_controller_unlock(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 660
c167b9c7e3d613 Maximilian Luz 2020-12-21 661 /*
c167b9c7e3d613 Maximilian Luz 2020-12-21 662 * Initial SAM requests: Log version and notify default/init power
c167b9c7e3d613 Maximilian Luz 2020-12-21 663 * states.
c167b9c7e3d613 Maximilian Luz 2020-12-21 664 */
c167b9c7e3d613 Maximilian Luz 2020-12-21 665 status = ssam_log_firmware_version(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 666 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 667 goto err_initrq;
c167b9c7e3d613 Maximilian Luz 2020-12-21 668
c167b9c7e3d613 Maximilian Luz 2020-12-21 669 status = ssam_ctrl_notif_d0_entry(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 670 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 671 goto err_initrq;
c167b9c7e3d613 Maximilian Luz 2020-12-21 672
c167b9c7e3d613 Maximilian Luz 2020-12-21 673 status = ssam_ctrl_notif_display_on(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 674 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 675 goto err_initrq;
c167b9c7e3d613 Maximilian Luz 2020-12-21 676
c167b9c7e3d613 Maximilian Luz 2020-12-21 677 status = sysfs_create_group(&serdev->dev.kobj, &ssam_sam_group);
c167b9c7e3d613 Maximilian Luz 2020-12-21 678 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 679 goto err_initrq;
c167b9c7e3d613 Maximilian Luz 2020-12-21 680
c167b9c7e3d613 Maximilian Luz 2020-12-21 681 /* Set up IRQ. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 682 status = ssam_irq_setup(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 683 if (status)
c167b9c7e3d613 Maximilian Luz 2020-12-21 684 goto err_irq;
c167b9c7e3d613 Maximilian Luz 2020-12-21 685
c167b9c7e3d613 Maximilian Luz 2020-12-21 686 /* Finally, set main controller reference. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 687 status = ssam_try_set_controller(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 688 if (WARN_ON(status)) /* Currently, we're the only provider. */
c167b9c7e3d613 Maximilian Luz 2020-12-21 689 goto err_mainref;
c167b9c7e3d613 Maximilian Luz 2020-12-21 690
c167b9c7e3d613 Maximilian Luz 2020-12-21 691 /*
c167b9c7e3d613 Maximilian Luz 2020-12-21 692 * TODO: The EC can wake up the system via the associated GPIO interrupt
c167b9c7e3d613 Maximilian Luz 2020-12-21 693 * in multiple situations. One of which is the remaining battery
c167b9c7e3d613 Maximilian Luz 2020-12-21 694 * capacity falling below a certain threshold. Normally, we should
c167b9c7e3d613 Maximilian Luz 2020-12-21 695 * use the device_init_wakeup function, however, the EC also seems
c167b9c7e3d613 Maximilian Luz 2020-12-21 696 * to have other reasons for waking up the system and it seems
c167b9c7e3d613 Maximilian Luz 2020-12-21 697 * that Windows has additional checks whether the system should be
c167b9c7e3d613 Maximilian Luz 2020-12-21 698 * resumed. In short, this causes some spurious unwanted wake-ups.
c167b9c7e3d613 Maximilian Luz 2020-12-21 699 * For now let's thus default power/wakeup to false.
c167b9c7e3d613 Maximilian Luz 2020-12-21 700 */
c167b9c7e3d613 Maximilian Luz 2020-12-21 701 device_set_wakeup_capable(&serdev->dev, true);
a9e10e58730432 Daniel Scally 2021-06-03 @702 acpi_dev_clear_dependencies(ssh);
c167b9c7e3d613 Maximilian Luz 2020-12-21 703
c167b9c7e3d613 Maximilian Luz 2020-12-21 704 return 0;
c167b9c7e3d613 Maximilian Luz 2020-12-21 705
c167b9c7e3d613 Maximilian Luz 2020-12-21 706 err_mainref:
c167b9c7e3d613 Maximilian Luz 2020-12-21 707 ssam_irq_free(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 708 err_irq:
c167b9c7e3d613 Maximilian Luz 2020-12-21 709 sysfs_remove_group(&serdev->dev.kobj, &ssam_sam_group);
c167b9c7e3d613 Maximilian Luz 2020-12-21 710 err_initrq:
c167b9c7e3d613 Maximilian Luz 2020-12-21 711 ssam_controller_lock(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 712 ssam_controller_shutdown(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 713 err_devinit:
c167b9c7e3d613 Maximilian Luz 2020-12-21 714 serdev_device_close(serdev);
c167b9c7e3d613 Maximilian Luz 2020-12-21 715 err_devopen:
c167b9c7e3d613 Maximilian Luz 2020-12-21 716 ssam_controller_destroy(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 717 ssam_controller_unlock(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 718 err_ctrl_init:
c167b9c7e3d613 Maximilian Luz 2020-12-21 719 kfree(ctrl);
c167b9c7e3d613 Maximilian Luz 2020-12-21 720 return status;
c167b9c7e3d613 Maximilian Luz 2020-12-21 721 }
c167b9c7e3d613 Maximilian Luz 2020-12-21 722 ---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
PATCH v2 3/5 platform: surface: Add surface xbl
In reply to Jarrett SchultzIntroduce support for the Extensible Boot Loader driver found on the Surface Duo. Makes device information available to users via sysfs.
---
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 | 78 +++++++ MAINTAINERS | 2 + drivers/platform/surface/Kconfig | 12 + drivers/platform/surface/Makefile | 1 + drivers/platform/surface/surface-xbl.c | 215 ++++++++++++++++++ 5 files changed, 308 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 +78 -0
@@ -0,0 +1,78 @@ +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 the 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 retries attempted to initialize the + discrete hardware circuit. + +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: + Read only. It returns whether the device is in manufacturing + 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 which power rail 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
@@ -12428,7 +12428,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
@@ -190,6 +190,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
@@ -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 +215 -0
215 lines not on this page · raw message
Signature · 1 line
2.25.1
Re: [PATCH v2 3/5] platform: surface: Add surface xbl
Quoted 32 lines · Jarrett Schultz
Introduce support for the Extensible Boot Loader driver found on the Surface Duo. Makes device information available to users via sysfs.
Signed-off-by Jarrett Schultz <[email protected]>---
Changes in v2:
- Addedtypes.hinclusion and removed unused inclusions
- Minor updates to code and acronym style
- Remove__packedattribute on driver struct
- Use .dev_groups for sysfs
- Added more in-depth description of driver inKconfig
- Changed target KernelVersion in sysfs documentation---
.../ABI/testing/sysfs-platform-surface-xbl | 78 +++++++ MAINTAINERS | 2 + drivers/platform/surface/Kconfig | 12 + drivers/platform/surface/Makefile | 1 + drivers/platform/surface/surface-xbl.c | 215 ++++++++++++++++++ 5 files changed, 308 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl create mode 100644 drivers/platform/surface/surface-xbl.cDocumentation/ABI/testing/sysfs-platform-surface-xbl new file +0
-0
Documentation/ABI/testing/sysfs-platform-surface-xbl line 9
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +9
-0@@ -0,0 +1,78 @@ +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
Would this information not be available from some battery driver, under /sys/class/power_supply?
Documentation/ABI/testing/sysfs-platform-surface-xbl line 16
Quoted 7 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +7
-0+ +What: /sys/devices/platform/146bfa94.xbl/board_id +Date: October 2021 +KernelVersion: 5.16 +Contact: [email protected] +Description: + Read only. It returns the board id.
Is this a Microsoft-specific board id, or does it relate to the Qualcomm socinfo property with the same name?
Documentation/ABI/testing/sysfs-platform-surface-xbl line 24
Quoted 8 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +8
-0+ +What: /sys/devices/platform/146bfa94.xbl/hw_init_retries +Date: October 2021 +KernelVersion: 5.16 +Contact: [email protected] +Description: + Read only. It returns retries attempted to initialize the + discrete hardware circuit.
Which description hardware circuit?
Quoted 20 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +20
-0+ +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: + Read only. It returns whether the device is in manufacturing
Documentation/ABI/testing/sysfs-platform-surface-xbl line 54
Quoted 10 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +10
-0+ 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 which power rail has the OCP error.
Sounds like the reason is singular, so why is this a bitmask?
Quoted 5 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +5
-0+ Valid values are: + Bit(s) Meaning + 15 More than one OCP error occurred + 14-12 PMIC + 11-7 SMPS
Documentation/ABI/testing/sysfs-platform-surface-xbl line 69
Quoted 10 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +10
-0+ 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:
Is this different from the PMIC reset reason that we read from the PMIC? Could we make sure to expose this generically for all Qualcomm PMICs?
Documentation/ABI/testing/sysfs-platform-surface-xbl line 78
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +9
-0+ 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.
Why isn't this exposed by the touchscreen driver instead?
Generally I wonder how you're consuming this information in userspace. Is it only for debugging purposes, i.e. would debugfs be a better place?
Quoted 42 lines
MAINTAINERS +2
-0@@ -12428,7 +12428,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@@ -190,6 +190,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_PLATFORMSdrivers/platform/surface/Makefile +0
-0
drivers/platform/surface/Makefile obj-$(CONFIG_SURFACE_GPE) += surface_gpe.o · line 19
Quoted 4 lines
drivers/platform/surface/Makefile +1
-0@@ -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
All other files in this directory are named with an underscore, would be nice to carry on with such convention.
Quoted 38 lines
drivers/platform/surface/surface-xbl.c new file +32
-0@@ -0,0 +1,215 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* Surface eXtensible Boot Loader (XBL) +* +* Copyright (C) 2021 Microsoft Corporation +* Author: Jarrett Schultz <[email protected]> +*/ + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/types.h> + +#define SURFACE_XBL_MAX_VERSION_LEN 16 +#define SURFACE_XBL_BOARD_ID 0 +#define SURFACE_XBL_BATTERY_PRESENT 1 +#define SURFACE_XBL_HW_INIT_RETRIES 2 +#define SURFACE_XBL_IS_CUSTOMER_MODE 3 +#define SURFACE_XBL_IS_ACT_MODE 4 +#define SURFACE_XBL_PMIC_RESET_REASON 5 +#define SURFACE_XBL_TOUCH_FW_VERSION 6 +#define SURFACE_XBL_OCP_ERROR_LOCATION \ + (SURFACE_XBL_TOUCH_FW_VERSION + \ + SURFACE_XBL_MAX_VERSION_LEN) + +struct surface_xbl { +struct device *dev; +void __iomem *regs; + +u8 board_id; +u8 battery_present;
drivers/platform/surface/surface-xbl.c line 42
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+u8 hw_init_retries; +u8 is_customer_mode; +u8 is_act_mode; +u8 pmic_reset_reason; +char touch_fw_version[SURFACE_XBL_MAX_VERSION_LEN ]; +u16 ocp_error_location; +}; + +static ssize_t +board_id_show (struct device *dev,struct device_attribute *attr,char *buf)
I think it would be nice to avoid some duplication by putting all these integer ones in a single show(), see e.g. soc_info_show()
Quoted 84 lines
drivers/platform/surface/surface-xbl.c new file +84
-0+{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->board_id); +} +static DEVICE_ATTR_RO (board_id); + +static ssize_t +battery_present_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->battery_present); +} +static DEVICE_ATTR_RO (battery_present); + +static ssize_t +hw_init_retries_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->hw_init_retries); +} +static DEVICE_ATTR_RO (hw_init_retries); + +static ssize_t +is_customer_mode_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->is_customer_mode); +} +static DEVICE_ATTR_RO (is_customer_mode); + +static ssize_t +is_act_mode_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->is_act_mode); +} +static DEVICE_ATTR_RO (is_act_mode); + +static ssize_t +pmic_reset_reason_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->pmic_reset_reason); +} +static DEVICE_ATTR_RO (pmic_reset_reason); + +static ssize_t +touch_fw_version_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"0x%s\n" , sxbl->touch_fw_version); +} +static DEVICE_ATTR_RO (touch_fw_version); + +static ssize_t +ocp_error_location_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->ocp_error_location); +} +static DEVICE_ATTR_RO (ocp_error_location); + +static struct attribute *xbl_attrs[] = { + &dev_attr_board_id.attr, + &dev_attr_battery_present.attr, + &dev_attr_hw_init_retries.attr, + &dev_attr_is_customer_mode.attr, + &dev_attr_is_act_mode.attr, + &dev_attr_pmic_reset_reason.attr, + &dev_attr_touch_fw_version.attr, + &dev_attr_ocp_error_location.attr, +NULL +}; + +static const struct attribute_group xbl_attr_group = { + .attrs = xbl_attrs,
drivers/platform/surface/surface-xbl.c line 136
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+}; + +const struct attribute_group *xbl_sysfs_groups[] = { + &xbl_attr_group, +NULL +}; + +static u8 surface_xbl_readb (void __iomem *base,u32 offset) +{ +return readb (base + offset);
Instead of having these helpers I think you should just call readb(base
+ offset) directly below.
The shorter function name (readb vs surface_xbl_readb) means that you don't even need to line wrap those lines.
Quoted 6 lines
drivers/platform/surface/surface-xbl.c new file +6
-0+} + +static u16 surface_xbl_readw (void __iomem *base,u32 offset) +{ +return readw (base + offset); +}
drivers/platform/surface/surface-xbl.c line 152
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ +static int surface_xbl_probe (struct platform_device *pdev) +{ +struct surface_xbl *sxbl; +struct device *dev; +void __iomem *regs; +int index; + + dev = &pdev->dev; + sxbl =devm_kzalloc (dev,sizeof (*sxbl),GFP_KERNEL );
This is the only use of &pdev->dev, so put that here and drop "dev" from sxbl (and the stack).
drivers/platform/surface/surface-xbl.c line 162
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+if (!sxbl) +return -ENOMEM ; + + sxbl->dev = dev; + + regs =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (regs)) +return PTR_ERR (regs); + + sxbl->regs = regs;
Seems only reason you stash "regs" in sxbl is so that you can pass sxbl->regs in below function calls. I.e. it's a local variable and you can omit it from struct surface_xbl...
Quoted 7 lines
drivers/platform/surface/surface-xbl.c new file +7
-0+ +platform_set_drvdata (pdev, sxbl); + + sxbl->board_id =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_BOARD_ID ); + sxbl->battery_present =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_BATTERY_PRESENT );
drivers/platform/surface/surface-xbl.c line 179
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ sxbl->hw_init_retries =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_HW_INIT_RETRIES ); + sxbl->is_customer_mode =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_IS_CUSTOMER_MODE ); + sxbl->is_act_mode =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_IS_ACT_MODE ); + sxbl->pmic_reset_reason =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_PMIC_RESET_REASON ); + +for (index =0 ; index <SURFACE_XBL_MAX_VERSION_LEN ; index++)
"i" is a good succinct variable name for an index.
drivers/platform/surface/surface-xbl.c line 189
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ sxbl->touch_fw_version[index] =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_TOUCH_FW_VERSION + index); + + sxbl->ocp_error_location =surface_xbl_readw (sxbl->regs, +SURFACE_XBL_OCP_ERROR_LOCATION ); + +return 0 ; +} + +static int surface_xbl_remove (struct platform_device *pdev)
Your remove function is empty, simply omit it...
Regards,
Bjorn
Quoted 29 lines
drivers/platform/surface/surface-xbl.c new file +26
-0+{ +return 0 ; +} + +static const struct of_device_id surface_xbl_of_match[] = { + { + .compatible ="microsoft,sm8150-surface-duo-xbl" + }, + { } +}; +MODULE_DEVICE_TABLE (of, surface_xbl_of_match); + +static struct platform_driver surface_xbl_driver = { + .driver = { + .name ="surface-xbl" , + .of_match_table = surface_xbl_of_match, + .dev_groups = xbl_sysfs_groups + }, + .probe = surface_xbl_probe, + .remove = surface_xbl_remove +}; +module_platform_driver (surface_xbl_driver); + +MODULE_AUTHOR ("Jarrett Schultz <[email protected]>" ); +MODULE_DESCRIPTION ("Surface Extensible Bootloader" ); +MODULE_LICENSE ("GPL" );Signature · 1 line
2.25.1
Re: [PATCH v2 3/5] platform: surface: Add surface xbl
Quoted 38 lines · Bjorn Andersson
Quoted 36 lines · Jarrett Schultz
Introduce support for the Extensible Boot Loader driver found on the Surface Duo. Makes device information available to users via sysfs.
Signed-off-by Jarrett Schultz <[email protected]>---
Changes in v2:
- Addedtypes.hinclusion and removed unused inclusions
- Minor updates to code and acronym style
- Remove__packedattribute on driver struct
- Use .dev_groups for sysfs
- Added more in-depth description of driver inKconfig
- Changed target KernelVersion in sysfs documentation---
.../ABI/testing/sysfs-platform-surface-xbl | 78 +++++++ MAINTAINERS | 2 + drivers/platform/surface/Kconfig | 12 + drivers/platform/surface/Makefile | 1 + drivers/platform/surface/surface-xbl.c | 215 ++++++++++++++++++ 5 files changed, 308 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl create mode 100644 drivers/platform/surface/surface-xbl.cDocumentation/ABI/testing/sysfs-platform-surface-xbl new file +4
-0@@ -0,0 +1,78 @@ +What: /sys/devices/platform/146bfa94.xbl/battery_present +Date: October 2021 +KernelVersion: 5.16 +Contact: [email protected]
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 9
Quoted 5 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +5
-0+Description: + Read only. It returns whether the battery is present. Valid + values are: + 0 - battery absent + 1 - battery presentWould this information not be available from some battery driver, under
/sys/class/power_supply?
See below under touch_fw_version.
Quoted 11 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 16
Quoted 7 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +7
-0+ +What: /sys/devices/platform/146bfa94.xbl/board_id +Date: October 2021 +KernelVersion: 5.16 +Contact: [email protected] +Description: + Read only. It returns the board id.Is this a Microsoft-specific board id, or does it relate to the Qualcomm socinfo property with the same name?
Microsoft-specific board id. I will include this in the new description.
Quoted 11 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 24
Quoted 8 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +8
-0+ +What: /sys/devices/platform/146bfa94.xbl/hw_init_retries +Date: October 2021 +KernelVersion: 5.16 +Contact: [email protected] +Description: + Read only. It returns retries attempted to initialize the + discrete hardware circuit.Which description hardware circuit?
This is a Microsoft specific value for the battery charging subsystem. I will include this in the new description.
Quoted 24 lines
Quoted 24 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +24
-0+ +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: + Read only. It returns whether the device is in manufacturing + mode. Valid values are: + 0 - Not in manufacturing mode + 1 - In manufacturing mode +
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 54
Quoted 6 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +6
-0+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 which power rail has the OCP error.Sounds like the reason is singular, so why is this a bitmask?
There are multiple power rails so if there is an error, the value will be the power rail which had the error.
Quoted 10 lines
Quoted 10 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +10
-0+ 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
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 69
Quoted 5 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +5
-0+KernelVersion: 5.16 +Contact: [email protected] +Description: + Read only. It returns the reason for the reset. Valid values + are:Is this different from the PMIC reset reason that we read from the PMIC? Could we make sure to expose this generically for all Qualcomm PMICs?
This is the same as the one read from PMIC. This information should be exposed generically for all Qualcomm PMICs. Here is some information from my colleague about the value:
- It is provided by QC (ResetRuntimeDxe driver in QComPkg)
- Protocol GUID: gEfiResetReasonProtocolGuid
- API: ReadPmicRegisterForResetReason
* Local Protocol: gQcomPmicPwrOnProtocolGuid
* Get Value: PmicPwrOnProtocol->GetSpareReg (PRIMARY_PMIC, EFI_PM_PON_SOFT_SPARE, &Value);
Quoted 13 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl line 78
Quoted 9 lines
Documentation/ABI/testing/sysfs-platform-surface-xbl new file +9
-0+ 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.Why isn't this exposed by the touchscreen driver instead?
Quoted 3 lines
Generally I wonder how you're consuming this information in userspace. Is it only for debugging purposes, i.e. would debugfs be a better place?
The information exposed through this driver is used in manufacturing mode when producing the device.
Please let me know if you need additional information. All your other other comments will be addressed in the next version, thank you for your continued feedback.
Quoted 303 lines
Quoted 42 lines
MAINTAINERS +2
-0@@ -12428,7 +12428,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@@ -190,6 +190,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_PLATFORMSdrivers/platform/surface/Makefile +0
-0drivers/platform/surface/Makefile obj-$(CONFIG_SURFACE_GPE) += surface_gpe.o · line 19
Quoted 4 lines
drivers/platform/surface/Makefile +1
-0@@ -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.oAll other files in this directory are named with an underscore, would be nice to carry on with such convention.
Quoted 38 lines
drivers/platform/surface/surface-xbl.c new file +32
-0@@ -0,0 +1,215 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* Surface eXtensible Boot Loader (XBL) +* +* Copyright (C) 2021 Microsoft Corporation +* Author: Jarrett Schultz <[email protected]> +*/ + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/types.h> + +#define SURFACE_XBL_MAX_VERSION_LEN 16 +#define SURFACE_XBL_BOARD_ID 0 +#define SURFACE_XBL_BATTERY_PRESENT 1 +#define SURFACE_XBL_HW_INIT_RETRIES 2 +#define SURFACE_XBL_IS_CUSTOMER_MODE 3 +#define SURFACE_XBL_IS_ACT_MODE 4 +#define SURFACE_XBL_PMIC_RESET_REASON 5 +#define SURFACE_XBL_TOUCH_FW_VERSION 6 +#define SURFACE_XBL_OCP_ERROR_LOCATION \ + (SURFACE_XBL_TOUCH_FW_VERSION + \ + SURFACE_XBL_MAX_VERSION_LEN) + +struct surface_xbl { +struct device *dev; +void __iomem *regs; + +u8 board_id; +u8 battery_present;drivers/platform/surface/surface-xbl.c line 42
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+u8 hw_init_retries; +u8 is_customer_mode; +u8 is_act_mode; +u8 pmic_reset_reason; +char touch_fw_version[SURFACE_XBL_MAX_VERSION_LEN ]; +u16 ocp_error_location; +}; + +static ssize_t +board_id_show (struct device *dev,struct device_attribute *attr,char *buf)I think it would be nice to avoid some duplication by putting all these integer ones in a single
show(), see e.g.soc_info_show()Quoted 84 lines
drivers/platform/surface/surface-xbl.c new file +84
-0+{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->board_id); +} +static DEVICE_ATTR_RO (board_id); + +static ssize_t +battery_present_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->battery_present); +} +static DEVICE_ATTR_RO (battery_present); + +static ssize_t +hw_init_retries_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->hw_init_retries); +} +static DEVICE_ATTR_RO (hw_init_retries); + +static ssize_t +is_customer_mode_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->is_customer_mode); +} +static DEVICE_ATTR_RO (is_customer_mode); + +static ssize_t +is_act_mode_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->is_act_mode); +} +static DEVICE_ATTR_RO (is_act_mode); + +static ssize_t +pmic_reset_reason_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->pmic_reset_reason); +} +static DEVICE_ATTR_RO (pmic_reset_reason); + +static ssize_t +touch_fw_version_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"0x%s\n" , sxbl->touch_fw_version); +} +static DEVICE_ATTR_RO (touch_fw_version); + +static ssize_t +ocp_error_location_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct surface_xbl *sxbl =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , sxbl->ocp_error_location); +} +static DEVICE_ATTR_RO (ocp_error_location); + +static struct attribute *xbl_attrs[] = { + &dev_attr_board_id.attr, + &dev_attr_battery_present.attr, + &dev_attr_hw_init_retries.attr, + &dev_attr_is_customer_mode.attr, + &dev_attr_is_act_mode.attr, + &dev_attr_pmic_reset_reason.attr, + &dev_attr_touch_fw_version.attr, + &dev_attr_ocp_error_location.attr, +NULL +}; + +static const struct attribute_group xbl_attr_group = { + .attrs = xbl_attrs,drivers/platform/surface/surface-xbl.c line 136
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+}; + +const struct attribute_group *xbl_sysfs_groups[] = { + &xbl_attr_group, +NULL +}; + +static u8 surface_xbl_readb (void __iomem *base,u32 offset) +{ +return readb (base + offset);Instead of having these helpers I think you should just call readb(base
+ offset) directly below.The shorter function name (readb vs
surface_xbl_readb) means that you don't even need to line wrap those lines.Quoted 6 lines
drivers/platform/surface/surface-xbl.c new file +6
-0+} + +static u16 surface_xbl_readw (void __iomem *base,u32 offset) +{ +return readw (base + offset); +}drivers/platform/surface/surface-xbl.c line 152
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ +static int surface_xbl_probe (struct platform_device *pdev) +{ +struct surface_xbl *sxbl; +struct device *dev; +void __iomem *regs; +int index; + + dev = &pdev->dev; + sxbl =devm_kzalloc (dev,sizeof (*sxbl),GFP_KERNEL );This is the only use of
&pdev->dev, so put that here and drop "dev" from sxbl (and the stack).drivers/platform/surface/surface-xbl.c line 162
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+if (!sxbl) +return -ENOMEM ; + + sxbl->dev = dev; + + regs =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (regs)) +return PTR_ERR (regs); + + sxbl->regs = regs;Seems only reason you stash "regs" in sxbl is so that you can pass
sxbl->regsin below function calls. I.e. it's a local variable and you can omit it from structsurface_xbl...Quoted 7 lines
drivers/platform/surface/surface-xbl.c new file +7
-0+ +platform_set_drvdata (pdev, sxbl); + + sxbl->board_id =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_BOARD_ID ); + sxbl->battery_present =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_BATTERY_PRESENT );drivers/platform/surface/surface-xbl.c line 179
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ sxbl->hw_init_retries =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_HW_INIT_RETRIES ); + sxbl->is_customer_mode =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_IS_CUSTOMER_MODE ); + sxbl->is_act_mode =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_IS_ACT_MODE ); + sxbl->pmic_reset_reason =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_PMIC_RESET_REASON ); + +for (index =0 ; index <SURFACE_XBL_MAX_VERSION_LEN ; index++)"i" is a good succinct variable name for an index.
drivers/platform/surface/surface-xbl.c line 189
Quoted 10 lines
drivers/platform/surface/surface-xbl.c new file +10
-0+ sxbl->touch_fw_version[index] =surface_xbl_readb (sxbl->regs, +SURFACE_XBL_TOUCH_FW_VERSION + index); + + sxbl->ocp_error_location =surface_xbl_readw (sxbl->regs, +SURFACE_XBL_OCP_ERROR_LOCATION ); + +return 0 ; +} + +static int surface_xbl_remove (struct platform_device *pdev)Your remove function is empty, simply omit it...
Regards,
BjornQuoted 29 lines
drivers/platform/surface/surface-xbl.c new file +26
-0+{ +return 0 ; +} + +static const struct of_device_id surface_xbl_of_match[] = { + { + .compatible ="microsoft,sm8150-surface-duo-xbl" + }, + { } +}; +MODULE_DEVICE_TABLE (of, surface_xbl_of_match); + +static struct platform_driver surface_xbl_driver = { + .driver = { + .name ="surface-xbl" , + .of_match_table = surface_xbl_of_match, + .dev_groups = xbl_sysfs_groups + }, + .probe = surface_xbl_probe, + .remove = surface_xbl_remove +}; +module_platform_driver (surface_xbl_driver); + +MODULE_AUTHOR ("Jarrett Schultz <[email protected]>" ); +MODULE_DESCRIPTION ("Surface Extensible Bootloader" ); +MODULE_LICENSE ("GPL" );Signature · 1 line
2.25.1
PATCH v2 4/5 arm64: dts: qcom: sm8150: Add imem section
In reply to Jarrett SchultzIntroduce the imem section in preparation for the surface xbl driver.
---
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
@@ -1176,6 +1176,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
Re: [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section
Quoted 4 lines · Jarrett Schultz
Introduce the imem section in preparation for the surface xbl driver.
Signed-off-by Jarrett Schultz <[email protected]>
Regards,
Bjorn
Quoted 32 lines
---
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@@ -1176,6 +1176,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
PATCH v2 5/5 arm64: dts: qcom: surface-duo: Add surface xbl
In reply to Jarrett SchultzIntroduce device tree source for the surface xbl driver.
---
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
@@ -429,6 +429,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