[PATCH 0/5] ARM: Broadcom BCM4760 support

[PATCH 0/5] ARM: Broadcom BCM4760 support

[Domenico Andreoli]

Hi all,

  I've refreshed this patchset to add the Broadcom BCM4760 SoC. It's
the bare minimum required to boot to the console.

Given the amount of Broadcom SoCs merged or being reviewed (and their
crazy and inconsistent names), I've decided to drop the BCM476X naming
I used before and stick with BCM4760.

Previous spin collected acked-by from Olof and Arnd but some time passed
and I cannot assume they have nothing new to comment.

Please give another glance to this smaller and smaller minimum patchset.

Best regards,
Domenico

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcm476x-add-infrastructure.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130721/1ddfe7cb/attachment.ksh>

[PATCH 2/5] ARM: bcm4760: Add system timer

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcm476x-add-system-timer.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130721/af8ec12f/attachment.ksh>

[PATCH 3/5] ARM: bcm4760: Add ripple counter

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcm476x-add-ripple-counter.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130721/b6173d51/attachment.ksh>

[PATCH 4/5] ARM: bcm4760: Add stub clock driver

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcm476x-add-stub-clock-driver.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130721/df7477dd/attachment.ksh>

[PATCH 5/5] ARM: bcm4760: Add restart hook

[Domenico Andreoli]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcm476x-add-restart-hook.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130721/c8074cc3/attachment.ksh>

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Arnd Bergmann]

On Sunday 21 July 2013, Domenico Andreoli wrote:
> From: Domenico Andreoli <domenico.andreoli@linux.com>
> 
> Platform infrastructure for the Broadcom BCM4760 based ARM11 SoCs.
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>

Looks very nice overall, thanks for following up on this!

> +#define BCM4760_PERIPH_PHYS   0x00080000
> +#define BCM4760_PERIPH_VIRT   IOMEM(0xd0080000)
> +#define BCM4760_PERIPH_SIZE   SZ_512K
> +
> +static struct map_desc io_map __initdata = {
> +	.virtual = (unsigned long) BCM4760_PERIPH_VIRT,
> +	.pfn = __phys_to_pfn(BCM4760_PERIPH_PHYS),
> +	.length = BCM4760_PERIPH_SIZE,
> +	.type = MT_DEVICE,
> +};
> +
> +static void __init bcm4760_map_io(void)
> +{
> +	iotable_init(&io_map, 1);
> +}

I would hope expect a comment here explaining what those registers are and why
they are mapped early.

> +
> +#define BCM4760_CPUID0 IOMEM(0xd00b0ff0)
> +#define BCM4760_CPUID1 IOMEM(0xd00b0ff4)

Better make these

#define BCM4760_CPUID0 (BCM4760_PERIPH_VIRT + 0x30ff0)
#define BCM4760_CPUID1 (BCM4760_PERIPH_VIRT + 0x30ff4)

for clarity.

> +static void __init bcm4760_system_rev(void)
> +{
> +	u32 id0, id1;
> +
> +	id0 = readl_relaxed(BCM4760_CPUID0);
> +	id1 = readl_relaxed(BCM4760_CPUID1);
> +
> +	if (id0 >> 16 != 0xbcbc)
> +		system_rev = 0xbc4760a0;
> +	else
> +		system_rev = id0 << 8 | (id1 & 0xff);
> +}

Or even better, change this function to do:

	struct device_node *node = of_find_compatible_node(NULL, NULL, "brcm,whatever");
	void __iomem *regs = of_iomap(node, 0);
	u32 id0, id1;

	id0 = readl_relaxed(regs + BCM4760_CPUID0);
	id1 = readl_relaxed(regs + BCM4760_CPUID1);

	...

	iounmap(regs);
	of_node_put(node);


bonus points if you use soc_device_register();

What is the system_rev variable actually used for?

> +		uart0 at c0000 {
> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
> +			reg = <0xc0000 0x1000>;
> +			interrupt-parent = <&vic0>;
> +			interrupts = <14>;
> +		};
> +
> +		uart1 at c1000 {
> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
> +			reg = <0xc1000 0x1000>;
> +			interrupt-parent = <&vic0>;
> +			interrupts = <15>;
> +		};
> +
> +		uart2 at b2000 {
> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
> +			reg = <0xb2000 0x1000>;
> +			interrupt-parent = <&vic0>;
> +			interrupts = <16>;
> +		};
> +	};

