All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/13] Device Tree support for RealView PB11MPCore
@ 2015-10-15 13:46 Linus Walleij
  2015-10-15 13:46 ` [PATCH 02/13] ARM: add DT bindings for the ARM11MPCore CPU cluster Linus Walleij
                   ` (11 more replies)
  0 siblings, 12 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

This patch set gives an almost feature-complete device tree boot
on the ARM PM11MPCore platform.

Remains to be done before deleting the board files:

- Flash protection: the board has flash protection through the
  system controller. Last time I tried to fix this I fried the
  flash on my PB1176 so I don't know if I dare to mess with this
  before I've recovered that board (which need to happen using
  JTAG and old Windows).

- CLCD: how the stuff merged for Vexpress device tree CLCD works
  is a mystery to me, and ideally the whole shebang should be
  rewritten to use DRM/KMS. But I guess I can fix it up some
  time.

The patches may have some rough edges but basically this is how
I imagine we should make this work. Most of the code will be
reusable for RealView PBx, EB and probably for tidying up some
of the Vexpress stuff as well.

Linus Walleij (13):
  ARM: add some L220 DT settings
  ARM: add DT bindings for the ARM11MPCore CPU cluster
  irqchips: fix ARM11MPCore GIC bindings
  irqchip/gic: support RealView variant setup
  irqchip/gic: assign irqchip dynamically
  clk: versatile-icst: convert to use regmap
  clk: versatile-icst: refactor to allocate regmap separately
  clk: add ARM syscon ICST device tree bindings
  clk: versatile-icst: add device tree support
  soc: versatile: add support for the PB11MPCore
  ARM: realview: select SP810 and ICST for the DT variant
  ARM: realview: add an DT SMP boot method
  ARM: realview: add device tree for PB11MPCore

 Documentation/devicetree/bindings/arm/cpus.txt     |   1 +
 Documentation/devicetree/bindings/arm/gic.txt      |   3 +-
 Documentation/devicetree/bindings/arm/l2cc.txt     |  10 +-
 Documentation/devicetree/bindings/arm/scu.txt      |   3 +
 .../devicetree/bindings/clock/arm-syscon-icst.txt  |  40 ++
 arch/arm/boot/dts/Makefile                         |   3 +-
 arch/arm/boot/dts/arm-realview-pb11mp.dts          | 681 +++++++++++++++++++++
 arch/arm/mach-realview/Kconfig                     |   4 +
 arch/arm/mach-realview/Makefile                    |   2 +-
 arch/arm/mach-realview/platsmp-dt.c                |  91 +++
 arch/arm/mm/cache-l2x0.c                           |  15 +
 drivers/clk/versatile/Kconfig                      |   1 +
 drivers/clk/versatile/clk-icst.c                   | 191 +++++-
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-gic-realview.c                 |  39 ++
 drivers/irqchip/irq-gic-realview.h                 |   5 +
 drivers/irqchip/irq-gic.c                          |  43 +-
 drivers/soc/versatile/soc-realview.c               |   4 +
 18 files changed, 1082 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
 create mode 100644 arch/arm/boot/dts/arm-realview-pb11mp.dts
 create mode 100644 arch/arm/mach-realview/platsmp-dt.c
 create mode 100644 drivers/irqchip/irq-gic-realview.c
 create mode 100644 drivers/irqchip/irq-gic-realview.h

-- 
2.4.3

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 01/13] ARM: add some L220 DT settings
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46     ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                       ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, devicetree-u79uwXL29TY76Z2rM5mHXA

The RealView ARM11MPCore enables parity, eventmon and shared
override in the cache controller through its current boardfile,
but the code and DT bindings for the ARM L220 is currently
lacking the ability to set this up from DT. Add the required
bool parameters.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
I know this patch mixes code and DT changes but it is silly to
split such a small patch. Will submit this to Russell's patch
tracker if it looks OK to the DT people. (Or if they are quiet.)
---
 Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
 arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index 06c88a4d28ac..4d262e9b3464 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -67,12 +67,14 @@ Optional properties:
   disable if zero.
 - arm,prefetch-offset : Override prefetch offset value. Valid values are
   0-7, 15, 23, and 31.
-- arm,shared-override : The default behavior of the pl310 cache controller with
-  respect to the shareable attribute is to transform "normal memory
-  non-cacheable transactions" into "cacheable no allocate" (for reads) or
-  "write through no write allocate" (for writes).
+- arm,shared-override : The default behavior of the PL220 or PL310 cache
+  controllers with respect to the shareable attribute is to transform "normal
+  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
+  or "write through no write allocate" (for writes).
   On systems where this may cause DMA buffer corruption, this property must be
   specified to indicate that such transforms are precluded.
+- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).
+- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).
 - prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1>
   (forcibly enable), property absent (retain settings set by firmware)
 - prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable),
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 493692d838c6..d4e9fa2594f3 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1060,6 +1060,21 @@ static void __init l2x0_of_parse(const struct device_node *np,
 		val |= (dirty - 1) << L2X0_AUX_CTRL_DIRTY_LATENCY_SHIFT;
 	}
 
+	if (of_property_read_bool(np, "arm,parity-enable")) {
+		mask &= ~L2C_AUX_CTRL_PARITY_ENABLE;
+		val |= L2C_AUX_CTRL_PARITY_ENABLE;
+	}
+
+	if (of_property_read_bool(np, "arm,eventmon-enable")) {
+		mask &= ~L2C_AUX_CTRL_EVTMON_ENABLE;
+		val |= L2C_AUX_CTRL_EVTMON_ENABLE;
+	}
+
+	if (of_property_read_bool(np, "arm,shared-override")) {
+		mask &= ~L2C_AUX_CTRL_SHARED_OVERRIDE;
+		val |= L2C_AUX_CTRL_SHARED_OVERRIDE;
+	}
+
 	ret = l2x0_cache_size_of_parse(np, aux_val, aux_mask, &assoc, SZ_256K);
 	if (ret)
 		return;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 01/13] ARM: add some L220 DT settings
@ 2015-10-15 13:46     ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The RealView ARM11MPCore enables parity, eventmon and shared
override in the cache controller through its current boardfile,
but the code and DT bindings for the ARM L220 is currently
lacking the ability to set this up from DT. Add the required
bool parameters.

Cc: devicetree at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I know this patch mixes code and DT changes but it is silly to
split such a small patch. Will submit this to Russell's patch
tracker if it looks OK to the DT people. (Or if they are quiet.)
---
 Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
 arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index 06c88a4d28ac..4d262e9b3464 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -67,12 +67,14 @@ Optional properties:
   disable if zero.
 - arm,prefetch-offset : Override prefetch offset value. Valid values are
   0-7, 15, 23, and 31.
-- arm,shared-override : The default behavior of the pl310 cache controller with
-  respect to the shareable attribute is to transform "normal memory
-  non-cacheable transactions" into "cacheable no allocate" (for reads) or
-  "write through no write allocate" (for writes).
+- arm,shared-override : The default behavior of the PL220 or PL310 cache
+  controllers with respect to the shareable attribute is to transform "normal
+  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
+  or "write through no write allocate" (for writes).
   On systems where this may cause DMA buffer corruption, this property must be
   specified to indicate that such transforms are precluded.
+- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).
+- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).
 - prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1>
   (forcibly enable), property absent (retain settings set by firmware)
 - prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable),
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 493692d838c6..d4e9fa2594f3 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1060,6 +1060,21 @@ static void __init l2x0_of_parse(const struct device_node *np,
 		val |= (dirty - 1) << L2X0_AUX_CTRL_DIRTY_LATENCY_SHIFT;
 	}
 
+	if (of_property_read_bool(np, "arm,parity-enable")) {
+		mask &= ~L2C_AUX_CTRL_PARITY_ENABLE;
+		val |= L2C_AUX_CTRL_PARITY_ENABLE;
+	}
+
+	if (of_property_read_bool(np, "arm,eventmon-enable")) {
+		mask &= ~L2C_AUX_CTRL_EVTMON_ENABLE;
+		val |= L2C_AUX_CTRL_EVTMON_ENABLE;
+	}
+
+	if (of_property_read_bool(np, "arm,shared-override")) {
+		mask &= ~L2C_AUX_CTRL_SHARED_OVERRIDE;
+		val |= L2C_AUX_CTRL_SHARED_OVERRIDE;
+	}
+
 	ret = l2x0_cache_size_of_parse(np, aux_val, aux_mask, &assoc, SZ_256K);
 	if (ret)
 		return;
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 02/13] ARM: add DT bindings for the ARM11MPCore CPU cluster
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM11MPCore has a Snoop Control Unit, but references to it
were missing from the DT specification. Define a compatible
string for this unit.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/arm/scu.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/scu.txt b/Documentation/devicetree/bindings/arm/scu.txt
index c447680519bb..08a587875996 100644
--- a/Documentation/devicetree/bindings/arm/scu.txt
+++ b/Documentation/devicetree/bindings/arm/scu.txt
@@ -10,10 +10,13 @@ References:
   Revision r2p0
 - Cortex-A5: see DDI0434B Cortex-A5 MPCore Technical Reference Manual
   Revision r0p1
+- ARM11 MPCore: see DDI0360F ARM 11 MPCore Processor Technical Reference
+  Manial Revision r2p0
 
 - compatible : Should be:
 	"arm,cortex-a9-scu"
 	"arm,cortex-a5-scu"
+	"arm,arm11mp-scu"
 
 - reg : Specify the base address and the size of the SCU register window.
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 03/13] irqchips: fix ARM11MPCore GIC bindings
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46     ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                       ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, devicetree-u79uwXL29TY76Z2rM5mHXA

The GIC bindings for the ARM11MPCore need to differentiate between
the GIC on the Test Chip and the one on the evaluation baseboard.
Split the binding in two and define new compatible-strings.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/gic.txt | 3 ++-
 drivers/irqchip/irq-gic.c                     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 2da059a4790c..a5445622c216 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -15,7 +15,8 @@ Main node required properties:
 	"arm,cortex-a15-gic"
 	"arm,cortex-a9-gic"
 	"arm,cortex-a7-gic"
-	"arm,arm11mp-gic"
+	"arm,tc11mp-gic"
+	"arm,pb11mp-gic"
 	"brcm,brahma-b15-gic"
 	"arm,arm1176jzf-devchip-gic"
 	"qcom,msm-8660-qgic"
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 982c09c2d791..5376d1cb0a4f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1184,7 +1184,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 	return 0;
 }
 IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
-IRQCHIP_DECLARE(arm11mp_gic, "arm,arm11mp-gic", gic_of_init);
+IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", gic_of_init);
+IRQCHIP_DECLARE(armpb11mp_gic, "arm,pb11mp-gic", gic_of_init);
 IRQCHIP_DECLARE(arm1176jzf_dc_gic, "arm,arm1176jzf-devchip-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 03/13] irqchips: fix ARM11MPCore GIC bindings
@ 2015-10-15 13:46     ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The GIC bindings for the ARM11MPCore need to differentiate between
the GIC on the Test Chip and the one on the evaluation baseboard.
Split the binding in two and define new compatible-strings.

Cc: devicetree at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/arm/gic.txt | 3 ++-
 drivers/irqchip/irq-gic.c                     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 2da059a4790c..a5445622c216 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -15,7 +15,8 @@ Main node required properties:
 	"arm,cortex-a15-gic"
 	"arm,cortex-a9-gic"
 	"arm,cortex-a7-gic"
-	"arm,arm11mp-gic"
+	"arm,tc11mp-gic"
+	"arm,pb11mp-gic"
 	"brcm,brahma-b15-gic"
 	"arm,arm1176jzf-devchip-gic"
 	"qcom,msm-8660-qgic"
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 982c09c2d791..5376d1cb0a4f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1184,7 +1184,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 	return 0;
 }
 IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
-IRQCHIP_DECLARE(arm11mp_gic, "arm,arm11mp-gic", gic_of_init);
+IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", gic_of_init);
+IRQCHIP_DECLARE(armpb11mp_gic, "arm,pb11mp-gic", gic_of_init);
 IRQCHIP_DECLARE(arm1176jzf_dc_gic, "arm,arm1176jzf-devchip-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 04/13] irqchip/gic: support RealView variant setup
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
  2015-10-15 13:46 ` [PATCH 02/13] ARM: add DT bindings for the ARM11MPCore CPU cluster Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-10-15 13:46 ` Linus Walleij
  2015-10-15 16:06   ` Marc Zyngier
  2015-10-16  8:28   ` Thomas Gleixner
  2015-10-15 13:46 ` [PATCH 05/13] irqchip/gic: assign irqchip dynamically Linus Walleij
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM RealView PB11MPCore reference design has some special
bits in a system controller register to set up the GIC in one
of three modes: legacy, new with DCC, new without DCC. The
register is also used to enable FIQ.

Since the platform will not boot unless this register is set
up to "new with DCC" mode, we need a special quirk to be
compiled-in for the RealView platforms.

If we find the right compatible string on the GIC TestChip,
we enable this quirk by looking up the system controller and
enabling the special bits.

We depend on the CONFIG_REALVIEW_DT Kconfig symbol as the old
boardfile code has the same fix hardcoded, and this is only
needed for the attempts to modernize the RealView code using
device tree.

After fixing this, the PB11MPCore boots with device tree
only.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Looking for an ACK for this (unless the irqchip maintainers
barf at it) so I can take it through the ARM SoC tree.
---
 drivers/irqchip/Makefile           |  1 +
 drivers/irqchip/irq-gic-realview.c | 39 ++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-gic-realview.h |  5 +++++
 drivers/irqchip/irq-gic.c          |  4 ++++
 4 files changed, 49 insertions(+)
 create mode 100644 drivers/irqchip/irq-gic-realview.c
 create mode 100644 drivers/irqchip/irq-gic-realview.h

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index bb3048f00e64..7a7d4182777d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi-nmi.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o irq-gic-common.o
+obj-$(CONFIG_REALVIEW_DT)		+= irq-gic-realview.o
 obj-$(CONFIG_ARM_GIC_V2M)		+= irq-gic-v2m.o
 obj-$(CONFIG_ARM_GIC_V3)		+= irq-gic-v3.o irq-gic-common.o
 obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
diff --git a/drivers/irqchip/irq-gic-realview.c b/drivers/irqchip/irq-gic-realview.c
new file mode 100644
index 000000000000..2e8a4b129f5f
--- /dev/null
+++ b/drivers/irqchip/irq-gic-realview.c
@@ -0,0 +1,39 @@
+/*
+ * Special GIC quirks for the ARM RealView
+ * Copyright (C) 2015 Linus Walleij
+ */
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/bitops.h>
+
+#define REALVIEW_SYS_LOCK_OFFSET	0x20
+#define REALVIEW_PB11MP_SYS_PLD_CTRL1	0x74
+#define VERSATILE_LOCK_VAL		0xA05F
+#define PLD_INTMODE_MASK		BIT(22)|BIT(23)|BIT(24)
+#define PLD_INTMODE_LEGACY		0x0
+#define PLD_INTMODE_NEW_DCC		BIT(22)
+#define PLD_INTMODE_NEW_NO_DCC		BIT(23)
+#define PLD_INTMODE_FIQ_ENABLE		BIT(24)
+
+void __init realview_gic_init(struct device_node *np)
+{
+	static struct regmap *map;
+
+	/* The PB11MPCore GIC needs to be configured in the syscon */
+	if (of_device_is_compatible(np, "arm,tc11mp-gic")) {
+		map = syscon_regmap_lookup_by_compatible("arm,realview-pb11mp-syscon");
+		if (!IS_ERR(map)) {
+			/* new irq mode with no DCC */
+			regmap_write(map, REALVIEW_SYS_LOCK_OFFSET,
+				     VERSATILE_LOCK_VAL);
+			regmap_update_bits(map, REALVIEW_PB11MP_SYS_PLD_CTRL1,
+					   PLD_INTMODE_NEW_NO_DCC,
+					   PLD_INTMODE_MASK);
+			regmap_write(map, REALVIEW_SYS_LOCK_OFFSET, 0x0000);
+			pr_info("TC11MP GIC: set up interrupt controller to NEW mode, no DCC\n");
+		} else {
+			pr_err("TC11MP GIC setup: could not find syscon\n");
+		}
+	}
+}
diff --git a/drivers/irqchip/irq-gic-realview.h b/drivers/irqchip/irq-gic-realview.h
new file mode 100644
index 000000000000..06ebdfc523a8
--- /dev/null
+++ b/drivers/irqchip/irq-gic-realview.h
@@ -0,0 +1,5 @@
+#ifdef CONFIG_REALVIEW_DT
+void realview_gic_init(struct device_node *np);
+#else
+static void realview_gic_init(struct device_node *np) {}
+#endif
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 5376d1cb0a4f..bd021e1e4847 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -50,6 +50,7 @@
 #include <asm/virt.h>
 
 #include "irq-gic-common.h"
+#include "irq-gic-realview.h"
 
 union gic_base {
 	void __iomem *common_base;
@@ -1158,6 +1159,9 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 	cpu_base = of_iomap(node, 1);
 	WARN(!cpu_base, "unable to map gic cpu registers\n");
 
+	/* Special quirk for ARM RealView */
+	realview_gic_init(node);
+
 	/*
 	 * Disable split EOI/Deactivate if either HYP is not available
 	 * or the CPU interface is too small.
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 05/13] irqchip/gic: assign irqchip dynamically
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
                   ` (2 preceding siblings ...)
  2015-10-15 13:46 ` [PATCH 04/13] irqchip/gic: support RealView variant setup Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
  2015-10-15 16:16   ` Marc Zyngier
  2015-10-15 13:46   ` Linus Walleij
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of having the irqchip being a static struct, make it part
of the per-instance data so we can assign it a dynamic name. This
has the usable side effect of displaying the GIC with an instance
number as GIC0, GIC1 ... GICn in /proc/interrupts, which is helpful
when debugging cascaded GICs, such as on the ARM PB11MPCore.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Fellas please say what you think about this:
Yes / No / Linus is an idiot
This can be applied directly to the irqchip tree if you like
it, AFAIK it has no dependencies.
---
 drivers/irqchip/irq-gic.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index bd021e1e4847..478279cf9517 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -58,6 +58,7 @@ union gic_base {
 };
 
 struct gic_chip_data {
+	struct irq_chip chip;
 	union gic_base dist_base;
 	union gic_base cpu_base;
 #ifdef CONFIG_CPU_PM
@@ -369,22 +370,6 @@ static void gic_handle_cascade_irq(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static struct irq_chip gic_chip = {
-	.name			= "GIC",
-	.irq_mask		= gic_mask_irq,
-	.irq_unmask		= gic_unmask_irq,
-	.irq_eoi		= gic_eoi_irq,
-	.irq_set_type		= gic_set_type,
-#ifdef CONFIG_SMP
-	.irq_set_affinity	= gic_set_affinity,
-#endif
-	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
-	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
-	.flags			= IRQCHIP_SET_TYPE_MASKED |
-				  IRQCHIP_SKIP_SET_WAKE |
-				  IRQCHIP_MASK_ON_SUSPEND,
-};
-
 static struct irq_chip gic_eoimode1_chip = {
 	.name			= "GICv2",
 	.irq_mask		= gic_eoimode1_mask_irq,
@@ -880,7 +865,8 @@ void __init gic_init_physaddr(struct device_node *node)
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				irq_hw_number_t hw)
 {
-	struct irq_chip *chip = &gic_chip;
+	struct gic_chip_data *gic = d->host_data;
+	struct irq_chip *chip = &gic->chip;
 
 	if (static_key_true(&supports_deactivate)) {
 		if (d->host_data == (void *)&gic_data[0])
@@ -989,6 +975,22 @@ static void __init __gic_init_bases(unsigned int gic_nr, int irq_start,
 	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	gic = &gic_data[gic_nr];
+
+	/* Initialize irq_chip */
+	gic->chip.name = kasprintf(GFP_KERNEL, "GIC%d", gic_nr);
+	gic->chip.irq_mask = gic_mask_irq;
+	gic->chip.irq_unmask = gic_unmask_irq;
+	gic->chip.irq_eoi = gic_eoi_irq;
+	gic->chip.irq_set_type	= gic_set_type;
+#ifdef CONFIG_SMP
+	gic->chip.irq_set_affinity = gic_set_affinity;
+#endif
+	gic->chip.irq_get_irqchip_state = gic_irq_get_irqchip_state;
+	gic->chip.irq_set_irqchip_state = gic_irq_set_irqchip_state;
+	gic->chip.flags = IRQCHIP_SET_TYPE_MASKED |
+		IRQCHIP_SKIP_SET_WAKE |
+		IRQCHIP_MASK_ON_SUSPEND;
+
 #ifdef CONFIG_GIC_NON_BANKED
 	if (percpu_offset) { /* Frankein-GIC without banked registers... */
 		unsigned int cpu;
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46   ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, Arnd Bergmann, Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, Michael Turquette, Stephen Boyd, linux-clk

Instead of passing around register bases, pass around a regmap
in this driver. This refactoring make things so much easier when
we later want to manage an ICST that is part of a syscon.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/Kconfig    |  1 +
 drivers/clk/versatile/clk-icst.c | 88 +++++++++++++++++++++++++++-------------
 2 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
index 1530c9352a76..e091bcec116c 100644
--- a/drivers/clk/versatile/Kconfig
+++ b/drivers/clk/versatile/Kconfig
@@ -1,6 +1,7 @@
 config COMMON_CLK_VERSATILE
 	bool "Clock driver for ARM Reference designs"
 	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
+	select REGMAP_MMIO
 	---help---
           Supports clocking on ARM Reference designs:
 	  - Integrator/AP and Integrator/CP
diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index a3893ea2199d..da5a3ccbbb96 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -19,9 +19,13 @@
 #include <linux/err.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/regmap.h>
 
 #include "clk-icst.h"
 
+/* Magic unlocking token used on all Versatile boards */
+#define VERSATILE_LOCK_VAL	0xA05F
+
 /**
  * struct clk_icst - ICST VCO clock wrapper
  * @hw: corresponding clock hardware entry
@@ -32,8 +36,9 @@
  */
 struct clk_icst {
 	struct clk_hw hw;
-	void __iomem *vcoreg;
-	void __iomem *lockreg;
+	struct regmap *map;
+	u32 vcoreg_off;
+	u32 lockreg_off;
 	struct icst_params *params;
 	unsigned long rate;
 };
@@ -41,53 +46,67 @@ struct clk_icst {
 #define to_icst(_hw) container_of(_hw, struct clk_icst, hw)
 
 /**
- * vco_get() - get ICST VCO settings from a certain register
- * @vcoreg: register containing the VCO settings
+ * vco_get() - get ICST VCO settings from a certain ICST
+ * @icst: the ICST clock to get
+ * @vco: the VCO struct to return the value in
  */
-static struct icst_vco vco_get(void __iomem *vcoreg)
+static int vco_get(struct clk_icst *icst, struct icst_vco *vco)
 {
 	u32 val;
-	struct icst_vco vco;
-
-	val = readl(vcoreg);
-	vco.v = val & 0x1ff;
-	vco.r = (val >> 9) & 0x7f;
-	vco.s = (val >> 16) & 03;
-	return vco;
+	int ret;
+
+	ret = regmap_read(icst->map, icst->vcoreg_off, &val);
+	if (ret)
+		return ret;
+	vco->v = val & 0x1ff;
+	vco->r = (val >> 9) & 0x7f;
+	vco->s = (val >> 16) & 03;
+	return 0;
 }
 
 /**
  * vco_set() - commit changes to an ICST VCO
- * @locreg: register to poke to unlock the VCO for writing
- * @vcoreg: register containing the VCO settings
- * @vco: ICST VCO parameters to commit
+ * @icst: the ICST clock to set
+ * @vco: the VCO struct to set the changes from
  */
-static void vco_set(void __iomem *lockreg,
-			void __iomem *vcoreg,
-			struct icst_vco vco)
+static int vco_set(struct clk_icst *icst, struct icst_vco vco)
 {
 	u32 val;
+	int ret;
 
-	val = readl(vcoreg) & ~0x7ffff;
+	ret = regmap_read(icst->map, icst->vcoreg_off, &val);
+	if (ret)
+		return ret;
 	val |= vco.v | (vco.r << 9) | (vco.s << 16);
 
 	/* This magic unlocks the VCO so it can be controlled */
-	writel(0xa05f, lockreg);
-	writel(val, vcoreg);
+	ret = regmap_write(icst->map, icst->lockreg_off, VERSATILE_LOCK_VAL);
+	if (ret)
+		return ret;
+	ret = regmap_write(icst->map, icst->vcoreg_off, val);
+	if (ret)
+		return ret;
 	/* This locks the VCO again */
-	writel(0, lockreg);
+	ret = regmap_write(icst->map, icst->lockreg_off, 0);
+	if (ret)
+		return ret;
+	return 0;
 }
 
-
 static unsigned long icst_recalc_rate(struct clk_hw *hw,
 				      unsigned long parent_rate)
 {
 	struct clk_icst *icst = to_icst(hw);
 	struct icst_vco vco;
+	int ret;
 
 	if (parent_rate)
 		icst->params->ref = parent_rate;
-	vco = vco_get(icst->vcoreg);
+	ret = vco_get(icst, &vco);
+	if (ret) {
+		pr_err("ICST: could not get VCO setting\n");
+		return 0;
+	}
 	icst->rate = icst_hz(icst->params, vco);
 	return icst->rate;
 }
@@ -112,8 +131,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate,
 		icst->params->ref = parent_rate;
 	vco = icst_hz_to_vco(icst->params, rate);
 	icst->rate = icst_hz(icst->params, vco);
-	vco_set(icst->lockreg, icst->vcoreg, vco);
-	return 0;
+	return vco_set(icst, vco);
 }
 
 static const struct clk_ops icst_ops = {
@@ -132,6 +150,11 @@ struct clk *icst_clk_register(struct device *dev,
 	struct clk_icst *icst;
 	struct clk_init_data init;
 	struct icst_params *pclone;
+	struct regmap_config icst_regmap_conf = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+	};
 
 	icst = kzalloc(sizeof(struct clk_icst), GFP_KERNEL);
 	if (!icst) {
@@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
 	init.flags = CLK_IS_ROOT;
 	init.parent_names = (parent_name ? &parent_name : NULL);
 	init.num_parents = (parent_name ? 1 : 0);
+	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
+	if (IS_ERR(icst->map)) {
+		int ret;
+
+		pr_err("could not initialize ICST regmap\n");
+		kfree(icst);
+		ret = PTR_ERR(icst->map);
+		return ERR_PTR(ret);
+	}
 	icst->hw.init = &init;
 	icst->params = pclone;
-	icst->vcoreg = base + desc->vco_offset;
-	icst->lockreg = base + desc->lock_offset;
+	icst->vcoreg_off = desc->vco_offset;
+	icst->lockreg_off = desc->lock_offset;
 
 	clk = clk_register(dev, &icst->hw);
 	if (IS_ERR(clk))
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-15 13:46   ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of passing around register bases, pass around a regmap
in this driver. This refactoring make things so much easier when
we later want to manage an ICST that is part of a syscon.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/Kconfig    |  1 +
 drivers/clk/versatile/clk-icst.c | 88 +++++++++++++++++++++++++++-------------
 2 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
index 1530c9352a76..e091bcec116c 100644
--- a/drivers/clk/versatile/Kconfig
+++ b/drivers/clk/versatile/Kconfig
@@ -1,6 +1,7 @@
 config COMMON_CLK_VERSATILE
 	bool "Clock driver for ARM Reference designs"
 	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
+	select REGMAP_MMIO
 	---help---
           Supports clocking on ARM Reference designs:
 	  - Integrator/AP and Integrator/CP
diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index a3893ea2199d..da5a3ccbbb96 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -19,9 +19,13 @@
 #include <linux/err.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/regmap.h>
 
 #include "clk-icst.h"
 
+/* Magic unlocking token used on all Versatile boards */
+#define VERSATILE_LOCK_VAL	0xA05F
+
 /**
  * struct clk_icst - ICST VCO clock wrapper
  * @hw: corresponding clock hardware entry
@@ -32,8 +36,9 @@
  */
 struct clk_icst {
 	struct clk_hw hw;
-	void __iomem *vcoreg;
-	void __iomem *lockreg;
+	struct regmap *map;
+	u32 vcoreg_off;
+	u32 lockreg_off;
 	struct icst_params *params;
 	unsigned long rate;
 };
@@ -41,53 +46,67 @@ struct clk_icst {
 #define to_icst(_hw) container_of(_hw, struct clk_icst, hw)
 
 /**
- * vco_get() - get ICST VCO settings from a certain register
- * @vcoreg: register containing the VCO settings
+ * vco_get() - get ICST VCO settings from a certain ICST
+ * @icst: the ICST clock to get
+ * @vco: the VCO struct to return the value in
  */
-static struct icst_vco vco_get(void __iomem *vcoreg)
+static int vco_get(struct clk_icst *icst, struct icst_vco *vco)
 {
 	u32 val;
-	struct icst_vco vco;
-
-	val = readl(vcoreg);
-	vco.v = val & 0x1ff;
-	vco.r = (val >> 9) & 0x7f;
-	vco.s = (val >> 16) & 03;
-	return vco;
+	int ret;
+
+	ret = regmap_read(icst->map, icst->vcoreg_off, &val);
+	if (ret)
+		return ret;
+	vco->v = val & 0x1ff;
+	vco->r = (val >> 9) & 0x7f;
+	vco->s = (val >> 16) & 03;
+	return 0;
 }
 
 /**
  * vco_set() - commit changes to an ICST VCO
- * @locreg: register to poke to unlock the VCO for writing
- * @vcoreg: register containing the VCO settings
- * @vco: ICST VCO parameters to commit
+ * @icst: the ICST clock to set
+ * @vco: the VCO struct to set the changes from
  */
-static void vco_set(void __iomem *lockreg,
-			void __iomem *vcoreg,
-			struct icst_vco vco)
+static int vco_set(struct clk_icst *icst, struct icst_vco vco)
 {
 	u32 val;
+	int ret;
 
-	val = readl(vcoreg) & ~0x7ffff;
+	ret = regmap_read(icst->map, icst->vcoreg_off, &val);
+	if (ret)
+		return ret;
 	val |= vco.v | (vco.r << 9) | (vco.s << 16);
 
 	/* This magic unlocks the VCO so it can be controlled */
-	writel(0xa05f, lockreg);
-	writel(val, vcoreg);
+	ret = regmap_write(icst->map, icst->lockreg_off, VERSATILE_LOCK_VAL);
+	if (ret)
+		return ret;
+	ret = regmap_write(icst->map, icst->vcoreg_off, val);
+	if (ret)
+		return ret;
 	/* This locks the VCO again */
-	writel(0, lockreg);
+	ret = regmap_write(icst->map, icst->lockreg_off, 0);
+	if (ret)
+		return ret;
+	return 0;
 }
 
-
 static unsigned long icst_recalc_rate(struct clk_hw *hw,
 				      unsigned long parent_rate)
 {
 	struct clk_icst *icst = to_icst(hw);
 	struct icst_vco vco;
+	int ret;
 
 	if (parent_rate)
 		icst->params->ref = parent_rate;
-	vco = vco_get(icst->vcoreg);
+	ret = vco_get(icst, &vco);
+	if (ret) {
+		pr_err("ICST: could not get VCO setting\n");
+		return 0;
+	}
 	icst->rate = icst_hz(icst->params, vco);
 	return icst->rate;
 }
@@ -112,8 +131,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate,
 		icst->params->ref = parent_rate;
 	vco = icst_hz_to_vco(icst->params, rate);
 	icst->rate = icst_hz(icst->params, vco);
-	vco_set(icst->lockreg, icst->vcoreg, vco);
-	return 0;
+	return vco_set(icst, vco);
 }
 
 static const struct clk_ops icst_ops = {
@@ -132,6 +150,11 @@ struct clk *icst_clk_register(struct device *dev,
 	struct clk_icst *icst;
 	struct clk_init_data init;
 	struct icst_params *pclone;
+	struct regmap_config icst_regmap_conf = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+	};
 
 	icst = kzalloc(sizeof(struct clk_icst), GFP_KERNEL);
 	if (!icst) {
@@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
 	init.flags = CLK_IS_ROOT;
 	init.parent_names = (parent_name ? &parent_name : NULL);
 	init.num_parents = (parent_name ? 1 : 0);
+	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
+	if (IS_ERR(icst->map)) {
+		int ret;
+
+		pr_err("could not initialize ICST regmap\n");
+		kfree(icst);
+		ret = PTR_ERR(icst->map);
+		return ERR_PTR(ret);
+	}
 	icst->hw.init = &init;
 	icst->params = pclone;
-	icst->vcoreg = base + desc->vco_offset;
-	icst->lockreg = base + desc->lock_offset;
+	icst->vcoreg_off = desc->vco_offset;
+	icst->lockreg_off = desc->lock_offset;
 
 	clk = clk_register(dev, &icst->hw);
 	if (IS_ERR(clk))
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46   ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, Arnd Bergmann, Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, Michael Turquette, Stephen Boyd, linux-clk

Break out the registration function so it creates a regmap and
pass to the setup function, so the latter can be shared with
a device tree probe function that already has a regmap.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/clk-icst.c | 44 +++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index da5a3ccbbb96..ab5d5f73818b 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -20,6 +20,7 @@
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include "clk-icst.h"
 
@@ -140,21 +141,16 @@ static const struct clk_ops icst_ops = {
 	.set_rate = icst_set_rate,
 };
 
-struct clk *icst_clk_register(struct device *dev,
+struct clk *icst_clk_setup(struct device *dev,
 			const struct clk_icst_desc *desc,
 			const char *name,
 			const char *parent_name,
-			void __iomem *base)
+			struct regmap *map)
 {
 	struct clk *clk;
 	struct clk_icst *icst;
 	struct clk_init_data init;
 	struct icst_params *pclone;
-	struct regmap_config icst_regmap_conf = {
-		.reg_bits = 32,
-		.val_bits = 32,
-		.reg_stride = 4,
-	};
 
 	icst = kzalloc(sizeof(struct clk_icst), GFP_KERNEL);
 	if (!icst) {
@@ -174,15 +170,7 @@ struct clk *icst_clk_register(struct device *dev,
 	init.flags = CLK_IS_ROOT;
 	init.parent_names = (parent_name ? &parent_name : NULL);
 	init.num_parents = (parent_name ? 1 : 0);
-	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
-	if (IS_ERR(icst->map)) {
-		int ret;
-
-		pr_err("could not initialize ICST regmap\n");
-		kfree(icst);
-		ret = PTR_ERR(icst->map);
-		return ERR_PTR(ret);
-	}
+	icst->map = map;
 	icst->hw.init = &init;
 	icst->params = pclone;
 	icst->vcoreg_off = desc->vco_offset;
@@ -194,4 +182,28 @@ struct clk *icst_clk_register(struct device *dev,
 
 	return clk;
 }
+
+struct clk *icst_clk_register(struct device *dev,
+			const struct clk_icst_desc *desc,
+			const char *name,
+			const char *parent_name,
+			void __iomem *base)
+{
+	struct regmap_config icst_regmap_conf = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+	};
+	struct regmap *map;
+
+	map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
+	if (IS_ERR(map)) {
+		int ret;
+
+		pr_err("could not initialize ICST regmap\n");
+		ret = PTR_ERR(map);
+		return ERR_PTR(ret);
+	}
+	return icst_clk_setup(dev, desc, name, parent_name, map);
+}
 EXPORT_SYMBOL_GPL(icst_clk_register);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
@ 2015-10-15 13:46   ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

Break out the registration function so it creates a regmap and
pass to the setup function, so the latter can be shared with
a device tree probe function that already has a regmap.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/clk-icst.c | 44 +++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index da5a3ccbbb96..ab5d5f73818b 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -20,6 +20,7 @@
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include "clk-icst.h"
 
@@ -140,21 +141,16 @@ static const struct clk_ops icst_ops = {
 	.set_rate = icst_set_rate,
 };
 
-struct clk *icst_clk_register(struct device *dev,
+struct clk *icst_clk_setup(struct device *dev,
 			const struct clk_icst_desc *desc,
 			const char *name,
 			const char *parent_name,
-			void __iomem *base)
+			struct regmap *map)
 {
 	struct clk *clk;
 	struct clk_icst *icst;
 	struct clk_init_data init;
 	struct icst_params *pclone;
-	struct regmap_config icst_regmap_conf = {
-		.reg_bits = 32,
-		.val_bits = 32,
-		.reg_stride = 4,
-	};
 
 	icst = kzalloc(sizeof(struct clk_icst), GFP_KERNEL);
 	if (!icst) {
@@ -174,15 +170,7 @@ struct clk *icst_clk_register(struct device *dev,
 	init.flags = CLK_IS_ROOT;
 	init.parent_names = (parent_name ? &parent_name : NULL);
 	init.num_parents = (parent_name ? 1 : 0);
-	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
-	if (IS_ERR(icst->map)) {
-		int ret;
-
-		pr_err("could not initialize ICST regmap\n");
-		kfree(icst);
-		ret = PTR_ERR(icst->map);
-		return ERR_PTR(ret);
-	}
+	icst->map = map;
 	icst->hw.init = &init;
 	icst->params = pclone;
 	icst->vcoreg_off = desc->vco_offset;
@@ -194,4 +182,28 @@ struct clk *icst_clk_register(struct device *dev,
 
 	return clk;
 }
+
+struct clk *icst_clk_register(struct device *dev,
+			const struct clk_icst_desc *desc,
+			const char *name,
+			const char *parent_name,
+			void __iomem *base)
+{
+	struct regmap_config icst_regmap_conf = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+	};
+	struct regmap *map;
+
+	map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
+	if (IS_ERR(map)) {
+		int ret;
+
+		pr_err("could not initialize ICST regmap\n");
+		ret = PTR_ERR(map);
+		return ERR_PTR(ret);
+	}
+	return icst_clk_setup(dev, desc, name, parent_name, map);
+}
 EXPORT_SYMBOL_GPL(icst_clk_register);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46   ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, Arnd Bergmann, Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, devicetree, Michael Turquette, Stephen Boyd,
	linux-clk

This adds the device tree bindings for the ARM Syscon ICST
oscillators, which is a register-level interface to the
Integrated Device Technology (IDT) ICS525 and ICS307
serially programmable oscillators.

Cc: devicetree@vger.kernel.org
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 .../devicetree/bindings/clock/arm-syscon-icst.txt  | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/arm-syscon-icst.txt

diff --git a/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
new file mode 100644
index 000000000000..19eb3aa765c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
@@ -0,0 +1,40 @@
+ARM System Controller ICST clocks
+
+The ICS525 and ICS307 oscillators are produced by Integrated Devices
+Technology (IDT). ARM integrated these oscillators deeply into their
+reference designs by adding special control registers that manage such
+oscillators to their system controllers.
+
+The ARM system controller contains logic to serialized and initialize
+an ICST clock request after a write to the 32 bit register at an offset
+into the system controller. Further, to even be able to alter one of
+these frequencies, the system controller must first be unlocked by
+writing a special token to another offset in the system controller.
+
+The ICST oscillator must be provided inside a system controller node.
+
+Required properties:
+- lock-offset: the offset address into the system controller where the
+  unlocking register is located
+- vco-offset: the offset address into the system controller where the
+  ICST control register is located (even 32 bit address)
+- compatible: must be one of "arm,syscon-icst525" or "arm,syscon-icst307"
+- #clock-cells: must be <0>
+- clocks: parent clock, since the ICST needs a parent clock to derive its
+  frequency from, this attribute is compulsory.
+
+Example:
+
+syscon: syscon@10000000 {
+	compatible = "syscon";
+	reg = <0x10000000 0x1000>;
+
+	oscclk0: osc0@0c {
+		compatible = "arm,syscon-icst307";
+		#clock-cells = <0>;
+		lock-offset = <0x20>;
+		vco-offset = <0x0C>;
+		clocks = <&xtal24mhz>;
+	};
+	(...)
+};
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
@ 2015-10-15 13:46   ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

This adds the device tree bindings for the ARM Syscon ICST
oscillators, which is a register-level interface to the
Integrated Device Technology (IDT) ICS525 and ICS307
serially programmable oscillators.

Cc: devicetree at vger.kernel.org
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 .../devicetree/bindings/clock/arm-syscon-icst.txt  | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/arm-syscon-icst.txt

diff --git a/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
new file mode 100644
index 000000000000..19eb3aa765c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
@@ -0,0 +1,40 @@
+ARM System Controller ICST clocks
+
+The ICS525 and ICS307 oscillators are produced by Integrated Devices
+Technology (IDT). ARM integrated these oscillators deeply into their
+reference designs by adding special control registers that manage such
+oscillators to their system controllers.
+
+The ARM system controller contains logic to serialized and initialize
+an ICST clock request after a write to the 32 bit register at an offset
+into the system controller. Further, to even be able to alter one of
+these frequencies, the system controller must first be unlocked by
+writing a special token to another offset in the system controller.
+
+The ICST oscillator must be provided inside a system controller node.
+
+Required properties:
+- lock-offset: the offset address into the system controller where the
+  unlocking register is located
+- vco-offset: the offset address into the system controller where the
+  ICST control register is located (even 32 bit address)
+- compatible: must be one of "arm,syscon-icst525" or "arm,syscon-icst307"
+- #clock-cells: must be <0>
+- clocks: parent clock, since the ICST needs a parent clock to derive its
+  frequency from, this attribute is compulsory.
+
+Example:
+
+syscon: syscon at 10000000 {
+	compatible = "syscon";
+	reg = <0x10000000 0x1000>;
+
+	oscclk0: osc0 at 0c {
+		compatible = "arm,syscon-icst307";
+		#clock-cells = <0>;
+		lock-offset = <0x20>;
+		vco-offset = <0x0C>;
+		clocks = <&xtal24mhz>;
+	};
+	(...)
+};
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
@ 2015-10-15 13:46   ` Linus Walleij
       [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel, Arnd Bergmann, Russell King
  Cc: Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Linus Walleij, Michael Turquette, Stephen Boyd, linux-clk

This adds support for the ARM syscon ICST clocks to initialized
directly from the device tree syscon node on ARM Integrator,
Versatile and RealView reference designs.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/clk-icst.c | 89 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index ab5d5f73818b..b2dc06923ac1 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -3,7 +3,7 @@
  * We wrap the custom interface from <asm/hardware/icst.h> into the generic
  * clock framework.
  *
- * Copyright (C) 2012 Linus Walleij
+ * Copyright (C) 2012-2015 Linus Walleij
  *
  * 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
@@ -207,3 +207,90 @@ struct clk *icst_clk_register(struct device *dev,
 	return icst_clk_setup(dev, desc, name, parent_name, map);
 }
 EXPORT_SYMBOL_GPL(icst_clk_register);
+
+#ifdef CONFIG_OF
+/*
+ * In a device tree, an memory-mapped ICST clock appear as a child
+ * of a syscon node. Assume this and probe it only as a child of a
+ * syscon.
+ */
+
+static const struct icst_params icst525_params = {
+	.vco_max	= ICST525_VCO_MAX_5V,
+	.vco_min	= ICST525_VCO_MIN,
+	.vd_min 	= 8,
+	.vd_max 	= 263,
+	.rd_min 	= 3,
+	.rd_max 	= 65,
+	.s2div		= icst525_s2div,
+	.idx2s		= icst525_idx2s,
+};
+
+static const struct icst_params icst307_params = {
+	.vco_max	= ICST307_VCO_MAX,
+	.vco_min	= ICST307_VCO_MIN,
+	.vd_min		= 4 + 8,
+	.vd_max		= 511 + 8,
+	.rd_min		= 1 + 2,
+	.rd_max		= 127 + 2,
+	.s2div		= icst307_s2div,
+	.idx2s		= icst307_idx2s,
+};
+
+static void __init of_syscon_icst_setup(struct device_node *np)
+{
+	struct device_node *parent;
+	struct regmap *map;
+	struct clk_icst_desc icst_desc;
+	const char *name = np->name;
+	const char *parent_name;
+	struct clk *regclk;
+
+	/* We do not release this reference, we are using it perpetually */
+	parent = of_get_parent(np);
+	if (!parent) {
+		pr_err("no parent node for syscon ICST clock\n");
+		return;
+	}
+	map = syscon_node_to_regmap(parent);
+	if (IS_ERR(map)) {
+		pr_err("no regmap for syscon ICST clock parent\n");
+		return;
+	}
+
+	if (of_property_read_u32(np, "vco-offset", &icst_desc.vco_offset)) {
+		pr_err("no VCO register offset for ICST clock\n");
+		return;
+	}
+	if (of_property_read_u32(np, "lock-offset", &icst_desc.lock_offset)) {
+		pr_err("no lock register offset for ICST clock\n");
+		return;
+	}
+
+	if (of_device_is_compatible(np, "arm,syscon-icst525"))
+		icst_desc.params = &icst525_params;
+	else if (of_device_is_compatible(np, "arm,syscon-icst307"))
+		icst_desc.params = &icst307_params;
+	else {
+		pr_info("unknown ICST clock %s\n", name);
+		return;
+	}
+
+	/* Parent clock name is not the same as node parent */
+	parent_name = of_clk_get_parent_name(np, 0);
+
+	regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map);
+	if (IS_ERR(regclk)) {
+		pr_err("error setting up syscon ICST clock %s\n", name);
+		return;
+	}
+	of_clk_add_provider(np, of_clk_src_simple_get, regclk);
+	pr_info("registered syscon ICST clock %s\n", name);
+}
+
+CLK_OF_DECLARE(arm_syscon_icst525_clk,
+	       "arm,syscon-icst525", of_syscon_icst_setup);
+CLK_OF_DECLARE(arm_syscon_icst307_clk,
+	       "arm,syscon-icst307", of_syscon_icst_setup);
+
+#endif
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-15 13:46   ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

This adds support for the ARM syscon ICST clocks to initialized
directly from the device tree syscon node on ARM Integrator,
Versatile and RealView reference designs.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-clk at vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I'm looking for an ACK from the CLK maintainers to take this
through the ARM SoC tree once the series stabilize.
---
 drivers/clk/versatile/clk-icst.c | 89 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index ab5d5f73818b..b2dc06923ac1 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -3,7 +3,7 @@
  * We wrap the custom interface from <asm/hardware/icst.h> into the generic
  * clock framework.
  *
- * Copyright (C) 2012 Linus Walleij
+ * Copyright (C) 2012-2015 Linus Walleij
  *
  * 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
@@ -207,3 +207,90 @@ struct clk *icst_clk_register(struct device *dev,
 	return icst_clk_setup(dev, desc, name, parent_name, map);
 }
 EXPORT_SYMBOL_GPL(icst_clk_register);
+
+#ifdef CONFIG_OF
+/*
+ * In a device tree, an memory-mapped ICST clock appear as a child
+ * of a syscon node. Assume this and probe it only as a child of a
+ * syscon.
+ */
+
+static const struct icst_params icst525_params = {
+	.vco_max	= ICST525_VCO_MAX_5V,
+	.vco_min	= ICST525_VCO_MIN,
+	.vd_min 	= 8,
+	.vd_max 	= 263,
+	.rd_min 	= 3,
+	.rd_max 	= 65,
+	.s2div		= icst525_s2div,
+	.idx2s		= icst525_idx2s,
+};
+
+static const struct icst_params icst307_params = {
+	.vco_max	= ICST307_VCO_MAX,
+	.vco_min	= ICST307_VCO_MIN,
+	.vd_min		= 4 + 8,
+	.vd_max		= 511 + 8,
+	.rd_min		= 1 + 2,
+	.rd_max		= 127 + 2,
+	.s2div		= icst307_s2div,
+	.idx2s		= icst307_idx2s,
+};
+
+static void __init of_syscon_icst_setup(struct device_node *np)
+{
+	struct device_node *parent;
+	struct regmap *map;
+	struct clk_icst_desc icst_desc;
+	const char *name = np->name;
+	const char *parent_name;
+	struct clk *regclk;
+
+	/* We do not release this reference, we are using it perpetually */
+	parent = of_get_parent(np);
+	if (!parent) {
+		pr_err("no parent node for syscon ICST clock\n");
+		return;
+	}
+	map = syscon_node_to_regmap(parent);
+	if (IS_ERR(map)) {
+		pr_err("no regmap for syscon ICST clock parent\n");
+		return;
+	}
+
+	if (of_property_read_u32(np, "vco-offset", &icst_desc.vco_offset)) {
+		pr_err("no VCO register offset for ICST clock\n");
+		return;
+	}
+	if (of_property_read_u32(np, "lock-offset", &icst_desc.lock_offset)) {
+		pr_err("no lock register offset for ICST clock\n");
+		return;
+	}
+
+	if (of_device_is_compatible(np, "arm,syscon-icst525"))
+		icst_desc.params = &icst525_params;
+	else if (of_device_is_compatible(np, "arm,syscon-icst307"))
+		icst_desc.params = &icst307_params;
+	else {
+		pr_info("unknown ICST clock %s\n", name);
+		return;
+	}
+
+	/* Parent clock name is not the same as node parent */
+	parent_name = of_clk_get_parent_name(np, 0);
+
+	regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map);
+	if (IS_ERR(regclk)) {
+		pr_err("error setting up syscon ICST clock %s\n", name);
+		return;
+	}
+	of_clk_add_provider(np, of_clk_src_simple_get, regclk);
+	pr_info("registered syscon ICST clock %s\n", name);
+}
+
+CLK_OF_DECLARE(arm_syscon_icst525_clk,
+	       "arm,syscon-icst525", of_syscon_icst_setup);
+CLK_OF_DECLARE(arm_syscon_icst307_clk,
+	       "arm,syscon-icst307", of_syscon_icst_setup);
+
+#endif
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 10/13] soc: versatile: add support for the PB11MPCore
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
                   ` (7 preceding siblings ...)
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
  2015-10-15 13:46 ` [PATCH 11/13] ARM: realview: select SP810 and ICST for the DT variant Linus Walleij
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The SoC driver needs a minor update to display the correct
sysfs information for the PB11MPCore.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I wonder if I should Cc: arm at kernel.org on patches like this.
They seem to be the default drivers/soc maintainers.
---
 drivers/soc/versatile/soc-realview.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soc/versatile/soc-realview.c b/drivers/soc/versatile/soc-realview.c
