| 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
| ||
|
Date: Sun, 20 Mar 2022 13:05:51 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ansuel Smith <ansuelsmth@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Andy Gross <agross@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-clk@...r.kernel.org
Subject: Re: [PATCH v2 16/16] dt-bindings: arm: msm: Convert kpss driver
Documentation to yaml
On 18/03/2022 17:08, Ansuel Smith wrote:
> Convert kpss-acc and kpss-gcc Documentation to yaml. Fix multiple
> Documentation error and provide additional example for kpss-gcc-v2.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> ---
> .../bindings/arm/msm/qcom,kpss-acc.txt | 49 ----------
> .../bindings/arm/msm/qcom,kpss-acc.yaml | 97 +++++++++++++++++++
> .../bindings/arm/msm/qcom,kpss-gcc.txt | 44 ---------
> .../bindings/arm/msm/qcom,kpss-gcc.yaml | 63 ++++++++++++
> 4 files changed, 160 insertions(+), 93 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
> delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> deleted file mode 100644
> index 7f696362a4a1..000000000000
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Krait Processor Sub-system (KPSS) Application Clock Controller (ACC)
> -
> -The KPSS ACC provides clock, power domain, and reset control to a Krait CPU.
> -There is one ACC register region per CPU within the KPSS remapped region as
> -well as an alias register region that remaps accesses to the ACC associated
> -with the CPU accessing the region.
> -
> -PROPERTIES
> -
> -- compatible:
> - Usage: required
> - Value type: <string>
> - Definition: should be one of:
> - "qcom,kpss-acc-v1"
> - "qcom,kpss-acc-v2"
> -
> -- reg:
> - Usage: required
> - Value type: <prop-encoded-array>
> - Definition: the first element specifies the base address and size of
> - the register region. An optional second element specifies
> - the base address and size of the alias register region.
> -
> -- clocks:
> - Usage: required
> - Value type: <prop-encoded-array>
> - Definition: reference to the pll parents.
> -
> -- clock-names:
> - Usage: required
> - Value type: <stringlist>
> - Definition: must be "pll8_vote", "pxo".
> -
> -- clock-output-names:
> - Usage: optional
> - Value type: <string>
> - Definition: Name of the output clock. Typically acpuX_aux where X is a
> - CPU number starting at 0.
> -
> -Example:
> -
> - clock-controller@...8000 {
> - compatible = "qcom,kpss-acc-v2";
> - reg = <0x02088000 0x1000>,
> - <0x02008000 0x1000>;
> - clocks = <&gcc PLL8_VOTE>, <&gcc PXO_SRC>;
> - clock-names = "pll8_vote", "pxo";
> - clock-output-names = "acpu0_aux";
> - };
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
> new file mode 100644
> index 000000000000..6e8ef4f85eab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/msm/qcom,kpss-acc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Krait Processor Sub-system (KPSS) Application Clock Controller (ACC)
> +
> +maintainers:
> + - Ansuel Smith <ansuelsmth@...il.com>
> +
> +description: |
> + The KPSS ACC provides clock, power domain, and reset control to a Krait CPU.
> + There is one ACC register region per CPU within the KPSS remapped region as
> + well as an alias register region that remaps accesses to the ACC associated
> + with the CPU accessing the region.
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,kpss-acc-v1
> + - qcom,kpss-acc-v2
> +
> + reg:
> + items:
> + - description: Base address and size of the register region
> + - description: Optional base address and size of the alias register region
> +
> + clocks:
> + items:
> + - description: phandle to pll8_vote
> + - description: phandle to pxo_board
> +
> + clock-names:
> + items:
> + - const: pll8_vote
> + - const: pxo
> +
> + clock-output-names:
> + description: Name of the aux clock. Krait can have at most 4 cpu.
> + enum:
> + - acpu0_aux
> + - acpu1_aux
> + - acpu2_aux
> + - acpu3_aux
> +
> + '#clock-cells':
> + const: 0
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
This is not obvious from the original bindings. Why are these required
only on v1? Whatr about v2? Are they allowed on v2? Why at least
clock-cells are not required on v2?
The previous bindings make them required which you change here. Your
commit does not explain this.
Please explicitly explain in commit msg all changes to the bindings
which are necessary for conversion.
> + const: qcom,kpss-acc-v1
> + then:
> + required:
> + - clocks
> + - clock-names
> + - clock-output-names
> + - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
> +
> + clock-controller@...8000 {
> + compatible = "qcom,kpss-acc-v1";
> + reg = <0x02088000 0x1000>, <0x02008000 0x1000>;
> + clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
> + clock-names = "pll8_vote", "pxo";
> + clock-output-names = "acpu0_aux";
> + #clock-cells = <0>;
> + };
> +
> + clock-controller@...8000 {
> + compatible = "qcom,kpss-acc-v1";
> + reg = <0x02098000 0x1000>, <0x02008000 0x1000>;
> + clock-output-names = "acpu1_aux";
> + clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
> + clock-names = "pll8_vote", "pxo";
> + #clock-cells = <0>;
It's the same example as above, no differences. Remove it.
> + };
> +
> + - |
> + clock-controller@...88000 {
> + compatible = "qcom,kpss-acc-v2";
> + reg = <0xf9088000 0x1000>,
> + <0xf9008000 0x1000>;
You now changed the example. This looks wrong.
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> deleted file mode 100644
> index e628758950e1..000000000000
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
> -
> -PROPERTIES
> -
> -- compatible:
> - Usage: required
> - Value type: <string>
> - Definition: should be one of the following. The generic compatible
> - "qcom,kpss-gcc" should also be included.
> - "qcom,kpss-gcc-ipq8064", "qcom,kpss-gcc"
> - "qcom,kpss-gcc-apq8064", "qcom,kpss-gcc"
> - "qcom,kpss-gcc-msm8974", "qcom,kpss-gcc"
> - "qcom,kpss-gcc-msm8960", "qcom,kpss-gcc"
> -
> -- reg:
> - Usage: required
> - Value type: <prop-encoded-array>
> - Definition: base address and size of the register region
> -
> -- clocks:
> - Usage: required
> - Value type: <prop-encoded-array>
> - Definition: reference to the pll parents.
> -
> -- clock-names:
> - Usage: required
> - Value type: <stringlist>
> - Definition: must be "pll8_vote", "pxo".
> -
> -- clock-output-names:
> - Usage: required
> - Value type: <string>
> - Definition: Name of the output clock. Typically acpu_l2_aux indicating
> - an L2 cache auxiliary clock.
> -
> -Example:
> -
> - l2cc: clock-controller@...1000 {
> - compatible = "qcom,kpss-gcc-ipq8064", "qcom,kpss-gcc";
> - reg = <0x2011000 0x1000>;
> - clocks = <&gcc PLL8_VOTE>, <&gcc PXO_SRC>;
> - clock-names = "pll8_vote", "pxo";
> - clock-output-names = "acpu_l2_aux";
> - };
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
> new file mode 100644
> index 000000000000..578e2eccb7db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/msm/qcom,kpss-gcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
> +
> +maintainers:
> + - Ansuel Smith <ansuelsmth@...il.com>
> +
> +description: |
> + Krait Processor Sub-system (KPSS) Global Clock Controller (GCC). Used
> + to control L2 mux (in the current implementation).
> +
> +properties:
> + compatible:
> + const: qcom,kpss-gcc
This is wrong conversion. You removed quite a lot.
Please split this commit into two separate conversions (acc and gcc).
> +
> + reg:
> + items:
> + - description: Base address and size of the register region
Just maxItems:1. No need for items and description - it's obvious.
> +
> + clocks:
> + items:
> + - description: phandle to pll8_vote
> + - description: phandle to pxo_board
> +
> + clock-names:
> + items:
> + - const: pll8_vote
> + - const: pxo
> +
> + clock-output-names:
> + const: acpu_l2_aux
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists