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: <20150506121941.GD11057@lukather>
Date:	Wed, 6 May 2015 14:19:41 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Jens Kuske <jenskuske@...il.com>
Cc:	Emilio López <emilio@...pez.com.ar>,
	Mike Turquette <mturquette@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Vinod Koul <vinod.koul@...el.com>,
	Rob Herring <robh+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-sunxi@...glegroups.com
Subject: Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske <jenskuske@...il.com>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 468 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 468 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 0000000..53aab95
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,468 @@
> +/*
> + * Copyright (C) 2015 Jens Kuske <jenskuske@...il.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA

Could you remove that last paragraph?
It generates a checkpatch warning.

> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "skeleton.dtsi"
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
> +
> +/ {
> +	interrupt-parent = <&gic>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <0>;
> +		};
> +
> +		cpu@1 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <1>;
> +		};
> +
> +		cpu@2 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <2>;
> +		};
> +
> +		cpu@3 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <3>;
> +		};
> +	};
> +
> +	memory {
> +		reg = <0x40000000 0x80000000>;
> +	};
> +
> +	clocks {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		osc24M: osc24M_clk {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <24000000>;
> +			clock-output-names = "osc24M";
> +		};
> +
> +		osc32k: osc32k_clk {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <32768>;
> +			clock-output-names = "osc32k";
> +		};
> +
> +		pll1: clk@...20000 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun8i-a23-pll1-clk";
> +			reg = <0x01c20000 0x4>;
> +			clocks = <&osc24M>;
> +			clock-output-names = "pll1";
> +		};
> +
> +		pll6: clk@...20028 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-pll6-clk";
> +			reg = <0x01c20028 0x4>;
> +			clocks = <&osc24M>;
> +			clock-output-names = "pll6", "pll6x2", "pll6d2";
> +		};
> +
> +		pll8: clk@...20044 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun8i-h3-pll8-clk";
> +			reg = <0x01c20044 0x4>;
> +			clocks = <&osc24M>;
> +			clock-output-names = "pll8";
> +		};
> +
> +		cpu: cpu_clk@...20050 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun4i-a10-cpu-clk";
> +			reg = <0x01c20050 0x4>;
> +			clocks = <&osc32k>, <&osc24M>, <&pll1>, <&pll1>;
> +			clock-output-names = "cpu";
> +		};
> +
> +		axi: axi_clk@...20050 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun4i-a10-axi-clk";
> +			reg = <0x01c20050 0x4>;
> +			clocks = <&cpu>;
> +			clock-output-names = "axi";
> +		};
> +
> +		ahb1: ahb1_clk@...20054 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun6i-a31-ahb1-clk";
> +			reg = <0x01c20054 0x4>;
> +			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
> +			clock-output-names = "ahb1";
> +		};
> +
> +		ahb2: ahb2_clk@...2005c {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun8i-h3-ahb2-clk";
> +			reg = <0x01c2005c 0x4>;
> +			clocks = <&ahb1>, <&pll6 2>;
> +			clock-output-names = "ahb2";
> +		};
> +
> +		apb1: apb1_clk@...20054 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun4i-a10-apb0-clk";
> +			reg = <0x01c20054 0x4>;
> +			clocks = <&ahb1>;
> +			clock-output-names = "apb1";
> +		};
> +
> +		apb2: apb2_clk@...20058 {
> +			#clock-cells = <0>;
> +			compatible = "allwinner,sun4i-a10-apb1-clk";
> +			reg = <0x01c20058 0x4>;
> +			clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
> +			clock-output-names = "apb2";
> +		};
> +
> +		ahb1_gates: ahb1_gates_clk@...20060 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-ahb1-gates-clk";
> +			reg = <0x01c20060 0x14>;
> +			clocks = <&ahb1>;
> +			clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
> +					"ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
> +					"ahb1_sdram", "ahb1_ts", "ahb1_hstimer",
> +					"ahb1_spi0", "ahb1_spi1", "ahb1_otg",
> +					"ahb1_otg_ehci0", "ahb1_ehic1",
> +					"ahb1_ehic2", "ahb1_ehic3",
> +					"ahb1_otg_ohci0", "ahb1_ve",
> +					"ahb1_lcd0", "ahb1_lcd1", "ahb1_deint",
> +					"ahb1_csi", "ahb1_tve", "ahb1_hdmi",
> +					"ahb1_de", "ahb1_gpu", "ahb1_msgbox",
> +					"ahb1_spinlock", "ahb1_ephy", "ahb1_dbg";
> +		};
> +
> +		ahb2_gates: ahb2_gates_clk@...20060 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-ahb2-gates-clk";
> +			reg = <0x01c20060 0x4>;
> +			clocks = <&ahb2>;
> +			clock-output-names = "ahb2_gmac", "ahb2_ohic1",
> +					"ahb2_ohic2", "ahb2_ohic3";
> +		};
> +
> +		apb1_gates: clk@...20068 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-apb1-gates-clk";
> +			reg = <0x01c20068 0x4>;
> +			clocks = <&apb1>;
> +			clock-output-names = "apb1_codec", "apb1_spdif",
> +					"apb1_pio", "apb1_ths", "apb1_i2s0",
> +					"apb1_i2s1", "apb1_i2s2";
> +		};
> +
> +		apb2_gates: clk@...2006c {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-apb2-gates-clk";
> +			reg = <0x01c2006c 0x4>;
> +			clocks = <&apb2>;
> +			clock-output-names = "apb2_i2c0", "apb2_i2c1",
> +					"apb2_i2c2", "apb2_uart0",
> +					"apb2_uart1", "apb2_uart2",
> +					"apb2_uart3", "apb2_sim";

I'd prefer if the clocks on a new line were right-aligned (like you
did for the mmc clocks just below).

> +		};
> +
> +		mmc0_clk: clk@...20088 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun4i-a10-mmc-clk";
> +			reg = <0x01c20088 0x4>;
> +			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
> +			clock-output-names = "mmc0",
> +					     "mmc0_output",
> +					     "mmc0_sample";
> +		};
> +
> +		mmc1_clk: clk@...2008c {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun4i-a10-mmc-clk";
> +			reg = <0x01c2008c 0x4>;
> +			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
> +			clock-output-names = "mmc1",
> +					     "mmc1_output",
> +					     "mmc1_sample";
> +		};
> +
> +		mmc2_clk: clk@...20090 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun4i-a10-mmc-clk";
> +			reg = <0x01c20090 0x4>;
> +			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
> +			clock-output-names = "mmc2",
> +					     "mmc2_output",
> +					     "mmc2_sample";
> +		};
> +	};
> +
> +	soc@...00000 {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		dma: dma-controller@...02000 {
> +			compatible = "allwinner,sun8i-h3-dma";
> +			reg = <0x01c02000 0x1000>;
> +			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ahb1_gates 6>;
> +			resets = <&ahb12_rst 6>;
> +			#dma-cells = <1>;
> +		};
> +
> +		mmc0: mmc@...0f000 {
> +			compatible = "allwinner,sun5i-a13-mmc";
> +			reg = <0x01c0f000 0x1000>;
> +			clocks = <&ahb1_gates 8>,
> +				 <&mmc0_clk 0>,
> +				 <&mmc0_clk 1>,
> +				 <&mmc0_clk 2>;
> +			clock-names = "ahb",
> +				      "mmc",
> +				      "output",
> +				      "sample";
> +			resets = <&ahb12_rst 8>;
> +			reset-names = "ahb";
> +			interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		mmc1: mmc@...10000 {
> +			compatible = "allwinner,sun5i-a13-mmc";
> +			reg = <0x01c10000 0x1000>;
> +			clocks = <&ahb1_gates 9>,
> +				 <&mmc1_clk 0>,
> +				 <&mmc1_clk 1>,
> +				 <&mmc1_clk 2>;
> +			clock-names = "ahb",
> +				      "mmc",
> +				      "output",
> +				      "sample";
> +			resets = <&ahb12_rst 9>;
> +			reset-names = "ahb";
> +			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		mmc2: mmc@...11000 {
> +			compatible = "allwinner,sun5i-a13-mmc";
> +			reg = <0x01c11000 0x1000>;
> +			clocks = <&ahb1_gates 10>,
> +				 <&mmc2_clk 0>,
> +				 <&mmc2_clk 1>,
> +				 <&mmc2_clk 2>;
> +			clock-names = "ahb",
> +				      "mmc",
> +				      "output",
> +				      "sample";
> +			resets = <&ahb12_rst 10>;
> +			reset-names = "ahb";
> +			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		pio: pinctrl@...20800 {
> +			compatible = "allwinner,sun8i-h3-pinctrl";
> +			reg = <0x01c20800 0x400>;
> +			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&apb1_gates 5>;
> +			gpio-controller;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			#size-cells = <0>;
> +			#gpio-cells = <3>;
> +
> +			uart0_pins_a: uart0@0 {
> +				allwinner,pins = "PA4", "PA5";
> +				allwinner,function = "uart0";
> +				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +			};
> +
> +			mmc0_pins_a: mmc0@0 {
> +				allwinner,pins = "PF0","PF1","PF2","PF3","PF4","PF5";

Could you have spaces between the commas, and wrap the line at 80
chars?

> +				allwinner,function = "mmc0";
> +				allwinner,drive = <SUN4I_PINCTRL_30_MA>;
> +				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +			};
> +
> +			mmc0_cd_pin: mmc0_cd_pin@0 {
> +				allwinner,pins = "PF6";
> +				allwinner,function = "gpio_in";
> +				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +				allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
> +			};
> +
> +			mmc1_pins_a: mmc1@0 {
> +				allwinner,pins = "PG0","PG1","PG2","PG3","PG4","PG5";
> +				allwinner,function = "mmc1";
> +				allwinner,drive = <SUN4I_PINCTRL_30_MA>;
> +				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +			};
> +		};
> +
> +		ahb12_rst: reset@...202c0 {
> +			#reset-cells = <1>;
> +			compatible = "allwinner,sun6i-a31-clock-reset";
> +			reg = <0x01c202c0 0xc>;
> +		};

This reset controller also resets the timers, it should be initialised
much earlier.

What about having an allwinner,sun8i-h3-bus-reset, and adding it to
the list of compatibles to initialise earlier in
drivers/reset/reset-sunxi.c?

Of course, it would cover the other reset controllers that you have
below.


> +		apb1_rst: reset@...202d0 {
> +			#reset-cells = <1>;
> +			compatible = "allwinner,sun6i-a31-clock-reset";
> +			reg = <0x01c202d0 0x4>;
> +		};
> +
> +		apb2_rst: reset@...202d8 {
> +			#reset-cells = <1>;
> +			compatible = "allwinner,sun6i-a31-clock-reset";
> +			reg = <0x01c202d8 0x4>;
> +		};
> +
> +		timer@...20c00 {
> +			compatible = "allwinner,sun4i-a10-timer";
> +			reg = <0x01c20c00 0xa0>;
> +			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&osc24M>;
> +		};
> +
> +		wdt0: watchdog@...20ca0 {
> +			compatible = "allwinner,sun6i-a31-wdt";
> +			reg = <0x01c20ca0 0x20>;
> +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +
> +		uart0: serial@...28000 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x01c28000 0x400>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-shift = <2>;
> +			reg-io-width = <4>;
> +			clocks = <&apb2_gates 16>;
> +			resets = <&apb2_rst 16>;
> +			dmas = <&dma 6>, <&dma 6>;
> +			dma-names = "rx", "tx";
> +			status = "disabled";
> +		};
> +
> +		uart1: serial@...28400 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x01c28400 0x400>;
> +			interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-shift = <2>;
> +			reg-io-width = <4>;
> +			clocks = <&apb2_gates 17>;
> +			resets = <&apb2_rst 17>;
> +			dmas = <&dma 7>, <&dma 7>;
> +			dma-names = "rx", "tx";
> +			status = "disabled";
> +		};
> +
> +		uart2: serial@...28800 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x01c28800 0x400>;
> +			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-shift = <2>;
> +			reg-io-width = <4>;
> +			clocks = <&apb2_gates 18>;
> +			resets = <&apb2_rst 18>;
> +			dmas = <&dma 8>, <&dma 8>;
> +			dma-names = "rx", "tx";
> +			status = "disabled";
> +		};
> +
> +		uart3: serial@...28c00 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x01c28c00 0x400>;
> +			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-shift = <2>;
> +			reg-io-width = <4>;
> +			clocks = <&apb2_gates 19>;
> +			resets = <&apb2_rst 19>;
> +			dmas = <&dma 9>, <&dma 9>;
> +			dma-names = "rx", "tx";
> +			status = "disabled";
> +		};
> +
> +		gic: interrupt-controller@...81000 {
> +			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
> +			reg = <0x01c81000 0x1000>,
> +			      <0x01c82000 0x1000>,
> +			      <0x01c84000 0x2000>,
> +			      <0x01c86000 0x2000>;
> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +		};
> +
> +		rtc: rtc@...00000 {
> +			compatible = "allwinner,sun6i-a31-rtc";
> +			reg = <0x01f00000 0x54>;
> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +	};
> +};
> -- 
> 2.3.7
> 

Have you tested the architected timers?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