ARM: CSR: Adding CSR SiRFprimaII board support

This patch adds the basic support for this SoC and EVB board based on device tree. It is following the ZYNQ of Grant Likely in some degree.

the device hierarchy in SiRFprimaII is as below

AXI(0-0x3FFFFFFF)
memory
AXI(0x40000000-0xC0000000)
CPUIF(sirf-iobus)
INTC
MEMC
LCD
VPP
GRAPHIC

MULTIMEDIA
GPS
UART
...
hard-coded PCI bridge
SD/MMC
rtc-iobrg
RTC

1. clock: use dev_id instead of con_id
2. Kconfig: delete ISA_DMA
3. irq: implement int controller based on DT
move to GENERIC_IRQ_CHIP framework
4. timer: implement timer based on DT
5. entry-macro.S: use get_irqnr_preamble to load the base address only once

per IRQ exception.
6. others:
use readl_relaxed instead of __raw_readl
adjust Kconfig order for PRIMA2
use clk_get_sys instead of clk_get for system level clock use BUG_ON(IS_ERR(clk)) instead of BUG_ON(IS_ERR_OR_NULL(clk)); fix typo for CLOCK_TICK_RATE
("sirf,xxx", "sirf,prima2-xxx") instead of ("sirf,xxx") in dts

new file mode 100644
index 0000000..1777e98

arch/arm/boot/dts/prima2-cb.dts new file +19 -0

@@ -0,0 +1,50 @@
+/dts-v1/;
+/ {
+	model = "SIRF Prima2 EVB";
+	compatible = "sirf,prima2-cb", "sirf,prima2";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&intc>;
+
+	memory {
+		reg = <0x00000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS1 earlyprintk";
+		linux,stdout-path = &uart1;
+	};
+
+	amba {
+		compatible = "simple-bus";

arch/arm/boot/dts/prima2-cb.dts new file +9 -0

+		timer0: timer at 0xb0020000 {
+			compatible = "sirf,tick", "sirf,prima2-tick";
+			reg = <0xb0020000 0x1000>;
+			interrupts = <0>;
+		};
+
+		uart0: uart at 0xb0050000 {
+			compatible = "sirf,uart", "sirf,prima2-uart";
+			reg = <0xb0050000 0x1000>;

+++ b/arch/arm/mach-prima2/board_dt.c

arch/arm/mach-prima2/Makefile new file +18 -0

@@ -0,0 +1,28 @@
+/*
+ * This file contains code for boards with device tree support.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/of.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <mach/hardware.h>
+#include "common.h"
+
+static const char *prima2cb_dt_match[] __initdata = {
+       "sirf,prima2-cb",
+       NULL
+};

arch/arm/mach-prima2/Makefile new file +5 -0

+#define IOTABLE_ENTRY(region) {\
+	.virtual 	= SIRFSOC_##region##_VA_BASE, \
+	.pfn	 	= __phys_to_pfn(SIRFSOC_##region##_PA_BASE),\
+	.length		= SIRFSOC_##region##_SIZE, \
+	.type 		= MT_DEVICE, \

arch/arm/mach-prima2/Makefile new file +22 -0

+void __init sirfsoc_map_io(void)
+{
+	iotable_init(sirfsoc_iodesc, ARRAY_SIZE(sirfsoc_iodesc));
+}
+
+#define L2X0_ADDR_FILTERING_START       0xC00
+#define L2X0_ADDR_FILTERING_END         0xC04
+
+static int __init sirfsoc_init(void)
+{
+#ifdef CONFIG_CACHE_L2X0
+	if (!(readl_relaxed(SIRFSOC_L2CC_VA_BASE + L2X0_CTRL) & 1)) {
+		/*
+		 * set the physical memory windows L2 cache will cover
+		 */
+		writel_relaxed(PHYS_OFFSET + 1024 * 1024 * 1024,
+			SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_END);
+		writel_relaxed(PHYS_OFFSET | 0x1,
+			SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_START);
+
+		writel_relaxed(0,
+			SIRFSOC_L2CC_VA_BASE + L2X0_TAG_LATENCY_CTRL);

+static void __init sirfsoc_of_timer_map(void);

Quoted 2 lines

This patch adds the basic support for this SoC and EVB board based on device tree. It is following the ZYNQ of Grant Likely in some degree.

Hi Barry,

There are a few more things I noticed this time around, and some comments about code that changed since v1.

Quoted 11 lines

the device hierarchy in SiRFprimaII is as below

AXI(0-0x3FFFFFFF)
? ? ? ? ? ? memory
AXI(0x40000000-0xC0000000)
? ? ? ? ? ? CPUIF(sirf-iobus)
? ? ? ? ? ? ? ? ? ? ? ? ?INTC
? ? ? ? ? ? ? ? ? ? ? ? ?MEMC
? ? ? ? ? ? ? ? ? ? ? ? ?LCD
? ? ? ? ? ? ? ? ? ? ? ? ?VPP
? ? ? ? ? ? ? ? ? ? ? ? ?GRAPHIC

Quoted 9 lines

? ? ? ? ? ? ? ? ? ? ? ? ?MULTIMEDIA
? ? ? ? ? ? ? ? ? ? ? ? ?GPS
? ? ? ? ? ? ? ? ? ? ? ? ?UART
? ? ? ? ? ? ? ? ? ? ? ? ?...
? ? ? ? ? ? ? ? ? ? ? ? ?hard-coded PCI bridge
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SD/MMC
? ? ? ? ? ? ? ? ? ? ? ? ?rtc-iobrg
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?RTC

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

This also includes using bus-local addresses for the 'reg' properties, rather than global addresses.

Quoted 6 lines

?1. clock: use dev_id instead of con_id
?2. Kconfig: delete ISA_DMA
?3. irq: implement int controller based on DT
? ? ? ?move to GENERIC_IRQ_CHIP framework
?4. timer: implement timer based on DT
?5. entry-macro.S: use get_irqnr_preamble to load the base address only once

Quoted 9 lines

? ? per IRQ exception.
?6. others:
? ? ? use readl_relaxed instead of __raw_readl
? ? ? adjust Kconfig order for PRIMA2
? ? ? use clk_get_sys instead of clk_get for system level clock ? ? ? use BUG_ON(IS_ERR(clk)) instead of BUG_ON(IS_ERR_OR_NULL(clk)); ? ? ? fix typo for CLOCK_TICK_RATE
? ? ? ("sirf,xxx", "sirf,prima2-xxx") ?instead of ("sirf,xxx") in dts

Good progress!

Quoted 24 lines

new file mode 100644
index 0000000..1777e98

arch/arm/boot/dts/prima2-cb.dts new file +19 -0

@@ -0,0 +1,50 @@
+/dts-v1/;
+/ {
+ ? ? model = "SIRF Prima2 EVB";
+ ? ? compatible = "sirf,prima2-cb", "sirf,prima2";
+ ? ? #address-cells = <1>;
+ ? ? #size-cells = <1>;
+ ? ? interrupt-parent = <&intc>;
+
+ ? ? memory {
+ ? ? ? ? ? ? reg = <0x00000000 0x20000000>;
+ ? ? };
+
+ ? ? chosen {
+ ? ? ? ? ? ? bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS1 earlyprintk";
+ ? ? ? ? ? ? linux,stdout-path = &uart1;
+ ? ? };
+
+ ? ? amba {
+ ? ? ? ? ? ? compatible = "simple-bus";

arch/arm/boot/dts/prima2-cb.dts line 29

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +10 -0

+ ? ? ? ? ? ? #address-cells = <1>;
+ ? ? ? ? ? ? #size-cells = <1>;
+ ? ? ? ? ? ? ranges;
+
+ ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 {
+ ? ? ? ? ? ? ? ? ? ? interrupt-controller;
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";
+ ? ? ? ? ? ? ? ? ? ? reg = <0x80020000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
+ ? ? ? ? ? ? };

Some more commends on the device tree properties:

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

Quoted 9 lines

arch/arm/boot/dts/prima2-cb.dts new file +9 -0

+ ? ? ? ? ? ? timer0: timer at 0xb0020000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,tick", "sirf,prima2-tick";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0020000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? interrupts = <0>;
+ ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? uart0: uart at 0xb0050000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,uart", "sirf,prima2-uart";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0050000 0x1000>;

arch/arm/boot/dts/prima2-cb.dts line 48

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +10 -0

+ ? ? ? ? ? ? ? ? ? ? interrupts = <17>;
+ ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? uart1: uart at 0xb0060000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,uart", "sirf,prima2-uart";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0060000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? interrupts = <18>;
+ ? ? ? ? ? ? };
+ ? ? };
+};

When you use local addresses, this would become something like

? ? ? ?axi1 {
? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ?ranges <0 0x80000000 0x10000000>;

? ? ? ? ? ? ? ?intc: interrupt-controller at 20000 {
? ? ? ? ? ? ? ? ? ? ? ?interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,intc", "sirf,prima2-intc"; ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x20000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ?#interrupt-cells = <1>;
? ? ? ? ? ? ? ?};
? ? ? ?}
? ? ? ?axi2 {
? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ?ranges <0 0xb0000000 0x10000000>;

? ? ? ? ? ? ? ?timer0: timer at 20000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x20000 0x1000>;
? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ?uart0: uart at 50000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x50000 0x1000>;
? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ?uart1: uart at 60000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x60000 0x1000>;
? ? ? ? ? ? ? ?};
? ? ? ?};

arch/arm/mach-prima2/Makefile line 1

Quoted 8 lines

arch/arm/boot/dts/prima2-cb.dts new file +1 -0

+

arch/arm/mach-prima2/Makefile new file +1 -0

new file mode 100644
index 0000000..c51d60d
@@ -0,0 +1 @@
+obj-y := timer.o irq.o clock.o common.o board_dt.o

Minor detail:

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

That makes the list more consistent when you add conditional files.

Quoted 26 lines

+++ b/arch/arm/mach-prima2/board_dt.c

arch/arm/mach-prima2/Makefile new file +24 -0

@@ -0,0 +1,28 @@
+/*
+ * This file contains code for boards with device tree support.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/of.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <mach/hardware.h>
+#include "common.h"
+
+static const char *prima2cb_dt_match[] __initdata = {
+ ? ? ? "sirf,prima2-cb",
+ ? ? ? NULL
+};
+
+MACHINE_START(PRIMA2_EVB, "prima2cb")
+ ? ? ? .boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100,
+ ? ? ? .init_early ? ? = sirfsoc_init_clk,
+ ? ? ? .map_io ? ? ? ? = sirfsoc_map_io,
+ ? ? ? .init_irq ? ? ? = sirfsoc_of_init_irq,

arch/arm/mach-prima2/Makefile line 28

Quoted 4 lines

arch/arm/mach-prima2/Makefile new file +4 -0

+ ? ? ? .timer ? ? ? ? ?= &sirfsoc_timer,
+ ? ? ? .init_machine ? = sirfsoc_mach_init,
+ ? ? ? .dt_compat ? ? ?= prima2cb_dt_match,
+MACHINE_END

How about removing the remaining board file entirely and merging the contents into common.c? We don't want to have board specific files any more, so the naming and the split is rather obsolete.

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

Quoted 5 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+#define IOTABLE_ENTRY(region) {\
+ ? ? .virtual ? ? ? ?= SIRFSOC_##region##_VA_BASE, \
+ ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(SIRFSOC_##region##_PA_BASE),\
+ ? ? .length ? ? ? ? = SIRFSOC_##region##_SIZE, \
+ ? ? .type ? ? ? ? ? = MT_DEVICE, \

arch/arm/mach-prima2/Makefile

Quoted 10 lines

arch/arm/mach-prima2/Makefile new file +10 -0

+}
+
+static struct map_desc sirfsoc_iodesc[] __initdata = {
+ ? ? IOTABLE_ENTRY(UART1),
+ ? ? IOTABLE_ENTRY(CLOCK),
+ ? ? IOTABLE_ENTRY(RSTC),
+#ifdef CONFIG_CACHE_L2X0
+ ? ? IOTABLE_ENTRY(L2CC),
+#endif
+};

I've thought about this some more, and I think you can get rid of most entries in the global table after all:

* sirfsoc_init_clk() can get the physical address from the device tree and call iotable_init for just one entry.

* RSTC does not seem to be needed early at all, you can simply ioremap it.

* L2CC is only accessed from an arch_initcall, which is late enough ?for ioremap.

* UART1 is only needed for early printk, so I would put the function ?that does this remaining iotable_init() call into a separate file ?that only gets compiled when that is enabled.

Quoted 22 lines

arch/arm/mach-prima2/Makefile new file +22 -0

+void __init sirfsoc_map_io(void)
+{
+ ? ? iotable_init(sirfsoc_iodesc, ARRAY_SIZE(sirfsoc_iodesc));
+}
+
+#define L2X0_ADDR_FILTERING_START ? ? ? 0xC00
+#define L2X0_ADDR_FILTERING_END ? ? ? ? 0xC04
+
+static int __init sirfsoc_init(void)
+{
+#ifdef CONFIG_CACHE_L2X0
+ ? ? if (!(readl_relaxed(SIRFSOC_L2CC_VA_BASE + L2X0_CTRL) & 1)) {
+ ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover
+ ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? writel_relaxed(PHYS_OFFSET + 1024 * 1024 * 1024,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_END);
+ ? ? ? ? ? ? writel_relaxed(PHYS_OFFSET | 0x1,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_START);
+
+ ? ? ? ? ? ? writel_relaxed(0,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_TAG_LATENCY_CTRL);

arch/arm/mach-prima2/Makefile

Quoted 10 lines

arch/arm/mach-prima2/Makefile new file +10 -0

+ ? ? ? ? ? ? writel_relaxed(0,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_DATA_LATENCY_CTRL);
+ ? ? }
+ ? ? l2x0_init((void __iomem *)SIRFSOC_L2CC_VA_BASE, 0x00040000,
+ ? ? ? ? ? ? 0x00000000);
+#endif
+
+ ? ? return 0;
+}
+arch_initcall(sirfsoc_init);

I would also suggest moving the l2x0 code into a new l2x0.c file that is conditionally compiled. In the header file, you then do

#ifdef CONFIG_CACHE_L2X0
extern void sirfsoc_l2x0_init(void);
#else
static inline void sirfsoc_l2x0_init(void) {}
#endif

arch/arm/mach-prima2/Makefile

Quoted 5 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+#ifndef __ASM_ARCH_IRQS_H
+#define __ASM_ARCH_IRQS_H
+
+#define SIRFSOC_INTENAL_IRQ_START ?0
+#define SIRFSOC_INTENAL_IRQ_END ? ?59

Typo, I assume: INTERNAL, not INTENAL

arch/arm/mach-prima2/Makefile

Quoted 5 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+
+static struct of_device_id intc_ids[] ?= {
+ ? ? { .compatible = "sirf,intc" },
+ ? ? { .compatible = "sirf,prima2-intc" },
+};

You should only list one of these in the driver: The idea is that as long as all variants of the hardware are identical, you can just list the most general name (sirf,intc), and if you need to tell the difference you list the more specific names and add a .data=... member that you use to differentiate between them. Note that it's usually easier to change the code than to change the device tree, so we want to have the full list in the device tree, but the code just needs to pick one that fits the purpose.

It's the same for the other drivers.

Quoted 1 line

+static void __init sirfsoc_of_timer_map(void);

You can remove this forward declaration if you move the sched_clock() function down.

? ? ? ?Arnd
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Quoted 2 lines

This patch adds the basic support for this SoC and EVB board based on device tree. It is following the ZYNQ of Grant Likely in some degree.

Hi Barry,

There are a few more things I noticed this time around, and some comments about code that changed since v1.

Quoted 11 lines

the device hierarchy in SiRFprimaII is as below

AXI(0-0x3FFFFFFF)
? ? ? ? ? ? memory
AXI(0x40000000-0xC0000000)
? ? ? ? ? ? CPUIF(sirf-iobus)
? ? ? ? ? ? ? ? ? ? ? ? ?INTC
? ? ? ? ? ? ? ? ? ? ? ? ?MEMC
? ? ? ? ? ? ? ? ? ? ? ? ?LCD
? ? ? ? ? ? ? ? ? ? ? ? ?VPP
? ? ? ? ? ? ? ? ? ? ? ? ?GRAPHIC

Quoted 9 lines

? ? ? ? ? ? ? ? ? ? ? ? ?MULTIMEDIA
? ? ? ? ? ? ? ? ? ? ? ? ?GPS
? ? ? ? ? ? ? ? ? ? ? ? ?UART
? ? ? ? ? ? ? ? ? ? ? ? ?...
? ? ? ? ? ? ? ? ? ? ? ? ?hard-coded PCI bridge
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SD/MMC
? ? ? ? ? ? ? ? ? ? ? ? ?rtc-iobrg
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?RTC

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

This also includes using bus-local addresses for the 'reg' properties, rather than global addresses.

Quoted 6 lines

?1. clock: use dev_id instead of con_id
?2. Kconfig: delete ISA_DMA
?3. irq: implement int controller based on DT
? ? ? ?move to GENERIC_IRQ_CHIP framework
?4. timer: implement timer based on DT
?5. entry-macro.S: use get_irqnr_preamble to load the base address only once

Quoted 9 lines

? ? per IRQ exception.
?6. others:
? ? ? use readl_relaxed instead of __raw_readl
? ? ? adjust Kconfig order for PRIMA2
? ? ? use clk_get_sys instead of clk_get for system level clock ? ? ? use BUG_ON(IS_ERR(clk)) instead of BUG_ON(IS_ERR_OR_NULL(clk)); ? ? ? fix typo for CLOCK_TICK_RATE
? ? ? ("sirf,xxx", "sirf,prima2-xxx") ?instead of ("sirf,xxx") in dts

Good progress!

Quoted 24 lines

new file mode 100644
index 0000000..1777e98

arch/arm/boot/dts/prima2-cb.dts new file +19 -0

@@ -0,0 +1,50 @@
+/dts-v1/;
+/ {
+ ? ? model = "SIRF Prima2 EVB";
+ ? ? compatible = "sirf,prima2-cb", "sirf,prima2";
+ ? ? #address-cells = <1>;
+ ? ? #size-cells = <1>;
+ ? ? interrupt-parent = <&intc>;
+
+ ? ? memory {
+ ? ? ? ? ? ? reg = <0x00000000 0x20000000>;
+ ? ? };
+
+ ? ? chosen {
+ ? ? ? ? ? ? bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS1 earlyprintk";
+ ? ? ? ? ? ? linux,stdout-path = &uart1;
+ ? ? };
+
+ ? ? amba {
+ ? ? ? ? ? ? compatible = "simple-bus";

arch/arm/boot/dts/prima2-cb.dts line 29

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +10 -0

+ ? ? ? ? ? ? #address-cells = <1>;
+ ? ? ? ? ? ? #size-cells = <1>;
+ ? ? ? ? ? ? ranges;
+
+ ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 {
+ ? ? ? ? ? ? ? ? ? ? interrupt-controller;
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";
+ ? ? ? ? ? ? ? ? ? ? reg = <0x80020000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
+ ? ? ? ? ? ? };

Some more commends on the device tree properties:

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +9 -0

+ ? ? ? ? ? ? timer0: timer at 0xb0020000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,tick", "sirf,prima2-tick";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0020000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? interrupts = <0>;
+ ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? uart0: uart at 0xb0050000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,uart", "sirf,prima2-uart";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0050000 0x1000>;

arch/arm/boot/dts/prima2-cb.dts line 48

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +10 -0

+ ? ? ? ? ? ? ? ? ? ? interrupts = <17>;
+ ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? uart1: uart at 0xb0060000 {
+ ? ? ? ? ? ? ? ? ? ? compatible = "sirf,uart", "sirf,prima2-uart";
+ ? ? ? ? ? ? ? ? ? ? reg = <0xb0060000 0x1000>;
+ ? ? ? ? ? ? ? ? ? ? interrupts = <18>;
+ ? ? ? ? ? ? };
+ ? ? };
+};

When you use local addresses, this would become something like

? ? ? ?axi1 {
? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ?ranges <0 0x80000000 0x10000000>;

? ? ? ? ? ? ? ?intc: interrupt-controller at 20000 {
? ? ? ? ? ? ? ? ? ? ? ?interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,intc", "sirf,prima2-intc"; ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x20000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ?#interrupt-cells = <1>;
? ? ? ? ? ? ? ?};
? ? ? ?}
? ? ? ?axi2 {
? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ?ranges <0 0xb0000000 0x10000000>;

? ? ? ? ? ? ? ?timer0: timer at 20000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x20000 0x1000>;
? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ?uart0: uart at 50000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x50000 0x1000>;
? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ?uart1: uart at 60000 {
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x60000 0x1000>;
? ? ? ? ? ? ? ?};
? ? ? ?};

arch/arm/mach-prima2/Makefile line 1

Quoted 10 lines

arch/arm/boot/dts/prima2-cb.dts new file +1 -0

+

arch/arm/mach-prima2/Makefile new file +1 -0

new file mode 100644
index 0000000..c51d60d
@@ -0,0 +1 @@
+obj-y := timer.o irq.o clock.o common.o board_dt.o

Minor detail:

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

That makes the list more consistent when you add conditional files.

Quoted 21 lines

+++ b/arch/arm/mach-prima2/board_dt.c

arch/arm/mach-prima2/Makefile new file +18 -0

@@ -0,0 +1,28 @@
+/*
+ * This file contains code for boards with device tree support.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/of.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <mach/hardware.h>
+#include "common.h"
+
+static const char *prima2cb_dt_match[] __initdata = {
+ ? ? ? "sirf,prima2-cb",
+ ? ? ? NULL
+};

arch/arm/mach-prima2/Makefile line 28

Quoted 10 lines

arch/arm/mach-prima2/Makefile new file +10 -0

+
+MACHINE_START(PRIMA2_EVB, "prima2cb")
+ ? ? ? .boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100,
+ ? ? ? .init_early ? ? = sirfsoc_init_clk,
+ ? ? ? .map_io ? ? ? ? = sirfsoc_map_io,
+ ? ? ? .init_irq ? ? ? = sirfsoc_of_init_irq,
+ ? ? ? .timer ? ? ? ? ?= &sirfsoc_timer,
+ ? ? ? .init_machine ? = sirfsoc_mach_init,
+ ? ? ? .dt_compat ? ? ?= prima2cb_dt_match,
+MACHINE_END

How about removing the remaining board file entirely and merging the contents into common.c? We don't want to have board specific files any more, so the naming and the split is rather obsolete.

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

Quoted 6 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+#define IOTABLE_ENTRY(region) {\
+ ? ? .virtual ? ? ? ?= SIRFSOC_##region##_VA_BASE, \
+ ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(SIRFSOC_##region##_PA_BASE),\
+ ? ? .length ? ? ? ? = SIRFSOC_##region##_SIZE, \
+ ? ? .type ? ? ? ? ? = MT_DEVICE, \

arch/arm/mach-prima2/Makefile

Quoted 10 lines

arch/arm/mach-prima2/Makefile new file +10 -0

+}
+
+static struct map_desc sirfsoc_iodesc[] __initdata = {
+ ? ? IOTABLE_ENTRY(UART1),
+ ? ? IOTABLE_ENTRY(CLOCK),
+ ? ? IOTABLE_ENTRY(RSTC),
+#ifdef CONFIG_CACHE_L2X0
+ ? ? IOTABLE_ENTRY(L2CC),
+#endif
+};

I've thought about this some more, and I think you can get rid of most entries in the global table after all:

* sirfsoc_init_clk() can get the physical address from the device tree and call iotable_init for just one entry.

* RSTC does not seem to be needed early at all, you can simply ioremap it.

* L2CC is only accessed from an arch_initcall, which is late enough ?for ioremap.

* UART1 is only needed for early printk, so I would put the function ?that does this remaining iotable_init() call into a separate file ?that only gets compiled when that is enabled.

Quoted 22 lines

arch/arm/mach-prima2/Makefile new file +22 -0

+void __init sirfsoc_map_io(void)
+{
+ ? ? iotable_init(sirfsoc_iodesc, ARRAY_SIZE(sirfsoc_iodesc));
+}
+
+#define L2X0_ADDR_FILTERING_START ? ? ? 0xC00
+#define L2X0_ADDR_FILTERING_END ? ? ? ? 0xC04
+
+static int __init sirfsoc_init(void)
+{
+#ifdef CONFIG_CACHE_L2X0
+ ? ? if (!(readl_relaxed(SIRFSOC_L2CC_VA_BASE + L2X0_CTRL) & 1)) {
+ ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover
+ ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? writel_relaxed(PHYS_OFFSET + 1024 * 1024 * 1024,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_END);
+ ? ? ? ? ? ? writel_relaxed(PHYS_OFFSET | 0x1,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_ADDR_FILTERING_START);
+
+ ? ? ? ? ? ? writel_relaxed(0,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_TAG_LATENCY_CTRL);

arch/arm/mach-prima2/Makefile

Quoted 10 lines

arch/arm/mach-prima2/Makefile new file +10 -0

+ ? ? ? ? ? ? writel_relaxed(0,
+ ? ? ? ? ? ? ? ? ? ? SIRFSOC_L2CC_VA_BASE + L2X0_DATA_LATENCY_CTRL);
+ ? ? }
+ ? ? l2x0_init((void __iomem *)SIRFSOC_L2CC_VA_BASE, 0x00040000,
+ ? ? ? ? ? ? 0x00000000);
+#endif
+
+ ? ? return 0;
+}
+arch_initcall(sirfsoc_init);

I would also suggest moving the l2x0 code into a new l2x0.c file that is conditionally compiled. In the header file, you then do

#ifdef CONFIG_CACHE_L2X0
extern void sirfsoc_l2x0_init(void);
#else
static inline void sirfsoc_l2x0_init(void) {}
#endif

arch/arm/mach-prima2/Makefile

Quoted 6 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+#ifndef __ASM_ARCH_IRQS_H
+#define __ASM_ARCH_IRQS_H
+
+#define SIRFSOC_INTENAL_IRQ_START ?0
+#define SIRFSOC_INTENAL_IRQ_END ? ?59

Typo, I assume: INTERNAL, not INTENAL

arch/arm/mach-prima2/Makefile

Quoted 7 lines

arch/arm/mach-prima2/Makefile new file +5 -0

+
+static struct of_device_id intc_ids[] ?= {
+ ? ? { .compatible = "sirf,intc" },
+ ? ? { .compatible = "sirf,prima2-intc" },
+};

You should only list one of these in the driver: The idea is that as long as all variants of the hardware are identical, you can just list the most general name (sirf,intc), and if you need to tell the difference you list the more specific names and add a .data=... member that you use to differentiate between them. Note that it's usually easier to change the code than to change the device tree, so we want to have the full list in the device tree, but the code just needs to pick one that fits the purpose.

It's the same for the other drivers.

Quoted 1 line

+static void __init sirfsoc_of_timer_map(void);

You can remove this forward declaration if you move the sched_clock() function down.

? ? ? ?Arnd
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Quoted 3 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
Memory
AXI-2

         interrupt controller
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  SYS2PCI
                            SD

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

Quoted 7 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name,
heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make
more senses than "sirf, intc".

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Quoted 5 lines

better put these in a list with one file per line, like

obj-y += timer.o
obj-y += irq.o

Quoted 1 line

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")

       .boot_params    = SIRFSOC_SDRAM_PA + 0x100,
       .init_early     = sirfsoc_init_clk,
       .map_io         = sirfsoc_map_io,
       .init_irq       = sirfsoc_of_init_irq,
       .timer          = &sirfsoc_timer,
       .init_machine   = sirfsoc_mach_init,
       .dt_compat      = prima2xxx_dt_match,

MACHINE_END

MACHINE_START(PRIMA2_YYY, "prima2yyy")

       .boot_params    = SIRFSOC_SDRAM_PA + 0x100,
       .init_early     = sirfsoc_init_clk,
       .map_io         = sirfsoc_map_io,
       .init_irq       = sirfsoc_of_init_irq,
       .timer          = &sirfsoc_timer,
       .init_machine   = sirfsoc_mach_init,
       .dt_compat      = prima2yyy_dt_match,

MACHINE_END

after creating a new file named mach-prima2/l2x0.c, it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)

{
        struct device_node *np;
        void __iomem *sirfsoc_l2x_base;

        np = of_find_matching_node(NULL, l2x_ids);
        if (!np)
                panic("unable to find compatible intc node in dtb\n");

        sirfsoc_l2x_base = of_iomap(np, 0);
        if (!sirfsoc_l2x_base)
                panic("unable to map l2x cpu registers\n");

        of_node_put(np);

        if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) {
                /*
                 * set the physical memory windows L2 cache will cover
                 */
                writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024,
                        sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END);
                writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
                        sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

                writel_relaxed(0,
                        sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL);
                writel_relaxed(0,
                        sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL);
        }
        l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000,
                0x00000000);

        return 0;
}

early_initcall(sirfsoc_of_l2x_init);

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Quoted 35 lines · Barry Song

Quoted 3 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
Memory
AXI-2

         interrupt controller
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  xxxx
         IOBG...
                  SYS2PCI
                            SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be referred to by other devices
* clock management for that device
* power management for that device

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

Quoted 15 lines

Quoted 7 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name,
heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make
more senses than "sirf, intc".

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Quoted 5 lines

Quoted 5 lines

better put these in a list with one file per line, like

obj-y += timer.o
obj-y += irq.o

Quoted 8 lines

Quoted 1 line

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Right. Note that you have a := in there, which needs to be +=.

Quoted 18 lines

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")

       .boot_params    = SIRFSOC_SDRAM_PA + 0x100,
       .init_early     = sirfsoc_init_clk,
       .map_io         = sirfsoc_map_io,
       .init_irq       = sirfsoc_of_init_irq,
       .timer          = &sirfsoc_timer,
       .init_machine   = sirfsoc_mach_init,
       .dt_compat      = prima2xxx_dt_match,

MACHINE_END

MACHINE_START(PRIMA2_YYY, "prima2yyy")

Quoted 8 lines

       .boot_params    = SIRFSOC_SDRAM_PA + 0x100,
       .init_early     = sirfsoc_init_clk,
       .map_io         = sirfsoc_map_io,
       .init_irq       = sirfsoc_of_init_irq,
       .timer          = &sirfsoc_timer,
       .init_machine   = sirfsoc_mach_init,
       .dt_compat      = prima2yyy_dt_match,

MACHINE_END

No, this wouldn't make any sense when the only difference is the dt_compat
field: At that point you would just list all the possible boards in the global dt_match table.

Quoted 40 lines

after creating a new file named mach-prima2/l2x0.c, it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)

{
        struct device_node *np;
        void __iomem *sirfsoc_l2x_base;

        np = of_find_matching_node(NULL, l2x_ids);
        if (!np)
                panic("unable to find compatible intc node in dtb\n");

        sirfsoc_l2x_base = of_iomap(np, 0);
        if (!sirfsoc_l2x_base)
                panic("unable to map l2x cpu registers\n");

        of_node_put(np);

        if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) {
                /*
                 * set the physical memory windows L2 cache will cover
                 */
                writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024,
                        sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END);
                writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
                        sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

                writel_relaxed(0,
                        sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL);
                writel_relaxed(0,
                        sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL);
        }
        l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000,
                0x00000000);

Quoted 6 lines

        return 0;
}

early_initcall(sirfsoc_of_l2x_init);

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Yes. Rob/Olof, what's the status of that patch?

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

Quoted 15 lines

Quoted 7 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name, ?heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make more senses than "sirf, intc".

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Quoted 6 lines

Quoted 6 lines

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

Quoted 8 lines

Quoted 1 line

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Right. Note that you have a := in there, which needs to be +=.

Quoted 17 lines

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2xxx_dt_match, MACHINE_END

Quoted 9 lines

MACHINE_START(PRIMA2_YYY, "prima2yyy")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2yyy_dt_match, MACHINE_END

No, this wouldn't make any sense when the only difference is the dt_compat
field: At that point you would just list all the possible boards in the global dt_match table.

Quoted 44 lines

after creating a new file named mach-prima2/l2x0.c, ?it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)
{
? ? ? ? struct device_node *np;
? ? ? ? void __iomem *sirfsoc_l2x_base;

? ? ? ? np = of_find_matching_node(NULL, l2x_ids);
? ? ? ? if (!np)
? ? ? ? ? ? ? ? panic("unable to find compatible intc node in dtb\n");

? ? ? ? sirfsoc_l2x_base = of_iomap(np, 0);
? ? ? ? if (!sirfsoc_l2x_base)
? ? ? ? ? ? ? ? panic("unable to map l2x cpu registers\n");

? ? ? ? of_node_put(np);

? ? ? ? if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) { ? ? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover ? ? ? ? ? ? ? ? ?*/
? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024, ? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END); ? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL); ? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL); ? ? ? ? }
? ? ? ? l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000, ? ? ? ? ? ? ? ? 0x00000000);

? ? ? ? return 0;
}
early_initcall(sirfsoc_of_l2x_init);

Quoted 2 lines

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Yes. Rob/Olof, what's the status of that patch?

? ? ? ?Arnd

except Wall St and stock, i really didn't realize CSR is something related with 'China Security & Surveillance Tech. Inc' . it seems CSR is much more extensively known as a company in Wireless Technology, Audio, Connectivity and GPS.
if we lost this name, people might not know it came from that CSR focusing on Wireless/Audio/Connectivity/GPS. people will ask, what is CSRXF? if we really use stock sticker symbols, CSR.L is the more normal symbol in UK market. if we move to stock sticker symbol, could we use CSR.L?

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 81 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

Quoted 15 lines

Quoted 7 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name, ?heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make more senses than "sirf, intc".

Quoted 7 lines

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Hi Arnd,

ha-ha.

except Wall St and stock, i really didn't realize CSR is something related with 'China Security & Surveillance Tech. Inc' . it seems CSR is much more extensively known as a company in Wireless Technology, Audio, Connectivity and GPS.
if we lost this name, people might not know it came from *that* CSR focusing on Wireless/Audio/Connectivity/GPS. people will ask, what is CSRXF? if we really use stock sticker symbols, CSR.L is the more normal symbol in UK market. if we move to stock sticker symbol, could we use CSR.L?

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

Quoted 16 lines

Quoted 8 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name, ?heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make more senses than "sirf, intc".

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Quoted 13 lines

Quoted 6 lines

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Right. Note that you have a := in there, which needs to be +=.

Quoted 17 lines

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2xxx_dt_match, MACHINE_END

Quoted 9 lines

MACHINE_START(PRIMA2_YYY, "prima2yyy")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2yyy_dt_match, MACHINE_END

No, this wouldn't make any sense when the only difference is the dt_compat
field: At that point you would just list all the possible boards in the global dt_match table.

Quoted 45 lines

after creating a new file named mach-prima2/l2x0.c, ?it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)
{
? ? ? ? struct device_node *np;
? ? ? ? void __iomem *sirfsoc_l2x_base;

? ? ? ? np = of_find_matching_node(NULL, l2x_ids);
? ? ? ? if (!np)
? ? ? ? ? ? ? ? panic("unable to find compatible intc node in dtb\n");

? ? ? ? sirfsoc_l2x_base = of_iomap(np, 0);
? ? ? ? if (!sirfsoc_l2x_base)
? ? ? ? ? ? ? ? panic("unable to map l2x cpu registers\n");

? ? ? ? of_node_put(np);

? ? ? ? if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) { ? ? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover ? ? ? ? ? ? ? ? ?*/
? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024, ? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END); ? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL); ? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL); ? ? ? ? }
? ? ? ? l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000, ? ? ? ? ? ? ? ? 0x00000000);

? ? ? ? return 0;
}
early_initcall(sirfsoc_of_l2x_init);

Quoted 2 lines

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Yes. Rob/Olof, what's the status of that patch?

? ? ? ?Arnd

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

For DEBUG_LL uart, i have moved the static mapping to a new file named lluart.c too:

(quoted diff) +18 -0

+#include <linux/kernel.h>
+#include <asm/page.h>
+#include <asm/mach/map.h>
+#include <mach/map.h>
+#include <mach/uart.h>
+
+void __init sirfsoc_map_lluart(void)
+{
+       struct map_desc sirfsoc_lluart_map = {
+               .virtual        = SIRFSOC_UART1_VA_BASE,
+               .pfn            = __phys_to_pfn(SIRFSOC_UART1_PA_BASE),
+               .length         = SIRFSOC_UART1_SIZE,
+               .type           = MT_DEVICE,
+       };
+
+       iotable_init(&sirfsoc_lluart_map, 1);
+}
+

only when DEBUG_LL is selected, this file will be compiled. Otherwise, an empty sirfsoc_map_lluart is used:

+/* TODO:
+ * add APIs to control reset of every module
+ */

arch/arm/mach-prima2/prima2.c new file +22 -0

+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "common.h"
+
+static struct of_device_id sirfsoc_of_bus_ids[] __initdata = {
+       { .compatible = "simple-bus", },
+       {},
+};
+
+void __init sirfsoc_mach_init(void)
+{
+       of_platform_bus_probe(NULL, sirfsoc_of_bus_ids, NULL);
+}
+
+static const char *prima2cb_dt_match[] __initdata = {
+       "sirf,prima2-cb",
+       NULL
+};

Quoted 6 lines · Barry Song

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

I think that's a good idea, but the ARM specific implementation cannot be in common code. Other architectures have stuff similar to iotable_init in asm/fixmap.h. If we decide on a function prototype for this, the implementation can be arch/*/.

Quoted 8 lines · Arnd Bergmann

Quoted 6 lines · Barry Song

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

Quoted 4 lines

I think that's a good idea, but the ARM specific implementation cannot be in common code. Other architectures have stuff similar to iotable_init in asm/fixmap.h. If we decide on a function prototype for this, the implementation can be arch/*/.

One of the issues with fixmap is that its based around single pages and indexing an area. It's idiotic to use such a thing if you have to map the ISA memory regions for VGA.

Plus, of course, forcing everything down the route of ioremap() and fixmap forces everyone to use 2-level page tables and 4K page table entries, avoiding the possibility of having just a single 1st level page table entry covering their IO space. Not only does it increase TLB pressure but it also makes page table walking more expensive.

Quoted 6 lines

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

I think that's a good idea, but the ARM specific implementation cannot be in common code. Other architectures have stuff similar to iotable_init in asm/fixmap.h. If we decide on a function prototype for this, the implementation can be arch/*/.

How about this:

extern void of_set_fixmap(struct device_node *np, int index, ? ? ? ?unsigned long of_fixmap_offset, pgprot_t flags);

Quoted 21 lines

For DEBUG_LL uart, i have moved the static mapping to a new file named lluart.c too:

(quoted diff) +18 -0

+#include <linux/kernel.h>
+#include <asm/page.h>
+#include <asm/mach/map.h>
+#include <mach/map.h>
+#include <mach/uart.h>
+
+void __init sirfsoc_map_lluart(void)
+{
+ ? ? ? struct map_desc sirfsoc_lluart_map = {
+ ? ? ? ? ? ? ? .virtual ? ? ? ?= SIRFSOC_UART1_VA_BASE,
+ ? ? ? ? ? ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(SIRFSOC_UART1_PA_BASE),
+ ? ? ? ? ? ? ? .length ? ? ? ? = SIRFSOC_UART1_SIZE,
+ ? ? ? ? ? ? ? .type ? ? ? ? ? = MT_DEVICE,
+ ? ? ? };
+
+ ? ? ? iotable_init(&sirfsoc_lluart_map, 1);
+}
+

Quoted 2 lines

only when DEBUG_LL is selected, this file will be compiled. Otherwise, an empty sirfsoc_map_lluart is used:

Ok, sounds good.

Quoted 3 lines

+/* TODO:
+ * add APIs to control reset of every module
+ */

Hmm, how about this: You enumerate every bit in the reset registers, and then add a device tree property to the devices where this is needed containing the number. Then you just need a simple interface like

void sirfsoc_reset_device(struct device *dev)
{
? ? ? ?int len, i;
? ? ? ?unsigned int *reset_bits = of_get_property(dev->of_node, "reset-bit", &len);

? ? ? ?for (i = 0; i<len/4; i++)
? ? ? ? ? ? ? ?sirfsoc_reset_line(reset_bits[i]);
}

Quoted 24 lines

arch/arm/mach-prima2/prima2.c new file +22 -0

+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "common.h"
+
+static struct of_device_id sirfsoc_of_bus_ids[] __initdata = {
+ ? ? ? { .compatible = "simple-bus", },
+ ? ? ? {},
+};
+
+void __init sirfsoc_mach_init(void)
+{
+ ? ? ? of_platform_bus_probe(NULL, sirfsoc_of_bus_ids, NULL);
+}
+
+static const char *prima2cb_dt_match[] __initdata = {
+ ? ? ? "sirf,prima2-cb",
+ ? ? ? NULL
+};

arch/arm/mach-prima2/prima2.c line 40

Quoted 10 lines

arch/arm/mach-prima2/prima2.c new file +10 -0

+
+MACHINE_START(PRIMA2_EVB, "prima2cb")
+ ? ? ? .boot_params ? ?= 0x00000100,
+ ? ? ? .init_early ? ? = sirfsoc_of_clk_init,
+ ? ? ? .map_io ? ? ? ? = sirfsoc_map_lluart,
+ ? ? ? .init_irq ? ? ? = sirfsoc_of_irq_init,
+ ? ? ? .timer ? ? ? ? ?= &sirfsoc_timer,
+ ? ? ? .init_machine ? = sirfsoc_mach_init,
+ ? ? ? .dt_compat ? ? ?= prima2cb_dt_match,
+MACHINE_END

Very nice!

? ? ? ?Arnd

Great idea. in fact there is only one reset bit for every device. So maybe the rstc.c can be
...

        /*
         * Writing 1 to this bit resets corresponding block.
         * Writing 0 to this bit de-asserts reset signal of
         * the corresponding block.
         */
        writel(sirfsoc_rstc_base + (reset_bit / 32) * 4,
                readl(sirfsoc_rstc_base + (reset_bit / 32) * 4) | reset_bit);
        msleep(10);
        writel(sirfsoc_rstc_base + (reset_bit / 32) * 4,
                readl(sirfsoc_rstc_base + (reset_bit / 32) * 4) & ~reset_bit);

Quoted 3 lines

Great idea. in fact there is only one reset bit for every device. So maybe the rstc.c can be
...

Yes, looks good.

Quoted 10 lines

? ? ? ? /*
? ? ? ? ?* Writing 1 to this bit resets corresponding block.
? ? ? ? ?* Writing 0 to this bit de-asserts reset signal of
? ? ? ? ?* the corresponding block.
? ? ? ? ?*/
? ? ? ? writel(sirfsoc_rstc_base + (reset_bit / 32) * 4,
? ? ? ? ? ? ? ? readl(sirfsoc_rstc_base + (reset_bit / 32) * 4) | reset_bit); ? ? ? ? msleep(10);
? ? ? ? writel(sirfsoc_rstc_base + (reset_bit / 32) * 4,
? ? ? ? ? ? ? ? readl(sirfsoc_rstc_base + (reset_bit / 32) * 4) & ~reset_bit);

One remark about the msleep here: I find arbitrary wait periods a bit unclean, and most hardware allows you to poll whether it's done by reading back the register you have just written.

If your hardware can do that, you can replace the msleep() with a single readl or a readl()/msleep(1) loop?

? ? ? ?Arnd

Quoted 2 lines

If your hardware can do that, you can replace the msleep() with a single readl or a readl()/msleep(1) loop?

i can't agree more. if there is such a register in chip, we would have used it. ic guys confirmed there isn't such a register. so..delay...

Hi Arnd,

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 62 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

Quoted 3 lines

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

IO bridges in prima2 don't have that. Configuration is hardcoded now. but we have learned so much from you and hope to improve our future IC design.

Quoted 17 lines

Quoted 17 lines

Quoted 9 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name, ?heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make more senses than "sirf, intc".

Quoted 21 lines

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Quoted 13 lines

Quoted 6 lines

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Now it is changed to:

(quoted diff) +7 -0

