ARM: add infra-structure for BCM2708/BCM2835 and Raspberry Pi

From: Simon Arlott <[email protected]>

The BCM2708 contains a custom interrupt controller, which supports 72 interrupt sources using a 2-level register scheme. The interrupt controller, or the HW block containing it, is referred to occasionally as "armctrl" in the SoC documentation, hence the symbol naming in the code.

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Added devicetree documentation, and hence removed list of IRQs from bcm2835.dtsi.
* Changed shift in MAKE_HWIRQ() and HWIRQ_BANK() from 8 to 5 to reduce the size of the hwirq space, and pass the total size of the hwirq space to irq_domain_add_linear(), rather than just the number of valid hwirqs; the two are different due to the hwirq space being sparse.

* Added the interrupt controller DT node to the top-level of the DT, rather than nesting it inside a /axi node. Hence, changed the reg value since /axi had a ranges property. This seems simpler to me, but I'm not sure if everyone will like this change or not.

TODO: Should of_address_to_resource(), ioremap(), request_mem_region() in armctrl_of_init() be collapsed into of_iomap(). This wouldn't request the region, but a quick grep implies that's quite common with DT.

arch/arm/mach-bcm2708/irq.c new file +12 -0

+asmlinkage void __exception_irq_entry bcm2708_armctrl_handle_irq(
+	struct pt_regs *regs)
+{
+	u32 stat, irq;
+
+	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
+		if (stat & BANK0_HWIRQ_MASK) {
+			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+		} else if (stat & SHORTCUT1_MASK) {
+			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
+		} else if (stat & SHORTCUT2_MASK) {

Quoted 11 lines · Stephen Warren

From: Simon Arlott <[email protected]>

The BCM2708 contains a custom interrupt controller, which supports 72 interrupt sources using a 2-level register scheme. The interrupt controller, or the HW block containing it, is referred to occasionally as "armctrl" in the SoC documentation, hence the symbol naming in the code.

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

Quoted 6 lines

* Added devicetree documentation, and hence removed list of IRQs from bcm2835.dtsi.
* Changed shift in MAKE_HWIRQ() and HWIRQ_BANK() from 8 to 5 to reduce the size of the hwirq space, and pass the total size of the hwirq space to irq_domain_add_linear(), rather than just the number of valid hwirqs; the two are different due to the hwirq space being sparse.

If the IRQ space is very sparse, isn't it better to use a tree domain rather than a linear one?

Quoted 4 lines

* Added the interrupt controller DT node to the top-level of the DT, rather than nesting it inside a /axi node. Hence, changed the reg value since /axi had a ranges property. This seems simpler to me, but I'm not sure if everyone will like this change or not.

The layout should follow what the hardware looks like. If the interrupt controller is connected through axi, then I'd suggest describing it there unless there is a strong reason not to. The interrupt-parent property of the root node can easily point anywhere.

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt line 6

Quoted 7 lines

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt new file +6 -0

@@ -0,0 +1,110 @@
+BCM2708 Top-Level ("ARMCTRL") Interrupt Controller
+
+The BCM2708 contains a custom top-level interrupt controller, which supports
+72 interrupt sources using a 2-level register scheme. The interrupt
+controller, or the HW block containing it, is referred to occasionally
+as "armctrl" in the SoC documentation, hence naming of this binding.

Do we actually know that BCM2708 has the same one, or could it be present just on bcm2835? It seem hard to find any information about bcm2708, so I don't feel too good about using that name in bindings.

Quoted 19 lines

arch/arm/mach-bcm2708/irq.c new file +19 -0

+asmlinkage void __exception_irq_entry bcm2708_armctrl_handle_irq(
+	struct pt_regs *regs)
+{
+	u32 stat, irq;
+
+	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
+		if (stat & BANK0_HWIRQ_MASK) {
+			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+		} else if (stat & SHORTCUT1_MASK) {
+			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
+		} else if (stat & SHORTCUT2_MASK) {
+			armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
+		} else if (stat & BANK1_HWIRQ) {
+			armctrl_handle_bank(1, regs);
+		} else if (stat & BANK2_HWIRQ) {
+			armctrl_handle_bank(2, regs);
+		} else {
+			BUG();

arch/arm/mach-bcm2708/irq.c line 132

Quoted 3 lines

arch/arm/mach-bcm2708/irq.c new file +3 -0

+		}
+	}
+}

I'm not sure if readl_relaxed() is appropriate here, or if you need readl(). If you have an MSI type interrupt signaling the completion of a DMA, you need to ensure ordering between the data transfer and the interrupt notification.

Quoted 2 lines · Arnd Bergmann

If the IRQ space is very sparse, isn't it better to use a tree domain rather than a linear one?

It's not very sparse. There are 3 banks, each containing up to 32 interrupts. However, the first bank actually only has 8 interrupts plus 2 cascade inputs (which are hidden inside the interrupt controller driver and so not exposed). So, it's more like there's one gap in the middle. I don't know much about the tree domain, but I figure it's probably not worth it.

Quoted 9 lines

Quoted 4 lines

* Added the interrupt controller DT node to the top-level of the DT, rather than nesting it inside a /axi node. Hence, changed the reg value since /axi had a ranges property. This seems simpler to me, but I'm not sure if everyone will like this change or not.

The layout should follow what the hardware looks like. If the interrupt controller is connected through axi, then I'd suggest describing it there unless there is a strong reason not to. The interrupt-parent property of the root node can easily point anywhere.

The problem is that there's no documentation of the actual bus structure. Simon's original patch placed all peripherals under a single top-level /axi bus/node, but the documentation mentions all of AXI, APB, and AHB in passing, but doesn't explicitly describe which peripherals are on which bus etc. I think I'd rather not represent the bus structure in the .dtsi file at all, rather than represent just part of the structure and hence be misleading.

Quoted 11 lines

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt line 6

Quoted 7 lines

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt new file +6 -0

@@ -0,0 +1,110 @@
+BCM2708 Top-Level ("ARMCTRL") Interrupt Controller
+
+The BCM2708 contains a custom top-level interrupt controller, which supports
+72 interrupt sources using a 2-level register scheme. The interrupt
+controller, or the HW block containing it, is referred to occasionally
+as "armctrl" in the SoC documentation, hence naming of this binding.

Do we actually know that BCM2708 has the same one, or could it be present just on bcm2835? It seem hard to find any information about bcm2708, so I don't feel too good about using that name in bindings.

I don't know anything at all about the BCM2708 really. Perhaps Dom at Broadcom can fill in some details?

A similar discussion was apparently held downstream, and IIRC the reported decision there was that BCM2708 was the "parent" of a family of SoCs, so they made all the DT stuff compatible with both 2708 and 2835. Given the lack of documentation, I'd be quite happy to rework all of this to say just BCM2835 instead, and drop any reference to BCM2708 at all. Should I just go ahead and do that?

Quoted 19 lines

Quoted 19 lines

arch/arm/mach-bcm2708/irq.c new file +19 -0

+asmlinkage void __exception_irq_entry bcm2708_armctrl_handle_irq(
+	struct pt_regs *regs)
+{
+	u32 stat, irq;
+
+	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
+		if (stat & BANK0_HWIRQ_MASK) {
+			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+		} else if (stat & SHORTCUT1_MASK) {
+			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
+		} else if (stat & SHORTCUT2_MASK) {
+			armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
+		} else if (stat & BANK1_HWIRQ) {
+			armctrl_handle_bank(1, regs);
+		} else if (stat & BANK2_HWIRQ) {
+			armctrl_handle_bank(2, regs);
+		} else {
+			BUG();

Quoted 8 lines

arch/arm/mach-bcm2708/irq.c line 132

Quoted 3 lines

arch/arm/mach-bcm2708/irq.c new file +3 -0

+		}
+	}
+}

