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]
Message-Id: <1582811295.3.1@crapouillou.net>
Date:   Thu, 27 Feb 2020 10:48:15 -0300
From:   Paul Cercueil <paul@...pouillou.net>
To:     周琰杰 <zhouyanjie@...yeetech.com>
Cc:     Rob Herring <robh@...nel.org>, linux-mips@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, tglx@...utronix.de,
        ralf@...ux-mips.org, paulburton@...nel.org,
        jiaxun.yang@...goat.com, chenhc@...ote.com, sboyd@...nel.org,
        mturquette@...libre.com, mark.rutland@....com,
        daniel.lezcano@...aro.org, geert+renesas@...der.be,
        krzk@...nel.org, ebiederm@...ssion.com, miquel.raynal@...tlin.com,
        keescook@...omium.org, sernia.zhou@...mail.com,
        zhenwenjin@...il.com, dongsheng.qiu@...enic.com
Subject: Re: [PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

Hi,

Le mer., févr. 26, 2020 at 10:29, Rob Herring <robh@...nel.org> a 
écrit :
> On Fri, Feb 21, 2020 at 12:24:47AM +0800, 周琰杰 (Zhou Yanjie) 
> wrote:
>>  Document the available properties for the SoC root node and the
>>  CPU nodes of the devicetree for the Ingenic XBurst SoCs.
>> 
>>  Tested-by: H. Nikolaus Schaller <hns@...delico.com>
>>  Tested-by: Paul Boddie <paul@...die.org.uk>
>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>>  ---
>> 
>>  Notes:
>>      v1->v2:
>>      Change the two Document from txt to yaml.
>> 
>>      v2->v3:
>>      Fix formatting errors.
>> 
>>      v3->v4:
>>      Fix bugs in the two yaml files.
>> 
>>      v4->v5:
>>      No change.
>> 
>>      v5->v6:
>>      Rewrite the two yaml files.
>> 
>>   .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 
>> ++++++++++++++++++++++
>>   .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 
>> ++++++++++++
>>   2 files changed, 95 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>   create mode 100644 
>> Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>> 
>>  diff --git 
>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>  new file mode 100644
>>  index 00000000..ad1fd86
>>  --- /dev/null
>>  +++ 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>  @@ -0,0 +1,61 @@
>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  +%YAML 1.2
>>  +---
>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>  +
>>  +title: Bindings for Ingenic XBurst family CPUs
>>  +
>>  +maintainers:
>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
> 
> Blank line here.
> 
>>  +description: |
> 
> Drop the '|'.
> 
>>  +  Ingenic XBurst family CPUs shall have the following properties.
>>  +
>>  +properties:
>>  +  compatible:
>>  +    oneOf:
>>  +
>>  +      - description: Ingenic XBurst®1 CPU Core
>>  +        items:
>>  +          - const: ingenic,xburst
>>  +
>>  +      - description: Ingenic XBurst®2 CPU Core
>>  +        items:
>>  +          - const: ingenic,xburst2
> 
> enum:
>   - ingenic,xburst  # Ingenic XBurst®1 CPU Core
>   - ingenic,xburst2 # Ingenic XBurst®2 CPU Core
> 
> Though I don't find the description really adds much.

About the enum values: shouldn't they be a bit more descriptive? There 
has been various versions of the Xburst1 chip, with slightly different 
instruction sets and hardware (FPU).

-Paul

>>  +
>>  +  reg:
>>  +    description: |
>>  +      The number of the CPU.
> 
> Drop this.
> 
> Add:
> 
> maxItems: 1
> 
>>  +
>>  +required:
>>  +  - device_type
>>  +  - compatible
>>  +  - reg
>>  +
>>  +examples:
>>  +  - |
>>  +    #include <dt-bindings/clock/jz4780-cgu.h>
>>  +
>>  +    cpus {
>>  +    	#address-cells = <1>;
>>  +    	#size-cells = <0>;
>>  +
>>  +    	cpu0: cpu@0 {
>>  +    		device_type = "cpu";
>>  +    		compatible = "ingenic,xburst";
>>  +    		reg = <0>;
>>  +
> 
>>  +    		clocks = <&cgu JZ4780_CLK_CPU>;
>>  +    		clock-names = "cpu";
> 
> Not documented.
> 
>>  +    	};
>>  +
>>  +    	cpu1: cpu@1 {
>>  +    		device_type = "cpu";
>>  +    		compatible = "ingenic,xburst";
>>  +    		reg = <1>;
>>  +
>>  +    		clocks = <&cgu JZ4780_CLK_CORE1>;
>>  +    		clock-names = "cpu";
>>  +    	};
>>  +    };
>>  +...
>>  diff --git 
>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>  new file mode 100644
>>  index 00000000..8943e73
>>  --- /dev/null
>>  +++ 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>  @@ -0,0 +1,34 @@
>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  +%YAML 1.2
>>  +---
>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>  +
>>  +title: Bindings for Ingenic SoCs with XBurst CPU inside.
>>  +
>>  +maintainers:
>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
> 
> Blank line.
> 
>>  +description: |
>>  +  Ingenic SoCs with XBurst CPU inside shall have the following 
>> properties.
>>  +
>>  +properties:
>>  +  $nodename:
>>  +    const: '/'
>>  +  compatible:
>>  +    oneOf:
>>  +
>>  +      - description: Ingenic JZ47 Series Mobile Application 
>> Processor
>>  +        items:
>>  +          - const: ingenic,jz4740
>>  +          - const: ingenic,jz4725b
>>  +          - const: ingenic,jz4760
>>  +          - const: ingenic,jz4760b
>>  +          - const: ingenic,jz4770
>>  +          - const: ingenic,jz4780
> 
> This is defining the root compatible is 6 strings. You want a enum 
> here
> I think.
> 
>>  +
>>  +      - description: Ingenic X Series IoT Application Processor
>>  +        items:
>>  +          - const: ingenic,x1000
>>  +          - const: ingenic,x1000e
>>  +          - const: ingenic,x1500
> 
> Same here.
> 
> Did you validate your dts file with this schema using 'make 
> dtbs_check'?
> 
> Rob


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