index e642c4540dda..c337764de867 100644
--- a/drivers/soc/versatile/soc-realview.c
+++ b/drivers/soc/versatile/soc-realview.c
@@ -36,6 +36,8 @@ static const char *realview_board_str(u32 id)
 	switch ((id >> 16) & 0xfff) {
 	case 0x0147:
 		return "HBI-0147";
+	case 0x0159:
+		return "HBI-0159";
 	default:
 		return "Unknown";
 	}
@@ -44,6 +46,8 @@ static const char *realview_board_str(u32 id)
 static const char *realview_arch_str(u32 id)
 {
 	switch ((id >> 8) & 0xf) {
+	case 0x04:
+		return "AHB";
 	case 0x05:
 		return "Multi-layer AXI";
 	default:
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 11/13] ARM: realview: select SP810 and ICST for the DT variant
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
                   ` (8 preceding siblings ...)
  2015-10-15 13:46 ` [PATCH 10/13] soc: versatile: add support for the PB11MPCore Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
  2015-10-15 13:46 ` [PATCH 12/13] ARM: realview: add an DT SMP boot method Linus Walleij
  2015-10-15 13:46 ` [PATCH 13/13] ARM: realview: add device tree for PB11MPCore Linus Walleij
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

The device tree boot for RealView need the SP810 system controller
(same as found on the Versatile Express) to set up the timers on the
board so the machine can tick. It further utilize the ICST307 through
its system controller for 6 other oscillators. We have to select these
from Kconfig or the machine does not boot.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-realview/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index 565925f37dc5..ea0b1d13eef6 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -4,6 +4,8 @@ menu "RealView platform type"
 config REALVIEW_DT
 	bool "Support RealView(R) Device Tree based boot"
 	select ARM_GIC
+	select CLK_SP810
+	select ICST
 	select MFD_SYSCON
 	select POWER_RESET
 	select POWER_RESET_VERSATILE
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 12/13] ARM: realview: add an DT SMP boot method
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
                   ` (9 preceding siblings ...)
  2015-10-15 13:46 ` [PATCH 11/13] ARM: realview: select SP810 and ICST for the DT variant Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
  2015-10-15 13:46 ` [PATCH 13/13] ARM: realview: add device tree for PB11MPCore Linus Walleij
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

This adds an SMP boot method for the ARM RealView reference
designs. We also select HAVE_SMP by default and make it use
SMP_ON_UP so we only need to support one single kernel across
the RealView reference designs when using DT.

The RealViews need to have the SCU (Snoop Control Unit)
activated on boot, and this is now done by looking up its
address from the device tree and initializing it and counting
the available cores.

The RealViews boot by using a magic address register in the
system controller (SYS_FLAGS) to store the boot address,
the ROM will then read this register to the PC when the CPUs
are taken out of WFI. This code uses a handle to the syscon
regmap to access this register.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/arm/cpus.txt |  1 +
 arch/arm/mach-realview/Kconfig                 |  2 +
 arch/arm/mach-realview/Makefile                |  2 +-
 arch/arm/mach-realview/platsmp-dt.c            | 91 ++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-realview/platsmp-dt.c

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 91e6e5c478d0..95735e1f83ae 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -190,6 +190,7 @@ nodes to be present and contain the properties described below.
 			    "allwinner,sun6i-a31"
 			    "allwinner,sun8i-a23"
 			    "arm,psci"
+			    "arm,realview-smp"
 			    "brcm,brahma-b15"
 			    "marvell,armada-375-smp"
 			    "marvell,armada-380-smp"
diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index ea0b1d13eef6..a41e0911c2fe 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -5,11 +5,13 @@ config REALVIEW_DT
 	bool "Support RealView(R) Device Tree based boot"
 	select ARM_GIC
 	select CLK_SP810
+	select HAVE_SMP
 	select ICST
 	select MFD_SYSCON
 	select POWER_RESET
 	select POWER_RESET_VERSATILE
 	select POWER_SUPPLY
+	select SMP_ON_UP
 	select SOC_REALVIEW
 	select USE_OF
 	help
diff --git a/arch/arm/mach-realview/Makefile b/arch/arm/mach-realview/Makefile
index e07fdf7ae8a7..a46fa694cf07 100644
--- a/arch/arm/mach-realview/Makefile
+++ b/arch/arm/mach-realview/Makefile
@@ -3,7 +3,7 @@
 #
 
 obj-y					:= core.o
-obj-$(CONFIG_REALVIEW_DT)		+= realview-dt.o
+obj-$(CONFIG_REALVIEW_DT)		+= realview-dt.o platsmp-dt.o
 obj-$(CONFIG_MACH_REALVIEW_EB)		+= realview_eb.o
 obj-$(CONFIG_MACH_REALVIEW_PB11MP)	+= realview_pb11mp.o
 obj-$(CONFIG_MACH_REALVIEW_PB1176)	+= realview_pb1176.o
diff --git a/arch/arm/mach-realview/platsmp-dt.c b/arch/arm/mach-realview/platsmp-dt.c
new file mode 100644
index 000000000000..65585392655b
--- /dev/null
+++ b/arch/arm/mach-realview/platsmp-dt.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2015 Linus Walleij
+ *
+ * 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.
+ */
+#include <linux/smp.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+#include <plat/platsmp.h>
+
+#include "core.h"
+
+#define REALVIEW_SYS_FLAGSSET_OFFSET	0x30
+
+static const struct of_device_id realview_scu_match[] = {
+	{ .compatible = "arm,arm11mp-scu", },
+	{ .compatible = "arm,cortex-a9-scu", },
+	{ .compatible = "arm,cortex-a5-scu", },
+	{ }
+};
+
+static const struct of_device_id realview_syscon_match[] = {
+        { .compatible = "arm,core-module-integrator", },
+        { .compatible = "arm,realview-eb-syscon", },
+        { .compatible = "arm,realview-pb11mp-syscon", },
+        { .compatible = "arm,realview-pbx-syscon", },
+        { },
+};
+
+static void __init realview_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *np;
+	void __iomem *scu_base;
+	struct regmap *map;
+	unsigned int ncores;
+	int i;
+
+	np = of_find_matching_node(NULL, realview_scu_match);
+	if (!np) {
+		pr_err("PLATSMP: No SCU base address\n");
+		return;
+	}
+	scu_base = of_iomap(np, 0);
+	of_node_put(np);
+	if (!scu_base) {
+		pr_err("PLATSMP: No SCU remap\n");
+		return;
+	}
+
+	scu_enable(scu_base);
+	ncores = scu_get_core_count(scu_base);
+	pr_info("SCU: %d cores detected\n", ncores);
+	for (i = 0; i < ncores; i++)
+		set_cpu_possible(i, true);
+	iounmap(scu_base);
+
+	/* The syscon contains the magic SMP start address registers */
+	np = of_find_matching_node(NULL, realview_syscon_match);
+	if (!np) {
+		pr_err("PLATSMP: No syscon match\n");
+		return;
+	}
+	map = syscon_node_to_regmap(np);
+	if (IS_ERR(map)) {
+		pr_err("PLATSMP: No syscon regmap\n");
+		return;
+	}
+	/* Put the boot address in this magic register */
+	regmap_write(map, REALVIEW_SYS_FLAGSSET_OFFSET,
+		     virt_to_phys(versatile_secondary_startup));
+}
+
+struct smp_operations realview_dt_smp_ops __initdata = {
+	.smp_prepare_cpus	= realview_smp_prepare_cpus,
+	.smp_secondary_init	= versatile_secondary_init,
+	.smp_boot_secondary	= versatile_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_die		= realview_cpu_die,
+#endif
+};
+CPU_METHOD_OF_DECLARE(realview_smp, "arm,realview-smp", &realview_dt_smp_ops);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 13/13] ARM: realview: add device tree for PB11MPCore
  2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
                   ` (10 preceding siblings ...)
  2015-10-15 13:46 ` [PATCH 12/13] ARM: realview: add an DT SMP boot method Linus Walleij
@ 2015-10-15 13:46 ` Linus Walleij
  11 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-15 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