+obj-y := timer.o
+obj-y += irq.o
+obj-y += clock.o
+obj-y += rstc.o
+obj-y += prima2.o
+obj-$(CONFIG_CACHE_L2X0) += l2x0.o
+obj-$(CONFIG_DEBUG_LL) += lluart.o

For clock, i have moved the static mapping to clock.c:

arch/arm/mach-prima2/clock.c new file +34 -0

new file mode 100644
index 0000000..f4676bc
...
+
+static void __init sirfsoc_clk_init(void)
+{
+ ? ? ? clkdev_add_table(onchip_clks, ARRAY_SIZE(onchip_clks));
+}
+
+static struct of_device_id clkc_ids[] = {
+ ? ? ? { .compatible = "sirf,clkc" },
+};
+
+void __init sirfsoc_of_clk_init(void)
+{
+ ? ? ? struct device_node *np;
+ ? ? ? struct resource res;
+ ? ? ? struct map_desc sirfsoc_clkc_iodesc = {
+ ? ? ? ? ? ? ? .virtual = SIRFSOC_CLOCK_VA_BASE,
+ ? ? ? ? ? ? ? .type ? ?= MT_DEVICE,
+ ? ? ? };
+
+ ? ? ? np = of_find_matching_node(NULL, clkc_ids);
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? panic("unable to find compatible clkc node in dtb\n");
+
+ ? ? ? if (of_address_to_resource(np, 0, &res))
+ ? ? ? ? ? ? ? panic("unable to find clkc range in dtb");
+ ? ? ? of_node_put(np);
+
+ ? ? ? sirfsoc_clkc_iodesc.pfn = __phys_to_pfn(res.start);
+ ? ? ? sirfsoc_clkc_iodesc.length = 1 + res.end - res.start;
+
+ ? ? ? iotable_init(&sirfsoc_clkc_iodesc, 1);
+
+ ? ? ? sirfsoc_clk_init();
+}

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

For DEBUG_LL uart, i have moved the static mapping to a new file named lluart.c too:

arch/arm/mach-prima2/clock.c new file +18 -0

+#include <linux/kernel.h>
+#include <asm/page.h>
+#include <asm/mach/map.h>
+#include <mach/map.h>
+#include <mach/uart.h>
+
+void __init sirfsoc_map_lluart(void)
+{
+ ? ? ? struct map_desc sirfsoc_lluart_map = {
+ ? ? ? ? ? ? ? .virtual ? ? ? ?= SIRFSOC_UART1_VA_BASE,
+ ? ? ? ? ? ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(SIRFSOC_UART1_PA_BASE),
+ ? ? ? ? ? ? ? .length ? ? ? ? = SIRFSOC_UART1_SIZE,
+ ? ? ? ? ? ? ? .type ? ? ? ? ? = MT_DEVICE,
+ ? ? ? };
+
+ ? ? ? iotable_init(&sirfsoc_lluart_map, 1);
+}
+

only when DEBUG_LL is selected, this file will be compiled. Otherwise, an empty sirfsoc_map_lluart is used:

arch/arm/mach-prima2/common.h new file +2 -0

@@ -0,0 +1,26 @@
+/*
+ * This file contains common function prototypes to avoid externs in

the c files.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */

arch/arm/mach-prima2/common.h new file +19 -0

+
+#ifndef __MACH_PRIMA2_COMMON_H__
+#define __MACH_PRIMA2_COMMON_H__
+
+#include <linux/init.h>
+#include <asm/mach/time.h>
+
+extern struct sys_timer sirfsoc_timer;
+
+extern void __init sirfsoc_of_irq_init(void);
+extern void __init sirfsoc_of_clk_init(void);
+
+#ifndef CONFIG_DEBUG_LL
+static inline void sirfsoc_map_lluart(void) ?{}
+#else
+extern void __init sirfsoc_map_lluart(void);
+#endif
+
+#endif

For rstc, it is the reset controller in prima2, every bit can reset a special component in the SoC. i move the mapping to a new rstc.c file too. But APIs, which every driver will call to reset itself, ?in rstc is not full finished:

arch/arm/mach-prima2/common.h new file +40 -0

+/*
+ * reset controller for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+void __iomem *sirfsoc_rstc_base;
+
+static struct of_device_id rstc_ids[] ?= {
+ ? ? ? { .compatible = "sirf,rstc" },
+};
+
+static int __init sirfsoc_of_rstc_init(void)
+{
+ ? ? ? struct device_node *np;
+
+ ? ? ? np = of_find_matching_node(NULL, rstc_ids);
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? panic("unable to find compatible rstc node in dtb\n");
+
+ ? ? ? sirfsoc_rstc_base = of_iomap(np, 0);
+ ? ? ? if (!sirfsoc_rstc_base)
+ ? ? ? ? ? ? ? panic("unable to map rstc cpu registers\n");
+
+ ? ? ? of_node_put(np);
+
+ ? ? ? return 0;
+}
+early_initcall(sirfsoc_of_rstc_init);
+
+/* TODO:
+ * add APIs to control reset of every module
+ */

Quoted 33 lines

Right. Note that you have a := in there, which needs to be +=.

Quoted 17 lines

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2xxx_dt_match, MACHINE_END

Quoted 9 lines

MACHINE_START(PRIMA2_YYY, "prima2yyy")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2yyy_dt_match, MACHINE_END

Quoted 3 lines

No, this wouldn't make any sense when the only difference is the dt_compat
field: At that point you would just list all the possible boards in the global dt_match table.

Yes. i have rename common.c to prima2.c and now there is no any map_desc table due to the above changes, so it is now much shorter:

arch/arm/mach-prima2/prima2.c new file +40 -0

new file mode 100644
index 0000000..f6b04a1
@@ -0,0 +1,40 @@
+/*
+ * Defines machines for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "common.h"
+
+static struct of_device_id sirfsoc_of_bus_ids[] __initdata = {
+ ? ? ? { .compatible = "simple-bus", },
+ ? ? ? {},
+};
+
+void __init sirfsoc_mach_init(void)
+{
+ ? ? ? of_platform_bus_probe(NULL, sirfsoc_of_bus_ids, NULL);
+}
+
+static const char *prima2cb_dt_match[] __initdata = {
+ ? ? ? "sirf,prima2-cb",
+ ? ? ? NULL
+};
+
+MACHINE_START(PRIMA2_EVB, "prima2cb")
+ ? ? ? .boot_params ? ?= 0x00000100,
+ ? ? ? .init_early ? ? = sirfsoc_of_clk_init,
+ ? ? ? .map_io ? ? ? ? = sirfsoc_map_lluart,
+ ? ? ? .init_irq ? ? ? = sirfsoc_of_irq_init,
+ ? ? ? .timer ? ? ? ? ?= &sirfsoc_timer,
+ ? ? ? .init_machine ? = sirfsoc_mach_init,
+ ? ? ? .dt_compat ? ? ?= prima2cb_dt_match,
+MACHINE_END

Quoted 50 lines

Quoted 42 lines

after creating a new file named mach-prima2/l2x0.c, ?it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)
{
? ? ? ? struct device_node *np;
? ? ? ? void __iomem *sirfsoc_l2x_base;

? ? ? ? np = of_find_matching_node(NULL, l2x_ids);
? ? ? ? if (!np)
? ? ? ? ? ? ? ? panic("unable to find compatible intc node in dtb\n");

? ? ? ? sirfsoc_l2x_base = of_iomap(np, 0);
? ? ? ? if (!sirfsoc_l2x_base)
? ? ? ? ? ? ? ? panic("unable to map l2x cpu registers\n");

? ? ? ? of_node_put(np);

? ? ? ? if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) { ? ? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover ? ? ? ? ? ? ? ? ?*/
? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024, ? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END); ? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL); ? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL); ? ? ? ? }
? ? ? ? l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000, ? ? ? ? ? ? ? ? 0x00000000);

Quoted 6 lines

? ? ? ? return 0;
}
early_initcall(sirfsoc_of_l2x_init);

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Yes. Rob/Olof, what's the status of that patch?

Quoted 2 lines

? ? ? ?Arnd

After we get aggrement on DT node name(csr/csrxf/csr.l ?), i will send patch v3 with all those all changes and full DT. Really thank you very much!

-barry

Hi Arnd,

2011/7/1 Barry Song <[email protected]>:

Quoted 65 lines

Hi Arnd,

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 62 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

Quoted 7 lines

Quoted 3 lines

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

IO bridges in prima2 don't have that. Configuration is hardcoded now. but we have learned so much from you and hope to improve our future IC design.

i have tried to figure out the full DT:

