* [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver
@ 2021-11-08 16:44 Jarrett Schultz
2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw)
To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede,
Mark Gross, Maximilian Luz
Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree,
Felipe Balbi, Jarrett Schultz, Jarrett Schultz
Introduce the Surface Extensible Boot Loader driver for the Surface Duo.
Exposes information about the driver to user space via sysfs.
Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
---
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
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz @ 2021-11-08 16:44 ` Jarrett Schultz 2021-11-08 18:48 ` Bjorn Andersson ` (2 more replies) 2021-11-08 16:44 ` [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency Jarrett Schultz ` (3 subsequent siblings) 4 siblings, 3 replies; 15+ messages in thread From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw) To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz, Jarrett Schultz Introduce yaml for surface xbl driver. Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> --- 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 diff --git a/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file mode 100644 index 000000000000..09f806f373bd --- /dev/null +++ b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml @@ -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 <jaschultzMS@gmail.com> + +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>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index eeb4c70b3d5b..8643546f8fab 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -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 <jaschultz@microsoft.com> +L: linux-arm-msm@vger.kernel.org +L: platform-driver-x86@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml + MICROSOFT SURFACE GPE LID SUPPORT DRIVER M: Maximilian Luz <luzmaximilian@gmail.com> L: platform-driver-x86@vger.kernel.org -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz @ 2021-11-08 18:48 ` Bjorn Andersson 2021-11-09 4:06 ` Rob Herring 2021-11-12 14:39 ` Rob Herring 2 siblings, 0 replies; 15+ messages in thread From: Bjorn Andersson @ 2021-11-08 18:48 UTC (permalink / raw) To: Jarrett Schultz Cc: Rob Herring, Andy Gross, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Mon 08 Nov 08:44 PST 2021, Jarrett Schultz wrote: > Introduce yaml for surface xbl driver. > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > --- > > 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 > > diff --git a/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > new file mode 100644 > index 000000000000..09f806f373bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > @@ -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 <jaschultzMS@gmail.com> > + > +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. > + * 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 I believe interrupts is a leftover... > + > +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 > + }; > + - | > + 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>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index eeb4c70b3d5b..8643546f8fab 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -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 <jaschultz@microsoft.com> > +L: linux-arm-msm@vger.kernel.org > +L: platform-driver-x86@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > + > MICROSOFT SURFACE GPE LID SUPPORT DRIVER > M: Maximilian Luz <luzmaximilian@gmail.com> > L: platform-driver-x86@vger.kernel.org > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz 2021-11-08 18:48 ` Bjorn Andersson @ 2021-11-09 4:06 ` Rob Herring 2021-11-12 14:39 ` Rob Herring 2 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2021-11-09 4:06 UTC (permalink / raw) To: Jarrett Schultz Cc: Maximilian Luz, Bjorn Andersson, linux-kernel, Felipe Balbi, Andy Gross, Rob Herring, Mark Gross, Jarrett Schultz, Hans de Goede, devicetree, linux-arm-msm, Jarrett Schultz, platform-driver-x86 On Mon, 08 Nov 2021 08:44:45 -0800, Jarrett Schultz wrote: > Introduce yaml for surface xbl driver. > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > --- > > 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 > 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.yaml Documentation/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.yaml Documentation/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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz 2021-11-08 18:48 ` Bjorn Andersson 2021-11-09 4:06 ` Rob Herring @ 2021-11-12 14:39 ` Rob Herring 2021-11-16 16:17 ` Jarrett Schultz 2 siblings, 1 reply; 15+ messages in thread From: Rob Herring @ 2021-11-12 14:39 UTC (permalink / raw) To: Jarrett Schultz Cc: Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Mon, Nov 08, 2021 at 08:44:45AM -0800, Jarrett Schultz wrote: > Introduce yaml for surface xbl driver. > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> Author and Sob emails need to match. > > --- > > 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 > > diff --git a/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > new file mode 100644 > index 000000000000..09f806f373bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > @@ -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 <jaschultzMS@gmail.com> > + > +description: | > + Exposes the following device information to user space via sysfs - What's sysfs? :) Linux details don't go in bindings. > + * 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"; 'simple-mfd' needs a specific compatible for the block. > + 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>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index eeb4c70b3d5b..8643546f8fab 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -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 <jaschultz@microsoft.com> > +L: linux-arm-msm@vger.kernel.org > +L: platform-driver-x86@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > + > MICROSOFT SURFACE GPE LID SUPPORT DRIVER > M: Maximilian Luz <luzmaximilian@gmail.com> > L: platform-driver-x86@vger.kernel.org > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl 2021-11-12 14:39 ` Rob Herring @ 2021-11-16 16:17 ` Jarrett Schultz 0 siblings, 0 replies; 15+ messages in thread From: Jarrett Schultz @ 2021-11-16 16:17 UTC (permalink / raw) To: Rob Herring Cc: Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Fri, Nov 12, 2021 at 08:39:24AM -0600, Rob Herring wrote: > On Mon, Nov 08, 2021 at 08:44:45AM -0800, Jarrett Schultz wrote: > > Introduce yaml for surface xbl driver. > > > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > Author and Sob emails need to match. > > > > > --- > > > > 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 > > > > diff --git a/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > > new file mode 100644 > > index 000000000000..09f806f373bd > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > > @@ -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 <jaschultzMS@gmail.com> > > + > > +description: | > > + Exposes the following device information to user space via sysfs - > > What's sysfs? :) Linux details don't go in bindings. > > > + * 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"; > > '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? > > + 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>; > > + }; > > + }; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index eeb4c70b3d5b..8643546f8fab 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -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 <jaschultz@microsoft.com> > > +L: linux-arm-msm@vger.kernel.org > > +L: platform-driver-x86@vger.kernel.org > > +S: Supported > > +F: Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml > > + > > MICROSOFT SURFACE GPE LID SUPPORT DRIVER > > M: Maximilian Luz <luzmaximilian@gmail.com> > > L: platform-driver-x86@vger.kernel.org > > -- > > 2.25.1 > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz @ 2021-11-08 16:44 ` Jarrett Schultz 2021-11-08 17:58 ` Maximilian Luz 2021-11-11 19:45 ` kernel test robot 2021-11-08 16:44 ` [PATCH v2 3/5] platform: surface: Add surface xbl Jarrett Schultz ` (2 subsequent siblings) 4 siblings, 2 replies; 15+ messages in thread From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw) To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz, Jarrett Schultz From: Jarrett Schultz <jaschultzMS@gmail.com> 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 <jaschultz@microsoft.com> --- Changes in v2: - Created to propagate ACPI dependency --- drivers/platform/surface/Kconfig | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig index 3105f651614f..0d3970e1d144 100644 --- a/drivers/platform/surface/Kconfig +++ b/drivers/platform/surface/Kconfig @@ -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. -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency 2021-11-08 16:44 ` [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency Jarrett Schultz @ 2021-11-08 17:58 ` Maximilian Luz 2021-11-11 19:45 ` kernel test robot 1 sibling, 0 replies; 15+ messages in thread From: Maximilian Luz @ 2021-11-08 17:58 UTC (permalink / raw) To: Jarrett Schultz, Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On 11/8/21 17:44, Jarrett Schultz wrote:> From: Jarrett Schultz <jaschultzMS@gmail.com> > > 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 <jaschultz@microsoft.com> Some remarks inline: > --- > > 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. > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig > index 3105f651614f..0d3970e1d144 100644 > --- a/drivers/platform/surface/Kconfig > +++ b/drivers/platform/surface/Kconfig > @@ -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 This is redundant, you can drop that. ACPI_WMI already 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 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. > 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. > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency 2021-11-08 16:44 ` [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency Jarrett Schultz 2021-11-08 17:58 ` Maximilian Luz @ 2021-11-11 19:45 ` kernel test robot 1 sibling, 0 replies; 15+ messages in thread From: kernel test robot @ 2021-11-11 19:45 UTC (permalink / raw) To: Jarrett Schultz, Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: kbuild-all, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 12615 bytes --] 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 Reported-by: kernel test robot <lkp@intel.com> 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@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 69244 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/5] platform: surface: Add surface xbl 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency Jarrett Schultz @ 2021-11-08 16:44 ` Jarrett Schultz 2021-11-08 18:39 ` Bjorn Andersson 2021-11-08 16:44 ` [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 5/5] arm64: dts: qcom: surface-duo: Add surface xbl Jarrett Schultz 4 siblings, 1 reply; 15+ messages in thread From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw) To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz, 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 <jaschultz@microsoft.com> --- 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 diff --git a/Documentation/ABI/testing/sysfs-platform-surface-xbl b/Documentation/ABI/testing/sysfs-platform-surface-xbl new file mode 100644 index 000000000000..2ae047b884d3 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-platform-surface-xbl @@ -0,0 +1,78 @@ +What: /sys/devices/platform/146bfa94.xbl/battery_present +Date: October 2021 +KernelVersion: 5.16 +Contact: jaschultz@microsoft.com +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: jaschultz@microsoft.com +Description: + Read only. It returns the board id. + +What: /sys/devices/platform/146bfa94.xbl/hw_init_retries +Date: October 2021 +KernelVersion: 5.16 +Contact: jaschultz@microsoft.com +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: jaschultz@microsoft.com +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: jaschultz@microsoft.com +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: jaschultz@microsoft.com +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: jaschultz@microsoft.com +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: jaschultz@microsoft.com +Description: + Read only. It returns the version of the firmware. diff --git a/MAINTAINERS b/MAINTAINERS index 8643546f8fab..d08b68d626f6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12428,7 +12428,9 @@ M: Jarrett Schultz <jaschultz@microsoft.com> L: linux-arm-msm@vger.kernel.org L: platform-driver-x86@vger.kernel.org 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 <luzmaximilian@gmail.com> diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig index 0d3970e1d144..3a1ced269d96 100644 --- a/drivers/platform/surface/Kconfig +++ b/drivers/platform/surface/Kconfig @@ -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 diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile index 32889482de55..0946266a8a73 100644 --- a/drivers/platform/surface/Makefile +++ b/drivers/platform/surface/Makefile @@ -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 diff --git a/drivers/platform/surface/surface-xbl.c b/drivers/platform/surface/surface-xbl.c new file mode 100644 index 000000000000..9ecec4e55a4d --- /dev/null +++ b/drivers/platform/surface/surface-xbl.c @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Surface eXtensible Boot Loader (XBL) + * + * Copyright (C) 2021 Microsoft Corporation + * Author: Jarrett Schultz <jaschultz@microsoft.com> + */ + +#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; + 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) +{ + 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, +}; + +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); +} + +static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{ + return readw(base + offset); +} + +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); + 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; + + 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); + 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++) + 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) +{ + 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 <jaschultz@microsoft.com>"); +MODULE_DESCRIPTION("Surface Extensible Bootloader"); +MODULE_LICENSE("GPL"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/5] platform: surface: Add surface xbl 2021-11-08 16:44 ` [PATCH v2 3/5] platform: surface: Add surface xbl Jarrett Schultz @ 2021-11-08 18:39 ` Bjorn Andersson 2021-11-16 22:05 ` Jarrett Schultz 0 siblings, 1 reply; 15+ messages in thread From: Bjorn Andersson @ 2021-11-08 18:39 UTC (permalink / raw) To: Jarrett Schultz Cc: Rob Herring, Andy Gross, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Mon 08 Nov 08:44 PST 2021, Jarrett Schultz wrote: > 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 <jaschultz@microsoft.com> > > --- > > 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 > > diff --git a/Documentation/ABI/testing/sysfs-platform-surface-xbl b/Documentation/ABI/testing/sysfs-platform-surface-xbl > new file mode 100644 > index 000000000000..2ae047b884d3 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-platform-surface-xbl > @@ -0,0 +1,78 @@ > +What: /sys/devices/platform/146bfa94.xbl/battery_present > +Date: October 2021 > +KernelVersion: 5.16 > +Contact: jaschultz@microsoft.com > +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? > + > +What: /sys/devices/platform/146bfa94.xbl/board_id > +Date: October 2021 > +KernelVersion: 5.16 > +Contact: jaschultz@microsoft.com > +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? > + > +What: /sys/devices/platform/146bfa94.xbl/hw_init_retries > +Date: October 2021 > +KernelVersion: 5.16 > +Contact: jaschultz@microsoft.com > +Description: > + Read only. It returns retries attempted to initialize the > + discrete hardware circuit. Which description hardware circuit? > + > +What: /sys/devices/platform/146bfa94.xbl/is_act_mode > +Date: October 2021 > +KernelVersion: 5.16 > +Contact: jaschultz@microsoft.com > +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: jaschultz@microsoft.com > +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: jaschultz@microsoft.com > +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? > + 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: jaschultz@microsoft.com > +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? > + 0 - no reason lol > + 9 - Battery driver triggered > + > +What: /sys/devices/platform/146bfa94.xbl/touch_fw_version > +Date: October 2021 > +KernelVersion: 5.16 > +Contact: jaschultz@microsoft.com > +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? > diff --git a/MAINTAINERS b/MAINTAINERS > index 8643546f8fab..d08b68d626f6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12428,7 +12428,9 @@ M: Jarrett Schultz <jaschultz@microsoft.com> > L: linux-arm-msm@vger.kernel.org > L: platform-driver-x86@vger.kernel.org > 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 <luzmaximilian@gmail.com> > diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig > index 0d3970e1d144..3a1ced269d96 100644 > --- a/drivers/platform/surface/Kconfig > +++ b/drivers/platform/surface/Kconfig > @@ -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 > diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile > index 32889482de55..0946266a8a73 100644 > --- a/drivers/platform/surface/Makefile > +++ b/drivers/platform/surface/Makefile > @@ -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. > diff --git a/drivers/platform/surface/surface-xbl.c b/drivers/platform/surface/surface-xbl.c > new file mode 100644 > index 000000000000..9ecec4e55a4d > --- /dev/null > +++ b/drivers/platform/surface/surface-xbl.c > @@ -0,0 +1,215 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Surface eXtensible Boot Loader (XBL) > + * > + * Copyright (C) 2021 Microsoft Corporation > + * Author: Jarrett Schultz <jaschultz@microsoft.com> > + */ > + > +#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; > + 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() > +{ > + 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, > +}; > + > +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. > +} > + > +static u16 surface_xbl_readw(void __iomem *base, u32 offset) > +{ > + return readw(base + offset); > +} > + > +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). > + 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... > + > + 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); > + 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. > + 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 > +{ > + 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 <jaschultz@microsoft.com>"); > +MODULE_DESCRIPTION("Surface Extensible Bootloader"); > +MODULE_LICENSE("GPL"); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/5] platform: surface: Add surface xbl 2021-11-08 18:39 ` Bjorn Andersson @ 2021-11-16 22:05 ` Jarrett Schultz 0 siblings, 0 replies; 15+ messages in thread From: Jarrett Schultz @ 2021-11-16 22:05 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Andy Gross, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Mon, Nov 08, 2021 at 10:39:52AM -0800, Bjorn Andersson wrote: > On Mon 08 Nov 08:44 PST 2021, Jarrett Schultz wrote: > > > 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 <jaschultz@microsoft.com> > > > > --- > > > > 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 > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-surface-xbl b/Documentation/ABI/testing/sysfs-platform-surface-xbl > > new file mode 100644 > > index 000000000000..2ae047b884d3 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-platform-surface-xbl > > @@ -0,0 +1,78 @@ > > +What: /sys/devices/platform/146bfa94.xbl/battery_present > > +Date: October 2021 > > +KernelVersion: 5.16 > > +Contact: jaschultz@microsoft.com > > +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? > See below under touch_fw_version. > > + > > +What: /sys/devices/platform/146bfa94.xbl/board_id > > +Date: October 2021 > > +KernelVersion: 5.16 > > +Contact: jaschultz@microsoft.com > > +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. > > + > > +What: /sys/devices/platform/146bfa94.xbl/hw_init_retries > > +Date: October 2021 > > +KernelVersion: 5.16 > > +Contact: jaschultz@microsoft.com > > +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. > > + > > +What: /sys/devices/platform/146bfa94.xbl/is_act_mode > > +Date: October 2021 > > +KernelVersion: 5.16 > > +Contact: jaschultz@microsoft.com > > +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: jaschultz@microsoft.com > > +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: jaschultz@microsoft.com > > +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. > > + 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: jaschultz@microsoft.com > > +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); > > + 0 - no reason lol > > + 9 - Battery driver triggered > > + > > +What: /sys/devices/platform/146bfa94.xbl/touch_fw_version > > +Date: October 2021 > > +KernelVersion: 5.16 > > +Contact: jaschultz@microsoft.com > > +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? > 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. > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 8643546f8fab..d08b68d626f6 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12428,7 +12428,9 @@ M: Jarrett Schultz <jaschultz@microsoft.com> > > L: linux-arm-msm@vger.kernel.org > > L: platform-driver-x86@vger.kernel.org > > 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 <luzmaximilian@gmail.com> > > diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig > > index 0d3970e1d144..3a1ced269d96 100644 > > --- a/drivers/platform/surface/Kconfig > > +++ b/drivers/platform/surface/Kconfig > > @@ -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 > > diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile > > index 32889482de55..0946266a8a73 100644 > > --- a/drivers/platform/surface/Makefile > > +++ b/drivers/platform/surface/Makefile > > @@ -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. > > > diff --git a/drivers/platform/surface/surface-xbl.c b/drivers/platform/surface/surface-xbl.c > > new file mode 100644 > > index 000000000000..9ecec4e55a4d > > --- /dev/null > > +++ b/drivers/platform/surface/surface-xbl.c > > @@ -0,0 +1,215 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Surface eXtensible Boot Loader (XBL) > > + * > > + * Copyright (C) 2021 Microsoft Corporation > > + * Author: Jarrett Schultz <jaschultz@microsoft.com> > > + */ > > + > > +#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; > > + 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() > > > +{ > > + 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, > > +}; > > + > > +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. > > > +} > > + > > +static u16 surface_xbl_readw(void __iomem *base, u32 offset) > > +{ > > + return readw(base + offset); > > +} > > + > > +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). > > > + 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... > > > + > > + 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); > > + 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. > > > + 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 > > > +{ > > + 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 <jaschultz@microsoft.com>"); > > +MODULE_DESCRIPTION("Surface Extensible Bootloader"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz ` (2 preceding siblings ...) 2021-11-08 16:44 ` [PATCH v2 3/5] platform: surface: Add surface xbl Jarrett Schultz @ 2021-11-08 16:44 ` Jarrett Schultz 2021-11-08 18:41 ` Bjorn Andersson 2021-11-08 16:44 ` [PATCH v2 5/5] arm64: dts: qcom: surface-duo: Add surface xbl Jarrett Schultz 4 siblings, 1 reply; 15+ messages in thread From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw) To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz, Jarrett Schultz Introduce the imem section in preparation for the surface xbl driver. Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> --- 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(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index ef0232c2cf45..1da327cd49ae 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -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>; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section 2021-11-08 16:44 ` [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section Jarrett Schultz @ 2021-11-08 18:41 ` Bjorn Andersson 0 siblings, 0 replies; 15+ messages in thread From: Bjorn Andersson @ 2021-11-08 18:41 UTC (permalink / raw) To: Jarrett Schultz Cc: Rob Herring, Andy Gross, Hans de Goede, Mark Gross, Maximilian Luz, linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz On Mon 08 Nov 08:44 PST 2021, Jarrett Schultz wrote: > Introduce the imem section in preparation for the surface xbl driver. > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > > 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(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index ef0232c2cf45..1da327cd49ae 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -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>; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 5/5] arm64: dts: qcom: surface-duo: Add surface xbl 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz ` (3 preceding siblings ...) 2021-11-08 16:44 ` [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section Jarrett Schultz @ 2021-11-08 16:44 ` Jarrett Schultz 4 siblings, 0 replies; 15+ messages in thread From: Jarrett Schultz @ 2021-11-08 16:44 UTC (permalink / raw) To: Rob Herring, Andy Gross, Bjorn Andersson, Hans de Goede, Mark Gross, Maximilian Luz Cc: linux-arm-msm, platform-driver-x86, linux-kernel, devicetree, Felipe Balbi, Jarrett Schultz, Jarrett Schultz Introduce device tree source for the surface xbl driver. Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> --- 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(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts b/arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts index 736da9af44e0..25a4ee05ee2b 100644 --- a/arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts +++ b/arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts @@ -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"; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-11-16 22:05 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-08 16:44 [PATCH v2 0/5] platform: surface: Introduce Surface XBL Driver Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 1/5] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz 2021-11-08 18:48 ` Bjorn Andersson 2021-11-09 4:06 ` Rob Herring 2021-11-12 14:39 ` Rob Herring 2021-11-16 16:17 ` Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency Jarrett Schultz 2021-11-08 17:58 ` Maximilian Luz 2021-11-11 19:45 ` kernel test robot 2021-11-08 16:44 ` [PATCH v2 3/5] platform: surface: Add surface xbl Jarrett Schultz 2021-11-08 18:39 ` Bjorn Andersson 2021-11-16 22:05 ` Jarrett Schultz 2021-11-08 16:44 ` [PATCH v2 4/5] arm64: dts: qcom: sm8150: Add imem section Jarrett Schultz 2021-11-08 18:41 ` Bjorn Andersson 2021-11-08 16:44 ` [PATCH v2 5/5] arm64: dts: qcom: surface-duo: Add surface xbl Jarrett Schultz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).