diff mbox series

[v3,2/2] dt-bindings: mailbox: qcom,apcs-kpss-global: Add clock-output-names

Message ID 20220717213645.1147342-3-bryan.odonoghue@linaro.org
State New
Headers show
Series Two apcs-kpss-global.yaml fixes | expand

Commit Message

Bryan O'Donoghue July 17, 2022, 9:36 p.m. UTC
Add clock-output-names as optional so that SoCs such as the msm8939 which
have multiple a53 PLLs can latch the appropriate output name in
drivers/clk/qcom/apcs-msm8916.c.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Rob Herring (Arm) July 18, 2022, 1:43 a.m. UTC | #1
On Sun, 17 Jul 2022 22:36:45 +0100, Bryan O'Donoghue wrote:
> Add clock-output-names as optional so that SoCs such as the msm8939 which
> have multiple a53 PLLs can latch the appropriate output name in
> drivers/clk/qcom/apcs-msm8916.c.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml: properties:clock-output-names: {'maxItems': 1, 'items': [{'const': 'a53mux_c0'}, {'const': 'a53mux_c1'}, {'const': 'a53mux_cci'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml: ignoring, error in schema: properties: clock-output-names

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Dmitry Baryshkov July 18, 2022, 10:33 a.m. UTC | #2
On Mon, 18 Jul 2022 at 00:37, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Add clock-output-names as optional so that SoCs such as the msm8939 which
> have multiple a53 PLLs can latch the appropriate output name in
> drivers/clk/qcom/apcs-msm8916.c.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> index f504652fc0ea2..7497e4c930ae7 100644
> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> @@ -63,6 +63,13 @@ properties:
>        - const: aux
>        - const: ref
>
> +  clock-output-names:
> +    maxItems: 1
> +    items:
> +      - const: a53mux_c0
> +      - const: a53mux_c1
> +      - const: a53mux_cci

You have probably meant to use enum here. However, is there any reason
why you would like to use fixed output names here? You are going to
use clocks DT properties (with clock-names or using indices) anyway,
so there is no dependency on system clock name.
Compare this with apcs-msm8916.c, which uses a53mux@unit_address.
Bryan O'Donoghue July 18, 2022, 11:04 a.m. UTC | #3
On 18/07/2022 11:33, Dmitry Baryshkov wrote:
> On Mon, 18 Jul 2022 at 00:37, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> Add clock-output-names as optional so that SoCs such as the msm8939 which
>> have multiple a53 PLLs can latch the appropriate output name in
>> drivers/clk/qcom/apcs-msm8916.c.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> index f504652fc0ea2..7497e4c930ae7 100644
>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> @@ -63,6 +63,13 @@ properties:
>>         - const: aux
>>         - const: ref
>>
>> +  clock-output-names:
>> +    maxItems: 1
>> +    items:
>> +      - const: a53mux_c0
>> +      - const: a53mux_c1
>> +      - const: a53mux_cci
> 
> You have probably meant to use enum here.

I do mean enum ...

  However, is there any reason
> why you would like to use fixed output names here? You are going to
> use clocks DT properties (with clock-names or using indices) anyway,
> so there is no dependency on system clock name.
> Compare this with apcs-msm8916.c, which uses a53mux@unit_address.
> 

The answer is because I have this in the dtsi forwarded ported from 4.19 
and like an idiot I didn't check the clock names

a53pll@b016000
a53pll@b116000
a53pll@b1d0000

a53mux@b1d1000
a53mux@b011000
a53mux@b111000

you're right this is old/dead code I don't need it.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
index f504652fc0ea2..7497e4c930ae7 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -63,6 +63,13 @@  properties:
       - const: aux
       - const: ref
 
+  clock-output-names:
+    maxItems: 1
+    items:
+      - const: a53mux_c0
+      - const: a53mux_c1
+      - const: a53mux_cci
+
 required:
   - compatible
   - reg