From: Krzysztof Kozlowski <krzk@kernel.org>
To: Ansuel Smith <ansuelsmth@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Andy Gross <agross@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org
Subject: Re: [PATCH v2 16/16] dt-bindings: arm: msm: Convert kpss driver Documentation to yaml
Date: Sun, 20 Mar 2022 13:05:51 +0100 [thread overview]
Message-ID: <45e4f763-d991-0939-3735-bfea05c48143@kernel.org> (raw)
In-Reply-To: <20220318160827.8860-17-ansuelsmth@gmail.com>
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@gmail.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@2088000 {
> - 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@gmail.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@2088000 {
> + 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@2098000 {
> + 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@f9088000 {
> + 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@2011000 {
> - 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@gmail.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
next prev parent reply other threads:[~2022-03-20 12:05 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-18 16:08 [PATCH v2 00/16] Modernize rest of the krait drivers Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 01/16] clk: introduce clk_hw_get_index_of_parent new API Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 02/16] clk: qcom: gcc-ipq806x: skip pxo/cxo fixed clk if already present Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 03/16] clk: qcom: gcc-ipq806x: add PXO_SRC in clk table Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 04/16] clk: qcom: clk-hfpll: use poll_timeout macro Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 05/16] clk: qcom: kpss-xcc: convert to parent data API Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 06/16] clk: qcom: clk-krait: unlock spin after mux completion Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 07/16] clk: qcom: clk-krait: add hw_parent check for div2_round_rate Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 08/16] clk: qcom: krait-cc: convert to parent_data API Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 09/16] clk: qcom: krait-cc: drop pr_info and register qsb only if needed Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 10/16] clk: qcom: krait-cc: drop hardcoded safe_sel Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 11/16] clk: qcom: krait-cc: force sec_mux to QSB Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 12/16] clk: qcom: clk-krait: add apq/ipq8064 errata workaround Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 13/16] clk: qcom: clk-krait: add enable disable ops Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 14/16] dt-bindings: clock: Convert qcom,krait-cc to yaml Ansuel Smith
2022-03-20 12:09 ` Krzysztof Kozlowski
2022-03-18 16:08 ` [PATCH v2 15/16] ARM: dts: qcom: qcom-ipq8064: add missing krait-cc compatible and clocks Ansuel Smith
2022-03-18 16:08 ` [PATCH v2 16/16] dt-bindings: arm: msm: Convert kpss driver Documentation to yaml Ansuel Smith
2022-03-20 12:05 ` Krzysztof Kozlowski [this message]
2022-03-20 9:11 ` Ansuel Smith
2022-03-20 14:47 ` Krzysztof Kozlowski
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=45e4f763-d991-0939-3735-bfea05c48143@kernel.org \
--to=krzk@kernel.org \
--cc=agross@kernel.org \
--cc=ansuelsmth@gmail.com \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@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