| 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
| ||
|
Message-ID: <54F4A0F7.80606@suse.de>
Date: Mon, 02 Mar 2015 18:42:15 +0100
From: Andreas Färber <afaerber@...e.de>
To: Maxime Coquelin <mcoquelin.stm32@...il.com>,
u.kleine-koenig@...gutronix.de, geert@...ux-m68k.org,
Rob Herring <robh+dt@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Jonathan Corbet <corbet@....net>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Russell King <linux@....linux.org.uk>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Linus Walleij <linus.walleij@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.cz>, Arnd Bergmann <arnd@...db.de>,
Andrew Morton <akpm@...ux-foundation.org>,
"David S. Miller" <davem@...emloft.net>,
Mauro Carvalho Chehab <mchehab@....samsung.com>,
Joe Perches <joe@...ches.com>, Antti Palosaari <crope@....fi>,
Tejun Heo <tj@...nel.org>, Will Deacon <will.deacon@....com>,
Nikolay Borisov <Nikolay.Borisov@....com>,
Rusty Russell <rusty@...tcorp.com.au>,
Kees Cook <keescook@...omium.org>,
Michal Marek <mmarek@...e.cz>, linux-doc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-serial@...r.kernel.org, linux-arch@...r.kernel.org,
linux-api@...r.kernel.org
Subject: Re: [PATCH v2 16/18] ARM: dts: Introduce STM32F429 MCU
Hi Maxime,
Please don't put the whole world in To, that makes replying to you much
harder. You can put maintainers in CC instead.
Am 20.02.2015 um 19:01 schrieb Maxime Coquelin:
> The STMicrolectornics's STM32F419 MCU has the following main features:
> - Cortex-M4 core running up to @180MHz
> - 2MB internal flash, 256KBytes internal RAM
> - FMC controller to connect SDRAM, NOR and NAND memories
> - SD/MMC/SDIO support
> - Ethernet controller
> - USB OTFG FS & HS controllers
> - I2C, SPI, CAN busses support
> - Several 16 & 32 bits general purpose timers
> - Serial Audio interface
> - LCD controller
>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@...il.com>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/stm32f429-disco.dts | 41 ++++
> arch/arm/boot/dts/stm32f429.dtsi | 396 ++++++++++++++++++++++++++++++++++
> 3 files changed, 438 insertions(+)
> create mode 100644 arch/arm/boot/dts/stm32f429-disco.dts
> create mode 100644 arch/arm/boot/dts/stm32f429.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 91bd5bd..d7da0ef 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -442,6 +442,7 @@ dtb-$(CONFIG_ARCH_STI)+= stih407-b2120.dtb \
> stih416-b2000.dtb \
> stih416-b2020.dtb \
> stih416-b2020e.dtb
> +dtb-$(CONFIG_ARCH_STM32)+= stm32f429-disco.dtb
> dtb-$(CONFIG_MACH_SUN4I) += \
> sun4i-a10-a1000.dtb \
> sun4i-a10-ba10-tvbox.dtb \
> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
> new file mode 100644
> index 0000000..0e79cc1
> --- /dev/null
> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
> @@ -0,0 +1,41 @@
Add a suitable license header here? There had been attempts to
dual-license as GPL and MIT/X11.
> +/dts-v1/;
> +#include "stm32f429.dtsi"
> +
> +/ {
> + model = "STMicroelectronics's STM32F429i-DISCO board";
"'s" seems uncommon here and "s's" looks wrong. Just use
"STMicroelectronics STM32F429I-DISCO board"?
> + compatible = "st,stm32f429i-disco", "st,stm32f429";
> +
> + chosen {
> + bootargs = "console=ttyS0,115200 root=/dev/ram rdinit=/linuxrc";
> + linux,stdout-path = &usart1;
I have actually been using USART3, following a guide I found on GitHub.
Which pins do you use for USART1, and is one better than the other?
> + };
> +
> + memory {
> + reg = <0xd0000000 0x800000>;
I have it at 0x90000000!
> + };
> +
> + aliases {
> + ttyS0 = &usart1;
Why ttyS0 here? Shouldn't that be serial0 by convention?
> + };
> +
> + soc {
> + usart1: usart@...11000 {
> + status = "okay";
> + };
This is a new target, so please use the new-style &usart1 {...};
reference rather than replicating the hierarchy.
> +
> + leds {
These LEDs are definitely not on the SoC, they're on the board. Please
move one level up.
> + compatible = "gpio-leds";
In this file you're being parsimonious with newline, yet in the .dtsi
you unnecessarily add newlines before the trailing status property.
> + red {
> + #gpio-cells = <2>;
This looks weird. Drop it?
> + label = "Front Panel LED";
"Front Panel"? Both LEDs are on the front, and several other
architectures avoid spaces in the label for accessing it from /sys.
> + gpios = <&gpiog 14 0>;
GPIO_ACTIVE_HIGH
> + linux,default-trigger = "heartbeat";
Suggest to leave both off by default.
> + };
> + green {
> + #gpio-cells = <2>;
Ditto.
> + gpios = <&gpiog 13 0>;
Ditto.
> + default-state = "off";
> + };
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> new file mode 100644
> index 0000000..5b3442a
> --- /dev/null
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -0,0 +1,396 @@
> +/*
> + * Device tree for STM32F429
License?
> + */
> +#include "armv7-m.dtsi"
> +#include <dt-bindings/pinctrl/pinctrl-stm32.h>
> +#include <dt-bindings/reset/st,stm32f429.h>
> +
> +/ {
> +
> + aliases {
> + gpio0 = &gpioa;
> + gpio1 = &gpiob;
> + gpio2 = &gpioc;
> + gpio3 = &gpiod;
> + gpio4 = &gpioe;
> + gpio5 = &gpiof;
> + gpio6 = &gpiog;
> + gpio7 = &gpioh;
> + gpio8 = &gpioi;
> + gpio9 = &gpioj;
> + gpio10 = &gpiok;
> + };
> +
> + clocks {
> + clk_sysclk: clk-sysclk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <180000000>;
> + };
> +
> + clk_hclk: clk-hclk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <180000000>;
> + };
> +
> + clk_pclk1: clk-pclk1 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <45000000>;
> + };
> +
> + clk_pclk2: clk-pclk2 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <90000000>;
> + };
> +
> + clk_pmtr1: clk-pmtr1 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <90000000>;
> + };
> +
> + clk_pmtr2: clk-pmtr2 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <180000000>;
> + };
> +
> + clk_systick: clk-systick {
> + compatible = "fixed-factor-clock";
> + clocks = <&clk_hclk>;
> + #clock-cells = <0>;
> + clock-div = <8>;
> + clock-mult = <1>;
> + };
Most of these should be replaceable with my clk driver.
> + };
> +
> + systick: timer@...0e010 {
> + clocks = <&clk_systick>;
> +
> + status = "okay";
> + };
&systick {...};
> +
> + soc {
> + reset: reset@...23810 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023810 0x18>;
> + };
Still, this feels wrong, given that it's an RCC IP block...
> +
> + pin-controller {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "st,stm32-pinctrl";
> + ranges = <0 0x40020000 0x3000>;
> +
> + gpioa: gpio@...20000 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x0 0x400>;
> + resets = <&reset GPIOA_RESET>;
> + st,bank-name = "GPIOA";
> + };
> +
> + gpiob: gpio@...20400 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x400 0x400>;
> + resets = <&reset GPIOB_RESET>;
> + st,bank-name = "GPIOB";
> + };
> +
> + gpioc: gpio@...20800 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x800 0x400>;
> + resets = <&reset GPIOC_RESET>;
> + st,bank-name = "GPIOC";
> + };
> +
> + gpiod: gpio@...20c00 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0xc00 0x400>;
> + resets = <&reset GPIOD_RESET>;
> + st,bank-name = "GPIOD";
> + };
> +
> + gpioe: gpio@...21000 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x1000 0x400>;
> + resets = <&reset GPIOE_RESET>;
> + st,bank-name = "GPIOE";
> + };
> +
> + gpiof: gpio@...21400 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x1400 0x400>;
> + resets = <&reset GPIOF_RESET>;
> + st,bank-name = "GPIOF";
> + };
> +
> + gpiog: gpio@...21800 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x1800 0x400>;
> + resets = <&reset GPIOG_RESET>;
> + st,bank-name = "GPIOG";
> + };
> +
> + gpioh: gpio@...21c00 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x1c00 0x400>;
> + resets = <&reset GPIOH_RESET>;
> + st,bank-name = "GPIOH";
> + };
> +
> + gpioi: gpio@...22000 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x2000 0x400>;
> + resets = <&reset GPIOI_RESET>;
> + st,bank-name = "GPIOI";
> + };
> +
> + gpioj: gpio@...22400 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x2400 0x400>;
> + resets = <&reset GPIOJ_RESET>;
> + st,bank-name = "GPIOJ";
> + };
> +
> + gpiok: gpio@...22800 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x2800 0x400>;
> + resets = <&reset GPIOK_RESET>;
> + st,bank-name = "GPIOK";
> + };
> +
> + usart1 {
> + pinctrl_usart1: usart1-0 {
> + st,pins {
> + tx = <&gpioa 9 ALT7 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioa 10 ALT7 NO_PULL>;
> + };
> + };
> + };
> +
> + usart2 {
> + pinctrl_usart2: usart2-0 {
> + st,pins {
> + tx = <&gpioa 2 ALT7 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioa 3 ALT7 NO_PULL>;
> + };
> + };
> + };
> +
> + usart3 {
> + pinctrl_usart3: usart3-0 {
> + st,pins {
> + tx = <&gpiob 10 ALT7 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpiob 11 ALT7 NO_PULL>;
> + };
> + };
> + };
> +
> + usart4 {
> + pinctrl_usart4: usart4-0 {
> + st,pins {
> + tx = <&gpioa 0 ALT8 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioa 1 ALT8 NO_PULL>;
> + };
> + };
> + };
> +
> + usart5 {
> + pinctrl_usart5: usart5-0 {
> + st,pins {
> + tx = <&gpioc 12 ALT8 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpiod 2 ALT8 NO_PULL>;
> + };
> + };
> + };
> +
> + usart6 {
> + pinctrl_usart6: usart6-0 {
> + st,pins {
> + tx = <&gpioc 6 ALT8 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioc 7 ALT8 NO_PULL>;
> + };
> + };
> + };
> +
> + usart7 {
> + pinctrl_usart7: usart7-0 {
> + st,pins {
> + tx = <&gpioe 8 ALT8 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioe 7 ALT8 NO_PULL>;
> + };
> + };
> + };
> +
> + usart8 {
> + pinctrl_usart8: usart8-0 {
> + st,pins {
> + tx = <&gpioe 1 ALT8 NO_PULL PUSH_PULL LOW_SPEED>;
> + rx = <&gpioe 0 ALT8 NO_PULL>;
> + };
> + };
> + };
Can't the outer level be avoided here to save space and indentation?
> + };
> +
> + timer2: timer@...00000 {
> + compatible = "st,stm32-timer";
> + reg = <0x40000000 0x400>;
> + interrupts = <28>;
> + resets = <&reset TIM2_RESET>;
> + clocks = <&clk_pmtr1>;
> +
> + status = "disabled";
> + };
> +
> + timer3: timer@...00400 {
> + compatible = "st,stm32-timer";
> + reg = <0x40000400 0x400>;
> + interrupts = <29>;
> + resets = <&reset TIM3_RESET>;
> + clocks = <&clk_pmtr1>;
> +
> + status = "disabled";
> + };
> +
> + timer4: timer@...00800 {
> + compatible = "st,stm32-timer";
> + reg = <0x40000800 0x400>;
> + interrupts = <30>;
> + resets = <&reset TIM4_RESET>;
> + clocks = <&clk_pmtr1>;
> +
> + status = "disabled";
> + };
> +
> + timer5: timer@...00c00 {
> + compatible = "st,stm32-timer";
> + reg = <0x40000c00 0x400>;
> + interrupts = <50>;
> + resets = <&reset TIM5_RESET>;
> + clocks = <&clk_pmtr1>;
> + };
> +
> + timer6: timer@...01000 {
> + compatible = "st,stm32-timer";
> + reg = <0x40001000 0x400>;
> + interrupts = <54>;
> + resets = <&reset TIM6_RESET>;
> + clocks = <&clk_pmtr1>;
> +
> + status = "disabled";
> + };
> +
> + timer7: timer@...01400 {
> + compatible = "st,stm32-timer";
> + reg = <0x40001400 0x400>;
> + interrupts = <55>;
> + resets = <&reset TIM7_RESET>;
> + clocks = <&clk_pmtr1>;
> +
> + status = "disabled";
> + };
> +
> + usart1: usart@...11000 {
> + compatible = "st,stm32-usart";
> + reg = <0x40011000 0x400>;
> + interrupts = <37>;
> + clocks = <&clk_pclk2>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart2: usart@...04400 {
> + compatible = "st,stm32-usart";
> + reg = <0x40004400 0x400>;
> + interrupts = <38>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
Copy&paste, also below. Is this really .dtsi material?
> +
> + status = "disabled";
> + };
> +
> + usart3: usart@...04800 {
> + compatible = "st,stm32-usart";
> + reg = <0x40004800 0x400>;
> + interrupts = <39>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart4: usart@...04c00 {
> + compatible = "st,stm32-usart";
> + reg = <0x40004c00 0x400>;
> + interrupts = <52>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart5: usart@...05000 {
> + compatible = "st,stm32-usart";
> + reg = <0x40005000 0x400>;
> + interrupts = <53>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart6: usart@...11400 {
> + compatible = "st,stm32-usart";
> + reg = <0x40011400 0x400>;
> + interrupts = <71>;
> + clocks = <&clk_pclk2>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart7: usart@...07800 {
Please order all nodes by unit address, not by label.
> + compatible = "st,stm32-usart";
> + reg = <0x40007800 0x400>;
> + interrupts = <82>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
> +
> + usart8: usart@...07c00 {
> + compatible = "st,stm32-usart";
> + reg = <0x40007c00 0x400>;
> + interrupts = <83>;
> + clocks = <&clk_pclk1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usart1>;
> +
> + status = "disabled";
> + };
I remember two of them not being USART but UART? Similarly, two of them
support flow control (or two don't?), for which we would either need a
property or two different compatible strings.
> + };
> +};
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists