Patchwork [v2] ARM: CSR: Adding CSR SiRFprimaII board support

login
register
mail settings
Submitter Barry Song
Date July 1, 2011, 6:20 a.m.
Message ID <BANLkTikoMu3ccUxj5KRmKzdReQcBqK4Y9Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/102871/
State New
Headers show

Comments

Barry Song - July 1, 2011, 6:20 a.m.
Hi Arnd,

2011/6/30 Arnd Bergmann <arnd@arndb.de>:
> On Thursday 30 June 2011, Barry Song wrote:
>
>> > 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 :-)
>
> 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.

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 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@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"?
>
> 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.
>
>> > 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:

+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:

diff --git a/arch/arm/mach-prima2/clock.c b/arch/arm/mach-prima2/clock.c
new file mode 100644
index 0000000..f4676bc
--- /dev/null
+++ b/arch/arm/mach-prima2/clock.c
...
+
+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:
+#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:

>
>> 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.
>>
> Yes. Rob/Olof, what's the status of that patch?
>
>        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
Arnd Bergmann - July 1, 2011, 4:19 p.m.
On Friday 01 July 2011, Barry Song wrote:
> 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); 

> For DEBUG_LL uart, i have moved the static mapping to a new file named
> lluart.c too:
> +#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:

Ok, sounds good.

