linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Jarrett Schultz <jaschultzms@gmail.com>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Mark Gross <markgross@kernel.org>,
	Maximilian Luz <luzmaximilian@gmail.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	platform-driver-x86@vger.kernel.org,
	Felipe Balbi <balbi@kernel.org>,
	Jarrett Schultz <jaschultz@microsoft.com>
Subject: Re: [PATCH v5 1/4] dt-bindings: platform: microsoft: Document surface xbl
Date: Wed, 13 Apr 2022 13:27:27 -0500	[thread overview]
Message-ID: <YlcWD3dnqotAEnfA@robh.at.kernel.org> (raw)
In-Reply-To: <20220405210750.619511-2-jaschultzMS@gmail.com>

On Tue, Apr 05, 2022 at 02:07:47PM -0700, Jarrett Schultz wrote:
> From: Jarrett Schultz <jaschultz@microsoft.com>
> 
> Introduce yaml for surface xbl driver.

From Bjorn's reply on v4, it sounds like the QCom folks need to work out 
how to describe 'imem' first. I'd think that would use 'mmio-sram' 
binding and then this could be a child of that. If it's DDR, then it 
belongs under /reserved-memory node. Either way, that's all kind of 
outside the scope of the binding unless there's something special about 
'imem'.

> 
> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> 
> ---
> 
> Changed in v5:
> 
>  - Updated description
>  - Added child node for imem
>  - Simplified example
> 
> ---
> 
> Changes in v4:
>  - Addressed small formatting changes
>  - Removed unnecessary lines
> 
> ---
> 
> Changes in v3:
>  - Updated description to only pertain to the hardware
>  - Updated the required field to properly reflect the binding
>  - Removed the first example
>  - Fixed the size of the reg field in the second example
> 
> ---
> 
> Changes in v2:
>  - Removed json-schema dependence
>  - Elaborated on description of driver
>  - Updated example
> ---
>  .../platform/microsoft/surface-xbl.yaml       | 70 +++++++++++++++++++

That's not a binding path. Probably bindings/nvmem/.

>  MAINTAINERS                                   |  7 ++
>  2 files changed, 77 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..648476670bbd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
> @@ -0,0 +1,70 @@
> +# 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 <jaschultz@microsoft.com>
> +
> +description: |
> +  Defined to expose information that is used during production when
> +  the device is in manufacturing mode. Some of the information included
> +  in the imem section is -
> +    * board_id
> +    * battery_present
> +    * hw_init_retries
> +    * is_customer_mode
> +    * is_act_mode
> +    * pmic_reset_reason
> +    * touch_fw_version
> +    * ocp_error_location
> +
> +properties:
> +  compatible:
> +    const: simple-mfd

This schema will never be applied (your example would fail if it did) 
because we filter out matching on 'simple-mfd' as it is not valid on its 
own.

> +
> +  reg:
> +    maxItems: 1
> +
> +  child-node:

This literally means you have a node called 'child-node'.

> +    description: A description of the xbl space within imem
> +
> +    type: object
> +
> +    properties:
> +      compatible: 
> +        const: microsoft,sm8150-surface-duo-xbl

This should be moved up a level removing 'simple-mfd'.

> +
> +      reg:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +      - reg
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges

> +  - address-cells
> +  - size-cells

Not documented and also the wrong names (missing '#').

> +
> +examples:
> +  - |
> +    imem@146bf000 {
> +      compatible = "simple-mfd";
> +      reg = <0x146bf000 0x1000>;
> +      ranges = <0x0 0x146bf000 0x1000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      xbl@a94 {
> +        compatible = "microsoft,sm8150-surface-duo-xbl";
> +        reg = <0xa94 0x100>;
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 13f9a84a617e..5d0ca2a98b57 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12649,6 +12649,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
> 
> 

  reply	other threads:[~2022-04-13 18:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-05 21:07 [PATCH v5 0/4] platform: surface: Introduce Surface XBL Driver Jarrett Schultz
2022-04-05 21:07 ` [PATCH v5 1/4] dt-bindings: platform: microsoft: Document surface xbl Jarrett Schultz
2022-04-13 18:27   ` Rob Herring [this message]
2022-04-18 22:53     ` [EXTERNAL] " Jarrett Schultz
2022-04-21 14:47       ` Jarrett Schultz
2022-04-05 21:07 ` [PATCH v5 2/4] platform: surface: Add " Jarrett Schultz
2022-04-05 21:07 ` [PATCH v5 3/4] arm64: dts: qcom: sm8150: Add imem section Jarrett Schultz
2022-04-05 21:07 ` [PATCH v5 4/4] arm64: dts: qcom: surface-duo: Add surface xbl Jarrett Schultz
2022-04-11 12:13 ` [PATCH v5 0/4] platform: surface: Introduce Surface XBL Driver Hans de Goede
2022-04-13 18:28   ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YlcWD3dnqotAEnfA@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=agross@kernel.org \
    --cc=balbi@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hdegoede@redhat.com \
    --cc=jaschultz@microsoft.com \
    --cc=jaschultzms@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luzmaximilian@gmail.com \
    --cc=markgross@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).