linux-kernel - Re: [PATCH 2/6] ARM: dts: nuc900: Add nuc970 dts files

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, 29 Jun 2016 17:24:10 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Wan Zongshun <vw@...mu.org>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Russell King <linux@...linux.org.uk>,
	devicetree@...r.kernel.org,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, Wan Zongshun <mcuos.com@...il.com>
Subject: Re: [PATCH 2/6] ARM: dts: nuc900: Add nuc970 dts files

On Saturday, June 25, 2016 6:37:18 PM CEST Wan Zongshun wrote:
> This patch is to add dts support for nuc970 platform.
> 
> Signed-off-by: Wan Zongshun <mcuos.com@...il.com>
> ---
>  .../devicetree/bindings/arm/nuvoton/nuc970.txt     |  30 +++
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/nuc970-evb.dts                   |  20 ++
>  arch/arm/boot/dts/nuc970.dtsi                      |  93 ++++++++
>  include/dt-bindings/clock/nuc970-clock.h           | 233 +++++++++++++++++++++
>  5 files changed, 377 insertions(+)

I'd suggest splitting this into multiple patches: the binding, the dts files and the header.

> +Boards with the NUC970 SoC shall have the following properties:
> +
> +Root node required properties:
> +- compatible: Should be "nuvoton,nuc970evb", "nuvoton,nuc970"

Better don't mention "nuvoton,nuc970evb", as that is the board specific
identifier: we don't want to have to list every single board.

> +Timer required properties:
> +- compatible: Should be "nuvoton,tmr"
> +- reg: Should contain registers location and length
> +- interrupts: hwirq is direct mapping to irq number
> +- clocks: phandle to input clock.
> +
> +Clock required properties:
> +- compatible: Should be "nuvoton,clk"
> +- reg: Should contain registers location and length
> +
> +Interrupt-controller required properties
> +- compatible: Should be "nuvoton,aic"
> +- reg: Should contain registers location and length
> +- interrupt-cells: set to 1
> +
> +GCR register required properties:
> +- compatible: Should be "nuvoton,gcr"
> +- reg: Should contain registers location and length

These compatible strings are all very generic, and should contain the
SoC name.

> diff --git a/arch/arm/boot/dts/nuc970.dtsi b/arch/arm/boot/dts/nuc970.dtsi
> new file mode 100644
> index 0000000..8a6c225
> --- /dev/null
> +++ b/arch/arm/boot/dts/nuc970.dtsi
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright 2016 Wan Zongshun <mcuos.com@...il.com>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include "skeleton.dtsi"
> +#include <dt-bindings/clock/nuc970-clock.h>
> +
> +/ {
> +	compatible = "nuvoton,nuc970evb", "nuvoton,nuc970";
> +
> +	interrupt-parent = <&aic>;
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	memory {
> +		reg = <0x00000000 0x04000000>;
> +	};

Better split this into SoC-specific and board-specific contents, the
aliases and memory should go into the board.dts file, along with the
alias for the serial port.

> +	ahb@...00000 {

Don't capatilize the hexadecimal numbers.

> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0xB0000000 0x9000>;
> +		ranges;

A simple-bus should not have a 'reg' property. You can have a 'ranges'
though, to translate the addresses in the child nodes.

	Arnd