linux-kernel - Re: [RFC V3 1/4] dt-bindings: firmware: Document bindings for ARM SCMI QCOM Vendor Protocol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zo1lqcjPEDRMXDJm@pluto>
Date: Tue, 9 Jul 2024 17:32:09 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Sibi Sankar <quic_sibis@...cinc.com>
Cc: sudeep.holla@....com, cristian.marussi@....com, andersson@...nel.org,
	konrad.dybcio@...aro.org, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, quic_rgottimu@...cinc.com,
	quic_kshivnan@...cinc.com, conor+dt@...nel.org
Subject: Re: [RFC V3 1/4] dt-bindings: firmware: Document bindings for ARM
 SCMI QCOM Vendor Protocol

On Wed, Jul 03, 2024 at 12:44:37AM +0530, Sibi Sankar wrote:
> Document the various memory buses that can be monitored and scaled by the
> memory latency governor hosted by the ARM SCMI QCOM Vendor protocol v1.0.
> 
> Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
> ---

Hi Sibi,

this series got a bit neglected...my bad...a few comments down below.

> 
> Adding a reg property in scmi-memlat.yaml seems incorrect/superfluous
> but without it I see the following errors:
> 
> Err Logs:
> protocol@80: 'reg' does not match any of the regexes: '^memory-[0-9]$', 'pinctrl-[0-9]+'
> protocol@80: Unevaluated properties are not allowed ('memory-0', 'memory-1', 'memory-2' were unexpected)
> 
> v2:
> * Drop container dvfs memlat container node. [Rob]
> * Move scmi-memlat.yaml to protocol level given that a lot of vendors might end up
>   using the same protocol number. [Rob]
> * Replace qcom,cpulist with the standard "cpus" property. [Rob]
> * Fix up compute-type/ipm-ceil required. [Rob]
> 

...so there has been a lot of work around Vendor protos recently (as you
have seen) and especially around the way we define the DT bindings to have
multiple vendor protocols coexist with the same overlapping numbers.
(the code-level coexistence is already in place as you've seen...)

I think some sort of agreement on HOW to render this in the bindings
side was reached around a series from NXP...not sure if I am missing something
here but this commit from Peng/NXP (if you have not seen it already):
https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-2-b85a6bf778cb@nxp.com/

...it is a good example of how you can define your vendor specific part in
a vendor specific binding files and then just add a single $ref line in
the core binding Documentation/devicetree/bindings/firmware/arm,scmi.yaml
(and that has been successfully reviewd...)

Moreover, in that same series from Peng/NXP you could have a look at
https://lore.kernel.org/linux-arm-kernel/20240621-imx95-bbm-misc-v2-v5-1-b85a6bf778cb@nxp.com/

that adds the Documentation for their Vendor protocols.
Beside the final location in the tree for such docs, which is a detail
we can settle later on our side too, I think that patch is a good example
of the kind of vendor-protos Documentation Sudeep is expecting.


>  .../bindings/firmware/arm,scmi.yaml           |  15 ++
>  .../bindings/soc/qcom/qcom,scmi-memlat.yaml   | 242 ++++++++++++++++++
>  include/dt-bindings/soc/qcom,scmi-vendor.h    |  22 ++
>  3 files changed, 279 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml
>  create mode 100644 include/dt-bindings/soc/qcom,scmi-vendor.h
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4d823f3b1f0e..a4022682e5ca 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -284,6 +284,21 @@ properties:
>      required:
>        - reg
>  
> +  protocol@80:
> +    type: object
> +    allOf:
> +      - $ref: '#/$defs/protocol-node'
> +      - $ref: /schemas/soc/qcom/qcom,scmi-memlat.yaml#
> +
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        const: 0x80
> +
> +    required:
> +      - reg
> +

..here you should be able to just plant your $ref without redefining the
protocol@80

>  additionalProperties: false
>  
>  $defs:
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml
> new file mode 100644
> index 000000000000..915a6bf5697f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,scmi-memlat.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/qcom/qcom,scmi-memlat.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SCMI Memory Bus nodes
> +
> +maintainers:
> +  - Sibi Sankar <quic_sibis@...cinc.com>
> +
> +description:
> +  This binding describes the various memory buses that can be monitored and scaled
> +  by memory latency governor running on the CPU Control Processor (SCMI controller).
> +

...and instead here you will define your protocols, compliant with the
main protocol-node def and any specific vendor sub-properties that you
should need....

...the above example from NXP is probably more clear than any attempt of mine
to explain this :P

Thanks,
Cristian