> +/* 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]);
}


> +#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

Very nice!

	Arnd
Russell King - ARM Linux - July 2, 2011, 12:25 p.m.
On Fri, Jul 01, 2011 at 06:19:43PM +0200, Arnd Bergmann wrote:
> On Friday 01 July 2011, Barry Song wrote:
> > 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/*/.

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.
Arnd Bergmann - July 2, 2011, 7:34 p.m.
On Saturday 02 July 2011 14:25:27 Russell King - ARM Linux wrote:
> On Fri, Jul 01, 2011 at 06:19:43PM +0200, Arnd Bergmann wrote:
> > On Friday 01 July 2011, Barry Song wrote:
> > > 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/*/.
> 
> 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.

I didn't mean converting ARM to use fixmap, but having a common prototype
for the function to do an early mapping of devices, regardless if it's
based on fixmap or iotable.

We already discussed the issue of large TLBs for iotable before. If
we ever want to conver that to the fixmap API, we would first have
to extend it with a more straightforward way to do large mappings,
and I'm not going to start that discussion.

I actually didn't know that we had fixmap support on ARM until I looked
now. In fact there are now at least three interfaces that would let you
establish early mappings:

* iotable (ARM only)
* fixmap (some architectures, unused on ARM except for kmap_atomic)
* early_ioremap (x86 only, built on top of fixmap)

Maybe we could implement early_ioremap on ARM on top of iotable,
with support for large pages, and build of_io_earlymap() as suggested
by Barry on top of that?

	Arnd
Barry Song - July 4, 2011, 2:55 a.m.
2011/7/2 Arnd Bergmann <arnd@arndb.de>:
> On Friday 01 July 2011, Barry Song wrote:
>> 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);
>
>> For DEBUG_LL uart, i have moved the static mapping to a new file named
>> lluart.c too:
>> +#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:
>
> Ok, sounds good.
>
>> +/* 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]);
> }

Great idea. in fact there is only one reset bit for every device. So
maybe the rstc.c can be:
/*
 * 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/mutex.h>
#include <linux/io.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/of.h>
#include <linux/of_address.h>

void __iomem *sirfsoc_rstc_base;
static DEFINE_MUTEX(rstc_lock);

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);

int sirfsoc_reset_device(struct device *dev)
{
	const unsigned int *prop = of_get_property(dev->of_node, "reset-bit", NULL);
	unsigned int reset_bit;

	if (!prop)
		return -ENODEV;

	reset_bit = be32_to_cpup(prop);

	mutex_lock(&rstc_lock);

	/*
	 * 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);

	mutex_unlock(&rstc_lock);

	return 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
>> +};
>> +
>> +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
Arnd Bergmann - July 4, 2011, 2:53 p.m.
On Monday 04 July 2011, Barry Song wrote:
> Great idea. in fact there is only one reset bit for every device. So
> maybe the rstc.c can be
> ...

Yes, looks good.

>         /*
>          * 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
Barry Song - July 5, 2011, 1:32 a.m.
2011/7/4 Arnd Bergmann <arnd@arndb.de>:
> On Monday 04 July 2011, Barry Song wrote:
>> Great idea. in fact there is only one reset bit for every device. So
>> maybe the rstc.c can be
>> ...
>
> Yes, looks good.
>
>>         /*
>>          * 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?

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...

>
>        Arnd
>
Barry Song - July 5, 2011, 8:34 a.m.
Hi Arnd,

2011/7/1 Barry Song <21cnbao@gmail.com>:
> Hi Arnd,
>
> 2011/6/30 Arnd Bergmann <arnd@arndb.de>:
>> On Thursday 30 June 2011, Barry Song wrote:
>>
>>> > 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 :-)
>>
>> 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.
>
> 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@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@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@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@0x88000000 {
					compatible = "sirf,clkc", "sirf,prima2-clkc";
					reg = <0x88000000 0x1000>;
					interrupts = <3>;
				};

				reset-controller@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@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@0x90010000 {
					compatible = "sirf,lcd", "sirf,prima2-lcd";
					reg = <0x90010000 0x20000>;
					interrupts = <30>;
				};

				vpp@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@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@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@0xa8000000 {
					compatible = "sirf,dspif", "sirf,prima2-dspif";
					reg = <0xa8000000 0x10000>;
					interrupts = <9>;
				};

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

				dsp@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@0xb0020000 {
					compatible = "sirf,tick", "sirf,prima2-tick";
					reg = <0xb0020000 0x1000>;
					interrupts = <0>;
				};

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

				pulsec@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@0x56000000 {
						cell-index = <0>;
						compatible = "sirf,sdhc", "sirf,prima2-sdhc";
						reg = <0x56000000 0x100000>;
						interrupts = <38>;
					};

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

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

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

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

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

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

					rom-interface@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@0x1000 {
					compatible = "sirf,gpsrtc", "sirf,prima2-gpsrtc";
					reg = <0x1000 0x1000>;
					interrupts = <55 56 57>;
				};

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

				pwrc@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@0xb00E0000 {
					compatible = "sirf,usb", "sirf,prima2-usb";
					reg = <0xb8000000 0x10000>;
					interrupts = <10>;
				};

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

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

				security@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.

>
>>
>>> > 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@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"?
>>
>> 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.
>>
>>> > 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:
>
> +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:
>
> diff --git a/arch/arm/mach-prima2/clock.c b/arch/arm/mach-prima2/clock.c
> new file mode 100644
> index 0000000..f4676bc
> --- /dev/null
> +++ b/arch/arm/mach-prima2/clock.c
> ...
> +
> +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:
> +#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:
> --- /dev/null
> +++ b/arch/arm/mach-prima2/common.h
> @@ -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.
> + */
> +
> +#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:
> +/*
> + * 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
> + */
>
>>
>>
>> Right. Note that you have a := in there, which needs to be +=.
>>
>>
>>> > 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
>>
>> 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:
>
> diff --git a/arch/arm/mach-prima2/prima2.c b/arch/arm/mach-prima2/prima2.c
> new file mode 100644
> index 0000000..f6b04a1
> --- /dev/null
> +++ b/arch/arm/mach-prima2/prima2.c
> @@ -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
>
>>
>>> 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.
>>>
>> Yes. Rob/Olof, what's the status of that patch?
>>
>>        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
Arnd Bergmann - July 5, 2011, 11:10 a.m.
On Tuesday 05 July 2011, Barry Song wrote:
> > 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...

Ok, fair enough. Maybe add a comment then to provide an explanation
why you are waiting for exactly 10ms. This will show up when people
try to optimize boot time, so it's good to know for the reader if it
can be made shorter or not.

	Arnd
Barry Song - July 6, 2011, 2:10 a.m.
2011/7/5 Barry Song <21cnbao@gmail.com>:
> Hi Arnd,
>
> 2011/7/1 Barry Song <21cnbao@gmail.com>:
>> Hi Arnd,
>>
>> 2011/6/30 Arnd Bergmann <arnd@arndb.de>:
>>> On Thursday 30 June 2011, Barry Song wrote:
>>>
>>>> > 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 :-)
>>>
>>> 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.
>>
>> 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@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@0x80040000 {
>                        compatible = "arm,pl310-cache", "cache";
>                        reg = <0x80040000 0x1000>;
>                        interrupts = <59>;
>                };