/dts-v1/;
/ {
? ? ? ?model = "SiRF Prima2 EVB";
? ? ? ?compatible = "sirf,prima2-cb", "sirf,prima2"; ? ? ? ?#address-cells = <1>;
? ? ? ?#size-cells = <1>;
? ? ? ?interrupt-parent = <&intc>;

? ? ? ?memory {
? ? ? ? ? ? ? ?reg = <0x00000000 0x20000000>;
? ? ? ?};

? ? ? ?chosen {
? ? ? ? ? ? ? ?bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS0 panel=1 bootsplash=true bpp=16 androidboot.console=ttyS1";
? ? ? ? ? ? ? ?linux,stdout-path = &uart1;
? ? ? ?};

? ? ? ?cpus {
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <0>;

? ? ? ? ? ? ? ?ARM-CortexA9,SiRFprimaII at 0 {
? ? ? ? ? ? ? ? ? ? ? ?device_type = "cpu";
? ? ? ? ? ? ? ? ? ? ? ?reg = <0x0>;
? ? ? ? ? ? ? ? ? ? ? ?d-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? ?i-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? ?d-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? ?i-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? ?/* from bootloader */
? ? ? ? ? ? ? ? ? ? ? ?timebase-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? ?bus-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? ?clock-frequency = <0>;
? ? ? ? ? ? ? ?};
? ? ? ?};

? ? ? ?axi {
? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ?ranges = <0x40000000 0x40000000 0x80000000>;

? ? ? ? ? ? ? ?l2-cache-controller at 0x80040000 {
? ? ? ? ? ? ? ? ? ? ? ?compatible = "arm,pl310-cache", "cache"; ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x80040000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ?interrupts = <59>;
? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ?sirfsoc-iobus {
? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x56000000 0x56000000 0x1b0000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x80010000 0x80010000 0x30000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x88000000 0x88000000 0x40040000>;

? ? ? ? ? ? ? ? ? ? ? ?intc: interrupt-controller at 0x80020000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#interrupt-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,intc", "sirf,prima2-intc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x80020000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?sys-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x88000000 0x88000000 0x40000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?clock-controller at 0x88000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,clkc", "sirf,prima2-clkc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x88000000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <3>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reset-controller at 0x88010000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,rstc", "sirf,prima2-rstc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x88010000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?mem-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x90000000 0x90000000 0x10000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?memory-controller at 0x90000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,memc", "sirf,prima2-memc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x90000000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <27>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?disp-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x90010000 0x90010000 0x30000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?display at 0x90010000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,lcd", "sirf,prima2-lcd"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x90010000 0x20000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <30>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?vpp at 0x90020000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,vpp", "sirf,prima2-vpp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x90020000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <31>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?graphics-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x98000000 0x98000000 0x8000000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?graphics at 0x98000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,graphics", "sirf,prima2-graphics"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x98000000 0x8000000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <6>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?multimedia-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0xa0000000 0xa0000000 0x8000000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?multimedia at 0xa0000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,multimedia", "sirf,prima2-multimedia"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xa0000000 0x8000000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <5>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?dsp-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0xa8000000 0xa8000000 0x2000000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dspif at 0xa8000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,dspif", "sirf,prima2-dspif"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xa8000000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <9>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gps at 0xa8010000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,gps", "sirf,prima2-gps"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xa8010000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <7>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dsp at 0xa9000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,dsp", "sirf,prima2-dsp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xa9000000 0x1000000>; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <8>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?peri-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0xb0000000 0xb0000000 0x180000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?timer at 0xb0020000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,tick", "sirf,prima2-tick"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0020000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?nand at 0xb0030000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,nand", "sirf,prima2-nand"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0030000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <41>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?audio at 0xb0040000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,audio", "sirf,prima2-audio"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0040000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <35>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uart0: uart at 0xb0050000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,uart", "sirf,prima2-uart"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0050000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <17>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uart1: uart at 0xb0060000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,uart", "sirf,prima2-uart"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0060000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <18>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uart2: uart at 0xb0070000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <2>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,uart", "sirf,prima2-uart"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0070000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <19>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usp0: usp at 0xb0080000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,usp", "sirf,prima2-usp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0080000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <20>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usp1: usp at 0xb0090000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,usp", "sirf,prima2-usp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0090000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <21>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usp2: usp at 0xb00a0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <2>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,usp", "sirf,prima2-usp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00a0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <22>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmac0: dma-controller at 0xb00b0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,dmac", "sirf,prima2-dmac"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00b0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <12>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmac1: dma-controller at 0xb0160000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,dmac", "sirf,prima2-dmac"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0160000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <13>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?vip at 0xb00C0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,vip", "sirf,prima2-vip"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00C0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?spi0: spi at 0xb00D0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,spi", "sirf,prima2-spi"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00D0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <15>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?spi1: spi at 0xb0170000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,spi", "sirf,prima2-spi"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0170000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <16>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?i2c0: i2c at 0xb00E0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,i2c", "sirf,prima2-i2c"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00E0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <24>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?i2c1: i2c at 0xb00f0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,i2c", "sirf,prima2-i2c"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb00f0000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <25>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tsc at 0xb0110000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,tsc", "sirf,prima2-tsc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0110000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <33>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gpio: gpio-controller at 0xb0120000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#gpio-cells = <2>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#interrupt-cells = <2>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,gpio", "sirf,prima2-gpio"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0120000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gpio-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pwm at 0xb0130000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,pwm", "sirf,prima2-pwm"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0130000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?efusesys at 0xb0140000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,efuse", "sirf,prima2-efuse"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0140000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pulsec at 0xb0150000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,pulsec", "sirf,prima2-pulsec"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb0150000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <48>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pci-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,pciiobg", "sirf,prima2-pciiobg", "simple-bus"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0x56000000 0x56000000 0x1b00000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd0: sdhci at 0x56000000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <0>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56000000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <38>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd1: sdhci at 0x56100000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56100000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <38>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd2: sdhci at 0x56200000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <2>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56200000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <23>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd3: sdhci at 0x56300000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <3>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56300000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <23>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd4: sdhci at 0x56400000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <4>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56400000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <39>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sd5: sdhci at 0x56500000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cell-index = <5>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sdhc", "sirf,prima2-sdhc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x56500000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <39>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pci-copy at 0x57900000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,pcicp", "sirf,prima2-pcicp"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x57900000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <40>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rom-interface at 0x57a00000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,romif", "sirf,prima2-romif"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x57a00000 0x100000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?rtc-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,rtciobg", "sirf,prima2-rtciobg", "simple-bus"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x80030000 0x10000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gpsrtc at 0x1000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,gpsrtc", "sirf,prima2-gpsrtc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x1000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <55 56 57>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sysrtc at 0x2000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sysrtc", "sirf,prima2-sysrtc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x2000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <52 53 54>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pwrc at 0x3000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,pwrc", "sirf,prima2-pwrc"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x3000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <32>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ?uus-iobg {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?#size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ranges = <0xb8000000 0xb8000000 0x40000>;

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usb0: usb at 0xb00E0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,usb", "sirf,prima2-usb"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb8000000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <10>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usb1: usb at 0xb00f0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,usb", "sirf,prima2-usb"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb8010000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <11>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sata at 0xb00f0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,sata", "sirf,prima2-sata"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb8020000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <37>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?security at 0xb00f0000 {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sirf,security", "sirf,prima2-security"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0xb8030000 0x10000>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <42>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ? ? ? ? ?};
? ? ? ? ? ? ? ?};
? ? ? ?};
};

if it looks ok to you, i will send v3 including it and other changes reviewed by you before.

Quoted 333 lines

Quoted 17 lines

Quoted 17 lines

Quoted 9 lines

I think the namespace for the compatible values is supposed to start with the stock ticker name of the company making the device as a unique identifier. This means you'd have to use
"csrxf,sirf-intc", "csrxf,sirf-prima2-intc" as the value, instead of starting with the product name. I don't know exactly how strictly we apply that rule, but I've taken the devicetree-discuss at lists.ozlabs.org mailing list on Cc, maybe someone can clarify.

in fact, SiRF is a company name. it was merged into CSR 4 years ago. Due to history reason, now the SoC names are still headed by sirf. the logo in SiRFprimaII chip is CSR.
So the "SiRF" of SiRFprimaII should mean two things: old company name, ?heritable CPU production-line. Anyway, "csr, sirf-intc" seems to make more senses than "sirf, intc".

Quoted 21 lines

Quoted 2 lines

could we change "csrxf,sirf-intc", "csrxf,sirf-prima2-intc" to "csr,sirf-intc", "csr,sirf-prima2-intc"?

Not sure how strict we interpret the rules about stock ticker symbols. 'CSR' on wallstreet is 'China Security & Surveillance Tech. Inc'. If they ever decide to produce embedded Linux machines, we'd get a conflict, unless they also use "csst" (their .com domain name) as a prefix.

Quoted 13 lines

Quoted 6 lines

better put these in a list with one file per line, like

obj-y ? += timer.o
obj-y ? += irq.o

That makes the list more consistent when you add conditional files.

Then it could be:
obj-y += timer.o
obj-y += irq.o
obj-y += clock.o
obj-y += common.o
obj-$(CONFIG_CACHE_L2X0) := l2x0.o

Now it is changed to:

(quoted diff) +7 -0

+obj-y := timer.o
+obj-y += irq.o
+obj-y += clock.o
+obj-y += rstc.o
+obj-y += prima2.o
+obj-$(CONFIG_CACHE_L2X0) += l2x0.o
+obj-$(CONFIG_DEBUG_LL) += lluart.o

For clock, i have moved the static mapping to clock.c:

arch/arm/mach-prima2/clock.c new file +34 -0

new file mode 100644
index 0000000..f4676bc
...
+
+static void __init sirfsoc_clk_init(void)
+{
+ ? ? ? clkdev_add_table(onchip_clks, ARRAY_SIZE(onchip_clks));
+}
+
+static struct of_device_id clkc_ids[] = {
+ ? ? ? { .compatible = "sirf,clkc" },
+};
+
+void __init sirfsoc_of_clk_init(void)
+{
+ ? ? ? struct device_node *np;
+ ? ? ? struct resource res;
+ ? ? ? struct map_desc sirfsoc_clkc_iodesc = {
+ ? ? ? ? ? ? ? .virtual = SIRFSOC_CLOCK_VA_BASE,
+ ? ? ? ? ? ? ? .type ? ?= MT_DEVICE,
+ ? ? ? };
+
+ ? ? ? np = of_find_matching_node(NULL, clkc_ids);
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? panic("unable to find compatible clkc node in dtb\n");
+
+ ? ? ? if (of_address_to_resource(np, 0, &res))
+ ? ? ? ? ? ? ? panic("unable to find clkc range in dtb");
+ ? ? ? of_node_put(np);
+
+ ? ? ? sirfsoc_clkc_iodesc.pfn = __phys_to_pfn(res.start);
+ ? ? ? sirfsoc_clkc_iodesc.length = 1 + res.end - res.start;
+
+ ? ? ? iotable_init(&sirfsoc_clkc_iodesc, 1);
+
+ ? ? ? sirfsoc_clk_init();
+}

It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping?
Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c?

For DEBUG_LL uart, i have moved the static mapping to a new file named lluart.c too:

arch/arm/mach-prima2/clock.c new file +18 -0

+#include <linux/kernel.h>
+#include <asm/page.h>
+#include <asm/mach/map.h>
+#include <mach/map.h>
+#include <mach/uart.h>
+
+void __init sirfsoc_map_lluart(void)
+{
+ ? ? ? struct map_desc sirfsoc_lluart_map = {
+ ? ? ? ? ? ? ? .virtual ? ? ? ?= SIRFSOC_UART1_VA_BASE,
+ ? ? ? ? ? ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(SIRFSOC_UART1_PA_BASE),
+ ? ? ? ? ? ? ? .length ? ? ? ? = SIRFSOC_UART1_SIZE,
+ ? ? ? ? ? ? ? .type ? ? ? ? ? = MT_DEVICE,
+ ? ? ? };
+
+ ? ? ? iotable_init(&sirfsoc_lluart_map, 1);
+}
+

only when DEBUG_LL is selected, this file will be compiled. Otherwise, an empty sirfsoc_map_lluart is used:

arch/arm/mach-prima2/common.h new file +2 -0

@@ -0,0 +1,26 @@
+/*
+ * This file contains common function prototypes to avoid externs in

the c files.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */

arch/arm/mach-prima2/common.h new file +19 -0

+
+#ifndef __MACH_PRIMA2_COMMON_H__
+#define __MACH_PRIMA2_COMMON_H__
+
+#include <linux/init.h>
+#include <asm/mach/time.h>
+
+extern struct sys_timer sirfsoc_timer;
+
+extern void __init sirfsoc_of_irq_init(void);
+extern void __init sirfsoc_of_clk_init(void);
+
+#ifndef CONFIG_DEBUG_LL
+static inline void sirfsoc_map_lluart(void) ?{}
+#else
+extern void __init sirfsoc_map_lluart(void);
+#endif
+
+#endif

For rstc, it is the reset controller in prima2, every bit can reset a special component in the SoC. i move the mapping to a new rstc.c file too. But APIs, which every driver will call to reset itself, ?in rstc is not full finished:

arch/arm/mach-prima2/common.h new file +40 -0

+/*
+ * reset controller for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+void __iomem *sirfsoc_rstc_base;
+
+static struct of_device_id rstc_ids[] ?= {
+ ? ? ? { .compatible = "sirf,rstc" },
+};
+
+static int __init sirfsoc_of_rstc_init(void)
+{
+ ? ? ? struct device_node *np;
+
+ ? ? ? np = of_find_matching_node(NULL, rstc_ids);
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? panic("unable to find compatible rstc node in dtb\n");
+
+ ? ? ? sirfsoc_rstc_base = of_iomap(np, 0);
+ ? ? ? if (!sirfsoc_rstc_base)
+ ? ? ? ? ? ? ? panic("unable to map rstc cpu registers\n");
+
+ ? ? ? of_node_put(np);
+
+ ? ? ? return 0;
+}
+early_initcall(sirfsoc_of_rstc_init);
+
+/* TODO:
+ * add APIs to control reset of every module
+ */

