Add platform support for LSI AXM55xx

arch/arm/mach-axxia/Kconfig new file +4 -0

new file mode 100644
index 0000000..336426a
@@ -0,0 +1,19 @@
+config ARCH_AXXIA
+	bool "LSI Axxia platforms" if (ARCH_MULTI_V7 && ARM_LPAE)
+	select ARM_GIC
+	select HAVE_SMP

arch/arm/mach-axxia/axxia.c new file +60 -0

new file mode 100644
index 0000000..8d85926
@@ -0,0 +1,117 @@
+/*
+ * Support for the LSI Axxia SoC devices based on ARM cores.
+ *
+ * Copyright (C) 2012 LSI
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of_platform.h>
+#include <linux/sizes.h>
+#include <asm/mach/arch.h>
+#include <asm/pmu.h>
+#include "axxia.h"
+
+/*
+ * The PMU IRQ lines of four cores are wired together into a single interrupt.
+ * Bounce the interrupt to other cores if it's not ours.
+ */
+static irqreturn_t axxia_pmu_handler(int irq, void *dev, irq_handler_t handler)
+{
+	irqreturn_t ret = handler(irq, dev);
+
+	if (ret == IRQ_NONE) {
+		int cpu = smp_processor_id();
+		int cluster = cpu / CORES_PER_CLUSTER;
+		int other;
+
+		/* Look until we find another cpu that's online. */
+		do {
+			other = (++cpu % CORES_PER_CLUSTER) +
+				(cluster * CORES_PER_CLUSTER);
+		} while (!cpu_online(other));
+
+		irq_set_affinity(irq, cpumask_of(other));
+	}
+
+	/*
+	 * We should be able to get away with the amount of IRQ_NONEs we give,
+	 * while still having the spurious IRQ detection code kick in if the
+	 * interrupt really starts hitting spuriously.
+	 */
+	return ret;

arch/arm/mach-axxia/axxia.c new file +13 -0

+static int
+axxia_bus_notifier(struct notifier_block *nb, unsigned long event, void *obj)
+{
+	struct device *dev = obj;
+
+	if (event != BUS_NOTIFY_ADD_DEVICE)
+		return NOTIFY_DONE;
+
+	if (!of_property_read_bool(dev->of_node, "dma-coherent"))
+		return NOTIFY_DONE;
+
+	set_dma_ops(dev, &arm_coherent_dma_ops);
+

arch/arm/mach-axxia/axxia.h new file +14 -0

new file mode 100644
index 0000000..594dd97
@@ -0,0 +1,42 @@
+/*
+ * Prototypes for platform functions.
+ *
+ *
+ * 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.
+ *

arch/arm/mach-axxia/platsmp.c new file +0 -0

new file mode 100644
index 0000000..fd7f507

arch/arm/mach-axxia/platsmp.c new file +13 -0

+static int axxia_boot_secondary(unsigned int cpu,
+					  struct task_struct *idle)
+{
+	unsigned long timeout;
+
+	/* Release the specified core */
+	write_pen_release(cpu_logical_map(cpu));
+
+	/* Send a wakeup event to get the idled cpu out of WFE state */
+	dsb_sev();
+
+	/* Wait for so long, then give up if nothing happens ... */
+	timeout = jiffies + (1 * HZ);

Quoted 14 lines · On Tuesday 15 April 2014 14:06:10 Anders Berg wrote:

arch/arm/mach-axxia/Kconfig new file +8 -0

new file mode 100644
index 0000000..336426a
@@ -0,0 +1,19 @@
+config ARCH_AXXIA
+	bool "LSI Axxia platforms" if (ARCH_MULTI_V7 && ARM_LPAE)
+	select ARM_GIC
+	select HAVE_SMP
+	select MFD_SYSCON
+	select ARM_AMBA
+	select ARCH_WANT_OPTIONAL_GPIOLIB
+	select HAVE_ARM_ARCH_TIMER

arch/arm/mach-axxia/Kconfig line 14

Quoted 6 lines

arch/arm/mach-axxia/Kconfig new file +6 -0

+	select ARM_TIMER_SP804
+	select ZONE_DMA
+	select ARCH_DMA_ADDR_T_64BIT
+	select ARCH_SUPPORTS_BIG_ENDIAN
+	select MIGHT_HAVE_PCI
+	select PCI_DOMAINS if PCI

No need to select HAVE_SMP or ARCH_WANT_OPTIONAL_GPIOLIB any more.

We should rethink the ARCH_SUPPORTS_BIG_ENDIAN option I guess. It makes little sense to select that from one platform in a multiplatform build if other platforms don't support it.

arch/arm/mach-axxia/Makefile.boot line 2

Quoted 5 lines

arch/arm/mach-axxia/Makefile.boot new file +2 -0

@@ -0,0 +1,2 @@
+   zreladdr-y	+= 0x00308000
+params_phys-y	:= 0x00300100

This won't be used, since AUTO_ZRELADDR is set by ARCH_MULTIPLATFORM.

Quoted 67 lines

arch/arm/mach-axxia/axxia.c new file +60 -0

new file mode 100644
index 0000000..8d85926
@@ -0,0 +1,117 @@
+/*
+ * Support for the LSI Axxia SoC devices based on ARM cores.
+ *
+ * Copyright (C) 2012 LSI
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of_platform.h>
+#include <linux/sizes.h>
+#include <asm/mach/arch.h>
+#include <asm/pmu.h>
+#include "axxia.h"
+
+/*
+ * The PMU IRQ lines of four cores are wired together into a single interrupt.
+ * Bounce the interrupt to other cores if it's not ours.
+ */
+static irqreturn_t axxia_pmu_handler(int irq, void *dev, irq_handler_t handler)
+{
+	irqreturn_t ret = handler(irq, dev);
+
+	if (ret == IRQ_NONE) {
+		int cpu = smp_processor_id();
+		int cluster = cpu / CORES_PER_CLUSTER;
+		int other;
+
+		/* Look until we find another cpu that's online. */
+		do {
+			other = (++cpu % CORES_PER_CLUSTER) +
+				(cluster * CORES_PER_CLUSTER);
+		} while (!cpu_online(other));
+
+		irq_set_affinity(irq, cpumask_of(other));
+	}
+
+	/*
+	 * We should be able to get away with the amount of IRQ_NONEs we give,
+	 * while still having the spurious IRQ detection code kick in if the
+	 * interrupt really starts hitting spuriously.
+	 */
+	return ret;

arch/arm/mach-axxia/axxia.c line 70

Quoted 10 lines

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

+}
+
+static struct arm_pmu_platdata pmu_pdata = {
+	.handle_irq = axxia_pmu_handler,
+};
+
+static struct of_dev_auxdata axxia_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("arm,cortex-a15-pmu", 0, "pmu", &pmu_pdata),
+	{}
+};

This looks similar to what we have in mach-ux500 as db8500_pmu_handler.

To avoid duplication, I'd prefer moving support for this into the perf_event code itself, and get rid of the auxdata.

Quoted 18 lines

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

+static int
+axxia_bus_notifier(struct notifier_block *nb, unsigned long event, void *obj)
+{
+	struct device *dev = obj;
+
+	if (event != BUS_NOTIFY_ADD_DEVICE)
+		return NOTIFY_DONE;
+
+	if (!of_property_read_bool(dev->of_node, "dma-coherent"))
+		return NOTIFY_DONE;
+
+	set_dma_ops(dev, &arm_coherent_dma_ops);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block axxia_platform_nb = {
+	.notifier_call = axxia_bus_notifier,

arch/arm/mach-axxia/axxia.c line 95

Quoted 5 lines

arch/arm/mach-axxia/axxia.c new file +5 -0

+};
+
+static struct notifier_block axxia_amba_nb = {
+	.notifier_call = axxia_bus_notifier,
+};

And we definitely want to do this in a generic fashion. That should not be in platform specific code.

arch/arm/mach-axxia/axxia.c line 111

Quoted 7 lines

arch/arm/mach-axxia/axxia.c new file +6 -0

+static const char *axxia_dt_match[] __initconst = {
+	"lsi,axm55xx",
+	"lsi,axm55xx-sim",
+	"lsi,axm55xx-emu",
+	NULL
+};

Please no 'xx' in compatible strings. You can list all the relevant model numbers here, or require that machines list a specific model as compatible, e.g.

	compatible = "lsi,axm5510-sim", "lsi,axm5510", "lsi,axm5502";

arch/arm/mach-axxia/axxia.c line 117

Quoted 6 lines

arch/arm/mach-axxia/axxia.c new file +6 -0

+
+DT_MACHINE_START(AXXIA_DT, "LSI Axxia AXM55XX")
+	.dt_compat	= axxia_dt_match,
+	.smp		= smp_ops(axxia_smp_ops),
+	.init_machine	= axxia_dt_init,
+MACHINE_END

Please have a look at how smp ops are hooked up in mach-qcom and see if you can do it that way as well.

Quoted 20 lines

arch/arm/mach-axxia/axxia.h new file +14 -0

new file mode 100644
index 0000000..594dd97
@@ -0,0 +1,42 @@
+/*
+ * Prototypes for platform functions.
+ *
+ *
+ * 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.
+ *

arch/arm/mach-axxia/axxia.h line 24

Quoted 10 lines

arch/arm/mach-axxia/axxia.h new file +10 -0

+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __AXXIA_H
+#define __AXXIA_H
+
+#define CORES_PER_CLUSTER      4
+
+#define AXXIA_PERIPH_PHYS      0x2000000000ULL
+#define AXXIA_SYSCON_PHYS      0x2010030000ULL

Remove these -- they have to come from DT.

arch/arm/mach-axxia/axxia.h line 36

Quoted 11 lines

arch/arm/mach-axxia/axxia.h new file +11 -0

+#if 0
+#define AXXIA_UART0_PHYS       0x2010080000ULL
+#define AXXIA_UART1_PHYS       0x2010081000ULL
+#define AXXIA_UART2_PHYS       0x2010082000ULL
+#define AXXIA_UART3_PHYS       0x2010083000ULL
+
+#ifdef CONFIG_DEBUG_LL
+#define AXXIA_DEBUG_UART_VIRT  0xf0080000
+#define AXXIA_DEBUG_UART_PHYS  AXXIA_UART0_PHYS
+#endif
+#endif

You can put these constants directly into debug code.

Quoted 7 lines

arch/arm/mach-axxia/platsmp.c new file +0 -0

new file mode 100644
index 0000000..fd7f507

arch/arm/mach-axxia/platsmp.c line 8

Quoted 8 lines

arch/arm/mach-axxia/platsmp.c new file +8 -0

@@ -0,0 +1,183 @@
+/*
+ *  linux/arch/arm/mach-axxia/platsmp.c
+ *
+ *  Copyright (C) 2012 LSI Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.

Have you considered using PSCI to bring up the secondary CPUs?

Quoted 17 lines

arch/arm/mach-axxia/platsmp.c new file +17 -0

+static int axxia_boot_secondary(unsigned int cpu,
+					  struct task_struct *idle)
+{
+	unsigned long timeout;
+
+	/* Release the specified core */
+	write_pen_release(cpu_logical_map(cpu));
+
+	/* Send a wakeup event to get the idled cpu out of WFE state */
+	dsb_sev();
+
+	/* Wait for so long, then give up if nothing happens ... */
+	timeout = jiffies + (1 * HZ);
+	while (time_before(jiffies, timeout)) {
+		/* Make sure stores to pen_release have completed */
+		smp_rmb();
+		if (pen_release == -1)

arch/arm/mach-axxia/platsmp.c line 116

Quoted 6 lines

arch/arm/mach-axxia/platsmp.c new file +6 -0

+			break;
+		udelay(1);
+	}
+
+	return pen_release != -1 ? -ENOSYS : 0;
+}

This is pretty sad. No hardware support for waking up CPUs?

Quoted 7 lines

arch/arm/mach-axxia/platsmp.c new file +7 -0

+static void __init axxia_smp_prepare_cpus(unsigned int max_cpus)
+{
+	void __iomem *syscon;
+	int cpu_count = 0;
+	int cpu;
+
+	syscon = ioremap(AXXIA_SYSCON_PHYS, SZ_64K);

arch/arm/mach-axxia/platsmp.c line 128

Quoted 4 lines

arch/arm/mach-axxia/platsmp.c new file +4 -0

+	if (WARN_ON(!syscon))
+		return;
+
+	check_fixup_sev(syscon);

Please use the syscon driver and regmap. That driver might need a little help to get syscon_node_to_regmap() to work before driver initialization, but other people need the same thing, and we should just implement it.

drivers/clk/clk-axxia.c line 2

Quoted 9 lines

drivers/clk/clk-axxia.c new file +2 -0

new file mode 100644
index 0000000..996b8f2
@@ -0,0 +1,281 @@
+/*
+ * arch/arm/mach-axxia/clock.c

This should be a separate patch, and get merged by the clk maintainer.

Arnd

arch/arm/mach-axxia/headsmp.S new file +8 -0

+	adr	r4, 2f
+	ldmia	r4, {r5, r6}
+	sub	r4, r4, r5
+	add	r6, r6, r4
+1:	ldr	r7, [r6]
+	cmp	r7, r0
+	wfene
+	bne	1b

+2: .long .

arch/arm/mach-axxia/headsmp.S new file +1 -0

+	.long	pen_release

arch/arm/mach-axxia/platsmp.c +0 -0

arch/arm/mach-axxia/platsmp.c +2 -0

+static void __init axxia_smp_prepare_cpus(unsigned int max_cpus)
+{

arch/arm/mach-axxia/platsmp.c +5 -0

+		/*
+		 * Release all physical cpus when not in hyp mode since we
+		 * might want to bring them online later.
+		 *
+		 * Also we need to get the execution into kernel code (it's

Add device tree for the Amarillo validation board with an AXM5516 SoC.

Signed-off-by Anders Berg <[email protected]>

---
 arch/arm/boot/dts/Makefile             |   1 +
 arch/arm/boot/dts/axm5516-amarillo.dts |  51 ++++++
 arch/arm/boot/dts/axm5516-cpus.dtsi    | 204 ++++++++++++++++++++++
 arch/arm/boot/dts/axm55xx.dtsi         | 306 +++++++++++++++++++++++++++++++++
 4 files changed, 562 insertions(+)
 create mode 100644 arch/arm/boot/dts/axm5516-amarillo.dts
 create mode 100644 arch/arm/boot/dts/axm5516-cpus.dtsi
 create mode 100644 arch/arm/boot/dts/axm55xx.dtsi

Hi Anders,

Quoted 13 lines · Anders Berg

Add device tree for the Amarillo validation board with an AXM5516 SoC.

Signed-off-by Anders Berg <[email protected]>

---
 arch/arm/boot/dts/Makefile             |   1 +
 arch/arm/boot/dts/axm5516-amarillo.dts |  51 ++++++
 arch/arm/boot/dts/axm5516-cpus.dtsi    | 204 ++++++++++++++++++++++
 arch/arm/boot/dts/axm55xx.dtsi         | 306 +++++++++++++++++++++++++++++++++
 4 files changed, 562 insertions(+)
 create mode 100644 arch/arm/boot/dts/axm5516-amarillo.dts
 create mode 100644 arch/arm/boot/dts/axm5516-cpus.dtsi
 create mode 100644 arch/arm/boot/dts/axm55xx.dtsi

[...]

arch/arm/boot/dts/axm55xx.dtsi line 177

Quoted 12 lines

arch/arm/boot/dts/axm55xx.dtsi new file +12 -0

+       gic: interrupt-controller at 2001001000 {
+               compatible = "arm,cortex-a15-gic";
+               #interrupt-cells = <3>;
+               #address-cells = <0>;
+               interrupt-controller;
+               reg = <0x20 0x01001000 0 0x1000>,
+                     <0x20 0x01002000 0 0x1000>,
+                     <0x20 0x01004000 0 0x2000>,
+                     <0x20 0x01006000 0 0x2000>;
+               interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+                               IRQ_TYPE_LEVEL_HIGH)>;
+       };

Given how many CPUs this system has, what's the catch regarding the GIC? Is there a second one shadowing this one at the same address for another set of 8 CPUs? Is there an additional mechanism to IPI the other CPUs?

arch/arm/boot/dts/axm5516-amarillo.dts new file +0 -0

new file mode 100644
index 0000000..1760d6c

arch/arm/boot/dts/axm5516-amarillo.dts new file +4 -0

+			timer0: timer at 2010091000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x20 0x10091000 0 0x1000>;
+				interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>,

arch/arm/boot/dts/axm5516-amarillo.dts new file +6 -0

+			gpio0: gpio at 2010092000 {
+				#gpio-cells = <2>;
+				compatible = "arm,pl061", "arm,primecell";
+				gpio-controller;
+				reg = <0x20 0x10092000 0x00 0x1000>;
+				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,

Quoted 11 lines · On Tuesday 15 April 2014 14:06:11 Anders Berg wrote:

arch/arm/boot/dts/axm5516-amarillo.dts new file +5 -0

new file mode 100644
index 0000000..1760d6c
@@ -0,0 +1,51 @@
+/*
+ * arch/arm/boot/dts/axm5516-amarillo.dts
+ *
+ * Copyright (C) 2013 LSI
+ *

arch/arm/boot/dts/axm5516-amarillo.dts line 10

Quoted 5 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +5 -0

+ * 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.
+ */

It's better to use a permissive license such as the BSD license for dts files, so they can be shared with other OSs.

The situation with the clocks is still very strange: either the bindings are all in need of update, or you got all devices wrong:

arch/arm/boot/dts/axm5516-amarillo.dts line 18

Quoted 8 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +8 -0

+			serial0: uart at 2010080000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x20 0x10080000 0 0x1000>;
+				interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk_per>, <&clk_per>;
+				clock-names = "uartclk", "apb_pclk";
+				status = "disabled";
+			};

"uartclk" is not a valid string for pl011, as per binding:

| - clocks: When present, must refer to exactly one clock named
| "apb_pclk"

I do see that a lot of platforms do the same thing you have here, not sure who is wrong.

Quoted 5 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +4 -0

+			timer0: timer at 2010091000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x20 0x10091000 0 0x1000>;
+				interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>,

arch/arm/boot/dts/axm5516-amarillo.dts line 32

Quoted 10 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +10 -0

+					     <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk_per>, <&clk_per>;
+				clock-names = "timclken1", "apb_pclk";

Citing the binding:

| - clocks: clocks driving the dual timer hardware. This list should be 1 or 3
|        clocks. With 3 clocks, the order is timer0 clock, timer1 clock,
|        apb_pclk. A single clock can also be specified if the same clock is
|        used for all clock inputs.

I think you only want to have one clock here and make that the "apb_pclk".

Quoted 12 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +11 -0

+			gpio0: gpio at 2010092000 {
+				#gpio-cells = <2>;
+				compatible = "arm,pl061", "arm,primecell";
+				gpio-controller;
+				reg = <0x20 0x10092000 0x00 0x1000>;
+				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,

arch/arm/boot/dts/axm5516-amarillo.dts line 48

Quoted 5 lines

arch/arm/boot/dts/axm5516-amarillo.dts new file +5 -0

+					     <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk_per>;
+				clock-names = "apb_pclk";
+				status = "disabled";

The pl061 binding does not specify any clocks at all. Do we need to update that?

Quoted 17 lines

Quoted 16 lines

arch/arm/boot/dts/axm55xx.dtsi new file +16 -0

+                   gpio0: gpio at 2010092000 {
+                           #gpio-cells = <2>;
+                           compatible = "arm,pl061", "arm,primecell";
+                           gpio-controller;
+                           reg = <0x20 0x10092000 0x00 0x1000>;
+                           interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+                           clocks = <&clk_per>;
+                           clock-names = "apb_pclk";
+                           status = "disabled";

Quoted 2 lines

The pl061 binding does not specify any clocks at all. Do we need to update that?

Doesn't all AMBA devices need at least one apb_pclk since the bus driver does clk_get(...,"apb_pclk") before calling probe()?

Quoted 20 lines · On Tuesday 15 April 2014 17:20:47 Anders Berg wrote:

Quoted 19 lines

Quoted 6 lines

arch/arm/boot/dts/axm55xx.dtsi new file +6 -0

+                   gpio0: gpio at 2010092000 {
+                           #gpio-cells = <2>;
+                           compatible = "arm,pl061", "arm,primecell";
+                           gpio-controller;
+                           reg = <0x20 0x10092000 0x00 0x1000>;
+                           interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,

arch/arm/boot/dts/axm55xx.dtsi line 285

Quoted 10 lines

arch/arm/boot/dts/axm55xx.dtsi new file +10 -0

+                                        <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+                                        <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+                           clocks = <&clk_per>;
+                           clock-names = "apb_pclk";
+                           status = "disabled";

The pl061 binding does not specify any clocks at all. Do we need to update that?

Quoted 2 lines

Doesn't all AMBA devices need at least one apb_pclk since the bus driver does clk_get(...,"apb_pclk") before calling probe()?

Yes, I was mostly wondering whether we had a policy about whether this clock should also be listed in the binding or not. My feeling is that it's better to make that explicit.

arch/arm/boot/dts/axm55xx.dtsi line 224

Quoted 8 lines · On Tuesday 15 April 2014 14:06:11 Anders Berg wrote:

arch/arm/boot/dts/axm55xx.dtsi new file +8 -0

+                     serial0: uart at 2010080000 {
+                             compatible = "arm,pl011", "arm,primecell";
+                             reg = <0x20 0x10080000 0 0x1000>;
+                             interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+                             clocks = <&clk_per>, <&clk_per>;
+                             clock-names = "uartclk", "apb_pclk";
+                             status = "disabled";
+                     };

"uartclk" is not a valid string for pl011, as per binding:

| - clocks: When present, must refer to exactly one clock named
| "apb_pclk"

I do see that a lot of platforms do the same thing you have here, not sure who is wrong.

"uartclk" is not a valid string for pl011, as per binding:

| - clocks: When present, must refer to exactly one clock named
| "apb_pclk"