I'm not sure if readl_relaxed() is appropriate here, or if you need readl(). If you have an MSI type interrupt signaling the completion of a DMA, you need to ensure ordering between the data transfer and the interrupt notification.

I did wonder about this. I suppose it would be safe to globally replace all readl/writel_relaxed with plain readl/writel, and fix this up later if we can justify it. Should I go ahead and do that?

Quoted 17 lines · Stephen Warren

Quoted 11 lines · Arnd Bergmann

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt line 6

Quoted 7 lines

Documentation/devicetree/bindings/arm/bcm2708/broadcom,bcm2708-armctrl-ic.txt new file +6 -0

@@ -0,0 +1,110 @@
+BCM2708 Top-Level ("ARMCTRL") Interrupt Controller
+
+The BCM2708 contains a custom top-level interrupt controller, which supports
+72 interrupt sources using a 2-level register scheme. The interrupt
+controller, or the HW block containing it, is referred to occasionally
+as "armctrl" in the SoC documentation, hence naming of this binding.

Do we actually know that BCM2708 has the same one, or could it be present just on bcm2835? It seem hard to find any information about bcm2708, so I don't feel too good about using that name in bindings.

I don't know anything at all about the BCM2708 really. Perhaps Dom at Broadcom can fill in some details?

Quoted 6 lines