This adds a device tree for the ARM RealView ARM11MPCore
reference design.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/Makefile                |   3 +-
 arch/arm/boot/dts/arm-realview-pb11mp.dts | 681 ++++++++++++++++++++++++++++++
 2 files changed, 683 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/arm-realview-pb11mp.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index bb8fa023d574..44d7b8436c4f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -496,7 +496,8 @@ dtb-$(CONFIG_ARCH_QCOM) += \
 	qcom-msm8960-cdp.dtb \
 	qcom-msm8974-sony-xperia-honami.dtb
 dtb-$(CONFIG_ARCH_REALVIEW) += \
-	arm-realview-pb1176.dtb
+	arm-realview-pb1176.dtb \
+	arm-realview-pb11mp.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += \
 	rk3066a-bqcurie2.dtb \
 	rk3066a-marsboard.dtb \
diff --git a/arch/arm/boot/dts/arm-realview-pb11mp.dts b/arch/arm/boot/dts/arm-realview-pb11mp.dts
new file mode 100644
index 000000000000..332bb0afb95f
--- /dev/null
+++ b/arch/arm/boot/dts/arm-realview-pb11mp.dts
@@ -0,0 +1,681 @@
+/*
+ * Copyright 2015 Linaro Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/gpio/gpio.h>
+#include "skeleton.dtsi"
+
+/ {
+	model = "ARM RealView PB11MPcore";
+	compatible = "arm,realview-pb11mp";
+
+	chosen { };
+
+	aliases {
+		serial0 = &pb11mp_serial0;
+		serial1 = &pb11mp_serial1;
+		serial2 = &pb11mp_serial2;
+		serial3 = &pb11mp_serial3;
+	};
+
+	memory {
+		/*
+		 * The PB11MPCore has 512 MiB memory @ 0x70000000
+		 * and the first 256 are also remapped @ 0x00000000
+		 */
+		reg = <0x70000000 0x20000000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "arm,realview-smp";
+
+		MP11_0: cpu at 0 {
+			device_type = "cpu";
+			compatible = "arm,arm11mpcore";
+			reg = <0>;
+			next-level-cache = <&L2>;
+		};
+
+		MP11_1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,arm11mpcore";
+			reg = <1>;
+			next-level-cache = <&L2>;
+		};
+
+		MP11_2: cpu at 2 {
+			device_type = "cpu";
+			compatible = "arm,arm11mpcore";
+			reg = <2>;
+			next-level-cache = <&L2>;
+		};
+
+		MP11_3: cpu at 3 {
+			device_type = "cpu";
+			compatible = "arm,arm11mpcore";
+			reg = <3>;
+			next-level-cache = <&L2>;
+		};
+	};
+
+	/* Primary TestChip GIC synthesized with the CPU */
+	intc_tc11mp: interrupt-controller at 1f000100 {
+		compatible = "arm,tc11mp-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <1>;
+		interrupt-controller;
+		reg = <0x1f001000 0x1000>,
+		      <0x1f000100 0x100>;
+	};
+
+	L2: l2-cache {
+		compatible = "arm,l220-cache";
+		reg = <0x1f002000 0x1000>;
+		interrupt-parent = <&intc_tc11mp>;
+		interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 30 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 31 IRQ_TYPE_LEVEL_HIGH>;
+		cache-unified;
+		cache-level = <2>;
+		/*
+		 * Override default cache size, sets and
+		 * associativity as these may be erroneously set
+		 * up by boot loader(s).
+		 */
+		cache-size = <1048576>; // 1MB
+		cache-sets = <4096>;
+		cache-line-size = <32>;
+		arm,shared-override;
+		arm,eventmon-enable;
+		arm,parity-enable;
+	};
+
+	scu at 1f000000 {
+		compatible = "arm,arm11mp-scu";
+		reg = <0x1f000000 0x100>;
+	};
+
+	timer at 1f000600 {
+		compatible = "arm,arm11mp-twd-timer";
+		reg = <0x1f000600 0x20>;
+		interrupt-parent = <&intc_tc11mp>;
+		interrupts = <1 13 0xf04>;
+	};
+
+	watchdog at 1f000620 {
+		compatible = "arm,arm11mp-twd-wdt";
+		reg = <0x1f000620 0x20>;
+		interrupt-parent = <&intc_tc11mp>;
+		interrupts = <1 14 0xf04>;
+	};
+
+	/* PMU with one IRQ line per core */
+	pmu {
+		compatible = "arm,arm11mpcore-pmu";
+		interrupt-parent = <&intc_tc11mp>;
+		interrupts = <0 17 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 18 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 19 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 20 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-affinity = <&MP11_0>, <&MP11_1>, <&MP11_2>, <&MP11_3>;
+	};
+
+	/* The voltage to the MMC card is hardwired at 3.3V */
+	vmmc: fixedregulator at 0 {
+		compatible = "regulator-fixed";
+		regulator-name = "vmmc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+        };
+
+	veth: fixedregulator at 0 {
+		compatible = "regulator-fixed";
+		regulator-name = "veth";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+	};
+
+	xtal24mhz: xtal24mhz at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+	};
+
+	refclk32khz: refclk32khz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
+
+	timclk: timclk at 1M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <24>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	mclk: mclk at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <1>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	kmiclk: kmiclk at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <1>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	sspclk: sspclk at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <1>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	uartclk: uartclk at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <1>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	wdogclk: wdogclk at 24M {
+		#clock-cells = <0>;
+		compatible = "fixed-factor-clock";
+		clock-div = <1>;
+		clock-mult = <1>;
+		clocks = <&xtal24mhz>;
+	};
+
+	/* FIXME: this actually hangs off the PLL clocks */
+	pclk: pclk at 0 {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <0>;
+	};
+
+	flash0 at 40000000 {
+		/* 2 * 32MiB NOR Flash memory */
+		compatible = "arm,vexpress-flash", "cfi-flash";
+		linux,part-probe = "afs";
+		reg = <0x40000000 0x04000000>;
+		bank-width = <4>;
+	};
+
+	flash1 at 44000000 {
+		// 2 * 32MiB NOR Flash memory
+		compatible = "arm,vexpress-flash", "cfi-flash";
+		linux,part-probe = "afs";
+		reg = <0x44000000 0x04000000>;
+		bank-width = <4>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "arm,realview-pb11mp-soc", "simple-bus";
+		regmap = <&pb11mp_syscon>;
+		ranges;
+
+		pb11mp_syscon: syscon at 10000000 {
+			compatible = "arm,realview-pb11mp-syscon", "syscon", "simple-mfd";
+			reg = <0x10000000 0x1000>;
+
+			led at 08.0 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x01>;
+				label = "versatile:0";
+				linux,default-trigger = "heartbeat";
+				default-state = "on";
+			};
+			led at 08.1 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x02>;
+				label = "versatile:1";
+				linux,default-trigger = "mmc0";
+				default-state = "off";
+			};
+			led at 08.2 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x04>;
+				label = "versatile:2";
+				linux,default-trigger = "cpu0";
+				default-state = "off";
+			};
+			led at 08.3 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x08>;
+				label = "versatile:3";
+				linux,default-trigger = "cpu1";
+				default-state = "off";
+			};
+			led at 08.4 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x10>;
+				label = "versatile:4";
+				linux,default-trigger = "cpu2";
+				default-state = "off";
+			};
+			led at 08.5 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x20>;
+				label = "versatile:5";
+				linux,default-trigger = "cpu3";
+				default-state = "off";
+			};
+			led at 08.6 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x40>;
+				label = "versatile:6";
+				default-state = "off";
+			};
+			led at 08.7 {
+				compatible = "register-bit-led";
+				offset = <0x08>;
+				mask = <0x80>;
+				label = "versatile:7";
+				default-state = "off";
+			};
+
+			oscclk0: osc0 at 0c {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0x0C>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk1: osc1 at 10 {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0x10>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk2: osc2 at 14 {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0x14>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk3: osc3 at 18 {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0x18>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk4: osc4 at 1c {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0x1c>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk5: osc5 at d4 {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0xd4>;
+				clocks = <&xtal24mhz>;
+			};
+			oscclk6: osc6 at d8 {
+				compatible = "arm,syscon-icst307";
+				#clock-cells = <0>;
+				lock-offset = <0x20>;
+				vco-offset = <0xd8>;
+				clocks = <&xtal24mhz>;
+			};
+		};
+
+		sp810_syscon: sysctl at 10001000 {
+			compatible = "arm,sp810", "arm,primecell";
+			reg = <0x10001000 0x1000>;
+			clocks = <&refclk32khz>, <&timclk>, <&xtal24mhz>;
+			clock-names = "refclk", "timclk", "apb_pclk";
+			#clock-cells = <1>;
+			clock-output-names = "timerclk0",
+					     "timerclk1",
+					     "timerclk2",
+					     "timerclk3";
+			assigned-clocks = <&sp810_syscon 0>,
+					  <&sp810_syscon 1>,
+					  <&sp810_syscon 2>,
+					  <&sp810_syscon 3>;
+			assigned-clock-parents = <&timclk>,
+					       <&timclk>,
+					       <&timclk>,
+					       <&timclk>;
+		};
+
+		i2c0: i2c at 10002000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "arm,versatile-i2c";
+			reg = <0x10002000 0x1000>;
+
+			rtc at 68 {
+				compatible = "dallas,ds1338";
+				reg = <0x68>;
+			};
+		};
+
+		aaci: aaci at 10004000 {
+			compatible = "arm,pl041", "arm,primecell";
+			reg = <0x10004000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+		};
+
+		mci: mmcsd at 10005000 {
+			compatible = "arm,pl18x", "arm,primecell";
+			reg = <0x10005000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 14 IRQ_TYPE_LEVEL_HIGH>,
+					<0 15 IRQ_TYPE_LEVEL_HIGH>;
+			/* Due to frequent FIFO overruns, use just 500 kHz */
+			max-frequency = <500000>;
+			bus-width = <4>;
+			cap-sd-highspeed;
+			cap-mmc-highspeed;
+			clocks = <&mclk>, <&pclk>;
+			clock-names = "mclk", "apb_pclk";
+			vmmc-supply = <&vmmc>;
+			cd-gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
+			wp-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+		};
+
+		kmi0: kmi at 10006000 {
+			compatible = "arm,pl050", "arm,primecell";
+			reg = <0x10006000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 7 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&kmiclk>, <&pclk>;
+			clock-names = "KMIREFCLK", "apb_pclk";
+		};
+
+		kmi1: kmi at 10007000 {
+			compatible = "arm,pl050", "arm,primecell";
+			reg = <0x10007000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&kmiclk>, <&pclk>;
+			clock-names = "KMIREFCLK", "apb_pclk";
+		};
+
+		pb11mp_serial0: serial at 10009000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x10009000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&uartclk>, <&pclk>;
+			clock-names = "uartclk", "apb_pclk";
+		};
+
+		pb11mp_serial1: serial at 1000a000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1000a000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&uartclk>, <&pclk>;
+			clock-names = "uartclk", "apb_pclk";
+		};
+
+		pb11mp_serial2: serial at 1000b000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1000b000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 14 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&uartclk>, <&pclk>;
+			clock-names = "uartclk", "apb_pclk";
+		};
+
+		pb11mp_serial3: serial at 1000c000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1000c000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 15 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&uartclk>, <&pclk>;
+			clock-names = "uartclk", "apb_pclk";
+		};
+
+		ssp at 1000d000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x1000d000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 11 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sspclk>, <&pclk>;
+			clock-names = "SSPCLK", "apb_pclk";
+		};
+
+		watchdog at 1000f000 {
+			compatible = "arm,sp805", "arm,primecell";
+			reg = <0x1000f000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&wdogclk>, <&pclk>;
+			clock-names = "wdogclk", "apb_pclk";
+			status = "disabled";
+		};
+
+		watchdog at 10010000 {
+			compatible = "arm,sp805", "arm,primecell";
+			reg = <0x10010000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&wdogclk>, <&pclk>;
+			clock-names = "wdogclk", "apb_pclk";
+		};
+
+		timer01: timer at 10011000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0x10011000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 1 IRQ_TYPE_LEVEL_HIGH>;
+			arm,sp804-has-irq = <1>;
+			clocks = <&sp810_syscon 0>,
+			         <&sp810_syscon 1>,
+				 <&pclk>;
+			clock-names = "timerclk0",
+				    "timerclk1",
+				    "apb_pclk";
+		};
+
+		timer23: timer at 10012000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0x10012000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>;
+			arm,sp804-has-irq = <1>;
+			clocks = <&sp810_syscon 2>,
+			         <&sp810_syscon 3>,
+				 <&pclk>;
+			clock-names = "timerclk2",
+				    "timerclk3",
+				    "apb_pclk";
+		};
+
+		gpio0: gpio at 10013000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x10013000 0x1000>;
+			gpio-controller;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 6 IRQ_TYPE_LEVEL_HIGH>;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+		};
+
+		gpio1: gpio at 10014000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x10014000 0x1000>;
+			gpio-controller;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 7 IRQ_TYPE_LEVEL_HIGH>;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+		};
+
+		gpio2: gpio at 10015000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x10015000 0x1000>;
+			gpio-controller;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+		};
+
+		rtc: rtc at 10017000 {
+			compatible = "arm,pl031", "arm,primecell";
+			reg = <0x10017000 0x1000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 6 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pclk>;
+			clock-names = "apb_pclk";
+		};
+
+		timer45: timer at 10018000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0x10018000 0x1000>;
+			clocks = <&timclk>, <&pclk>;
+			clock-names = "timer", "apb_pclk";
+			status = "disabled";
+		};
+
+		timer67: timer at 10019000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0x10019000 0x1000>;
+			clocks = <&timclk>, <&pclk>;
+			clock-names = "timer", "apb_pclk";
+			status = "disabled";
+		};
+
+
+		clcd at 10020000 {
+			compatible = "arm,pl111", "arm,primecell";
+			reg = <0x10020000 0x1000>;
+			interrupt-parent = <&intc_pb11mp>;
+			interrupt-names = "combined";
+			interrupts = <0 23 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&oscclk4>, <&pclk>;
+			clock-names = "clcdclk", "apb_pclk";
+			max-memory-bandwidth = <130000000>; /* 16bpp @ 63.5MHz */
+
+			port {
+				clcd_pads: endpoint {
+					remote-endpoint = <&clcd_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
+				};
+			};
+
+			panel {
+				compatible = "panel-dpi";
+
+				port {
+					clcd_panel: endpoint {
+						remote-endpoint = <&clcd_pads>;
+					};
+				};
+
+				panel-timing {
+					clock-frequency = <63500127>;
+					hactive = <1024>;
+					hback-porch = <152>;
+					hfront-porch = <48>;
+					hsync-len = <104>;
+					vactive = <768>;
+					vback-porch = <23>;
+					vfront-porch = <3>;
+					vsync-len = <4>;
+				};
+			};
+		};
+
+		/*
+		 * This GIC on the Platform Baseboard is cascaded off the
+		 * TestChip GIC
+		 */
+		intc_pb11mp: interrupt-controller at 1e000000 {
+			compatible = "arm,pb11mp-gic";
+			#interrupt-cells = <3>;
+			#address-cells = <1>;
+			interrupt-controller;
+			reg = <0x1e001000 0x1000>,
+			      <0x1e000000 0x100>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 10 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		/* SMSC 9118 ethernet with PHY and EEPROM */
+		ethernet at 4e000000 {
+			compatible = "smsc,lan9118", "smsc,lan9115";
+			reg = <0x4e000000 0x10000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
+			phy-mode = "mii";
+			reg-io-width = <4>;
+			smsc,irq-active-high;
+			smsc,irq-push-pull;
+			vdd33a-supply = <&veth>;
+			vddvario-supply = <&veth>;
+		};
+
+		usb at 4f000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <0x4f000000 0x20000>;
+			interrupt-parent = <&intc_tc11mp>;
+			interrupts = <0 3 IRQ_TYPE_LEVEL_HIGH>;
+			port1-otg;
+		};
+	};
+};
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/13] ARM: add some L220 DT settings
  2015-10-15 13:46     ` Linus Walleij
@ 2015-10-15 13:57         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 62+ messages in thread
From: Russell King - ARM Linux @ 2015-10-15 13:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 15, 2015 at 03:46:41PM +0200, Linus Walleij wrote:
> The RealView ARM11MPCore enables parity, eventmon and shared
> override in the cache controller through its current boardfile,
> but the code and DT bindings for the ARM L220 is currently
> lacking the ability to set this up from DT. Add the required
> bool parameters.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> I know this patch mixes code and DT changes but it is silly to
> split such a small patch. Will submit this to Russell's patch
> tracker if it looks OK to the DT people. (Or if they are quiet.)
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
>  arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index 06c88a4d28ac..4d262e9b3464 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -67,12 +67,14 @@ Optional properties:
>    disable if zero.
>  - arm,prefetch-offset : Override prefetch offset value. Valid values are
>    0-7, 15, 23, and 31.
> -- arm,shared-override : The default behavior of the pl310 cache controller with
> -  respect to the shareable attribute is to transform "normal memory
> -  non-cacheable transactions" into "cacheable no allocate" (for reads) or
> -  "write through no write allocate" (for writes).
> +- arm,shared-override : The default behavior of the PL220 or PL310 cache
> +  controllers with respect to the shareable attribute is to transform "normal
> +  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
> +  or "write through no write allocate" (for writes).
>    On systems where this may cause DMA buffer corruption, this property must be
>    specified to indicate that such transforms are precluded.
> +- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).
> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).

I don't think we should introduce a DT property for this: if we support
the event monitor, then the event monitor support code should be
controlling this bit.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 01/13] ARM: add some L220 DT settings
@ 2015-10-15 13:57         ` Russell King - ARM Linux
  0 siblings, 0 replies; 62+ messages in thread
From: Russell King - ARM Linux @ 2015-10-15 13:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 03:46:41PM +0200, Linus Walleij wrote:
> The RealView ARM11MPCore enables parity, eventmon and shared
> override in the cache controller through its current boardfile,
> but the code and DT bindings for the ARM L220 is currently
> lacking the ability to set this up from DT. Add the required
> bool parameters.
> 
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I know this patch mixes code and DT changes but it is silly to
> split such a small patch. Will submit this to Russell's patch
> tracker if it looks OK to the DT people. (Or if they are quiet.)
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
>  arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index 06c88a4d28ac..4d262e9b3464 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -67,12 +67,14 @@ Optional properties:
>    disable if zero.
>  - arm,prefetch-offset : Override prefetch offset value. Valid values are
>    0-7, 15, 23, and 31.
> -- arm,shared-override : The default behavior of the pl310 cache controller with
> -  respect to the shareable attribute is to transform "normal memory
> -  non-cacheable transactions" into "cacheable no allocate" (for reads) or
> -  "write through no write allocate" (for writes).
> +- arm,shared-override : The default behavior of the PL220 or PL310 cache
> +  controllers with respect to the shareable attribute is to transform "normal
> +  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
> +  or "write through no write allocate" (for writes).
>    On systems where this may cause DMA buffer corruption, this property must be
>    specified to indicate that such transforms are precluded.
> +- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).
> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).

I don't think we should introduce a DT property for this: if we support
the event monitor, then the event monitor support code should be
controlling this bit.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/13] ARM: add some L220 DT settings
  2015-10-15 13:46     ` Linus Walleij
@ 2015-10-15 13:58         ` Rob Herring
  -1 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2015-10-15 13:58 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Russell King, Pawel Moll, Mark Rutland, Marc Zyngier,
	Will Deacon, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 15, 2015 at 8:46 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> The RealView ARM11MPCore enables parity, eventmon and shared
> override in the cache controller through its current boardfile,
> but the code and DT bindings for the ARM L220 is currently
> lacking the ability to set this up from DT. Add the required
> bool parameters.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> I know this patch mixes code and DT changes but it is silly to
> split such a small patch. Will submit this to Russell's patch
> tracker if it looks OK to the DT people. (Or if they are quiet.)
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
>  arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index 06c88a4d28ac..4d262e9b3464 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -67,12 +67,14 @@ Optional properties:
>    disable if zero.
>  - arm,prefetch-offset : Override prefetch offset value. Valid values are
>    0-7, 15, 23, and 31.
> -- arm,shared-override : The default behavior of the pl310 cache controller with
> -  respect to the shareable attribute is to transform "normal memory
> -  non-cacheable transactions" into "cacheable no allocate" (for reads) or
> -  "write through no write allocate" (for writes).
> +- arm,shared-override : The default behavior of the PL220 or PL310 cache

PL220 is something else:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0249b/CHDIIEAD.html


> +  controllers with respect to the shareable attribute is to transform "normal
> +  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
> +  or "write through no write allocate" (for writes).

I seem to recall the PL310 TRM says this bit is different from the
L220 or the default is.

>    On systems where this may cause DMA buffer corruption, this property must be
>    specified to indicate that such transforms are precluded.
> +- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).

PL310 has parity.

> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).

and eventmon.

There's a slight problem here in that you can turn on these with DT,
but you can't turn them off as absence means don't touch. Maybe a
value of 0 should be allowed for disabling.

>  - prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1>
>    (forcibly enable), property absent (retain settings set by firmware)
>  - prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable),
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 493692d838c6..d4e9fa2594f3 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1060,6 +1060,21 @@ static void __init l2x0_of_parse(const struct device_node *np,
>                 val |= (dirty - 1) << L2X0_AUX_CTRL_DIRTY_LATENCY_SHIFT;
>         }
>
> +       if (of_property_read_bool(np, "arm,parity-enable")) {
> +               mask &= ~L2C_AUX_CTRL_PARITY_ENABLE;
> +               val |= L2C_AUX_CTRL_PARITY_ENABLE;
> +       }
> +
> +       if (of_property_read_bool(np, "arm,eventmon-enable")) {
> +               mask &= ~L2C_AUX_CTRL_EVTMON_ENABLE;
> +               val |= L2C_AUX_CTRL_EVTMON_ENABLE;
> +       }
> +
> +       if (of_property_read_bool(np, "arm,shared-override")) {
> +               mask &= ~L2C_AUX_CTRL_SHARED_OVERRIDE;
> +               val |= L2C_AUX_CTRL_SHARED_OVERRIDE;
> +       }
> +
>         ret = l2x0_cache_size_of_parse(np, aux_val, aux_mask, &assoc, SZ_256K);
>         if (ret)
>                 return;
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 01/13] ARM: add some L220 DT settings
@ 2015-10-15 13:58         ` Rob Herring
  0 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2015-10-15 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 8:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> The RealView ARM11MPCore enables parity, eventmon and shared
> override in the cache controller through its current boardfile,
> but the code and DT bindings for the ARM L220 is currently
> lacking the ability to set this up from DT. Add the required
> bool parameters.
>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I know this patch mixes code and DT changes but it is silly to
> split such a small patch. Will submit this to Russell's patch
> tracker if it looks OK to the DT people. (Or if they are quiet.)
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++----
>  arch/arm/mm/cache-l2x0.c                       | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index 06c88a4d28ac..4d262e9b3464 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -67,12 +67,14 @@ Optional properties:
>    disable if zero.
>  - arm,prefetch-offset : Override prefetch offset value. Valid values are
>    0-7, 15, 23, and 31.
> -- arm,shared-override : The default behavior of the pl310 cache controller with
> -  respect to the shareable attribute is to transform "normal memory
> -  non-cacheable transactions" into "cacheable no allocate" (for reads) or
> -  "write through no write allocate" (for writes).
> +- arm,shared-override : The default behavior of the PL220 or PL310 cache

PL220 is something else:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0249b/CHDIIEAD.html


> +  controllers with respect to the shareable attribute is to transform "normal
> +  memory non-cacheable transactions" into "cacheable no allocate" (for reads)
> +  or "write through no write allocate" (for writes).

I seem to recall the PL310 TRM says this bit is different from the
L220 or the default is.

>    On systems where this may cause DMA buffer corruption, this property must be
>    specified to indicate that such transforms are precluded.
> +- arm,parity-enable : enable parity checking on the L2 cache (PL220 only).

PL310 has parity.

> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).

and eventmon.

There's a slight problem here in that you can turn on these with DT,
but you can't turn them off as absence means don't touch. Maybe a
value of 0 should be allowed for disabling.

>  - prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1>
>    (forcibly enable), property absent (retain settings set by firmware)
>  - prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable),
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 493692d838c6..d4e9fa2594f3 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1060,6 +1060,21 @@ static void __init l2x0_of_parse(const struct device_node *np,
>                 val |= (dirty - 1) << L2X0_AUX_CTRL_DIRTY_LATENCY_SHIFT;
>         }
>
> +       if (of_property_read_bool(np, "arm,parity-enable")) {
> +               mask &= ~L2C_AUX_CTRL_PARITY_ENABLE;
> +               val |= L2C_AUX_CTRL_PARITY_ENABLE;
> +       }
> +
> +       if (of_property_read_bool(np, "arm,eventmon-enable")) {
> +               mask &= ~L2C_AUX_CTRL_EVTMON_ENABLE;
> +               val |= L2C_AUX_CTRL_EVTMON_ENABLE;
> +       }
> +
> +       if (of_property_read_bool(np, "arm,shared-override")) {
> +               mask &= ~L2C_AUX_CTRL_SHARED_OVERRIDE;
> +               val |= L2C_AUX_CTRL_SHARED_OVERRIDE;
> +       }
> +
>         ret = l2x0_cache_size_of_parse(np, aux_val, aux_mask, &assoc, SZ_256K);
>         if (ret)
>                 return;
> --
> 2.4.3
>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 04/13] irqchip/gic: support RealView variant setup
  2015-10-15 13:46 ` [PATCH 04/13] irqchip/gic: support RealView variant setup Linus Walleij
@ 2015-10-15 16:06   ` Marc Zyngier
  2015-10-16  8:28   ` Thomas Gleixner
  1 sibling, 0 replies; 62+ messages in thread
