linux-kernel - Re: [PATCH v4 1/6] dt-bindings: clock: meson: add A1 PLL clock controller bindings

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 18 Dec 2019 16:00:20 +0800
From:   Jian Hu <jian.hu@...ogic.com>
To:     Maxime Ripard <maxime@...no.tech>
CC:     Jerome Brunet <jbrunet@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Rob Herring <robh@...nel.org>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Qiufang Dai <qiufang.dai@...ogic.com>,
        Jianxin Pan <jianxin.pan@...ogic.com>,
        Victor Wan <victor.wan@...ogic.com>,
        Chandle Zou <chandle.zou@...ogic.com>,
        <linux-clk@...r.kernel.org>, <linux-amlogic@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v4 1/6] dt-bindings: clock: meson: add A1 PLL clock
 controller bindings

Hi Maxime

Thanks for your review

On 2019/12/13 18:38, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Dec 06, 2019 at 03:40:47PM +0800, Jian Hu wrote:
>> Add the documentation to support Amlogic A1 PLL clock driver,
>> and add A1 PLL clock controller bindings.
>>
>> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
>> ---
>>   .../bindings/clock/amlogic,a1-pll-clkc.yaml   | 59 +++++++++++++++++++
>>   include/dt-bindings/clock/a1-pll-clkc.h       | 16 +++++
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
>>   create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
>> new file mode 100644
>> index 000000000000..7feeef5abf1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
>> @@ -0,0 +1,59 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-pll-clkc.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Amlogic Meson A/C serials PLL Clock Control Unit Device Tree Bindings
>> +
>> +maintainers:
>> +  - Neil Armstrong <narmstrong@...libre.com>
>> +  - Jerome Brunet <jbrunet@...libre.com>
>> +  - Jian Hu <jian.hu@...n.hu.com>
>> +
>> +properties:
>> +  compatible:
>> +    - enum:
>> +        - amlogic,a1-pll-clkc
> 
> I'm not sure this works, compatible shouldn't contain a list.
> 
I refered to 
Documentation/devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml.

I have used 'dt-doc-validate' tools to check, it will report something 
wrong below.

properties:compatible: [{'enum': ['amlogic,a1-pll-clkc']}] is not of 
type 'object', 'boolean'

Refer to
https://github.com/robherring/dt-schema/blob/master/example-schema.yaml

I will change it like this:

properties:
   compatible:
     oneOf:
       - enum:
          - amlogic,a1-pll-clkc

And It has been passed by 'dt-doc-validate' tools.

Is it right?

> You can write this like:
> compatible:
>    const: amlogic,a1-pll-clkc
> 
>> +  "#clock-cells":
>> +    const: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +clocks:
>> +  minItems: 2
>> +  maxItems: 2
> 
> This is redundant, it will be added automatically by the tools ...
If I remove the minItems, it will pass by dt-doc-validate.

Would please tell how to use dt-schema to generate automatically it?

> 
>> +  items:
>> +   - description: Input xtal_fixpll
>> +   - description: Input xtal_hifipll
> 
> ... When you have a list of items :)
> 
>> +
>> +clock-names:
>> +  minItems: 2
>> +  maxItems: 2
>> +  items:
>> +     - const: xtal_fixpll
>> +     - const: xtal_hifipll
> 
> Same story here
OK, I will change it when I find the right way.
> 
> Maxime
>