A similar discussion was apparently held downstream, and IIRC the reported decision there was that BCM2708 was the "parent" of a family of SoCs, so they made all the DT stuff compatible with both 2708 and 2835. Given the lack of documentation, I'd be quite happy to rework all of this to say just BCM2835 instead, and drop any reference to BCM2708 at all. Should I just go ahead and do that?

That's probably safer, yes.

Quoted 28 lines

Quoted 27 lines

Quoted 12 lines

arch/arm/mach-bcm2708/irq.c new file +12 -0

+asmlinkage void __exception_irq_entry bcm2708_armctrl_handle_irq(
+	struct pt_regs *regs)
+{
+	u32 stat, irq;
+
+	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
+		if (stat & BANK0_HWIRQ_MASK) {
+			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+		} else if (stat & SHORTCUT1_MASK) {
+			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
+		} else if (stat & SHORTCUT2_MASK) {

arch/arm/mach-bcm2708/irq.c line 234

Quoted 10 lines

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

+			armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
+		} else if (stat & BANK1_HWIRQ) {
+			armctrl_handle_bank(1, regs);
+		} else if (stat & BANK2_HWIRQ) {
+			armctrl_handle_bank(2, regs);
+		} else {
+			BUG();
+		}
+	}
+}

I'm not sure if readl_relaxed() is appropriate here, or if you need readl(). If you have an MSI type interrupt signaling the completion of a DMA, you need to ensure ordering between the data transfer and the interrupt notification.

Quoted 3 lines

I did wonder about this. I suppose it would be safe to globally replace all readl/writel_relaxed with plain readl/writel, and fix this up later if we can justify it. Should I go ahead and do that?

The synchronizations can be a bit expensive, so in the interrupt controller driver it makes sense to use at least writel_relaxed, which should always be fine because you don't have to worry about outgoing DMAs.

From: Simon Arlott <[email protected]>

The System Timer peripheral provides four 32-bit timer channels and a single 64-bit free running counter. Each channel has an output compare register, which is compared against the 32 least significant bits of the free running counter values, and generates an interrupt.

Timer 3 is used as the Linux timer.