sorry, here i made an mistake. l2-cache is located in AXI, but
l2-cache registers <0x80040000 0x1000> are in sirfsoc-iobus, so it can
be moved to below in the same level with interrupt-controller.

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

And the ranges can be  <0x40000000 0x40000000 0x80000000> directly. in
fact,  except that memory is located in a single AXI, all registers in
all controllers are in sirfsoc-iobus transferred from another AXI.

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

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

>                        sys-iobg {
>                                compatible = "simple-bus";
>                                #address-cells = <1>;
>                                #size-cells = <1>;
>                                ranges = <0x88000000 0x88000000 0x40000>;
>
>                                clock-controller@0x88000000 {
>                                        compatible = "sirf,clkc", "sirf,prima2-clkc";
>                                        reg = <0x88000000 0x1000>;
>                                        interrupts = <3>;
>                                };
>
>                                reset-controller@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@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@0x90010000 {
>                                        compatible = "sirf,lcd", "sirf,prima2-lcd";
>                                        reg = <0x90010000 0x20000>;
>                                        interrupts = <30>;
>                                };
>
>                                vpp@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@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@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@0xa8000000 {
>                                        compatible = "sirf,dspif", "sirf,prima2-dspif";
>                                        reg = <0xa8000000 0x10000>;
>                                        interrupts = <9>;
>                                };
>
>                                gps@0xa8010000 {
>                                        compatible = "sirf,gps", "sirf,prima2-gps";
>                                        reg = <0xa8010000 0x10000>;
>                                        interrupts = <7>;
>                                };
>
>                                dsp@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@0xb0020000 {
>                                        compatible = "sirf,tick", "sirf,prima2-tick";
>                                        reg = <0xb0020000 0x1000>;
>                                        interrupts = <0>;
>                                };
>
>                                nand@0xb0030000 {
>                                        compatible = "sirf,nand", "sirf,prima2-nand";
>                                        reg = <0xb0030000 0x10000>;
>                                        interrupts = <41>;
>                                };
>
>                                audio@0xb0040000 {
>                                        compatible = "sirf,audio", "sirf,prima2-audio";
>                                        reg = <0xb0040000 0x10000>;
>                                        interrupts = <35>;
>                                };
>
>                                uart0: uart@0xb0050000 {
>                                        cell-index = <0>;
>                                        compatible = "sirf,uart", "sirf,prima2-uart";
>                                        reg = <0xb0050000 0x10000>;
>                                        interrupts = <17>;
>                                };
>
>                                uart1: uart@0xb0060000 {
>                                        cell-index = <1>;
>                                        compatible = "sirf,uart", "sirf,prima2-uart";
>                                        reg = <0xb0060000 0x10000>;
>                                        interrupts = <18>;
>                                };
>
>                                uart2: uart@0xb0070000 {
>                                        cell-index = <2>;
>                                        compatible = "sirf,uart", "sirf,prima2-uart";
>                                        reg = <0xb0070000 0x10000>;
>                                        interrupts = <19>;
>                                };
>
>                                usp0: usp@0xb0080000 {
>                                        cell-index = <0>;
>                                        compatible = "sirf,usp", "sirf,prima2-usp";
>                                        reg = <0xb0080000 0x10000>;
>                                        interrupts = <20>;
>                                };
>
>                                usp1: usp@0xb0090000 {
>                                        cell-index = <1>;
>                                        compatible = "sirf,usp", "sirf,prima2-usp";
>                                        reg = <0xb0090000 0x10000>;
>                                        interrupts = <21>;
>                                };
>
>                                usp2: usp@0xb00a0000 {
>                                        cell-index = <2>;
>                                        compatible = "sirf,usp", "sirf,prima2-usp";
>                                        reg = <0xb00a0000 0x10000>;
>                                        interrupts = <22>;
>                                };
>
>                                dmac0: dma-controller@0xb00b0000 {
>                                        cell-index = <0>;
>                                        compatible = "sirf,dmac", "sirf,prima2-dmac";
>                                        reg = <0xb00b0000 0x10000>;
>                                        interrupts = <12>;
>                                };
>
>                                dmac1: dma-controller@0xb0160000 {
>                                        cell-index = <1>;
>                                        compatible = "sirf,dmac", "sirf,prima2-dmac";
>                                        reg = <0xb0160000 0x10000>;
>                                        interrupts = <13>;
>                                };
>
>                                vip@0xb00C0000 {
>                                        compatible = "sirf,vip", "sirf,prima2-vip";
>                                        reg = <0xb00C0000 0x10000>;
>                                };
>
>                                spi0: spi@0xb00D0000 {
>                                        cell-index = <0>;
>                                        compatible = "sirf,spi", "sirf,prima2-spi";
>                                        reg = <0xb00D0000 0x10000>;
>                                        interrupts = <15>;
>                                };
>
>                                spi1: spi@0xb0170000 {
>                                        cell-index = <1>;
>                                        compatible = "sirf,spi", "sirf,prima2-spi";
>                                        reg = <0xb0170000 0x10000>;
>                                        interrupts = <16>;
>                                };
>
>                                i2c0: i2c@0xb00E0000 {
>                                        cell-index = <0>;
>                                        compatible = "sirf,i2c", "sirf,prima2-i2c";
>                                        reg = <0xb00E0000 0x10000>;
>                                        interrupts = <24>;
>                                };
>
>                                i2c1: i2c@0xb00f0000 {
>                                        cell-index = <1>;
>                                        compatible = "sirf,i2c", "sirf,prima2-i2c";
>                                        reg = <0xb00f0000 0x10000>;
>                                        interrupts = <25>;
>                                };
>
>                                tsc@0xb0110000 {
>                                        compatible = "sirf,tsc", "sirf,prima2-tsc";
>                                        reg = <0xb0110000 0x10000>;
>                                        interrupts = <33>;
>                                };
>
>                                gpio: gpio-controller@0xb0120000 {
>                                        #gpio-cells = <2>;
>                                        #interrupt-cells = <2>;
>                                        compatible = "sirf,gpio", "sirf,prima2-gpio";
>                                        reg = <0xb0120000 0x10000>;
>                                        gpio-controller;
>                                        interrupt-controller;
>                                };
>
>                                pwm@0xb0130000 {
>                                        compatible = "sirf,pwm", "sirf,prima2-pwm";
>                                        reg = <0xb0130000 0x10000>;
>                                };
>
>                                efusesys@0xb0140000 {
>                                        compatible = "sirf,efuse", "sirf,prima2-efuse";
>                                        reg = <0xb0140000 0x10000>;
>                                };
>
>                                pulsec@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@0x56000000 {
>                                                cell-index = <0>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56000000 0x100000>;
>                                                interrupts = <38>;
>                                        };
>
>                                        sd1: sdhci@0x56100000 {
>                                                cell-index = <1>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56100000 0x100000>;
>                                                interrupts = <38>;
>                                        };
>
>                                        sd2: sdhci@0x56200000 {
>                                                cell-index = <2>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56200000 0x100000>;
>                                                interrupts = <23>;
>                                        };
>
>                                        sd3: sdhci@0x56300000 {
>                                                cell-index = <3>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56300000 0x100000>;
>                                                interrupts = <23>;
>                                        };
>
>                                        sd4: sdhci@0x56400000 {
>                                                cell-index = <4>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56400000 0x100000>;
>                                                interrupts = <39>;
>                                        };
>
>                                        sd5: sdhci@0x56500000 {
>                                                cell-index = <5>;
>                                                compatible = "sirf,sdhc", "sirf,prima2-sdhc";
>                                                reg = <0x56500000 0x100000>;
>                                                interrupts = <39>;
>                                        };
>
>                                        pci-copy@0x57900000 {
>                                                compatible = "sirf,pcicp", "sirf,prima2-pcicp";
>                                                reg = <0x57900000 0x100000>;
>                                                interrupts = <40>;
>                                        };
>
>                                        rom-interface@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@0x1000 {
>                                        compatible = "sirf,gpsrtc", "sirf,prima2-gpsrtc";
>                                        reg = <0x1000 0x1000>;
>                                        interrupts = <55 56 57>;
>                                };
>
>                                sysrtc@0x2000 {
>                                        compatible = "sirf,sysrtc", "sirf,prima2-sysrtc";
>                                        reg = <0x2000 0x1000>;
>                                        interrupts = <52 53 54>;
>                                };
>
>                                pwrc@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@0xb00E0000 {
>                                        compatible = "sirf,usb", "sirf,prima2-usb";
>                                        reg = <0xb8000000 0x10000>;
>                                        interrupts = <10>;
>                                };
>
>                                usb1: usb@0xb00f0000 {
>                                        compatible = "sirf,usb", "sirf,prima2-usb";
>                                        reg = <0xb8010000 0x10000>;
>                                        interrupts = <11>;
>                                };
>
>                                sata@0xb00f0000 {
>                                        compatible = "sirf,sata", "sirf,prima2-sata";
>                                        reg = <0xb8020000 0x10000>;
>                                        interrupts = <37>;
>                                };
>
>                                security@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.
>
>>
>>>
>>>> > 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@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"?
>>>
>>> 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.
>>>
>>>> > 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:
>>
>> +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:
>>
>> diff --git a/arch/arm/mach-prima2/clock.c b/arch/arm/mach-prima2/clock.c
>> new file mode 100644
>> index 0000000..f4676bc
>> --- /dev/null
>> +++ b/arch/arm/mach-prima2/clock.c
>> ...
>> +
>> +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:
>> +#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:
>> --- /dev/null
>> +++ b/arch/arm/mach-prima2/common.h
>> @@ -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.
>> + */
>> +
>> +#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:
>> +/*
>> + * 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
>> + */
>>
>>>
>>>
>>> Right. Note that you have a := in there, which needs to be +=.
>>>
>>>
>>>> > 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
>>>
>>> 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:
>>
>> diff --git a/arch/arm/mach-prima2/prima2.c b/arch/arm/mach-prima2/prima2.c
>> new file mode 100644
>> index 0000000..f6b04a1
>> --- /dev/null
>> +++ b/arch/arm/mach-prima2/prima2.c
>> @@ -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
>>
>>>
>>>> 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.
>>>>
>>> Yes. Rob/Olof, what's the status of that patch?
>>>
>>>        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
>
Grant Likely - July 6, 2011, 5:30 a.m.
On Tue, Jul 05, 2011 at 04:34:05PM +0800, Barry Song wrote:
> Hi Arnd,
> 
> 2011/7/1 Barry Song <21cnbao@gmail.com>:
> > Hi Arnd,
> >
> > 2011/6/30 Arnd Bergmann <arnd@arndb.de>:
> >> On Thursday 30 June 2011, Barry Song wrote:
> >>
> >>> > 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 :-)
> >>
> >> 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.
> >
> > 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 {

memory@0

> 		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@0 {

cpu@0 {

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

> 			device_type = "cpu";

Drop device_type here.

> 			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@0x80040000 {

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

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

Drop the "cache" compatible value here.

> 			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@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.
Barry Song - July 6, 2011, 5:58 a.m.
2011/7/6 Grant Likely <grant.likely@secretlab.ca>:
> On Tue, Jul 05, 2011 at 04:34:05PM +0800, Barry Song wrote:
>> Hi Arnd,
>>
>> 2011/7/1 Barry Song <21cnbao@gmail.com>:
>> > Hi Arnd,
>> >
>> > 2011/6/30 Arnd Bergmann <arnd@arndb.de>:
>> >> On Thursday 30 June 2011, Barry Song wrote:
>> >>
>> >>> > 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 :-)
>> >>
>> >> 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.
>> >
>> > 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 {
>
> memory@0
>
>>               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@0 {
>
> cpu@0 {
>
> Don't duplicate the crack that powerpc did for cpu node names.  Use a
> compatible property to identify the specific CPU model.
>
>>                       device_type = "cpu";
>
> Drop device_type here.
>
>>                       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@0x80040000 {
>
> l2-cache-controller@80040000  (Drop the '0x' from node names; address
> in the node name is always hex.
>
>>                       compatible = "arm,pl310-cache", "cache";
>
> Drop the "cache" compatible value here.
>
>>                       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@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.

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?

>
> 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.
>
>
Barry Song - July 6, 2011, 6:01 a.m.
2011/7/6 Barry Song <21cnbao@gmail.com>:
> 2011/7/6 Grant Likely <grant.likely@secretlab.ca>:
>> On Tue, Jul 05, 2011 at 04:34:05PM +0800, Barry Song wrote:
>>> Hi Arnd,
>>>
>>> 2011/7/1 Barry Song <21cnbao@gmail.com>:
>>> > Hi Arnd,
>>> >
>>> > 2011/6/30 Arnd Bergmann <arnd@arndb.de>:
>>> >> On Thursday 30 June 2011, Barry Song wrote:
>>> >>
>>> >>> > 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 :-)
>>> >>
>>> >> 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.
>>> >
>>> > 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 {
>>
>> memory@0
>>
>>>               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@0 {
>>
>> cpu@0 {
>>
>> Don't duplicate the crack that powerpc did for cpu node names.  Use a
>> compatible property to identify the specific CPU model.
>>
>>>                       device_type = "cpu";
>>
>> Drop device_type here.
>>
>>>                       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@0x80040000 {
>>
>> l2-cache-controller@80040000  (Drop the '0x' from node names; address
>> in the node name is always hex.
>>
>>>                       compatible = "arm,pl310-cache", "cache";
>>
>> Drop the "cache" compatible value here.
>>
>>>                       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@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.
>
> 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";

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

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

>
> 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?
>
>>
>> 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.
>>
>>
>
Grant Likely - July 6, 2011, 6:28 a.m.
On Wed, Jul 06, 2011 at 02:01:24PM +0800, Barry Song wrote:
> 2011/7/6 Barry Song <21cnbao@gmail.com>:
> > 2011/7/6 Grant Likely <grant.likely@secretlab.ca>:
> >>>                       intc: interrupt-controller@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.
> >
> > 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";
> 
> > 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.

> >
> > 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?
> >
> >>
> >> 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.
> >>
> >>
> >
Barry Song - July 6, 2011, 7:03 a.m.
Hi Grant,
Thanks.

2011/7/6 Grant Likely <grant.likely@secretlab.ca>:
> On Wed, Jul 06, 2011 at 02:01:24PM +0800, Barry Song wrote:
>> 2011/7/6 Barry Song <21cnbao@gmail.com>:
>> > 2011/7/6 Grant Likely <grant.likely@secretlab.ca>:
>> >>>                       intc: interrupt-controller@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.
>> >
>> > 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";
>>
>> > 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.

it seems you mean if we only support prima2 for the moment:
prima2 .dts:    compatible = "sirf,prima2-intc"
intc driver:       compatible = "sirf,prima2-intc"

then after some time, we bring up atlas6 and find atlas6 can use
prima2 intc driver due to shared ip core, then we let alsta6 .dts
include:
compatible =  "sirf,alsta6-intc", "sirf,prima2-intc"
no matter we change compatible in intc driver to <"sirf,prima2-intc",
"sirf,alsta6-intc"> or not, the driver will always work for alsta6
even only with "sirf,prima2-intc" .

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.

anyway, both prima2.dts and drivers use "sirf,prima2-xxx" should be
the right way for the moment. "sirf, xxx" is too generic now. i will
drop all "sirf, xxx" in both drivers and dts.

>
> 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.
>
>> >
>> > 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?
>> >
>> >>
>> >> 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.
>> >>
>> >>
>> >
-barry
Arnd Bergmann - July 6, 2011, 7:40 a.m.
On Wednesday 06 July 2011 09:03:45 Barry Song wrote:
> 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.

I don't think there is a need for that. I'd only add the second
one if there is a need in the driver to tell the difference.
In that case, you can add a data field to the device_id and use
that for making decisions in the driver.

	Arnd

Patch

--- /dev/null
+++ b/arch/arm/mach-prima2/common.h
@@ -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.
+ */
+
+#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:
+/*
+ * 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
+ */

>
>
> Right. Note that you have a := in there, which needs to be +=.
>
>
>> > 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
>
> 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:

diff --git a/arch/arm/mach-prima2/prima2.c b/arch/arm/mach-prima2/prima2.c
new file mode 100644
index 0000000..f6b04a1
--- /dev/null
+++ b/arch/arm/mach-prima2/prima2.c
@@ -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