Quoted 33 lines

Right. Note that you have a := in there, which needs to be +=.

Quoted 17 lines

Quoted 2 lines

It probably makes sense to pick a new name for the combined file, too, but I can't think of a good one. Maybe one of platform.c, prima2.c or core.c.

i am not sure the original purpose of board_dt.c. and i am guessing whether Grant created that single board file to contain multiple boards. For example:

MACHINE_START(PRIMA2_XXX, "prima2xxx")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2xxx_dt_match, MACHINE_END

Quoted 9 lines

MACHINE_START(PRIMA2_YYY, "prima2yyy")
? ? ? ?.boot_params ? ?= SIRFSOC_SDRAM_PA + 0x100, ? ? ? ?.init_early ? ? = sirfsoc_init_clk, ? ? ? ?.map_io ? ? ? ? = sirfsoc_map_io,
? ? ? ?.init_irq ? ? ? = sirfsoc_of_init_irq, ? ? ? ?.timer ? ? ? ? ?= &sirfsoc_timer,
? ? ? ?.init_machine ? = sirfsoc_mach_init, ? ? ? ?.dt_compat ? ? ?= prima2yyy_dt_match, MACHINE_END

Quoted 3 lines

No, this wouldn't make any sense when the only difference is the dt_compat
field: At that point you would just list all the possible boards in the global dt_match table.

Yes. i have rename common.c to prima2.c and now there is no any map_desc table due to the above changes, so it is now much shorter:

arch/arm/mach-prima2/prima2.c new file +40 -0

new file mode 100644
index 0000000..f6b04a1
@@ -0,0 +1,40 @@
+/*
+ * Defines machines for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include "common.h"
+
+static struct of_device_id sirfsoc_of_bus_ids[] __initdata = {
+ ? ? ? { .compatible = "simple-bus", },
+ ? ? ? {},
+};
+
+void __init sirfsoc_mach_init(void)
+{
+ ? ? ? of_platform_bus_probe(NULL, sirfsoc_of_bus_ids, NULL);
+}
+
+static const char *prima2cb_dt_match[] __initdata = {
+ ? ? ? "sirf,prima2-cb",
+ ? ? ? NULL
+};
+
+MACHINE_START(PRIMA2_EVB, "prima2cb")
+ ? ? ? .boot_params ? ?= 0x00000100,
+ ? ? ? .init_early ? ? = sirfsoc_of_clk_init,
+ ? ? ? .map_io ? ? ? ? = sirfsoc_map_lluart,
+ ? ? ? .init_irq ? ? ? = sirfsoc_of_irq_init,
+ ? ? ? .timer ? ? ? ? ?= &sirfsoc_timer,
+ ? ? ? .init_machine ? = sirfsoc_mach_init,
+ ? ? ? .dt_compat ? ? ?= prima2cb_dt_match,
+MACHINE_END

Quoted 50 lines

Quoted 42 lines

after creating a new file named mach-prima2/l2x0.c, ?it seems we only need to change Makefile to:
obj-$(CONFIG_CACHE_L2X0) := l2x0.o
the head file is not needed.

Currently, rob's OF-based L2 cache is not merged yet. then i only write the following:

static int __init sirfsoc_of_l2x_init(void)
{
? ? ? ? struct device_node *np;
? ? ? ? void __iomem *sirfsoc_l2x_base;

? ? ? ? np = of_find_matching_node(NULL, l2x_ids);
? ? ? ? if (!np)
? ? ? ? ? ? ? ? panic("unable to find compatible intc node in dtb\n");

? ? ? ? sirfsoc_l2x_base = of_iomap(np, 0);
? ? ? ? if (!sirfsoc_l2x_base)
? ? ? ? ? ? ? ? panic("unable to map l2x cpu registers\n");

? ? ? ? of_node_put(np);

? ? ? ? if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) { ? ? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ? ?* set the physical memory windows L2 cache will cover ? ? ? ? ? ? ? ? ?*/
? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024, ? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END); ? ? ? ? ? ? ? ? writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);

? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL); ? ? ? ? ? ? ? ? writel_relaxed(0,
? ? ? ? ? ? ? ? ? ? ? ? sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL); ? ? ? ? }
? ? ? ? l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000, ? ? ? ? ? ? ? ? 0x00000000);

Quoted 6 lines

? ? ? ? return 0;
}
early_initcall(sirfsoc_of_l2x_init);

After Rob's patch is merged, i think sirfsoc_of_l2x_init can be much simpler.

Yes. Rob/Olof, what's the status of that patch?

Quoted 9 lines

Quoted 2 lines

? ? ? ?Arnd

After we get aggrement on DT node name(csr/csrxf/csr.l ?), i will send patch v3 with all those all changes and full DT. Really thank you very much!

-barry

-barry

Hi Arnd,

2011/7/1 Barry Song <[email protected]>:

Quoted 65 lines

Hi Arnd,

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 62 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

Quoted 7 lines

Quoted 3 lines

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

IO bridges in prima2 don't have that. Configuration is hardcoded now. but we have learned so much from you and hope to improve our future IC design.

i have tried to figure out the full DT:

/dts-v1/;