From: Marc Zyngier @ 2015-10-15 16:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 15/10/15 14:46, Linus Walleij wrote:
> The ARM RealView PB11MPCore reference design has some special
> bits in a system controller register to set up the GIC in one
> of three modes: legacy, new with DCC, new without DCC. The
> register is also used to enable FIQ.
> 
> Since the platform will not boot unless this register is set
> up to "new with DCC" mode, we need a special quirk to be
> compiled-in for the RealView platforms.
> 
> If we find the right compatible string on the GIC TestChip,
> we enable this quirk by looking up the system controller and
> enabling the special bits.
> 
> We depend on the CONFIG_REALVIEW_DT Kconfig symbol as the old
> boardfile code has the same fix hardcoded, and this is only
> needed for the attempts to modernize the RealView code using
> device tree.
> 
> After fixing this, the PB11MPCore boots with device tree
> only.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Looking for an ACK for this (unless the irqchip maintainers
> barf at it) so I can take it through the ARM SoC tree.
> ---
>  drivers/irqchip/Makefile           |  1 +
>  drivers/irqchip/irq-gic-realview.c | 39 ++++++++++++++++++++++++++++++++++++++
>  drivers/irqchip/irq-gic-realview.h |  5 +++++
>  drivers/irqchip/irq-gic.c          |  4 ++++
>  4 files changed, 49 insertions(+)
>  create mode 100644 drivers/irqchip/irq-gic-realview.c
>  create mode 100644 drivers/irqchip/irq-gic-realview.h
> 
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index bb3048f00e64..7a7d4182777d 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
>  obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi-nmi.o
>  obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
>  obj-$(CONFIG_ARM_GIC)			+= irq-gic.o irq-gic-common.o
> +obj-$(CONFIG_REALVIEW_DT)		+= irq-gic-realview.o
>  obj-$(CONFIG_ARM_GIC_V2M)		+= irq-gic-v2m.o
>  obj-$(CONFIG_ARM_GIC_V3)		+= irq-gic-v3.o irq-gic-common.o
>  obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
> diff --git a/drivers/irqchip/irq-gic-realview.c b/drivers/irqchip/irq-gic-realview.c
> new file mode 100644
> index 000000000000..2e8a4b129f5f
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-realview.c
> @@ -0,0 +1,39 @@
> +/*
> + * Special GIC quirks for the ARM RealView
> + * Copyright (C) 2015 Linus Walleij
> + */
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/bitops.h>
> +
> +#define REALVIEW_SYS_LOCK_OFFSET	0x20
> +#define REALVIEW_PB11MP_SYS_PLD_CTRL1	0x74
> +#define VERSATILE_LOCK_VAL		0xA05F
> +#define PLD_INTMODE_MASK		BIT(22)|BIT(23)|BIT(24)
> +#define PLD_INTMODE_LEGACY		0x0
> +#define PLD_INTMODE_NEW_DCC		BIT(22)
> +#define PLD_INTMODE_NEW_NO_DCC		BIT(23)
> +#define PLD_INTMODE_FIQ_ENABLE		BIT(24)
> +
> +void __init realview_gic_init(struct device_node *np)
> +{
> +	static struct regmap *map;
> +
> +	/* The PB11MPCore GIC needs to be configured in the syscon */
> +	if (of_device_is_compatible(np, "arm,tc11mp-gic")) {
> +		map = syscon_regmap_lookup_by_compatible("arm,realview-pb11mp-syscon");
> +		if (!IS_ERR(map)) {
> +			/* new irq mode with no DCC */
> +			regmap_write(map, REALVIEW_SYS_LOCK_OFFSET,
> +				     VERSATILE_LOCK_VAL);
> +			regmap_update_bits(map, REALVIEW_PB11MP_SYS_PLD_CTRL1,
> +					   PLD_INTMODE_NEW_NO_DCC,
> +					   PLD_INTMODE_MASK);
> +			regmap_write(map, REALVIEW_SYS_LOCK_OFFSET, 0x0000);
> +			pr_info("TC11MP GIC: set up interrupt controller to NEW mode, no DCC\n");
> +		} else {
> +			pr_err("TC11MP GIC setup: could not find syscon\n");
> +		}
> +	}
> +}
> diff --git a/drivers/irqchip/irq-gic-realview.h b/drivers/irqchip/irq-gic-realview.h
> new file mode 100644
> index 000000000000..06ebdfc523a8
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-realview.h
> @@ -0,0 +1,5 @@
> +#ifdef CONFIG_REALVIEW_DT
> +void realview_gic_init(struct device_node *np);
> +#else
> +static void realview_gic_init(struct device_node *np) {}
> +#endif
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 5376d1cb0a4f..bd021e1e4847 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -50,6 +50,7 @@
>  #include <asm/virt.h>
>  
>  #include "irq-gic-common.h"
> +#include "irq-gic-realview.h"
>  
>  union gic_base {
>  	void __iomem *common_base;
> @@ -1158,6 +1159,9 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>  	cpu_base = of_iomap(node, 1);
>  	WARN(!cpu_base, "unable to map gic cpu registers\n");
>  
> +	/* Special quirk for ARM RealView */
> +	realview_gic_init(node);
> +

How about having a special entry in the IRQCHIP_OF_DECLARE section,
which would call this, and then gic_of_init? It would look slightly
nicer, and you could from the check in realview_gic_init...

Thoughts?

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 05/13] irqchip/gic: assign irqchip dynamically
  2015-10-15 13:46 ` [PATCH 05/13] irqchip/gic: assign irqchip dynamically Linus Walleij
@ 2015-10-15 16:16   ` Marc Zyngier
  0 siblings, 0 replies; 62+ messages in thread
From: Marc Zyngier @ 2015-10-15 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Linus,