The BCM2708 also contains an SP804-based timer module. However, it apparently has significant differences from the standard SP804 IP block, and Broadcom's documentation recommends using the system timer instead.

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Moved struct sys_timer bcm2708_timer into time.c to encapsulate it more.
* Added DT binding docs.
* Moved to drivers/clocksource/. This looks like the desired location for such code now.
* Simplified bcm2708_time_init() to find one matching node and operate on it, rather than looping over all matching nodes. This seems more consistent with other clocksource code.
* Renamed struct bcm2708_timer.index to match_mask to better represent its purpose.
* s/printk(PR_INFO/pr_info(/

Signed-off-by Chris Boot <[email protected]>

Signed-off-by Simon Arlott <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Stephen Warren <[email protected]>

From: Simon Arlott <[email protected]>

This patch adds a minimal stub clock driver for the BCM2708. Its sole purpose is to allow the PL011 AMBA clk_get() API calls to provide something that looks enough like a clock that the driver probes and operates correctly.

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Modified .dev_id for UART clocks to match UART DT node names.

Signed-off-by Chris Boot <[email protected]>

Signed-off-by Simon Arlott <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Stephen Warren <[email protected]>

--
v3: New patch.

---
 arch/arm/mach-bcm2708/Makefile  |    1 +
 arch/arm/mach-bcm2708/bcm2708.c |    3 +++
 arch/arm/mach-bcm2708/clock.c   |   53 +++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-bcm2708/clock.h   |   24 ++++++++++++++++++
 4 files changed, 81 insertions(+)
 create mode 100644 arch/arm/mach-bcm2708/clock.c
 create mode 100644 arch/arm/mach-bcm2708/clock.h

From: Simon Arlott <[email protected]>

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Modified UART DT node to use a unit-address to differentiate UART node names, rather than using different base names "uart0" and "uart1". Note that UART 1 (the Broadcom "mini UART") is not yet present, but I'm naming the DT node in anticipation that it will be added.

Quoted 10 lines · Stephen Warren

From: Simon Arlott <[email protected]>

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Modified UART DT node to use a unit-address to differentiate UART node names, rather than using different base names "uart0" and "uart1". Note that UART 1 (the Broadcom "mini UART") is not yet present, but I'm naming the DT node in anticipation that it will be added.

Shouldn't the selection be done using an alias like this?

	aliases {
		serial0 = &/uart at 20201000;
	};

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · Stephen Warren

From: Simon Arlott <[email protected]>

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split as of 2012/09/08, and modified as follows:

* Modified UART DT node to use a unit-address to differentiate UART node names, rather than using different base names "uart0" and "uart1". Note that UART 1 (the Broadcom "mini UART") is not yet present, but I'm naming the DT node in anticipation that it will be added.

Quoted 5 lines

Shouldn't the selection be done using an alias like this?

      aliases {
              serial0 = &/uart at 20201000;
      };

The "differentiate" was more "ensure that node names in DT are unique" than anything to do with user-visible selection/identification.

I can certainly add the alias if you want, but I don't know what benefit it will have, since I've never used aliases before.

From: Simon Arlott <[email protected]>

The BCM2708 is an ARM SoC from Broadcom. It is the primary SoC in a series which contains the BCM2835 amongst other variants. This patch adds very basic support for this series of SoCs, under the BCM2708 name.

http://www.broadcom.com/products/BCM2835
http://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pd

Note that the documentation in the latter .pdf assumes the MMU setup that's used on the "VideoCore" companion processor, and does not document physical peripheral addresses. Subtract 0x5e000000 to obtain the physical addresses.

The BCM2835 SoC is used in the Raspberry Pi. This patch also adds a minimal device tree for this board; enough to see some very early kernel boot messages through earlyprintk. However, this patch does not yet provide a useful booting system.

http://www.raspberrypi.org/.

This patch was extracted from git://github.com/lp0/linux.git branch rpi-split from 3-4 months ago, and significantly stripped down and modified since.

Signed-off-by Chris Boot <[email protected]>

Signed-off-by Simon Arlott <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Dom Cobley <[email protected]>

Signed-off-by Stephen Warren <[email protected]>

From: Simon Arlott <[email protected]>

Documentation/devicetree/bindings/arm/bcm2708.txt new file +5 -0

new file mode 100644
index 0000000..49eec61
@@ -0,0 +1,15 @@
+Broadcom BCM2708 device tree bindings
+-------------------------------------------
+
+Boards with the BCM2708 SoC, or another SoC in the BCM2708 family, shall have
+the following properties:

Hi Stephen,

Quoted 18 lines · Stephen Warren

From: Simon Arlott <[email protected]>

Documentation/devicetree/bindings/arm/bcm2708.txt new file +10 -0

new file mode 100644
index 0000000..49eec61
@@ -0,0 +1,15 @@
+Broadcom BCM2708 device tree bindings
+-------------------------------------------
+
+Boards with the BCM2708 SoC, or another SoC in the BCM2708 family, shall have
+the following properties:
+
+Required root node property:
+
+compatible = "broadcom,bcm2708";
+

Documentation/devicetree/bindings/arm/bcm2708.txt line 15

Quoted 5 lines

Documentation/devicetree/bindings/arm/bcm2708.txt new file +5 -0

+Boards with BCM2835 SoC shall *additionally* have the following properties:
+
+Required root node property:
+
+compatible = "broadcom,bcm2835";

Please update also the vendor prefixes index at Documentation/devicetree/bindings/vendor-prefixes.txt.

BTW is it possible to change it to "bcm"?

Quoted 1 line

BTW is it possible to change it to "bcm"?

I believe standard practice is to use the stock ticker symbol if not using the full company name, so that'd be "brcm". Does anyone have an objection? I'll assume not and make the change...