Re: [PATCHv2 1/8] ARM: at91: Add at91sam9rl DT SoC support

From: Boris BREZILLON
Date: Wed Feb 19 2014 - 12:35:36 EST


Hi Mark,

On 19/02/2014 18:00, Mark Rutland wrote:
On Wed, Feb 19, 2014 at 03:32:24PM +0000, Alexandre Belloni wrote:
This adds preliminary DT support for the at91sam9rl.

Signed-off-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx>
---
arch/arm/boot/dts/at91sam9rl.dtsi | 628 ++++++++++++++++++++++++++++++++++++++
arch/arm/mach-at91/at91sam9rl.c | 16 +
2 files changed, 644 insertions(+)
create mode 100644 arch/arm/boot/dts/at91sam9rl.dtsi
[...]

+ tcb0: timer@fffa0000 {
+ compatible = "atmel,at91rm9200-tcb";
+ reg = <0xfffa0000 0x100>;
+ interrupts = <16 IRQ_TYPE_LEVEL_HIGH 0
+ 17 IRQ_TYPE_LEVEL_HIGH 0
+ 18 IRQ_TYPE_LEVEL_HIGH 0>;
+ };
Nit: please bracket list entries individually. Also for other list
properties like reg and (*-)gpio(s).

[...]

+ adc0: adc@fffd0000 {
+ compatible = "atmel,at91sam9260-adc";
+ reg = <0xfffd0000 0x100>;
+ interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
+ atmel,adc-use-external-triggers;
+ atmel,adc-channels-used = <0xf>;
+ atmel,adc-vref = <3300>;
+ atmel,adc-num-channels = <4>;
+ atmel,adc-startup-time = <15>;
+ atmel,adc-channel-base = <0x30>;
+ atmel,adc-drdy-mask = <0x10000>;
+ atmel,adc-status-register = <0x1c>;
+ atmel,adc-trigger-register = <0x04>;
+ atmel,adc-res = <8 10>;
+ atmel,adc-res-names = "lowres", "highres";
+ atmel,adc-use-res = "highres";
+
+ trigger@0 {
+ trigger-name = "timer-counter-0";
+ trigger-value = <0x1>;
+ };
A unit-address should go with a reg value. Either this needs a reg and
the parent node needs #address-cells and #size-cells, or the
unit-address should go, and the names made unique through other means.

[...]

+ pinctrl@fffff400 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "atmel,at91rm9200-pinctrl", "simple-bus";
NAK. Either this is a atmel,at91rm9200-pinctrl node or a simple-bus. Not
both; that doesn't make any sense.

diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index 3651517abedf..d6ee8bb47213 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -196,6 +196,22 @@ static struct clk_lookup periph_clocks_lookups[] = {
CLKDEV_CON_ID("pioB", &pioB_clk),
CLKDEV_CON_ID("pioC", &pioC_clk),
CLKDEV_CON_ID("pioD", &pioD_clk),
+ /* more lookup table for DT entries */
+ CLKDEV_CON_DEV_ID("usart", "fffff200.serial", &mck),
+ CLKDEV_CON_DEV_ID("usart", "fffb0000.serial", &usart0_clk),
+ CLKDEV_CON_DEV_ID("usart", "ffffb400.serial", &usart1_clk),
+ CLKDEV_CON_DEV_ID("usart", "ffffb800.serial", &usart2_clk),
+ CLKDEV_CON_DEV_ID("usart", "ffffbc00.serial", &usart3_clk),
+ CLKDEV_CON_DEV_ID("t0_clk", "fffa0000.timer", &tc0_clk),
+ CLKDEV_CON_DEV_ID("t1_clk", "fffa0000.timer", &tc1_clk),
+ CLKDEV_CON_DEV_ID("t2_clk", "fffa0000.timer", &tc2_clk),
+ CLKDEV_CON_DEV_ID("mci_clk", "fffa4000.mmc", &mmc_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffa8000.i2c", &twi0_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffac000.i2c", &twi1_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffff400.gpio", &pioA_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffff600.gpio", &pioB_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffff800.gpio", &pioC_clk),
+ CLKDEV_CON_DEV_ID(NULL, "fffffa00.gpio", &pioD_clk),
Why can't these clocks be described in the DT?

They're described in the DT in patch 6/8, but we need these static definitions
in case the user wants to support both legacy and DT boards.

The at91 CCF based implementation is only compatible with DT, and when legacy
board support is enabled this disables the CCF implementation in favor of the
old clk implementation.

Best Regards,

Boris

Cheers,
Mark.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/