/ {
	model = "SiRF Prima2 EVB";
	compatible = "sirf,prima2-cb", "sirf,prima2";
	#address-cells = <1>;
	#size-cells = <1>;
	interrupt-parent = <&intc>;

	memory {

		reg = <0x00000000 0x20000000>;
	};

	chosen {
		bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS0 panel=1
bootsplash=true bpp=16 androidboot.console=ttyS1";

		linux,stdout-path = &uart1;
	};

	cpus {
		#address-cells = <1>;
		#size-cells = <0>;

		ARM-CortexA9,SiRFprimaII at 0 {

			device_type = "cpu";

			reg = <0x0>;
			d-cache-line-size = <32>;
			i-cache-line-size = <32>;
			d-cache-size = <32768>;
			i-cache-size = <32768>;
			/* from bootloader */
			timebase-frequency = <0>;
			bus-frequency = <0>;
			clock-frequency = <0>;
		};
	};

	axi {
		compatible = "simple-bus";
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x40000000 0x40000000 0x80000000>;

		l2-cache-controller at 0x80040000 {

			compatible = "arm,pl310-cache", "cache";

			reg = <0x80040000 0x1000>;
			interrupts = <59>;
		};

		sirfsoc-iobus {
			compatible = "simple-bus";
			#address-cells = <1>;
			#size-cells = <1>;

			ranges = <0x56000000 0x56000000 0x1b0000
				0x80010000 0x80010000 0x30000
				0x88000000 0x88000000 0x40040000>;

			intc: interrupt-controller at 0x80020000 {
				#interrupt-cells = <1>;
				interrupt-controller;
				compatible = "sirf,intc", "sirf,prima2-intc";

Quoted 87 lines

Hi Arnd,

2011/7/1 Barry Song <[email protected]>:

Quoted 65 lines

Hi Arnd,

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 62 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

Quoted 7 lines

Quoted 3 lines

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

IO bridges in prima2 don't have that. Configuration is hardcoded now. but we have learned so much from you and hope to improve our future IC design.

i have tried to figure out the full DT:

/dts-v1/;
/ {
? ? ? model = "SiRF Prima2 EVB";
? ? ? compatible = "sirf,prima2-cb", "sirf,prima2"; ? ? ? #address-cells = <1>;
? ? ? #size-cells = <1>;
? ? ? interrupt-parent = <&intc>;

Quoted 1 line

? ? ? memory {

memory at 0

Quoted 9 lines

? ? ? ? ? ? ? reg = <0x00000000 0x20000000>;
? ? ? };

? ? ? chosen {
? ? ? ? ? ? ? bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS0 panel=1 bootsplash=true bpp=16 androidboot.console=ttyS1";
? ? ? ? ? ? ? linux,stdout-path = &uart1;
? ? ? };

Quoted 5 lines

? ? ? cpus {
? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? #size-cells = <0>;

? ? ? ? ? ? ? ARM-CortexA9,SiRFprimaII at 0 {

cpu at 0 {

Don't duplicate the crack that powerpc did for cpu node names. ?Use a compatible property to identify the specific CPU model.

Quoted 1 line

? ? ? ? ? ? ? ? ? ? ? device_type = "cpu";

Drop device_type here.

Quoted 12 lines

? ? ? ? ? ? ? ? ? ? ? reg = <0x0>;
? ? ? ? ? ? ? ? ? ? ? d-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? i-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? d-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? i-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? /* from bootloader */
? ? ? ? ? ? ? ? ? ? ? timebase-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? bus-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? clock-frequency = <0>;
? ? ? ? ? ? ? };
? ? ? };

Quoted 7 lines

? ? ? axi {
? ? ? ? ? ? ? compatible = "simple-bus";
? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? #size-cells = <1>;
? ? ? ? ? ? ? ranges = <0x40000000 0x40000000 0x80000000>;

? ? ? ? ? ? ? l2-cache-controller at 0x80040000 {

l2-cache-controller at 80040000 ?(Drop the '0x' from node names; address in the node name is always hex.

Quoted 1 line

? ? ? ? ? ? ? ? ? ? ? compatible = "arm,pl310-cache", "cache";

Drop the "cache" compatible value here.

Quoted 12 lines

? ? ? ? ? ? ? ? ? ? ? reg = <0x80040000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? interrupts = <59>;
? ? ? ? ? ? ? };

? ? ? ? ? ? ? sirfsoc-iobus {
? ? ? ? ? ? ? ? ? ? ? compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? #size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ranges = <0x56000000 0x56000000 0x1b0000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x80010000 0x80010000 0x30000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x88000000 0x88000000 0x40040000>;

Quoted 4 lines

? ? ? ? ? ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";

This looks backwards. ?Most specific values should appear first. "sirf,intc" looks to be generic when compared to "sirf,prima2-intc". I'm also concerned that "sirf,intc" is trying to be too generic. ?As much as possible compatible values should reflect a real implementation of the interface, not a generic abstract name. ?Future hardware can always claim compatibility with existing older hardware. You're probably better off dropping "sirf,intc" entirely, and doing the same for similar properties throughout the file.

Also, for every new compatible value make sure you add documentation for it to Documentation/devicetree/bindings. (You may have already done so, it's just something I remind people about a lot).

These comments also apply through the rest of the file.

2011/7/6 Grant Likely <[email protected]>:

Quoted 160 lines · Barry Song

Quoted 87 lines

Hi Arnd,

2011/7/1 Barry Song <[email protected]>:

Quoted 65 lines

Hi Arnd,

2011/6/30 Arnd Bergmann <[email protected]>:

Quoted 62 lines · Barry Song

Quoted 36 lines

Quoted 4 lines

Is this really just one bus with a huge address space, or rather some nested buses? I'd prefer to have the device tree representation as close as possible to the actual layout.

there are two AXI buses in prima2. AXI-1 connect to memory, AXI-2 is transferred to CSR self-defined IOBUS by CPUIF, then 1 intterupt controller and 9 IO bridges are connected to the IOBUS . The 9 IO bridges are SYSIOBG, PERIIOBG,CPURTCIOBG, UUSIOBG, GRAPHIOBG, MEDIAIOBG, DSPIOBG, DISPIOBG, MEMIOBG. Every iobrg connect to a group of controllers.
For example, DISPIOBG connect to VPP and LCD, SYSIOBG connect to CLKC, RSTC, RSC and CPHIFBG, DSPIOBG connect to DSPIF, GPS and DSP. PERIIOBG connect to TIMER, NAND, AUDIO, UART0, UART1, UART2, USP0, USP1, USP2, SPI0, I2C0, I2C1, GPIO, *SYS2PCI* and so on. Then *SYS2PCI* connect to SD.

The indendation descible the device hierarchy
AXI-1
? ? ? ? ?Memory
AXI-2
? ? ? ? ?interrupt controller
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? xxxx
? ? ? ? ?IOBG...
? ? ? ? ? ? ? ? ? SYS2PCI
? ? ? ? ? ? ? ? ? ? ? ? ? ? SD

Quoted 1 line

i have get the IC guy Weizeng involved, maybe he can explain better than me :-)

I think it would be good to represent the IOBG devices in the device tree then. You don't need to represent AXI-1 because memory is special anyway, and I would not bother to list SYS2PCI if the intention of that block was to hide the fact that it's PCI behind it. Properly instantiating it as a PCI bridge would be a lot of work that is probably not worth it.

My usual plea to hardware developers: Please make the registers autodiscoverable from software! On an AMBA bus, please use the PrimeCell register layout. If you always have an IOBG device behind, they should all have the same identifier for that kind of bus bridge.

For the IOBG, it would be ideal to have a similar way of finding and configuring the connected hardware, including:

* unique identifier for each distinct IP block
* revision of that block
* MMIO ranges and sizes, relative to the bus
* interrupt numbers, relative to a local interrupt controller
* location identifier (like PCI bus/device/fn number) that can be ?referred to by other devices
* clock management for that device
* power management for that device

Quoted 7 lines

Quoted 3 lines

If your IODB infrastructure already has this, you should create a new bus-type for this in Linux, which will let you detect all devices in a consistent manner without having to list them in the device tree.

IO bridges in prima2 don't have that. Configuration is hardcoded now. but we have learned so much from you and hope to improve our future IC design.

i have tried to figure out the full DT:

/dts-v1/;
/ {
? ? ? model = "SiRF Prima2 EVB";
? ? ? compatible = "sirf,prima2-cb", "sirf,prima2"; ? ? ? #address-cells = <1>;
? ? ? #size-cells = <1>;
? ? ? interrupt-parent = <&intc>;

Quoted 1 line

? ? ? memory {

memory at 0

Quoted 9 lines

? ? ? ? ? ? ? reg = <0x00000000 0x20000000>;
? ? ? };

? ? ? chosen {
? ? ? ? ? ? ? bootargs = "mem=512M real_root=/dev/mmcblk0p2 console=ttyS0 panel=1 bootsplash=true bpp=16 androidboot.console=ttyS1";
? ? ? ? ? ? ? linux,stdout-path = &uart1;
? ? ? };

Quoted 5 lines

? ? ? cpus {
? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? #size-cells = <0>;

? ? ? ? ? ? ? ARM-CortexA9,SiRFprimaII at 0 {

cpu at 0 {

Don't duplicate the crack that powerpc did for cpu node names. ?Use a compatible property to identify the specific CPU model.

Quoted 1 line

? ? ? ? ? ? ? ? ? ? ? device_type = "cpu";

Drop device_type here.

Quoted 12 lines

? ? ? ? ? ? ? ? ? ? ? reg = <0x0>;
? ? ? ? ? ? ? ? ? ? ? d-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? i-cache-line-size = <32>;
? ? ? ? ? ? ? ? ? ? ? d-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? i-cache-size = <32768>;
? ? ? ? ? ? ? ? ? ? ? /* from bootloader */
? ? ? ? ? ? ? ? ? ? ? timebase-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? bus-frequency = <0>;
? ? ? ? ? ? ? ? ? ? ? clock-frequency = <0>;
? ? ? ? ? ? ? };
? ? ? };

Quoted 7 lines

? ? ? axi {
? ? ? ? ? ? ? compatible = "simple-bus";
? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? #size-cells = <1>;
? ? ? ? ? ? ? ranges = <0x40000000 0x40000000 0x80000000>;

? ? ? ? ? ? ? l2-cache-controller at 0x80040000 {

l2-cache-controller at 80040000 ?(Drop the '0x' from node names; address in the node name is always hex.

Quoted 1 line

? ? ? ? ? ? ? ? ? ? ? compatible = "arm,pl310-cache", "cache";

Drop the "cache" compatible value here.

Quoted 12 lines

? ? ? ? ? ? ? ? ? ? ? reg = <0x80040000 0x1000>;
? ? ? ? ? ? ? ? ? ? ? interrupts = <59>;
? ? ? ? ? ? ? };

? ? ? ? ? ? ? sirfsoc-iobus {
? ? ? ? ? ? ? ? ? ? ? compatible = "simple-bus";
? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? #size-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ranges = <0x56000000 0x56000000 0x1b0000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x80010000 0x80010000 0x30000 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x88000000 0x88000000 0x40040000>;

Quoted 4 lines

? ? ? ? ? ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";

Quoted 8 lines

This looks backwards. ?Most specific values should appear first. "sirf,intc" looks to be generic when compared to "sirf,prima2-intc". I'm also concerned that "sirf,intc" is trying to be too generic. ?As much as possible compatible values should reflect a real implementation of the interface, not a generic abstract name. ?Future hardware can always claim compatibility with existing older hardware. You're probably better off dropping "sirf,intc" entirely, and doing the same for similar properties throughout the file.

i have been confused by compatible in dts and drivers for some time. if an intc ip core is shared by two chips, for example, prima2 and atlas6. is the following right?

in dts for prima2: compatible = "sirf,prima2-intc"; in dts for atlas6: ?compatible = "sirf,prima2-atlas6";

in intc drivers shared for both: ? ? compatible = "sirf,prima2-intc", "sirf,prima2-atlas6";

in my understanding, dts for special soc/board contains special compatible for the chip, drivers shared by multi-soc/board contain all compatible lists which can be supported by them?

what's the generic way for this?

Quoted 8 lines

Also, for every new compatible value make sure you add documentation for it to Documentation/devicetree/bindings. (You may have already done so, it's just something I remind people about a lot).

These comments also apply through the rest of the file.

2011/7/6 Barry Song <[email protected]>:

Quoted 15 lines

2011/7/6 Grant Likely <[email protected]>:

Quoted 5 lines

Quoted 4 lines

? ? ? ? ? ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";

Quoted 8 lines

This looks backwards. ?Most specific values should appear first. "sirf,intc" looks to be generic when compared to "sirf,prima2-intc". I'm also concerned that "sirf,intc" is trying to be too generic. ?As much as possible compatible values should reflect a real implementation of the interface, not a generic abstract name. ?Future hardware can always claim compatibility with existing older hardware. You're probably better off dropping "sirf,intc" entirely, and doing the same for similar properties throughout the file.

Quoted 6 lines

i have been confused by compatible in dts and drivers for some time. if an intc ip core is shared by two chips, for example, prima2 and atlas6. is the following right?

in dts for prima2: compatible = "sirf,prima2-intc"; in dts for atlas6: ?compatible = "sirf,prima2-atlas6";

sorry for typo. in dts for atlas6: compatible = "sirf,atlas6-intc";

Quoted 2 lines

in intc drivers shared for both: ? ? compatible = "sirf,prima2-intc", "sirf,prima2-atlas6";

sorry for typo. compatible = "sirf,prima2-intc", "sirf,atlas6-intc";

Quoted 16 lines

in my understanding, dts for special soc/board contains special compatible for the chip, drivers shared by multi-soc/board contain all compatible lists which can be supported by them?

what's the generic way for this?

Quoted 8 lines

Also, for every new compatible value make sure you add documentation for it to Documentation/devicetree/bindings. (You may have already done so, it's just something I remind people about a lot).

These comments also apply through the rest of the file.

Quoted 23 lines

2011/7/6 Barry Song <[email protected]>:

Quoted 15 lines

2011/7/6 Grant Likely <[email protected]>:

Quoted 5 lines

Quoted 4 lines

? ? ? ? ? ? ? ? ? ? ? intc: interrupt-controller at 0x80020000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? #interrupt-cells = <1>;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? interrupt-controller;
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "sirf,intc", "sirf,prima2-intc";

Quoted 8 lines

This looks backwards. ?Most specific values should appear first. "sirf,intc" looks to be generic when compared to "sirf,prima2-intc". I'm also concerned that "sirf,intc" is trying to be too generic. ?As much as possible compatible values should reflect a real implementation of the interface, not a generic abstract name. ?Future hardware can always claim compatibility with existing older hardware. You're probably better off dropping "sirf,intc" entirely, and doing the same for similar properties throughout the file.

Quoted 6 lines

i have been confused by compatible in dts and drivers for some time. if an intc ip core is shared by two chips, for example, prima2 and atlas6. is the following right?

in dts for prima2: compatible = "sirf,prima2-intc"; in dts for atlas6: ?compatible = "sirf,prima2-atlas6";

Quoted 7 lines

sorry for typo. in dts for atlas6: ?compatible = "sirf,atlas6-intc";

Quoted 2 lines

in intc drivers shared for both: ? ? compatible = "sirf,prima2-intc", "sirf,prima2-atlas6";

sorry for typo. ?compatible = "sirf,prima2-intc", "sirf,atlas6-intc";

Yes, that is one way to do it, just specify all the implementations using the IP core. ?If there are only a few, then this is easy. ?If there are a lot of implementations using the same core, then the list gets rather large in a hurry.

The intent of compatible is to allow one device to claim compatibility with another. ?So, if the prima2 was already supported, and you were adding atlas6 support, you could put the following into the atlas6 .dts:

? ? ? ?compatible = "sirf,atlas6-intc", "airf,prima2-intc";

and the existing driver would work.

The other option when working with IP cores it so encode the version of the IP block into the compatible values, and make both boards use that. ?ie. ?"sirf,intc-1.32". ?*however* doing so requires that the hardware folks actually do a good job of documenting the core versions so that the compatible value you use accurately describes the hardware.

It is also completely valid for a newer IP core to claim compatibility with an older one in the .dts:

? ? ? ?ie: compatible = "sirf,intc-1.32", "sirf,intc-1.00";

Personally, I prefer anchoring compatible values to the specific hardware implmentation rather than the IP core version.

Quoted 16 lines

Quoted 16 lines

in my understanding, dts for special soc/board contains special compatible for the chip, drivers shared by multi-soc/board contain all compatible lists which can be supported by them?

what's the generic way for this?

Quoted 8 lines

Also, for every new compatible value make sure you add documentation for it to Documentation/devicetree/bindings. (You may have already done so, it's just something I remind people about a lot).

These comments also apply through the rest of the file.

but it seems we will still add "sirf,alsta6-intc" in intc driver if that really happen in the future. in my understanding, drivers should explicitly claim all chips or ip cores they can support.