[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6324d1be.050a0220.9d842.7b47@mx.google.com>
Date: Fri, 16 Sep 2022 21:42:52 +0200
From: Christian Marangi <ansuelsmth@...il.com>
To: Rob Herring <robh@...nel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Alexey Dobriyan <adobriyan@...il.com>,
Takashi Iwai <tiwai@...e.de>,
Christian Brauner <brauner@...nel.org>,
Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
Marc Herbert <marc.herbert@...el.com>,
James Smart <jsmart2021@...il.com>,
Justin Tee <justin.tee@...adcom.com>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v5 2/5] dt-bindings: arm: msm: Convert kpss-acc driver
Documentation to yaml
On Fri, Sep 16, 2022 at 02:17:15PM -0500, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 04:22:53PM +0200, Christian Marangi wrote:
> > Convert kpss-acc driver Documentation to yaml.
> > The original Documentation was wrong all along. Fix it while we are
> > converting it.
> > The example was wrong as kpss-acc-v2 should only expose the regs but we
> > don't have any driver that expose additional clocks. The kpss-acc driver
> > is only specific to v1. For this exact reason, limit all the additional
> > bindings (clocks, clock-names, clock-output-names and #clock-cells) to
> > v1 and also flag that these bindings should NOT be used for v2.
>
> Odd that a clock controller has no clocks, but okay.
>
As said in the commit v2 is only used for regs. v2 it's only used in
arch/arm/mach-qcom/platsmp.c to setup stuff cpu hotplug and bringup.
Should we split the 2 driver? To me the acc naming seems to be just
recycled for v2 and it's not really a clk controller.
So keeping v2 in arm/msm/qcom,kpss-acc-v2.yaml and v1 moved to clock?
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> > ---
> > .../bindings/arm/msm/qcom,kpss-acc.txt | 49 ----------
> > .../bindings/arm/msm/qcom,kpss-acc.yaml | 93 +++++++++++++++++++
>
> As this is a clock controller, please move to bindings/clock/
>
> > 2 files changed, 93 insertions(+), 49 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
>
> > 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..5e16121d9f0d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
> > @@ -0,0 +1,93 @@
> > +# 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:
> > + - Christian Marangi <ansuelsmth@...il.com>
> > +
> > +description: |
>
> Don't need '|' if no formatting to preserve.
>
> > + 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
>
> Always a phandle (and arg), so that's redundant. Really, if there's not
> more to add that what clock-names says, then just 'maxItems: 2' is fine.
>
> > + - 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
> > +
> > +if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: qcom,kpss-acc-v1
> > +then:
> > + required:
> > + - clocks
> > + - clock-names
> > + - clock-output-names
> > + - '#clock-cells'
> > +else:
> > + properties:
> > + clocks: false
> > + clock-names: false
> > + clock-output-names: false
> > + '#clock-cells': false
> > +
> > +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@...88000 {
> > + compatible = "qcom,kpss-acc-v2";
> > + reg = <0xf9088000 0x1000>,
> > + <0xf9008000 0x1000>;
> > + };
> > +...
> > --
> > 2.37.2
> >
> >
--
Ansuel
Powered by blists - more mailing lists