On 15/10/15 14:46, Linus Walleij wrote:
> Instead of having the irqchip being a static struct, make it part
> of the per-instance data so we can assign it a dynamic name. This
> has the usable side effect of displaying the GIC with an instance
> number as GIC0, GIC1 ... GICn in /proc/interrupts, which is helpful
> when debugging cascaded GICs, such as on the ARM PB11MPCore.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Fellas please say what you think about this:
> Yes / No / Linus is an idiot
> This can be applied directly to the irqchip tree if you like
> it, AFAIK it has no dependencies.
> ---
>  drivers/irqchip/irq-gic.c | 36 +++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index bd021e1e4847..478279cf9517 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -58,6 +58,7 @@ union gic_base {
>  };
>  
>  struct gic_chip_data {
> +	struct irq_chip chip;
>  	union gic_base dist_base;
>  	union gic_base cpu_base;
>  #ifdef CONFIG_CPU_PM
> @@ -369,22 +370,6 @@ static void gic_handle_cascade_irq(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> -static struct irq_chip gic_chip = {
> -	.name			= "GIC",
> -	.irq_mask		= gic_mask_irq,
> -	.irq_unmask		= gic_unmask_irq,
> -	.irq_eoi		= gic_eoi_irq,
> -	.irq_set_type		= gic_set_type,
> -#ifdef CONFIG_SMP
> -	.irq_set_affinity	= gic_set_affinity,
> -#endif
> -	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
> -	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
> -	.flags			= IRQCHIP_SET_TYPE_MASKED |
> -				  IRQCHIP_SKIP_SET_WAKE |
> -				  IRQCHIP_MASK_ON_SUSPEND,
> -};
> -
>  static struct irq_chip gic_eoimode1_chip = {
>  	.name			= "GICv2",
>  	.irq_mask		= gic_eoimode1_mask_irq,
> @@ -880,7 +865,8 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> -	struct irq_chip *chip = &gic_chip;
> +	struct gic_chip_data *gic = d->host_data;
> +	struct irq_chip *chip = &gic->chip;
>  
>  	if (static_key_true(&supports_deactivate)) {
>  		if (d->host_data == (void *)&gic_data[0])
> @@ -989,6 +975,22 @@ static void __init __gic_init_bases(unsigned int gic_nr, int irq_start,
>  	BUG_ON(gic_nr >= MAX_GIC_NR);
>  
>  	gic = &gic_data[gic_nr];
> +
> +	/* Initialize irq_chip */
> +	gic->chip.name = kasprintf(GFP_KERNEL, "GIC%d", gic_nr);
> +	gic->chip.irq_mask = gic_mask_irq;
> +	gic->chip.irq_unmask = gic_unmask_irq;
> +	gic->chip.irq_eoi = gic_eoi_irq;
> +	gic->chip.irq_set_type	= gic_set_type;
> +#ifdef CONFIG_SMP
> +	gic->chip.irq_set_affinity = gic_set_affinity;
> +#endif
> +	gic->chip.irq_get_irqchip_state = gic_irq_get_irqchip_state;
> +	gic->chip.irq_set_irqchip_state = gic_irq_set_irqchip_state;
> +	gic->chip.flags = IRQCHIP_SET_TYPE_MASKED |
> +		IRQCHIP_SKIP_SET_WAKE |
> +		IRQCHIP_MASK_ON_SUSPEND;
> +

How does it work when we want to use the ops from gic_eoimode1_chip? I
don't think it breaks, but it is a bit weird.

You could also replace this sequence by keeping the original structure
and assigning it to the embedded one (I don't think you save much by
replacing this with discreet field assignment).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:08     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>  	init.flags = CLK_IS_ROOT;
>  	init.parent_names = (parent_name ? &parent_name : NULL);
>  	init.num_parents = (parent_name ? 1 : 0);
> +	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> +	if (IS_ERR(icst->map)) {
> +		int ret;
> +
> +		pr_err("could not initialize ICST regmap\n");
> +		kfree(icst);
> +		ret = PTR_ERR(icst->map);

drivers/clk/versatile/clk-icst.c:183
icst_clk_register() error: dereferencing freed memory 'icst'
drivers/clk/versatile/clk-icst.c:184
icst_clk_register() warn: possible memory leak of 'pclone'


> +		return ERR_PTR(ret);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-15 19:08     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>  	init.flags = CLK_IS_ROOT;
>  	init.parent_names = (parent_name ? &parent_name : NULL);
>  	init.num_parents = (parent_name ? 1 : 0);
> +	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> +	if (IS_ERR(icst->map)) {
> +		int ret;
> +
> +		pr_err("could not initialize ICST regmap\n");
> +		kfree(icst);
> +		ret = PTR_ERR(icst->map);

drivers/clk/versatile/clk-icst.c:183
icst_clk_register() error: dereferencing freed memory 'icst'
drivers/clk/versatile/clk-icst.c:184
icst_clk_register() warn: possible memory leak of 'pclone'


> +		return ERR_PTR(ret);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:10     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:10 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> @@ -174,15 +170,7 @@ struct clk *icst_clk_register(struct device *dev,
>  	init.flags = CLK_IS_ROOT;
>  	init.parent_names = (parent_name ? &parent_name : NULL);
>  	init.num_parents = (parent_name ? 1 : 0);
> -	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> -	if (IS_ERR(icst->map)) {
> -		int ret;
> -
> -		pr_err("could not initialize ICST regmap\n");
> -		kfree(icst);
> -		ret = PTR_ERR(icst->map);
> -		return ERR_PTR(ret);

Oh now it gets moved.

> -	}
> +	icst->map = map;
>  	icst->hw.init = &init;
>  	icst->params = pclone;
>  	icst->vcoreg_off = desc->vco_offset;
> @@ -194,4 +182,28 @@ struct clk *icst_clk_register(struct device *dev,
>  
>  	return clk;
>  }
> +
> +struct clk *icst_clk_register(struct device *dev,
> +			const struct clk_icst_desc *desc,
> +			const char *name,
> +			const char *parent_name,
> +			void __iomem *base)
> +{
> +	struct regmap_config icst_regmap_conf = {
> +		.reg_bits = 32,
> +		.val_bits = 32,
> +		.reg_stride = 4,
> +	};
> +	struct regmap *map;
> +
> +	map = regmap_init_mmio(NULL, base, &icst_regmap_conf);

Any reason we aren't passing dev as the first argument? Same
comment applies to patch 6.

> +	if (IS_ERR(map)) {
> +		int ret;
> +
> +		pr_err("could not initialize ICST regmap\n");
> +		ret = PTR_ERR(map);
> +		return ERR_PTR(ret);

Use ERR_CAST(map) instead.

> +	}
> +	return icst_clk_setup(dev, desc, name, parent_name, map);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
@ 2015-10-15 19:10     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> @@ -174,15 +170,7 @@ struct clk *icst_clk_register(struct device *dev,
>  	init.flags = CLK_IS_ROOT;
>  	init.parent_names = (parent_name ? &parent_name : NULL);
>  	init.num_parents = (parent_name ? 1 : 0);
> -	icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> -	if (IS_ERR(icst->map)) {
> -		int ret;
> -
> -		pr_err("could not initialize ICST regmap\n");
> -		kfree(icst);
> -		ret = PTR_ERR(icst->map);
> -		return ERR_PTR(ret);

Oh now it gets moved.

> -	}
> +	icst->map = map;
>  	icst->hw.init = &init;
>  	icst->params = pclone;
>  	icst->vcoreg_off = desc->vco_offset;
> @@ -194,4 +182,28 @@ struct clk *icst_clk_register(struct device *dev,
>  
>  	return clk;
>  }
> +
> +struct clk *icst_clk_register(struct device *dev,
> +			const struct clk_icst_desc *desc,
> +			const char *name,
> +			const char *parent_name,
> +			void __iomem *base)
> +{
> +	struct regmap_config icst_regmap_conf = {
> +		.reg_bits = 32,
> +		.val_bits = 32,
> +		.reg_stride = 4,
> +	};
> +	struct regmap *map;
> +
> +	map = regmap_init_mmio(NULL, base, &icst_regmap_conf);

Any reason we aren't passing dev as the first argument? Same
comment applies to patch 6.

> +	if (IS_ERR(map)) {
> +		int ret;
> +
> +		pr_err("could not initialize ICST regmap\n");
> +		ret = PTR_ERR(map);
> +		return ERR_PTR(ret);

Use ERR_CAST(map) instead.

> +	}
> +	return icst_clk_setup(dev, desc, name, parent_name, map);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:23     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring, devicetree,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> diff --git a/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
> new file mode 100644
> index 000000000000..19eb3aa765c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
> @@ -0,0 +1,40 @@
> +ARM System Controller ICST clocks
> +
> +The ICS525 and ICS307 oscillators are produced by Integrated Devices
> +Technology (IDT). ARM integrated these oscillators deeply into their
> +reference designs by adding special control registers that manage such
> +oscillators to their system controllers.
> +
> +The ARM system controller contains logic to serialized and initialize

serialize ?

> +an ICST clock request after a write to the 32 bit register at an offset
> +into the system controller. Further, to even be able to alter one of

Furthermore?

> +these frequencies, the system controller must first be unlocked by
> +writing a special token to another offset in the system controller.

Sounds like a great design!

> +
> +The ICST oscillator must be provided inside a system controller node.
> +
> +Required properties:
> +- lock-offset: the offset address into the system controller where the
> +  unlocking register is located
> +- vco-offset: the offset address into the system controller where the
> +  ICST control register is located (even 32 bit address)

Is there any reason why we don't use a reg property for this?

> +- compatible: must be one of "arm,syscon-icst525" or "arm,syscon-icst307"
> +- #clock-cells: must be <0>
> +- clocks: parent clock, since the ICST needs a parent clock to derive its
> +  frequency from, this attribute is compulsory.
> +
> +Example:
> +
> +syscon: syscon@10000000 {
> +	compatible = "syscon";
> +	reg = <0x10000000 0x1000>;
> +
> +	oscclk0: osc0@0c {
> +		compatible = "arm,syscon-icst307";
> +		#clock-cells = <0>;
> +		lock-offset = <0x20>;
> +		vco-offset = <0x0C>;

lowercase the C?

> +		clocks = <&xtal24mhz>;
> +	};
> +	(...)
> +};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
@ 2015-10-15 19:23     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> diff --git a/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
> new file mode 100644
> index 000000000000..19eb3aa765c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/arm-syscon-icst.txt
> @@ -0,0 +1,40 @@
> +ARM System Controller ICST clocks
> +
> +The ICS525 and ICS307 oscillators are produced by Integrated Devices
> +Technology (IDT). ARM integrated these oscillators deeply into their
> +reference designs by adding special control registers that manage such
> +oscillators to their system controllers.
> +
> +The ARM system controller contains logic to serialized and initialize

serialize ?

> +an ICST clock request after a write to the 32 bit register at an offset
> +into the system controller. Further, to even be able to alter one of

Furthermore?

> +these frequencies, the system controller must first be unlocked by
> +writing a special token to another offset in the system controller.

Sounds like a great design!

> +
> +The ICST oscillator must be provided inside a system controller node.
> +
> +Required properties:
> +- lock-offset: the offset address into the system controller where the
> +  unlocking register is located
> +- vco-offset: the offset address into the system controller where the
> +  ICST control register is located (even 32 bit address)

Is there any reason why we don't use a reg property for this?

> +- compatible: must be one of "arm,syscon-icst525" or "arm,syscon-icst307"
> +- #clock-cells: must be <0>
> +- clocks: parent clock, since the ICST needs a parent clock to derive its
> +  frequency from, this attribute is compulsory.
> +
> +Example:
> +
> +syscon: syscon at 10000000 {
> +	compatible = "syscon";
> +	reg = <0x10000000 0x1000>;
> +
> +	oscclk0: osc0 at 0c {
> +		compatible = "arm,syscon-icst307";
> +		#clock-cells = <0>;
> +		lock-offset = <0x20>;
> +		vco-offset = <0x0C>;

lowercase the C?

> +		clocks = <&xtal24mhz>;
> +	};
> +	(...)
> +};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:26     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:26 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> +
> +	if (of_device_is_compatible(np, "arm,syscon-icst525"))
> +		icst_desc.params = &icst525_params;
> +	else if (of_device_is_compatible(np, "arm,syscon-icst307"))
> +		icst_desc.params = &icst307_params;

I guess if we add anymore here we should use an of_device_id
array instead.

> +	else {
> +		pr_info("unknown ICST clock %s\n", name);

pr_warn? pr_err?

> +		return;
> +	}
> +
> +	/* Parent clock name is not the same as node parent */
> +	parent_name = of_clk_get_parent_name(np, 0);
> +
> +	regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map);
> +	if (IS_ERR(regclk)) {
> +		pr_err("error setting up syscon ICST clock %s\n", name);
> +		return;
> +	}
> +	of_clk_add_provider(np, of_clk_src_simple_get, regclk);
> +	pr_info("registered syscon ICST clock %s\n", name);

debug print? Please remove debug noise.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-15 19:26     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:26 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> +
> +	if (of_device_is_compatible(np, "arm,syscon-icst525"))
> +		icst_desc.params = &icst525_params;
> +	else if (of_device_is_compatible(np, "arm,syscon-icst307"))
> +		icst_desc.params = &icst307_params;

I guess if we add anymore here we should use an of_device_id
array instead.

> +	else {
> +		pr_info("unknown ICST clock %s\n", name);

pr_warn? pr_err?

> +		return;
> +	}
> +
> +	/* Parent clock name is not the same as node parent */
> +	parent_name = of_clk_get_parent_name(np, 0);
> +
> +	regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map);
> +	if (IS_ERR(regclk)) {
> +		pr_err("error setting up syscon ICST clock %s\n", name);
> +		return;
> +	}
> +	of_clk_add_provider(np, of_clk_src_simple_get, regclk);
> +	pr_info("registered syscon ICST clock %s\n", name);

debug print? Please remove debug noise.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:28     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> This adds support for the ARM syscon ICST clocks to initialized
> directly from the device tree syscon node on ARM Integrator,
> Versatile and RealView reference designs.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Also:

WARNING: please, no space before tabs
#47: FILE: drivers/clk/versatile/clk-icst.c:221:
+^I.vd_min ^I= 8,$

WARNING: please, no space before tabs
#48: FILE: drivers/clk/versatile/clk-icst.c:222:
+^I.vd_max ^I= 263,$

WARNING: please, no space before tabs
#49: FILE: drivers/clk/versatile/clk-icst.c:223:
+^I.rd_min ^I= 3,$

WARNING: please, no space before tabs
#50: FILE: drivers/clk/versatile/clk-icst.c:224:
+^I.rd_max ^I= 65,$

total: 0 errors, 4 warnings, 98 lines checked

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-15 19:28     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> This adds support for the ARM syscon ICST clocks to initialized
> directly from the device tree syscon node on ARM Integrator,
> Versatile and RealView reference designs.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: linux-clk at vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Also:

WARNING: please, no space before tabs
#47: FILE: drivers/clk/versatile/clk-icst.c:221:
+^I.vd_min ^I= 8,$

WARNING: please, no space before tabs
#48: FILE: drivers/clk/versatile/clk-icst.c:222:
+^I.vd_max ^I= 263,$

WARNING: please, no space before tabs
#49: FILE: drivers/clk/versatile/clk-icst.c:223:
+^I.rd_min ^I= 3,$

WARNING: please, no space before tabs
#50: FILE: drivers/clk/versatile/clk-icst.c:224:
+^I.rd_max ^I= 65,$

total: 0 errors, 4 warnings, 98 lines checked

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
  2015-10-15 13:46   ` Linus Walleij
@ 2015-10-15 19:28     ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/15, Linus Walleij wrote:
> @@ -140,21 +141,16 @@ static const struct clk_ops icst_ops = {
>  	.set_rate = icst_set_rate,
>  };
>  
> -struct clk *icst_clk_register(struct device *dev,
> +struct clk *icst_clk_setup(struct device *dev,

This needs static now.

>  			const struct clk_icst_desc *desc,
>  			const char *name,
>  			const char *parent_name,
> -			void __iomem *base)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately
@ 2015-10-15 19:28     ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-15 19:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15, Linus Walleij wrote:
> @@ -140,21 +141,16 @@ static const struct clk_ops icst_ops = {
>  	.set_rate = icst_set_rate,
>  };
>  
> -struct clk *icst_clk_register(struct device *dev,
> +struct clk *icst_clk_setup(struct device *dev,

This needs static now.

>  			const struct clk_icst_desc *desc,
>  			const char *name,
>  			const char *parent_name,
> -			void __iomem *base)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 04/13] irqchip/gic: support RealView variant setup
  2015-10-15 13:46 ` [PATCH 04/13] irqchip/gic: support RealView variant setup Linus Walleij
  2015-10-15 16:06   ` Marc Zyngier
@ 2015-10-16  8:28   ` Thomas Gleixner
  1 sibling, 0 replies; 62+ messages in thread
From: Thomas Gleixner @ 2015-10-16  8:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 15 Oct 2015, Linus Walleij wrote:

   http://marc.info/?l=linux-arm-kernel&m=144250191426653&w=2

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/13] ARM: add some L220 DT settings
  2015-10-15 13:57         ` Russell King - ARM Linux
@ 2015-10-22 12:57             ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-22 12:57 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Pawel Moll, Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 15, 2015 at 3:57 PM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Thu, Oct 15, 2015 at 03:46:41PM +0200, Linus Walleij wrote:

>> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).
>
> I don't think we should introduce a DT property for this: if we support
> the event monitor, then the event monitor support code should be
> controlling this bit.

OK that's reasonable. I'll make a patch for the parity enable and think
about how to implement eventmonitor bit setting.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 01/13] ARM: add some L220 DT settings
@ 2015-10-22 12:57             ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-22 12:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 3:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Oct 15, 2015 at 03:46:41PM +0200, Linus Walleij wrote:

>> +- arm,eventmon-enable : enable the event monitor on the L2 cache (PL220 only).
>
> I don't think we should introduce a DT property for this: if we support
> the event monitor, then the event monitor support code should be
> controlling this bit.

OK that's reasonable. I'll make a patch for the parity enable and think
about how to implement eventmonitor bit setting.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-15 19:08     ` Stephen Boyd
@ 2015-10-23  9:27       ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:
>> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>>       init.flags = CLK_IS_ROOT;
>>       init.parent_names = (parent_name ? &parent_name : NULL);
>>       init.num_parents = (parent_name ? 1 : 0);
>> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>> +     if (IS_ERR(icst->map)) {
>> +             int ret;
>> +
>> +             pr_err("could not initialize ICST regmap\n");
>> +             kfree(icst);
>> +             ret = PTR_ERR(icst->map);
>
> drivers/clk/versatile/clk-icst.c:183
> icst_clk_register() error: dereferencing freed memory 'icst'
> drivers/clk/versatile/clk-icst.c:184
> icst_clk_register() warn: possible memory leak of 'pclone'

The pclone warning is correct, nice catch. (Fixing it.)

But for the second warning, whatever static checker you're
using for this is unable to handle error pointers:

    clk = clk_register(dev, &icst->hw);
    if (IS_ERR(clk))
        kfree(icst);

    return clk;

It is quite obvious that returning clk (which may be an error code)
is OK here.

If you want, I may need to add some specific annotation to shut
up the static checker, any hints?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-23  9:27       ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:
>> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>>       init.flags = CLK_IS_ROOT;
>>       init.parent_names = (parent_name ? &parent_name : NULL);
>>       init.num_parents = (parent_name ? 1 : 0);
>> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>> +     if (IS_ERR(icst->map)) {
>> +             int ret;
>> +
>> +             pr_err("could not initialize ICST regmap\n");
>> +             kfree(icst);
>> +             ret = PTR_ERR(icst->map);
>
> drivers/clk/versatile/clk-icst.c:183
> icst_clk_register() error: dereferencing freed memory 'icst'
> drivers/clk/versatile/clk-icst.c:184
> icst_clk_register() warn: possible memory leak of 'pclone'

The pclone warning is correct, nice catch. (Fixing it.)

But for the second warning, whatever static checker you're
using for this is unable to handle error pointers:

    clk = clk_register(dev, &icst->hw);
    if (IS_ERR(clk))
        kfree(icst);

    return clk;

It is quite obvious that returning clk (which may be an error code)
is OK here.

If you want, I may need to add some specific annotation to shut
up the static checker, any hints?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-23  9:27       ` Linus Walleij
@ 2015-10-23  9:37         ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:37 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Fri, Oct 23, 2015 at 11:27 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/15, Linus Walleij wrote:
>>> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>>>       init.flags = CLK_IS_ROOT;
>>>       init.parent_names = (parent_name ? &parent_name : NULL);
>>>       init.num_parents = (parent_name ? 1 : 0);
>>> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>>> +     if (IS_ERR(icst->map)) {
>>> +             int ret;
>>> +
>>> +             pr_err("could not initialize ICST regmap\n");
>>> +             kfree(icst);
>>> +             ret = PTR_ERR(icst->map);
>>
>> drivers/clk/versatile/clk-icst.c:183
>> icst_clk_register() error: dereferencing freed memory 'icst'
>> drivers/clk/versatile/clk-icst.c:184
>> icst_clk_register() warn: possible memory leak of 'pclone'
>
> The pclone warning is correct, nice catch. (Fixing it.)

Turns out that this was around for ages, so sent a separate
fix for -stable.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-23  9:37         ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 23, 2015 at 11:27 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/15, Linus Walleij wrote:
>>> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>>>       init.flags = CLK_IS_ROOT;
>>>       init.parent_names = (parent_name ? &parent_name : NULL);
>>>       init.num_parents = (parent_name ? 1 : 0);
>>> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>>> +     if (IS_ERR(icst->map)) {
>>> +             int ret;
>>> +
>>> +             pr_err("could not initialize ICST regmap\n");
>>> +             kfree(icst);
>>> +             ret = PTR_ERR(icst->map);
>>
>> drivers/clk/versatile/clk-icst.c:183
>> icst_clk_register() error: dereferencing freed memory 'icst'
>> drivers/clk/versatile/clk-icst.c:184
>> icst_clk_register() warn: possible memory leak of 'pclone'
>
> The pclone warning is correct, nice catch. (Fixing it.)

Turns out that this was around for ages, so sent a separate
fix for -stable.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
  2015-10-15 19:23     ` Stephen Boyd
  (?)
@ 2015-10-23  9:48         ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:48 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Russell King, Pawel Moll, Mark Rutland, Marc Zyngier,
	Will Deacon, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 15, 2015 at 9:23 PM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 10/15, Linus Walleij wrote:

>> +Required properties:
>> +- lock-offset: the offset address into the system controller where the
>> +  unlocking register is located
>> +- vco-offset: the offset address into the system controller where the
>> +  ICST control register is located (even 32 bit address)
>
> Is there any reason why we don't use a reg property for this?

Usually reg = <> is used with two (or more) tokens:

reg = <phys_addr size>;

The exception being things like I2C addresses which
are just one token.

Since in this case, there is a "mother" reg property in the
syscon-compatible node, which we are indexing into,
it is confusing to use the same name for subnodes.

Also there is a bunch of precedents doing it like this
for sybdevices to system controllers, just
git grep offset Documentation/devicetree/bindings
will give you a bunch of them.

(Fixing the spelling comments.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
@ 2015-10-23  9:48         ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:48 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring, devicetree,
	Michael Turquette, linux-clk

On Thu, Oct 15, 2015 at 9:23 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:

>> +Required properties:
>> +- lock-offset: the offset address into the system controller where the
>> +  unlocking register is located
>> +- vco-offset: the offset address into the system controller where the
>> +  ICST control register is located (even 32 bit address)
>
> Is there any reason why we don't use a reg property for this?

Usually reg = <> is used with two (or more) tokens:

reg = <phys_addr size>;

The exception being things like I2C addresses which
are just one token.

Since in this case, there is a "mother" reg property in the
syscon-compatible node, which we are indexing into,
it is confusing to use the same name for subnodes.

Also there is a bunch of precedents doing it like this
for sybdevices to system controllers, just
git grep offset Documentation/devicetree/bindings
will give you a bunch of them.

(Fixing the spelling comments.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
@ 2015-10-23  9:48         ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-23  9:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 9:23 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:

>> +Required properties:
>> +- lock-offset: the offset address into the system controller where the
>> +  unlocking register is located
>> +- vco-offset: the offset address into the system controller where the
>> +  ICST control register is located (even 32 bit address)
>
> Is there any reason why we don't use a reg property for this?

Usually reg = <> is used with two (or more) tokens:

reg = <phys_addr size>;

The exception being things like I2C addresses which
are just one token.

Since in this case, there is a "mother" reg property in the
syscon-compatible node, which we are indexing into,
it is confusing to use the same name for subnodes.

Also there is a bunch of precedents doing it like this
for sybdevices to system controllers, just
git grep offset Documentation/devicetree/bindings
will give you a bunch of them.

(Fixing the spelling comments.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-23  9:27       ` Linus Walleij
@ 2015-10-23 16:24         ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-23 16:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On 10/23, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> >> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
> >>       init.flags = CLK_IS_ROOT;
> >>       init.parent_names = (parent_name ? &parent_name : NULL);
> >>       init.num_parents = (parent_name ? 1 : 0);
> >> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> >> +     if (IS_ERR(icst->map)) {
> >> +             int ret;
> >> +
> >> +             pr_err("could not initialize ICST regmap\n");
> >> +             kfree(icst);
> >> +             ret = PTR_ERR(icst->map);
> >
> > drivers/clk/versatile/clk-icst.c:183
> > icst_clk_register() error: dereferencing freed memory 'icst'
> > drivers/clk/versatile/clk-icst.c:184
> > icst_clk_register() warn: possible memory leak of 'pclone'
> 
> The pclone warning is correct, nice catch. (Fixing it.)
> 
> But for the second warning, whatever static checker you're
> using for this is unable to handle error pointers:
> 
>     clk = clk_register(dev, &icst->hw);
>     if (IS_ERR(clk))
>         kfree(icst);
> 
>     return clk;
> 
> It is quite obvious that returning clk (which may be an error code)
> is OK here.
> 
> If you want, I may need to add some specific annotation to shut
> up the static checker, any hints?
> 

Huh? I'm totally lost. I use sparse and smatch.

> >> +             kfree(icst);
> >> +             ret = PTR_ERR(icst->map);

We just freed icst, and then we dereferenced it in the next line.
What does that have to do with error pointers?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-23 16:24         ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-23 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/23, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> >> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
> >>       init.flags = CLK_IS_ROOT;
> >>       init.parent_names = (parent_name ? &parent_name : NULL);
> >>       init.num_parents = (parent_name ? 1 : 0);
> >> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
> >> +     if (IS_ERR(icst->map)) {
> >> +             int ret;
> >> +
> >> +             pr_err("could not initialize ICST regmap\n");
> >> +             kfree(icst);
> >> +             ret = PTR_ERR(icst->map);
> >
> > drivers/clk/versatile/clk-icst.c:183
> > icst_clk_register() error: dereferencing freed memory 'icst'
> > drivers/clk/versatile/clk-icst.c:184
> > icst_clk_register() warn: possible memory leak of 'pclone'
> 
> The pclone warning is correct, nice catch. (Fixing it.)
> 
> But for the second warning, whatever static checker you're
> using for this is unable to handle error pointers:
> 
>     clk = clk_register(dev, &icst->hw);
>     if (IS_ERR(clk))
>         kfree(icst);
> 
>     return clk;
> 
> It is quite obvious that returning clk (which may be an error code)
> is OK here.
> 
> If you want, I may need to add some specific annotation to shut
> up the static checker, any hints?
> 

Huh? I'm totally lost. I use sparse and smatch.

> >> +             kfree(icst);
> >> +             ret = PTR_ERR(icst->map);

We just freed icst, and then we dereferenced it in the next line.
What does that have to do with error pointers?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
  2015-10-23  9:48         ` Linus Walleij
@ 2015-10-23 16:43           ` Stephen Boyd
  -1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-23 16:43 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring, devicetree,
	Michael Turquette, linux-clk

On 10/23, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:23 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> 
> >> +Required properties:
> >> +- lock-offset: the offset address into the system controller where the
> >> +  unlocking register is located
> >> +- vco-offset: the offset address into the system controller where the
> >> +  ICST control register is located (even 32 bit address)
> >
> > Is there any reason why we don't use a reg property for this?
> 
> Usually reg = <> is used with two (or more) tokens:
> 
> reg = <phys_addr size>;
> 
> The exception being things like I2C addresses which
> are just one token.
> 
> Since in this case, there is a "mother" reg property in the
> syscon-compatible node, which we are indexing into,
> it is confusing to use the same name for subnodes.
> 
> Also there is a bunch of precedents doing it like this
> for sybdevices to system controllers, just
> git grep offset Documentation/devicetree/bindings
> will give you a bunch of them.
> 

Ok. I'm no DT expert, but it seems odd to have subnodes without a
reg property.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 08/13] clk: add ARM syscon ICST device tree bindings
@ 2015-10-23 16:43           ` Stephen Boyd
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2015-10-23 16:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/23, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:23 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> 
> >> +Required properties:
> >> +- lock-offset: the offset address into the system controller where the
> >> +  unlocking register is located
> >> +- vco-offset: the offset address into the system controller where the
> >> +  ICST control register is located (even 32 bit address)
> >
> > Is there any reason why we don't use a reg property for this?
> 
> Usually reg = <> is used with two (or more) tokens:
> 
> reg = <phys_addr size>;
> 
> The exception being things like I2C addresses which
> are just one token.
> 
> Since in this case, there is a "mother" reg property in the
> syscon-compatible node, which we are indexing into,
> it is confusing to use the same name for subnodes.
> 
> Also there is a bunch of precedents doing it like this
> for sybdevices to system controllers, just
> git grep offset Documentation/devicetree/bindings
> will give you a bunch of them.
> 

Ok. I'm no DT expert, but it seems odd to have subnodes without a
reg property.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-15 19:26     ` Stephen Boyd
@ 2015-10-26 13:14       ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-26 13:14 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Thu, Oct 15, 2015 at 9:26 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:
>> +
>> +     if (of_device_is_compatible(np, "arm,syscon-icst525"))
>> +             icst_desc.params = &icst525_params;
>> +     else if (of_device_is_compatible(np, "arm,syscon-icst307"))
>> +             icst_desc.params = &icst307_params;
>
> I guess if we add anymore here we should use an of_device_id
> array instead.

As it happens those two are gonna be it.

ARM never created any more integrated ICST devices, and
they stopped using them since. Those two are the required
ones.

(Fixing all other comments.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-26 13:14       ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-26 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 9:26 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/15, Linus Walleij wrote:
>> +
>> +     if (of_device_is_compatible(np, "arm,syscon-icst525"))
>> +             icst_desc.params = &icst525_params;
>> +     else if (of_device_is_compatible(np, "arm,syscon-icst307"))
>> +             icst_desc.params = &icst307_params;
>
> I guess if we add anymore here we should use an of_device_id
> array instead.

As it happens those two are gonna be it.

ARM never created any more integrated ICST devices, and
they stopped using them since. Those two are the required
ones.

(Fixing all other comments.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-26 13:14       ` Linus Walleij
@ 2015-10-26 13:31         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 62+ messages in thread
From: Russell King - ARM Linux @ 2015-10-26 13:31 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, linux-arm-kernel, Arnd Bergmann, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Mon, Oct 26, 2015 at 02:14:15PM +0100, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:26 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> >> +
> >> +     if (of_device_is_compatible(np, "arm,syscon-icst525"))
> >> +             icst_desc.params = &icst525_params;
> >> +     else if (of_device_is_compatible(np, "arm,syscon-icst307"))
> >> +             icst_desc.params = &icst307_params;
> >
> > I guess if we add anymore here we should use an of_device_id
> > array instead.
> 
> As it happens those two are gonna be it.
> 
> ARM never created any more integrated ICST devices, and
> they stopped using them since. Those two are the required
> ones.

As ARM didn't create any ICST devices at all, that's hardly surprising.
These devices are created by Integrated Circuit Systems, Inc.  The 525
is a parallel-loaded clock generator, the 307 is a serial-loaded clock
generator.

However, they have no "standard" software interface - indeed, the 525
is marketed as a device that needs no processor or software to be used,
so it doesn't have a "software" interface as such.

ARM Ltd's hardware on these boards provides interfaces to these, however
the underlying ICST support I wrote was factored to separate out the
interface from the chip support - I haven't been tracking what's been
going on with these, but I hope that separation has been kept as it's
entirely logical, and describing these things in DT as an ARM Ltd device,
combining the ICST device itself with its interface would be wrong.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-26 13:31         ` Russell King - ARM Linux
  0 siblings, 0 replies; 62+ messages in thread
From: Russell King - ARM Linux @ 2015-10-26 13:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 26, 2015 at 02:14:15PM +0100, Linus Walleij wrote:
> On Thu, Oct 15, 2015 at 9:26 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/15, Linus Walleij wrote:
> >> +
> >> +     if (of_device_is_compatible(np, "arm,syscon-icst525"))
> >> +             icst_desc.params = &icst525_params;
> >> +     else if (of_device_is_compatible(np, "arm,syscon-icst307"))
> >> +             icst_desc.params = &icst307_params;
> >
> > I guess if we add anymore here we should use an of_device_id
> > array instead.
> 
> As it happens those two are gonna be it.
> 
> ARM never created any more integrated ICST devices, and
> they stopped using them since. Those two are the required
> ones.

As ARM didn't create any ICST devices at all, that's hardly surprising.
These devices are created by Integrated Circuit Systems, Inc.  The 525
is a parallel-loaded clock generator, the 307 is a serial-loaded clock
generator.

However, they have no "standard" software interface - indeed, the 525
is marketed as a device that needs no processor or software to be used,
so it doesn't have a "software" interface as such.

ARM Ltd's hardware on these boards provides interfaces to these, however
the underlying ICST support I wrote was factored to separate out the
interface from the chip support - I haven't been tracking what's been
going on with these, but I hope that separation has been kept as it's
entirely logical, and describing these things in DT as an ARM Ltd device,
combining the ICST device itself with its interface would be wrong.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/13] clk: versatile-icst: convert to use regmap
  2015-10-23 16:24         ` Stephen Boyd
@ 2015-10-27 15:56           ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-27 15:56 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-arm-kernel, Arnd Bergmann, Russell King, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Fri, Oct 23, 2015 at 6:24 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/23, Linus Walleij wrote:
>> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 10/15, Linus Walleij wrote:
>> >> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>> >>       init.flags = CLK_IS_ROOT;
>> >>       init.parent_names = (parent_name ? &parent_name : NULL);
>> >>       init.num_parents = (parent_name ? 1 : 0);
>> >> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>> >> +     if (IS_ERR(icst->map)) {
>> >> +             int ret;
>> >> +
>> >> +             pr_err("could not initialize ICST regmap\n");
>> >> +             kfree(icst);
>> >> +             ret = PTR_ERR(icst->map);
>> >
>> > drivers/clk/versatile/clk-icst.c:183
>> > icst_clk_register() error: dereferencing freed memory 'icst'
>> > drivers/clk/versatile/clk-icst.c:184
>> > icst_clk_register() warn: possible memory leak of 'pclone'
>>
>> The pclone warning is correct, nice catch. (Fixing it.)
>>
>> But for the second warning, whatever static checker you're
>> using for this is unable to handle error pointers:
>>
>>     clk = clk_register(dev, &icst->hw);
>>     if (IS_ERR(clk))
>>         kfree(icst);
>>
>>     return clk;
>>
>> It is quite obvious that returning clk (which may be an error code)
>> is OK here.
>>
>> If you want, I may need to add some specific annotation to shut
>> up the static checker, any hints?
>>
>
> Huh? I'm totally lost. I use sparse and smatch.
>
>> >> +             kfree(icst);
>> >> +             ret = PTR_ERR(icst->map);
>
> We just freed icst, and then we dereferenced it in the next line.
> What does that have to do with error pointers?

Sorry I must have confused patch versions.  :(

I see the real problem now, so will make a v3 of this.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 06/13] clk: versatile-icst: convert to use regmap
@ 2015-10-27 15:56           ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-27 15:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 23, 2015 at 6:24 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/23, Linus Walleij wrote:
>> On Thu, Oct 15, 2015 at 9:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 10/15, Linus Walleij wrote:
>> >> @@ -151,10 +174,19 @@ struct clk *icst_clk_register(struct device *dev,
>> >>       init.flags = CLK_IS_ROOT;
>> >>       init.parent_names = (parent_name ? &parent_name : NULL);
>> >>       init.num_parents = (parent_name ? 1 : 0);
>> >> +     icst->map = regmap_init_mmio(NULL, base, &icst_regmap_conf);
>> >> +     if (IS_ERR(icst->map)) {
>> >> +             int ret;
>> >> +
>> >> +             pr_err("could not initialize ICST regmap\n");
>> >> +             kfree(icst);
>> >> +             ret = PTR_ERR(icst->map);
>> >
>> > drivers/clk/versatile/clk-icst.c:183
>> > icst_clk_register() error: dereferencing freed memory 'icst'
>> > drivers/clk/versatile/clk-icst.c:184
>> > icst_clk_register() warn: possible memory leak of 'pclone'
>>
>> The pclone warning is correct, nice catch. (Fixing it.)
>>
>> But for the second warning, whatever static checker you're
>> using for this is unable to handle error pointers:
>>
>>     clk = clk_register(dev, &icst->hw);
>>     if (IS_ERR(clk))
>>         kfree(icst);
>>
>>     return clk;
>>
>> It is quite obvious that returning clk (which may be an error code)
>> is OK here.
>>
>> If you want, I may need to add some specific annotation to shut
>> up the static checker, any hints?
>>
>
> Huh? I'm totally lost. I use sparse and smatch.
>
>> >> +             kfree(icst);
>> >> +             ret = PTR_ERR(icst->map);
>
> We just freed icst, and then we dereferenced it in the next line.
> What does that have to do with error pointers?

Sorry I must have confused patch versions.  :(

I see the real problem now, so will make a v3 of this.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/13] clk: versatile-icst: add device tree support
  2015-10-26 13:31         ` Russell King - ARM Linux
@ 2015-10-29 13:00           ` Linus Walleij
  -1 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-29 13:00 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Stephen Boyd, linux-arm-kernel, Arnd Bergmann, Pawel Moll,
	Mark Rutland, Marc Zyngier, Will Deacon, Rob Herring,
	Michael Turquette, linux-clk

On Mon, Oct 26, 2015 at 2:31 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> [Me]
>> ARM never created any more integrated ICST devices, and
>> they stopped using them since. Those two are the required
>> ones.
>
> As ARM didn't create any ICST devices at all, that's hardly surprising.
> These devices are created by Integrated Circuit Systems, Inc.  The 525
> is a parallel-loaded clock generator, the 307 is a serial-loaded clock
> generator.
>
> However, they have no "standard" software interface - indeed, the 525
> is marketed as a device that needs no processor or software to be used,
> so it doesn't have a "software" interface as such.

RIght, I wrote a blurb with the more elaborate and correct story
for the device tree bindings. I get sloppy sometimes.

> ARM Ltd's hardware on these boards provides interfaces to these, however
> the underlying ICST support I wrote was factored to separate out the
> interface from the chip support - I haven't been tracking what's been
> going on with these, but I hope that separation has been kept as it's
> entirely logical, and describing these things in DT as an ARM Ltd device,
> combining the ICST device itself with its interface would be wrong.

So the device tree bindings does say that, and the compatible strings
are "arm,syscon-icst525" or "arm,syscon-icst307" indicating that it is
indeed the ARM syscon register-mapped thing, and the ICST sits on
the back of that register.

The logical separation is indeed kept, if someone ever decides to
interface the ICST clocks in some other way, the code is reusable.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 09/13] clk: versatile-icst: add device tree support
@ 2015-10-29 13:00           ` Linus Walleij
  0 siblings, 0 replies; 62+ messages in thread
From: Linus Walleij @ 2015-10-29 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 26, 2015 at 2:31 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> [Me]
>> ARM never created any more integrated ICST devices, and
>> they stopped using them since. Those two are the required
>> ones.
>
> As ARM didn't create any ICST devices at all, that's hardly surprising.
> These devices are created by Integrated Circuit Systems, Inc.  The 525
> is a parallel-loaded clock generator, the 307 is a serial-loaded clock
> generator.
>
> However, they have no "standard" software interface - indeed, the 525
> is marketed as a device that needs no processor or software to be used,
> so it doesn't have a "software" interface as such.

RIght, I wrote a blurb with the more elaborate and correct story
for the device tree bindings. I get sloppy sometimes.

> ARM Ltd's hardware on these boards provides interfaces to these, however
> the underlying ICST support I wrote was factored to separate out the
> interface from the chip support - I haven't been tracking what's been
> going on with these, but I hope that separation has been kept as it's
> entirely logical, and describing these things in DT as an ARM Ltd device,
> combining the ICST device itself with its interface would be wrong.

So the device tree bindings does say that, and the compatible strings
are "arm,syscon-icst525" or "arm,syscon-icst307" indicating that it is
indeed the ARM syscon register-mapped thing, and the ICST sits on
the back of that register.

The logical separation is indeed kept, if someone ever decides to
interface the ICST clocks in some other way, the code is reusable.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 03/13] irqchips: fix ARM11MPCore GIC bindings
  2015-10-15 13:46     ` Linus Walleij
@ 2015-11-02 14:35         ` Rob Herring
  -1 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2015-11-02 14:35 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Arnd Bergmann,
	Russell King, Pawel Moll, Mark Rutland, Marc Zyngier,
	Will Deacon, devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 15, 2015 at 8:46 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> The GIC bindings for the ARM11MPCore need to differentiate between
> the GIC on the Test Chip and the one on the evaluation baseboard.
> Split the binding in two and define new compatible-strings.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  Documentation/devicetree/bindings/arm/gic.txt | 3 ++-
>  drivers/irqchip/irq-gic.c                     | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 2da059a4790c..a5445622c216 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -15,7 +15,8 @@ Main node required properties:
>         "arm,cortex-a15-gic"
>         "arm,cortex-a9-gic"
>         "arm,cortex-a7-gic"
> -       "arm,arm11mp-gic"
> +       "arm,tc11mp-gic"
> +       "arm,pb11mp-gic"
>         "brcm,brahma-b15-gic"
>         "arm,arm1176jzf-devchip-gic"
>         "qcom,msm-8660-qgic"
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 982c09c2d791..5376d1cb0a4f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1184,7 +1184,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>         return 0;
>  }
>  IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> -IRQCHIP_DECLARE(arm11mp_gic, "arm,arm11mp-gic", gic_of_init);
> +IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", gic_of_init);
> +IRQCHIP_DECLARE(armpb11mp_gic, "arm,pb11mp-gic", gic_of_init);
>  IRQCHIP_DECLARE(arm1176jzf_dc_gic, "arm,arm1176jzf-devchip-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 03/13] irqchips: fix ARM11MPCore GIC bindings
@ 2015-11-02 14:35         ` Rob Herring
  0 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2015-11-02 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 8:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> The GIC bindings for the ARM11MPCore need to differentiate between
> the GIC on the Test Chip and the one on the evaluation baseboard.
> Split the binding in two and define new compatible-strings.
>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  Documentation/devicetree/bindings/arm/gic.txt | 3 ++-
>  drivers/irqchip/irq-gic.c                     | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 2da059a4790c..a5445622c216 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -15,7 +15,8 @@ Main node required properties:
>         "arm,cortex-a15-gic"
>         "arm,cortex-a9-gic"
>         "arm,cortex-a7-gic"
> -       "arm,arm11mp-gic"
> +       "arm,tc11mp-gic"
> +       "arm,pb11mp-gic"
>         "brcm,brahma-b15-gic"
>         "arm,arm1176jzf-devchip-gic"
>         "qcom,msm-8660-qgic"
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 982c09c2d791..5376d1cb0a4f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1184,7 +1184,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>         return 0;
>  }
>  IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> -IRQCHIP_DECLARE(arm11mp_gic, "arm,arm11mp-gic", gic_of_init);
> +IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", gic_of_init);
> +IRQCHIP_DECLARE(armpb11mp_gic, "arm,pb11mp-gic", gic_of_init);
>  IRQCHIP_DECLARE(arm1176jzf_dc_gic, "arm,arm1176jzf-devchip-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> --
> 2.4.3
>

^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2015-11-02 14:35 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-15 13:46 [PATCH 00/13] Device Tree support for RealView PB11MPCore Linus Walleij
2015-10-15 13:46 ` [PATCH 02/13] ARM: add DT bindings for the ARM11MPCore CPU cluster Linus Walleij
     [not found] ` <1444916813-31024-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-10-15 13:46   ` [PATCH 01/13] ARM: add some L220 DT settings Linus Walleij
2015-10-15 13:46     ` Linus Walleij
     [not found]     ` <1444916813-31024-2-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-10-15 13:57       ` Russell King - ARM Linux
2015-10-15 13:57         ` Russell King - ARM Linux
     [not found]         ` <20151015135730.GC32532-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2015-10-22 12:57           ` Linus Walleij
2015-10-22 12:57             ` Linus Walleij
2015-10-15 13:58       ` Rob Herring
2015-10-15 13:58         ` Rob Herring
2015-10-15 13:46   ` [PATCH 03/13] irqchips: fix ARM11MPCore GIC bindings Linus Walleij
2015-10-15 13:46     ` Linus Walleij
     [not found]     ` <1444916813-31024-4-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-11-02 14:35       ` Rob Herring
2015-11-02 14:35         ` Rob Herring
2015-10-15 13:46 ` [PATCH 04/13] irqchip/gic: support RealView variant setup Linus Walleij
2015-10-15 16:06   ` Marc Zyngier
2015-10-16  8:28   ` Thomas Gleixner
2015-10-15 13:46 ` [PATCH 05/13] irqchip/gic: assign irqchip dynamically Linus Walleij
2015-10-15 16:16   ` Marc Zyngier
2015-10-15 13:46 ` [PATCH 06/13] clk: versatile-icst: convert to use regmap Linus Walleij
2015-10-15 13:46   ` Linus Walleij
2015-10-15 19:08   ` Stephen Boyd
2015-10-15 19:08     ` Stephen Boyd
2015-10-23  9:27     ` Linus Walleij
2015-10-23  9:27       ` Linus Walleij
2015-10-23  9:37       ` Linus Walleij
2015-10-23  9:37         ` Linus Walleij
2015-10-23 16:24       ` Stephen Boyd
2015-10-23 16:24         ` Stephen Boyd
2015-10-27 15:56         ` Linus Walleij
2015-10-27 15:56           ` Linus Walleij
2015-10-15 13:46 ` [PATCH 07/13] clk: versatile-icst: refactor to allocate regmap separately Linus Walleij
2015-10-15 13:46   ` Linus Walleij
2015-10-15 19:10   ` Stephen Boyd
2015-10-15 19:10     ` Stephen Boyd
2015-10-15 19:28   ` Stephen Boyd
2015-10-15 19:28     ` Stephen Boyd
2015-10-15 13:46 ` [PATCH 08/13] clk: add ARM syscon ICST device tree bindings Linus Walleij
2015-10-15 13:46   ` Linus Walleij
2015-10-15 19:23   ` Stephen Boyd
2015-10-15 19:23     ` Stephen Boyd
     [not found]     ` <20151015192325.GN4558-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2015-10-23  9:48       ` Linus Walleij
2015-10-23  9:48         ` Linus Walleij
2015-10-23  9:48         ` Linus Walleij
2015-10-23 16:43         ` Stephen Boyd
2015-10-23 16:43           ` Stephen Boyd
2015-10-15 13:46 ` [PATCH 09/13] clk: versatile-icst: add device tree support Linus Walleij
2015-10-15 13:46   ` Linus Walleij
2015-10-15 19:26   ` Stephen Boyd
2015-10-15 19:26     ` Stephen Boyd
2015-10-26 13:14     ` Linus Walleij
2015-10-26 13:14       ` Linus Walleij
2015-10-26 13:31       ` Russell King - ARM Linux
2015-10-26 13:31         ` Russell King - ARM Linux
2015-10-29 13:00         ` Linus Walleij
2015-10-29 13:00           ` Linus Walleij
2015-10-15 19:28   ` Stephen Boyd
2015-10-15 19:28     ` Stephen Boyd
2015-10-15 13:46 ` [PATCH 10/13] soc: versatile: add support for the PB11MPCore Linus Walleij
2015-10-15 13:46 ` [PATCH 11/13] ARM: realview: select SP810 and ICST for the DT variant Linus Walleij
2015-10-15 13:46 ` [PATCH 12/13] ARM: realview: add an DT SMP boot method Linus Walleij
2015-10-15 13:46 ` [PATCH 13/13] ARM: realview: add device tree for PB11MPCore Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.