Please change the names here to say "serial" rather than "uartX". The name should not
have an index in it (that's what the @address part is for) and there are conventions
for common devices.

If some of the serial ports are not connected on all boars, best mark them as
status="disabled"; here and only enable them in the board specific file.

> +config ARCH_BCM4760
> +	bool "Broadcom BCM4760 based SoCs (ARM11)" if ARCH_MULTI_V6
> +	select ARCH_WANT_OPTIONAL_GPIOLIB
> +	select ARM_AMBA
> +	select ARM_VIC
> +	select CLKDEV_LOOKUP
> +	select CLKSRC_OF
> +	select COMMON_CLK
> +	select CPU_V6
> +	select GENERIC_CLOCKEVENTS
> +	select GENERIC_IRQ_CHIP
> +	select MULTI_IRQ_HANDLER
> +	select NO_IOPORT
> +	select PINCTRL
> +	select PINMUX
> +	select SPARSE_IRQ
> +	select USE_OF

A lot of these are implied by ARCH_MULTIPLATFORM or ARCH_MULTI_V6 and can be removed
here. I don't think you should select 'PINCTRL and PINMUX' here, as long as the code
builds without them.

	Arnd

[PATCH 2/5] ARM: bcm4760: Add system timer

[Arnd Bergmann]

+Daniel Lezcano

On Sunday 21 July 2013, Domenico Andreoli wrote:

> Index: b/Documentation/devicetree/bindings/timer/brcm,bcm4760-system-timer.txt
> ===================================================================
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/brcm,bcm4760-system-timer.txt
> @@ -0,0 +1,23 @@
> +Broadcom BCM4760 System Timer device tree bindings
> +--------------------------------------------------
> +
> +The BCM4760 Timer peripheral provides either two or four 32-bit timer
> +channels. Three timer blocks are available at 0xba000, 0xbb000 and
> +0xd1000. The first two provide four channels, the last (in the AON -
> +Always ON power domain) provides only two.
> +
> +Required properties:
> +
> +- compatible : should be "brcm,bcm4760-system-timer"
> +- reg : Specifies base physical address and size of the registers.
> +- interrupts : A list of 2 or 4 interrupt sinks; one per timer channel.
> +- clock-frequency : The frequency of the clock that drives the counter, in Hz.

I think the current consensus is that if you have a clock driver (as added in
patch 4), you should use a 'clocks' reference and clk_get_rate() to find the
frequency rather than an explicit clock-frequency property.

I have no comments on the driver, the rest is quoted so Daniel can find it
more easily.

	Arnd

> +Example:
> +
> +timer at ba000 {
> +	compatible = "brcm,bcm4760-system-timer";
> +	reg = <0xba000 0x1000>;
> +	interrupts = <4>, <11>;
> +	clock-frequency = <24000000>;
> +};
> Index: b/arch/arm/boot/dts/bcm4760.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/bcm4760.dtsi
> +++ b/arch/arm/boot/dts/bcm4760.dtsi
> @@ -10,6 +10,14 @@
>  		#size-cells = <1>;
>  		ranges;
>  
> +		timer at ba000 {
> +			compatible = "brcm,bcm4760-system-timer";
> +			reg = <0xba000 0x1000>;
> +			interrupt-parent = <&vic0>;
> +			interrupts = <4>, <11>;
> +			clock-frequency = <24000000>;
> +		};
> +
>  		vic0: interrupt-controller at 80000 {
>  			compatible = "brcm,bcm4760-pl192", "arm,pl192-vic", "arm,primecell";
>  			reg = <0x80000 0x1000>;
> Index: b/drivers/clocksource/Makefile
> ===================================================================
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clk
>  obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
>  obj-$(CONFIG_ORION_TIMER)	+= time-orion.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
> +obj-$(CONFIG_ARCH_BCM4760)	+= bcm4760_timer.o
>  obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
>  obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
>  obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
> Index: b/drivers/clocksource/bcm4760_timer.c
> ===================================================================
> --- /dev/null
> +++ b/drivers/clocksource/bcm4760_timer.c
> @@ -0,0 +1,170 @@
> +/*
> + * Broadcom BCM4760 based ARM11 SoCs system timer
> + *
> + * Copyright (C) 2012 Domenico Andreoli <domenico.andreoli@linux.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/clockchips.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +
> +#define TIMER_LOAD_OFFSET          0x00          /* load */
> +#define TIMER_VALUE_OFFSET         0x04          /* value */
> +#define TIMER_CONTROL_OFFSET       0x08          /* control */
> +#define TIMER_INTCLR_OFFSET        0x0c          /* interrupt clear */
> +#define TIMER_RIS_OFFSET           0x10          /* raw interrupt */
> +#define TIMER_MIS_OFFSET           0x14          /* masked interrupt status */
> +#define TIMER_BGLOAD_OFFSET        0x18          /* background load */
> +
> +#define TIMER_CTRL_ONESHOTMODE     BIT(0)        /* One shot mode */
> +#define TIMER_CTRL_32BIT           BIT(1)        /* 32-bit counter mode */
> +#define TIMER_CTRL_IE              BIT(5)        /* Interrupt enable */
> +#define TIMER_CTRL_PERIODIC        BIT(6)        /* Periodic mode */
> +#define TIMER_CTRL_EN              BIT(7)        /* Timer enable */
> +#define TIMER_CTRL_CLK2            BIT(9)        /* Clock 2 selected */
> +#define TIMER_CTRL_PREBY16         (1 << 2)      /* prescale divide by 16 */
> +#define TIMER_CTRL_PREBY256        (2 << 2)      /* prescale divide by 256 */
> +
> +struct bcm4760_timer {
> +	void __iomem *base;
> +	struct clock_event_device evt;
> +	struct irqaction act;
> +};
> +
> +static inline void __iomem *to_load(struct bcm4760_timer *timer)
> +{
> +	return timer->base + TIMER_LOAD_OFFSET;
> +}
> +
> +static inline void __iomem *to_control(struct bcm4760_timer *timer)
> +{
> +	return timer->base + TIMER_CONTROL_OFFSET;
> +}
> +
> +static inline void __iomem *to_intclr(struct bcm4760_timer *timer)
> +{
> +	return timer->base + TIMER_INTCLR_OFFSET;
> +}
> +
> +static inline void __iomem *to_ris(struct bcm4760_timer *timer)
> +{
> +	return timer->base + TIMER_RIS_OFFSET;
> +}
> +
> +static inline void __iomem *to_mis(struct bcm4760_timer *timer)
> +{
> +	return timer->base + TIMER_MIS_OFFSET;
> +}
> +
> +static void bcm4760_timer_set_mode(enum clock_event_mode mode,
> +	struct clock_event_device *evt_dev)
> +{
> +	struct bcm4760_timer *timer;
> +	u32 val;
> +
> +	timer = container_of(evt_dev, struct bcm4760_timer, evt);
> +	val = TIMER_CTRL_CLK2 | TIMER_CTRL_32BIT |
> +	      TIMER_CTRL_IE | TIMER_CTRL_EN;
> +
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		writel(val | TIMER_CTRL_ONESHOTMODE, to_control(timer));
> +		break;
> +	case CLOCK_EVT_MODE_RESUME:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +		break;
> +	default:
> +		WARN(1, "%s: unhandled event mode %d\n", __func__, mode);
> +		break;
> +	}
> +}
> +
> +static int bcm4760_timer_set_next_event(unsigned long event,
> +	struct clock_event_device *evt_dev)
> +{
> +	struct bcm4760_timer *timer;
> +
> +	timer = container_of(evt_dev, struct bcm4760_timer, evt);
> +	writel(event, to_load(timer));
> +	return 0;
> +}
> +
> +static irqreturn_t bcm4760_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct bcm4760_timer *timer = dev_id;
> +	void (*event_handler)(struct clock_event_device *);
> +
> +	/* check the (masked) interrupt status */
> +	if (!readl_relaxed(to_mis(timer)))
> +		return IRQ_NONE;
> +
> +	/* clear the timer interrupt */
> +	writel_relaxed(1, to_intclr(timer));
> +
> +	event_handler = ACCESS_ONCE(timer->evt.event_handler);
> +	if (event_handler)
> +		event_handler(&timer->evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void __init bcm4760_init_time(struct device_node *node)
> +{
> +	void __iomem *base;
> +	u32 freq;
> +	int irq;
> +	struct bcm4760_timer *timer;
> +
> +	base = of_iomap(node, 0);
> +	if (!base)
> +		panic("Can't remap timer registers");
> +
> +	if (of_property_read_u32(node, "clock-frequency", &freq))
> +		panic("Can't read timer frequency");
> +
> +	/* TODO allow other frequences by using pre-scaling parameters */
> +	if (freq != 24000000)
> +		panic("Invalid timer frequency");
> +
> +	timer = kzalloc(sizeof(*timer), GFP_KERNEL);
> +	if (!timer)
> +		panic("Can't allocate timer struct\n");
> +
> +	irq = irq_of_parse_and_map(node, 0);
> +	if (irq <= 0)
> +		panic("Can't parse timer IRQ");
> +
> +	timer->base = base;
> +	timer->evt.name = node->name;
> +	timer->evt.rating = 300;
> +	timer->evt.features = CLOCK_EVT_FEAT_ONESHOT;
> +	timer->evt.set_mode = bcm4760_timer_set_mode;
> +	timer->evt.set_next_event = bcm4760_timer_set_next_event;
> +	timer->evt.cpumask = cpumask_of(0);
> +	timer->act.name = node->name;
> +	timer->act.flags = IRQF_TIMER | IRQF_SHARED;
> +	timer->act.dev_id = timer;
> +	timer->act.handler = bcm4760_timer_interrupt;
> +
> +	if (setup_irq(irq, &timer->act))
> +		panic("Can't set up timer IRQ\n");
> +
> +	clockevents_config_and_register(&timer->evt, freq, 0xf, 0xffffffff);
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(bcm4760, "brcm,bcm4760-system-timer", bcm4760_init_time);
> 
> 

[PATCH 4/5] ARM: bcm4760: Add stub clock driver

[Arnd Bergmann]

On Sunday 21 July 2013, Domenico Andreoli wrote:
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk/bcm4760.h>
> +
> +void __init bcm4760_init_clocks(void)
> +{
> +       struct clk *clk;
> +
> +       clk = clk_register_fixed_rate(NULL, "pclk", NULL, CLK_IS_ROOT, 78000000);
> +       if (IS_ERR(clk))
> +               pr_err("pclk not registered\n");
> +
> +       if (clk_register_clkdev(clk, NULL, "c0000.uart0"))
> +               pr_err("uart0 clk alias not registered\n");
> +       if (clk_register_clkdev(clk, NULL, "c1000.uart1"))
> +               pr_err("uart1 clk alias not registered\n");
> +       if (clk_register_clkdev(clk, NULL, "b2000.uart2"))
> +               pr_err("uart2 clk alias not registered\n");
> +}


Since these are all fixed-rate clocks, I think what you should do here is to
put them into the device tree as compatible="fixed-clock" and remove the code
here.

	Arnd

[PATCH 5/5] ARM: bcm4760: Add restart hook

[Arnd Bergmann]

On Sunday 21 July 2013, Domenico Andreoli wrote:
> From: Domenico Andreoli <domenico.andreoli@linux.com>
> 
> Restart hook implementation for the Broadcom BCM4760 based ARM11 SoCs.
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> ---
>  Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt |   14 ++
>  arch/arm/boot/dts/bcm4760.dtsi                                     |    5 +
>  arch/arm/mach-bcm/bcm4760.c                                        |   78 ++++++++++
>  3 files changed, 97 insertions(+)

The implementation looks ok to me, but I would prefer this to be part of a
watchdog device driver that overrides the pm_power_off hook.

It can probably be done at a later stage when you submit a watchdog driver.

	Arnd

> Index: b/Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt
> ===================================================================
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt
> @@ -0,0 +1,14 @@
> +Broadcom BCM4760 watchdog timer device tree bindings
> +----------------------------------------------------
> +
> +Required properties:
> +
> +- compatible : should be "brcm,bcm4760-pm-wdt"
> +- reg : Specifies base physical address and size of the registers.
> +
> +Example:
> +
> +watchdog {
> +	compatible = "brcm,bcm4760-pm-wdt";
> +	reg = <0xbd000 0x1000>;
> +};
> Index: b/arch/arm/boot/dts/bcm4760.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/bcm4760.dtsi
> +++ b/arch/arm/boot/dts/bcm4760.dtsi
> @@ -23,6 +23,11 @@
>  			reg = <0xbc000 0x1000>;
>  		};
>  
> +		watchdog {
> +			compatible = "brcm,bcm4760-pm-wdt";
> +			reg = <0xbd000 0x1000>;
> +		};
> +
>  		vic0: interrupt-controller at 80000 {
>  			compatible = "brcm,bcm4760-pl192", "arm,pl192-vic", "arm,primecell";
>  			reg = <0x80000 0x1000>;
> Index: b/arch/arm/mach-bcm/bcm4760.c
> ===================================================================
> --- a/arch/arm/mach-bcm/bcm4760.c
> +++ b/arch/arm/mach-bcm/bcm4760.c
> @@ -16,7 +16,9 @@
>  
>  #include <linux/init.h>
>  #include <linux/irqchip.h>
> +#include <linux/delay.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  #include <linux/clk/bcm4760.h>
>  
> @@ -28,6 +30,17 @@
>  #define BCM4760_PERIPH_VIRT   IOMEM(0xd0080000)
>  #define BCM4760_PERIPH_SIZE   SZ_512K
>  
> +#define BCM4760_WDT_LOAD      0x000
> +#define BCM4760_WDT_CTRL      0x008
> +#define BCM4760_WDT_INTCLR    0x00c
> +#define BCM4760_WDT_LOCK      0xc00
> +
> +#define BCM4760_WDT_PASSWORD  0x1acce551
> +#define BCM4760_WDT_INTEN     BIT(0)
> +#define BCM4760_WDT_RESEN     BIT(1)
> +
> +static void __iomem *wdt_regs;
> +
>  static struct map_desc io_map __initdata = {
>  	.virtual = (unsigned long) BCM4760_PERIPH_VIRT,
>  	.pfn = __phys_to_pfn(BCM4760_PERIPH_PHYS),
> @@ -56,11 +69,75 @@ static void __init bcm4760_system_rev(vo
>  		system_rev = id0 << 8 | (id1 & 0xff);
>  }
>  
> +static const struct of_device_id bcm4760_pm_wdt_match[] __initconst = {
> +	{ .compatible = "brcm,bcm4760-pm-wdt" },
> +	{}
> +};
> +
> +/*
> + * The machine restart method can be called from an atomic context so we won't
> + * be able to ioremap the regs then.
> + */
> +static void __init bcm4760_setup_restart(void)
> +{
> +	struct device_node *node;
> +
> +	node = of_find_matching_node(NULL, bcm4760_pm_wdt_match);
> +	if (!node) {
> +		pr_info("No bcm4760 watchdog node\n");
> +		return;
> +	}
> +
> +	wdt_regs = of_iomap(node, 0);
> +	if (!wdt_regs) {
> +		pr_err("Can't remap watchdog registers\n");
> +		return;
> +	}
> +
> +	/* unlock watchdog registers */
> +	writel(BCM4760_WDT_PASSWORD, wdt_regs + BCM4760_WDT_LOCK);
> +	/* disable watchdog */
> +	writel(0, wdt_regs + BCM4760_WDT_CTRL);
> +	/* lock watchdog registers */
> +	writel(1, wdt_regs + BCM4760_WDT_LOCK);
> +}
> +
> +static void bcm4760_restart(enum reboot_mode mode, const char *cmd)
> +{
> +	if (!wdt_regs) {
> +		pr_err("No restart hook installed. ");
> +		return;
> +	}
> +
> +	/* unlock watchdog registers */
> +	writel(BCM4760_WDT_PASSWORD, wdt_regs + BCM4760_WDT_LOCK);
> +
> +	/* disable watchdog */
> +	writel(0, wdt_regs + BCM4760_WDT_CTRL);
> +	udelay(20);
> +
> +	/* clear the irq status */
> +	writel(1, wdt_regs + BCM4760_WDT_INTCLR);
> +	udelay(20);
> +
> +	/* expire after 5 cycles (~156us) */
> +	writel(5, wdt_regs + BCM4760_WDT_LOAD);
> +	/* enable watchdog */
> +	writel(BCM4760_WDT_INTEN | BCM4760_WDT_RESEN,
> +		wdt_regs + BCM4760_WDT_CTRL);
> +
> +	/* lock watchdog registers */
> +	writel(1, wdt_regs + BCM4760_WDT_LOCK);
> +	/* wait the bite */
> +	udelay(400);
> +}
> +
>  static void __init bcm4760_init(void)
>  {
>  	int err;
>  
>  	bcm4760_system_rev();
> +	bcm4760_setup_restart();
>  	bcm4760_init_clocks();
>  
>  	err = of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> @@ -78,5 +155,6 @@ static const char * const bcm4760_compat
>  DT_MACHINE_START(BCM4760, "Broadcom BCM4760")
>  	.map_io = bcm4760_map_io,
>  	.init_machine = bcm4760_init,
> +	.restart = bcm4760_restart,
>  	.dt_compat = bcm4760_compat
>  MACHINE_END
> 
> 

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Domenico Andreoli]

Arnd Bergmann <arnd@arndb.de> wrote:
>On Sunday 21 July 2013, Domenico Andreoli wrote:
>> From: Domenico Andreoli <domenico.andreoli@linux.com>
>> 
>> Platform infrastructure for the Broadcom BCM4760 based ARM11 SoCs.
>> 
>> Cc: linux-arm-kernel at lists.infradead.org
>> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
>
>Looks very nice overall, thanks for following up on this!

Thank you for reviewing again.

>
>> +#define BCM4760_PERIPH_PHYS   0x00080000
>> +#define BCM4760_PERIPH_VIRT   IOMEM(0xd0080000)
>> +#define BCM4760_PERIPH_SIZE   SZ_512K
>> +
>> +static struct map_desc io_map __initdata = {
>> +	.virtual = (unsigned long) BCM4760_PERIPH_VIRT,
>> +	.pfn = __phys_to_pfn(BCM4760_PERIPH_PHYS),
>> +	.length = BCM4760_PERIPH_SIZE,
>> +	.type = MT_DEVICE,
>> +};
>> +
>> +static void __init bcm4760_map_io(void)
>> +{
>> +	iotable_init(&io_map, 1);
>> +}
>
>I would hope expect a comment here explaining what those registers are
>and why
>they are mapped early.

Without these the board doesn't boot so I must be doing something
wrong somewhere else. I'll investigate.

>
>> +
>> +#define BCM4760_CPUID0 IOMEM(0xd00b0ff0)
>> +#define BCM4760_CPUID1 IOMEM(0xd00b0ff4)
>
>Better make these
>
>#define BCM4760_CPUID0 (BCM4760_PERIPH_VIRT + 0x30ff0)
>#define BCM4760_CPUID1 (BCM4760_PERIPH_VIRT + 0x30ff4)
>
>for clarity.
>
>> +static void __init bcm4760_system_rev(void)
>> +{
>> +	u32 id0, id1;
>> +
>> +	id0 = readl_relaxed(BCM4760_CPUID0);
>> +	id1 = readl_relaxed(BCM4760_CPUID1);
>> +
>> +	if (id0 >> 16 != 0xbcbc)
>> +		system_rev = 0xbc4760a0;
>> +	else
>> +		system_rev = id0 << 8 | (id1 & 0xff);
>> +}
>
>Or even better, change this function to do:
>
>	struct device_node *node = of_find_compatible_node(NULL, NULL,
>"brcm,whatever");
>	void __iomem *regs = of_iomap(node, 0);
>	u32 id0, id1;
>
>	id0 = readl_relaxed(regs + BCM4760_CPUID0);
>	id1 = readl_relaxed(regs + BCM4760_CPUID1);
>
>	...
>
>	iounmap(regs);
>	of_node_put(node);
>
>
>bonus points if you use soc_device_register();
>
>What is the system_rev variable actually used for?

I use it to show something sensible in /proc/cpuinfo, to report
whether the soc is  a 4760 or 4761 and the silicon revision.

If the proper way to deal with this has so much boilerplate as
you say, I think I'll drop it altogether. I'd like to print the info
somewhere in the bootlog anyway.

btw this could well be one of the accesses requiring the io
mapping above.


>> +		uart0 at c0000 {
>> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
>> +			reg = <0xc0000 0x1000>;
>> +			interrupt-parent = <&vic0>;
>> +			interrupts = <14>;
>> +		};
>> +
>> +		uart1 at c1000 {
>> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
>> +			reg = <0xc1000 0x1000>;
>> +			interrupt-parent = <&vic0>;
>> +			interrupts = <15>;
>> +		};
>> +
>> +		uart2 at b2000 {
>> +			compatible = "brcm,bcm4760-pl011", "arm,pl011", "arm,primecell";
>> +			reg = <0xb2000 0x1000>;
>> +			interrupt-parent = <&vic0>;
>> +			interrupts = <16>;
>> +		};
>> +	};
>
>Please change the names here to say "serial" rather than "uartX". The
>name should not
>have an index in it (that's what the @address part is for) and there
>are conventions
>for common devices.

ok, will fix

>If some of the serial ports are not connected on all boars, best mark
>them as
>status="disabled"; here and only enable them in the board specific
>file.

uhm..  interesting

>> +config ARCH_BCM4760
>> +	bool "Broadcom BCM4760 based SoCs (ARM11)" if ARCH_MULTI_V6
>> +	select ARCH_WANT_OPTIONAL_GPIOLIB
>> +	select ARM_AMBA
>> +	select ARM_VIC
>> +	select CLKDEV_LOOKUP
>> +	select CLKSRC_OF
>> +	select COMMON_CLK
>> +	select CPU_V6
>> +	select GENERIC_CLOCKEVENTS
>> +	select GENERIC_IRQ_CHIP
>> +	select MULTI_IRQ_HANDLER
>> +	select NO_IOPORT
>> +	select PINCTRL
>> +	select PINMUX
>> +	select SPARSE_IRQ
>> +	select USE_OF
>
>A lot of these are implied by ARCH_MULTIPLATFORM or ARCH_MULTI_V6 and
>can be removed
>here. I don't think you should select 'PINCTRL and PINMUX' here, as
>long as the code
>builds without them.
>
>	Arnd
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Arnd Bergmann]

On Sunday 21 July 2013, Domenico Andreoli wrote:
> >bonus points if you use soc_device_register();
> >
> >What is the system_rev variable actually used for?
> 
> I use it to show something sensible in /proc/cpuinfo, to report
> whether the soc is  a 4760 or 4761 and the silicon revision.
> 
> If the proper way to deal with this has so much boilerplate as
> you say, I think I'll drop it altogether. I'd like to print the info
> somewhere in the bootlog anyway.
> 
> btw this could well be one of the accesses requiring the io
> mapping above.

If it's just for informational purposes, soc_device_register() sounds like
the correct place to put that information, you can register a couple
of strings there that get put into sysfs at a known location for the
SoC, and keeps the cpuinfo for the actual CPU core.

	Arnd

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Domenico Andreoli]

+ Christian Daudt  (Christian, you've not an entry in MAINTAINERS)

On Sun, Jul 21, 2013 at 02:23:21AM +0200, Domenico Andreoli wrote:
> From: Domenico Andreoli <domenico.andreoli@linux.com>
> 
> Platform infrastructure for the Broadcom BCM4760 based ARM11 SoCs.
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> ---
> Index: b/arch/arm/Makefile
> ===================================================================
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -147,6 +147,7 @@ textofs-$(CONFIG_ARCH_MSM8960) := 0x0020
>  machine-$(CONFIG_ARCH_AT91)		+= at91
>  machine-$(CONFIG_ARCH_BCM)		+= bcm
>  machine-$(CONFIG_ARCH_BCM2835)		+= bcm2835
> +machine-$(CONFIG_ARCH_BCM4760)		+= bcm
>  machine-$(CONFIG_ARCH_CLPS711X)		+= clps711x
>  machine-$(CONFIG_ARCH_CNS3XXX)		+= cns3xxx
>  machine-$(CONFIG_ARCH_DAVINCI)		+= davinci

I've noticed that enabling both CONFIG_ARCH_BCM and CONFIG_ARCH_BCM4760
makes the linker complain of duplicate symbols.

Shouldn't the mach-bcm subdir be included always and let then Makefile
therein sort what to build?

As I side note, I think this CONFIG_ARCH_BCM option is misnamed, shouldn't be
something like CONFIG_ARCH_BCM281XX? It looks about this ARMv7 family only.

Thanks,
Domenico

[PATCH 5/5] ARM: bcm4760: Add restart hook

[Domenico Andreoli]

On Sun, Jul 21, 2013 at 10:20:25AM +0200, Arnd Bergmann wrote:
> On Sunday 21 July 2013, Domenico Andreoli wrote:
> > From: Domenico Andreoli <domenico.andreoli@linux.com>
> > 
> > Restart hook implementation for the Broadcom BCM4760 based ARM11 SoCs.
> > 
> > Cc: linux-arm-kernel at lists.infradead.org
> > Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> > ---
> >  Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt |   14 ++
> >  arch/arm/boot/dts/bcm4760.dtsi                                     |    5 +
> >  arch/arm/mach-bcm/bcm4760.c                                        |   78 ++++++++++
> >  3 files changed, 97 insertions(+)
> 
> The implementation looks ok to me, but I would prefer this to be part of a
> watchdog device driver that overrides the pm_power_off hook.

I'd appreciate if I could keep this implementation, which I don't plan to
develop further, but won't cry if it's left out.

> It can probably be done at a later stage when you submit a watchdog driver.

I should be able to reuse an existing ARM watchdog driver but I need to
investigate further.

BTW is it acceptable to loose the ability to reboot the board in case the
proper watchdog driver is not configured?

Domenico

> 
> 	Arnd
> 
> > Index: b/Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt
> > ===================================================================
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm4760-pm-wdt.txt
> > @@ -0,0 +1,14 @@
> > +Broadcom BCM4760 watchdog timer device tree bindings
> > +----------------------------------------------------
> > +
> > +Required properties:
> > +
> > +- compatible : should be "brcm,bcm4760-pm-wdt"
> > +- reg : Specifies base physical address and size of the registers.
> > +
> > +Example:
> > +
> > +watchdog {
> > +	compatible = "brcm,bcm4760-pm-wdt";
> > +	reg = <0xbd000 0x1000>;
> > +};
> > Index: b/arch/arm/boot/dts/bcm4760.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/bcm4760.dtsi
> > +++ b/arch/arm/boot/dts/bcm4760.dtsi
> > @@ -23,6 +23,11 @@
> >  			reg = <0xbc000 0x1000>;
> >  		};
> >  
> > +		watchdog {
> > +			compatible = "brcm,bcm4760-pm-wdt";
> > +			reg = <0xbd000 0x1000>;
> > +		};
> > +
> >  		vic0: interrupt-controller at 80000 {
> >  			compatible = "brcm,bcm4760-pl192", "arm,pl192-vic", "arm,primecell";
> >  			reg = <0x80000 0x1000>;
> > Index: b/arch/arm/mach-bcm/bcm4760.c
> > ===================================================================
> > --- a/arch/arm/mach-bcm/bcm4760.c
> > +++ b/arch/arm/mach-bcm/bcm4760.c
> > @@ -16,7 +16,9 @@
> >  
> >  #include <linux/init.h>
> >  #include <linux/irqchip.h>
> > +#include <linux/delay.h>
> >  #include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/clk/bcm4760.h>
> >  
> > @@ -28,6 +30,17 @@
> >  #define BCM4760_PERIPH_VIRT   IOMEM(0xd0080000)
> >  #define BCM4760_PERIPH_SIZE   SZ_512K
> >  
> > +#define BCM4760_WDT_LOAD      0x000
> > +#define BCM4760_WDT_CTRL      0x008
> > +#define BCM4760_WDT_INTCLR    0x00c
> > +#define BCM4760_WDT_LOCK      0xc00
> > +
> > +#define BCM4760_WDT_PASSWORD  0x1acce551
> > +#define BCM4760_WDT_INTEN     BIT(0)
> > +#define BCM4760_WDT_RESEN     BIT(1)
> > +
> > +static void __iomem *wdt_regs;
> > +
> >  static struct map_desc io_map __initdata = {
> >  	.virtual = (unsigned long) BCM4760_PERIPH_VIRT,
> >  	.pfn = __phys_to_pfn(BCM4760_PERIPH_PHYS),
> > @@ -56,11 +69,75 @@ static void __init bcm4760_system_rev(vo
> >  		system_rev = id0 << 8 | (id1 & 0xff);
> >  }
> >  
> > +static const struct of_device_id bcm4760_pm_wdt_match[] __initconst = {
> > +	{ .compatible = "brcm,bcm4760-pm-wdt" },
> > +	{}
> > +};
> > +
> > +/*
> > + * The machine restart method can be called from an atomic context so we won't
> > + * be able to ioremap the regs then.
> > + */
> > +static void __init bcm4760_setup_restart(void)
> > +{
> > +	struct device_node *node;
> > +
> > +	node = of_find_matching_node(NULL, bcm4760_pm_wdt_match);
> > +	if (!node) {
> > +		pr_info("No bcm4760 watchdog node\n");
> > +		return;
> > +	}
> > +
> > +	wdt_regs = of_iomap(node, 0);
> > +	if (!wdt_regs) {
> > +		pr_err("Can't remap watchdog registers\n");
> > +		return;
> > +	}
> > +
> > +	/* unlock watchdog registers */
> > +	writel(BCM4760_WDT_PASSWORD, wdt_regs + BCM4760_WDT_LOCK);
> > +	/* disable watchdog */
> > +	writel(0, wdt_regs + BCM4760_WDT_CTRL);
> > +	/* lock watchdog registers */
> > +	writel(1, wdt_regs + BCM4760_WDT_LOCK);
> > +}
> > +
> > +static void bcm4760_restart(enum reboot_mode mode, const char *cmd)
> > +{
> > +	if (!wdt_regs) {
> > +		pr_err("No restart hook installed. ");
> > +		return;
> > +	}
> > +
> > +	/* unlock watchdog registers */
> > +	writel(BCM4760_WDT_PASSWORD, wdt_regs + BCM4760_WDT_LOCK);
> > +
> > +	/* disable watchdog */
> > +	writel(0, wdt_regs + BCM4760_WDT_CTRL);
> > +	udelay(20);
> > +
> > +	/* clear the irq status */
> > +	writel(1, wdt_regs + BCM4760_WDT_INTCLR);
> > +	udelay(20);
> > +
> > +	/* expire after 5 cycles (~156us) */
> > +	writel(5, wdt_regs + BCM4760_WDT_LOAD);
> > +	/* enable watchdog */
> > +	writel(BCM4760_WDT_INTEN | BCM4760_WDT_RESEN,
> > +		wdt_regs + BCM4760_WDT_CTRL);
> > +
> > +	/* lock watchdog registers */
> > +	writel(1, wdt_regs + BCM4760_WDT_LOCK);
> > +	/* wait the bite */
> > +	udelay(400);
> > +}
> > +
> >  static void __init bcm4760_init(void)
> >  {
> >  	int err;
> >  
> >  	bcm4760_system_rev();
> > +	bcm4760_setup_restart();
> >  	bcm4760_init_clocks();
> >  
> >  	err = of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > @@ -78,5 +155,6 @@ static const char * const bcm4760_compat
> >  DT_MACHINE_START(BCM4760, "Broadcom BCM4760")
> >  	.map_io = bcm4760_map_io,
> >  	.init_machine = bcm4760_init,
> > +	.restart = bcm4760_restart,
> >  	.dt_compat = bcm4760_compat
> >  MACHINE_END
> > 
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] ARM: bcm4760: Add system timer

[Domenico Andreoli]

On Sun, Jul 21, 2013 at 10:14:20AM +0200, Arnd Bergmann wrote:
> +Daniel Lezcano
> 
> On Sunday 21 July 2013, Domenico Andreoli wrote:
> 
> > Index: b/Documentation/devicetree/bindings/timer/brcm,bcm4760-system-timer.txt
> > ===================================================================
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/brcm,bcm4760-system-timer.txt
> > @@ -0,0 +1,23 @@
> > +Broadcom BCM4760 System Timer device tree bindings
> > +--------------------------------------------------
> > +
> > +The BCM4760 Timer peripheral provides either two or four 32-bit timer
> > +channels. Three timer blocks are available at 0xba000, 0xbb000 and
> > +0xd1000. The first two provide four channels, the last (in the AON -
> > +Always ON power domain) provides only two.
> > +
> > +Required properties:
> > +
> > +- compatible : should be "brcm,bcm4760-system-timer"
> > +- reg : Specifies base physical address and size of the registers.
> > +- interrupts : A list of 2 or 4 interrupt sinks; one per timer channel.
> > +- clock-frequency : The frequency of the clock that drives the counter, in Hz.
> 
> I think the current consensus is that if you have a clock driver (as added in
> patch 4), you should use a 'clocks' reference and clk_get_rate() to find the
> frequency rather than an explicit clock-frequency property.

This frequency can be either 32KHz or 24MHz (currently only 24MHz is
accepted) and is selected flipping bit TIMER_CTRL_CLK2 in the timer's control
register. It depends somehow on the pclk but I don't know the details.

I'm not sure a fake fixed-rate clock is needed here. Do you still think
I should add it? 

Domenico

> 
> I have no comments on the driver, the rest is quoted so Daniel can find it
> more easily.
> 
> 	Arnd
> 
> > +Example:
> > +
> > +timer at ba000 {
> > +	compatible = "brcm,bcm4760-system-timer";
> > +	reg = <0xba000 0x1000>;
> > +	interrupts = <4>, <11>;
> > +	clock-frequency = <24000000>;
> > +};
> > Index: b/arch/arm/boot/dts/bcm4760.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/bcm4760.dtsi
> > +++ b/arch/arm/boot/dts/bcm4760.dtsi
> > @@ -10,6 +10,14 @@
> >  		#size-cells = <1>;
> >  		ranges;
> >  
> > +		timer at ba000 {
> > +			compatible = "brcm,bcm4760-system-timer";
> > +			reg = <0xba000 0x1000>;
> > +			interrupt-parent = <&vic0>;
> > +			interrupts = <4>, <11>;
> > +			clock-frequency = <24000000>;
> > +		};
> > +
> >  		vic0: interrupt-controller at 80000 {
> >  			compatible = "brcm,bcm4760-pl192", "arm,pl192-vic", "arm,primecell";
> >  			reg = <0x80000 0x1000>;
> > Index: b/drivers/clocksource/Makefile
> > ===================================================================
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clk
> >  obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
> >  obj-$(CONFIG_ORION_TIMER)	+= time-orion.o
> >  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
> > +obj-$(CONFIG_ARCH_BCM4760)	+= bcm4760_timer.o
> >  obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
> >  obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
> >  obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
> > Index: b/drivers/clocksource/bcm4760_timer.c
> > ===================================================================
> > --- /dev/null
> > +++ b/drivers/clocksource/bcm4760_timer.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * Broadcom BCM4760 based ARM11 SoCs system timer
> > + *
> > + * Copyright (C) 2012 Domenico Andreoli <domenico.andreoli@linux.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +
> > +#define TIMER_LOAD_OFFSET          0x00          /* load */
> > +#define TIMER_VALUE_OFFSET         0x04          /* value */
> > +#define TIMER_CONTROL_OFFSET       0x08          /* control */
> > +#define TIMER_INTCLR_OFFSET        0x0c          /* interrupt clear */
> > +#define TIMER_RIS_OFFSET           0x10          /* raw interrupt */
> > +#define TIMER_MIS_OFFSET           0x14          /* masked interrupt status */
> > +#define TIMER_BGLOAD_OFFSET        0x18          /* background load */
> > +
> > +#define TIMER_CTRL_ONESHOTMODE     BIT(0)        /* One shot mode */
> > +#define TIMER_CTRL_32BIT           BIT(1)        /* 32-bit counter mode */
> > +#define TIMER_CTRL_IE              BIT(5)        /* Interrupt enable */
> > +#define TIMER_CTRL_PERIODIC        BIT(6)        /* Periodic mode */
> > +#define TIMER_CTRL_EN              BIT(7)        /* Timer enable */
> > +#define TIMER_CTRL_CLK2            BIT(9)        /* Clock 2 selected */
> > +#define TIMER_CTRL_PREBY16         (1 << 2)      /* prescale divide by 16 */
> > +#define TIMER_CTRL_PREBY256        (2 << 2)      /* prescale divide by 256 */
> > +
> > +struct bcm4760_timer {
> > +	void __iomem *base;
> > +	struct clock_event_device evt;
> > +	struct irqaction act;
> > +};
> > +
> > +static inline void __iomem *to_load(struct bcm4760_timer *timer)
> > +{
> > +	return timer->base + TIMER_LOAD_OFFSET;
> > +}
> > +
> > +static inline void __iomem *to_control(struct bcm4760_timer *timer)
> > +{
> > +	return timer->base + TIMER_CONTROL_OFFSET;
> > +}
> > +
> > +static inline void __iomem *to_intclr(struct bcm4760_timer *timer)
> > +{
> > +	return timer->base + TIMER_INTCLR_OFFSET;
> > +}
> > +
> > +static inline void __iomem *to_ris(struct bcm4760_timer *timer)
> > +{
> > +	return timer->base + TIMER_RIS_OFFSET;
> > +}
> > +
> > +static inline void __iomem *to_mis(struct bcm4760_timer *timer)
> > +{
> > +	return timer->base + TIMER_MIS_OFFSET;
> > +}
> > +
> > +static void bcm4760_timer_set_mode(enum clock_event_mode mode,
> > +	struct clock_event_device *evt_dev)
> > +{
> > +	struct bcm4760_timer *timer;
> > +	u32 val;
> > +
> > +	timer = container_of(evt_dev, struct bcm4760_timer, evt);
> > +	val = TIMER_CTRL_CLK2 | TIMER_CTRL_32BIT |
> > +	      TIMER_CTRL_IE | TIMER_CTRL_EN;
> > +
> > +	switch (mode) {
> > +	case CLOCK_EVT_MODE_ONESHOT:
> > +		writel(val | TIMER_CTRL_ONESHOTMODE, to_control(timer));
> > +		break;
> > +	case CLOCK_EVT_MODE_RESUME:
> > +	case CLOCK_EVT_MODE_SHUTDOWN:
> > +		break;
> > +	default:
> > +		WARN(1, "%s: unhandled event mode %d\n", __func__, mode);
> > +		break;
> > +	}
> > +}
> > +
> > +static int bcm4760_timer_set_next_event(unsigned long event,
> > +	struct clock_event_device *evt_dev)
> > +{
> > +	struct bcm4760_timer *timer;
> > +
> > +	timer = container_of(evt_dev, struct bcm4760_timer, evt);
> > +	writel(event, to_load(timer));
> > +	return 0;
> > +}
> > +
> > +static irqreturn_t bcm4760_timer_interrupt(int irq, void *dev_id)
> > +{
> > +	struct bcm4760_timer *timer = dev_id;
> > +	void (*event_handler)(struct clock_event_device *);
> > +
> > +	/* check the (masked) interrupt status */
> > +	if (!readl_relaxed(to_mis(timer)))
> > +		return IRQ_NONE;
> > +
> > +	/* clear the timer interrupt */
> > +	writel_relaxed(1, to_intclr(timer));
> > +
> > +	event_handler = ACCESS_ONCE(timer->evt.event_handler);
> > +	if (event_handler)
> > +		event_handler(&timer->evt);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void __init bcm4760_init_time(struct device_node *node)
> > +{
> > +	void __iomem *base;
> > +	u32 freq;
> > +	int irq;
> > +	struct bcm4760_timer *timer;
> > +
> > +	base = of_iomap(node, 0);
> > +	if (!base)
> > +		panic("Can't remap timer registers");
> > +
> > +	if (of_property_read_u32(node, "clock-frequency", &freq))
> > +		panic("Can't read timer frequency");
> > +
> > +	/* TODO allow other frequences by using pre-scaling parameters */
> > +	if (freq != 24000000)
> > +		panic("Invalid timer frequency");
> > +
> > +	timer = kzalloc(sizeof(*timer), GFP_KERNEL);
> > +	if (!timer)
> > +		panic("Can't allocate timer struct\n");
> > +
> > +	irq = irq_of_parse_and_map(node, 0);
> > +	if (irq <= 0)
> > +		panic("Can't parse timer IRQ");
> > +
> > +	timer->base = base;
> > +	timer->evt.name = node->name;
> > +	timer->evt.rating = 300;
> > +	timer->evt.features = CLOCK_EVT_FEAT_ONESHOT;
> > +	timer->evt.set_mode = bcm4760_timer_set_mode;
> > +	timer->evt.set_next_event = bcm4760_timer_set_next_event;
> > +	timer->evt.cpumask = cpumask_of(0);
> > +	timer->act.name = node->name;
> > +	timer->act.flags = IRQF_TIMER | IRQF_SHARED;
> > +	timer->act.dev_id = timer;
> > +	timer->act.handler = bcm4760_timer_interrupt;
> > +
> > +	if (setup_irq(irq, &timer->act))
> > +		panic("Can't set up timer IRQ\n");
> > +
> > +	clockevents_config_and_register(&timer->evt, freq, 0xf, 0xffffffff);
> > +}
> > +
> > +CLOCKSOURCE_OF_DECLARE(bcm4760, "brcm,bcm4760-system-timer", bcm4760_init_time);
> > 
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] ARM: bcm4760: Add platform infrastructure

[Domenico Andreoli]

On Sun, Jul 21, 2013 at 02:00:21PM +0200, Arnd Bergmann wrote:
> On Sunday 21 July 2013, Domenico Andreoli wrote:
> > >bonus points if you use soc_device_register();
> > >
> > >What is the system_rev variable actually used for?
> > 
> > I use it to show something sensible in /proc/cpuinfo, to report
> > whether the soc is  a 4760 or 4761 and the silicon revision.
> > 
> > If the proper way to deal with this has so much boilerplate as
> > you say, I think I'll drop it altogether. I'd like to print the info
> > somewhere in the bootlog anyway.
> > 
> > btw this could well be one of the accesses requiring the io
> > mapping above.
> 
> If it's just for informational purposes, soc_device_register() sounds like
> the correct place to put that information, you can register a couple
> of strings there that get put into sysfs at a known location for the
> SoC, and keeps the cpuinfo for the actual CPU core.
> 
> 	Arnd

I've implemented the registration using soc_device_register() but I've
postponed to a later stage, these registers are in block I need to add
later also for the clocks.

This showed to be also the only user of the early io mapping, which is
now gone.

I'll respin the patchset as soon as you suggest what to do with the
clock-frequency of the system timer.

BRs,
Domenico