public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Handle non-secure L2C initialization on Exynos4
@ 2014-06-11 15:30 Tomasz Figa
  2014-06-11 15:30 ` [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback Tomasz Figa
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

This series intends to add support for L2 cache on Exynos4 SoCs on boards
running under secure firmware, which requires certain initialization steps
to be done with help of firmware, as selected registers are writable only
from secure mode.

First three patches extend existing support for secure write in L2C driver
to account for design of secure firmware running on Exynos. Namely:
 1) direct read access to certain registers is needed on Exynos, because
    secure firmware calls set several registers at once,
 2) not all boards are running secure firmware, so .write_sec callback
    needs to be installed in Exynos firmware ops initialization code,
 3) write access to {DATA,TAG}_LATENCY_CTRL registers fron non-secure world
    is not allowed and so must use l2c_write_sec as well.
Those patches might affect other platforms using .write_sec callback, so
I'd like to kindly ask any interested people for testing.

Further two patches add impelmentation of .write_sec for Exynos secure
firmware and necessary DT nodes to enable L2 cache.

Tested on Exynos4210-based Universal C210 board (without secure firmware)
and Exynos4412-based TRATS2 board (with secure firmware).

Tomasz Figa (5):
  ARM: mm: cache-l2x0: Add base address argument to write_sec callback
  ARM: Get outer cache .write_sec callback from mach_desc only if not
    NULL
  ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers
  ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310
  ARM: dts: exynos4: Add nodes for L2 cache controller

 arch/arm/boot/dts/exynos4210.dtsi  |  9 ++++++
 arch/arm/boot/dts/exynos4x12.dtsi  |  9 ++++++
 arch/arm/include/asm/mach/arch.h   |  3 +-
 arch/arm/include/asm/outercache.h  |  2 +-
 arch/arm/kernel/irq.c              |  3 +-
 arch/arm/mach-exynos/firmware.c    | 61 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-highbank/highbank.c  |  3 +-
 arch/arm/mach-omap2/omap4-common.c |  3 +-
 arch/arm/mach-ux500/cache-l2x0.c   |  3 +-
 arch/arm/mm/cache-l2x0.c           | 10 +++----
 10 files changed, 95 insertions(+), 11 deletions(-)

-- 
1.9.3


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

* [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback
  2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
@ 2014-06-11 15:30 ` Tomasz Figa
  2014-06-11 16:00   ` Jon Loeliger
  2014-06-11 15:30 ` [PATCH 2/5] ARM: Get outer cache .write_sec callback from mach_desc only if not NULL Tomasz Figa
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

For certain platforms (e.g. Exynos) it is necessary to read back some
values from registers before they can be written (i.e. SMC calls that
set multiple registers per call), so base address of L2C controller is
needed for .write_sec operation. This patch adds base argument to
.write_sec callback so that its implementation can also access registers
directly.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/include/asm/mach/arch.h   | 3 ++-
 arch/arm/include/asm/outercache.h  | 2 +-
 arch/arm/mach-highbank/highbank.c  | 3 ++-
 arch/arm/mach-omap2/omap4-common.c | 3 ++-
 arch/arm/mach-ux500/cache-l2x0.c   | 3 ++-
 arch/arm/mm/cache-l2x0.c           | 2 +-
 6 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
index 060a75e..ddaebcd 100644
--- a/arch/arm/include/asm/mach/arch.h
+++ b/arch/arm/include/asm/mach/arch.h
@@ -46,7 +46,8 @@ struct machine_desc {
 	enum reboot_mode	reboot_mode;	/* default restart mode	*/
 	unsigned		l2c_aux_val;	/* L2 cache aux value	*/
 	unsigned		l2c_aux_mask;	/* L2 cache aux mask	*/
-	void			(*l2c_write_sec)(unsigned long, unsigned);
+	void			(*l2c_write_sec)(void __iomem *,
+						 unsigned long, unsigned);
 	struct smp_operations	*smp;		/* SMP operations	*/
 	bool			(*smp_init)(void);
 	void			(*fixup)(struct tag *, char **);
diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h
index 891a56b..5cc703b 100644
--- a/arch/arm/include/asm/outercache.h
+++ b/arch/arm/include/asm/outercache.h
@@ -35,7 +35,7 @@ struct outer_cache_fns {
 	void (*resume)(void);
 
 	/* This is an ARM L2C thing */
-	void (*write_sec)(unsigned long, unsigned);
+	void (*write_sec)(void __iomem *, unsigned long, unsigned);
 };
 
 extern struct outer_cache_fns outer_cache;
diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 8c35ae4..2bd3243 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -51,7 +51,8 @@ static void __init highbank_scu_map_io(void)
 }
 
 
-static void highbank_l2c310_write_sec(unsigned long val, unsigned reg)
+static void highbank_l2c310_write_sec(void __iomem *base,
+				      unsigned long val, unsigned reg)
 {
 	if (reg == L2X0_CTRL)
 		highbank_smc1(0x102, val);
diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c
index 326cd98..bdbe658 100644
--- a/arch/arm/mach-omap2/omap4-common.c
+++ b/arch/arm/mach-omap2/omap4-common.c
@@ -167,7 +167,8 @@ void __iomem *omap4_get_l2cache_base(void)
 	return l2cache_base;
 }
 
-static void omap4_l2c310_write_sec(unsigned long val, unsigned reg)
+static void omap4_l2c310_write_sec(void __iomem *base,
+				   unsigned long val, unsigned reg)
 {
 	unsigned smc_op;
 
diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
index 842ebed..35c2623 100644
--- a/arch/arm/mach-ux500/cache-l2x0.c
+++ b/arch/arm/mach-ux500/cache-l2x0.c
@@ -35,7 +35,8 @@ static int __init ux500_l2x0_unlock(void)
 	return 0;
 }
 
-static void ux500_l2c310_write_sec(unsigned long val, unsigned reg)
+static void ux500_l2c310_write_sec(void __iomem *base,
+				   unsigned long val, unsigned reg)
 {
 	/*
 	 * We can't write to secure registers as we are in non-secure
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index efc5cab..1695eab 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg)
 	if (val == readl_relaxed(base + reg))
 		return;
 	if (outer_cache.write_sec)
-		outer_cache.write_sec(val, reg);
+		outer_cache.write_sec(base, val, reg);
 	else
 		writel_relaxed(val, base + reg);
 }
-- 
1.9.3


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

* [PATCH 2/5] ARM: Get outer cache .write_sec callback from mach_desc only if not NULL
  2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
  2014-06-11 15:30 ` [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback Tomasz Figa
@ 2014-06-11 15:30 ` Tomasz Figa
  2014-06-11 15:30 ` [PATCH 3/5] ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers Tomasz Figa
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

Certain platforms (i.e. Exynos) might need to set .write_sec callback
from firmware initialization which is happenning in .init_early callback
of machine descriptor. However current code will overwrite the pointer
with whatever is present in machine descriptor, even though it can be
already set earlier. This patch fixes this by making the assignment
conditional, depending on whether current .write_sec callback is NULL.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/kernel/irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 2c42576..e7383b9 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -125,7 +125,8 @@ void __init init_IRQ(void)
 
 	if (IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_CACHE_L2X0) &&
 	    (machine_desc->l2c_aux_mask || machine_desc->l2c_aux_val)) {
-		outer_cache.write_sec = machine_desc->l2c_write_sec;
+		if (!outer_cache.write_sec)
+			outer_cache.write_sec = machine_desc->l2c_write_sec;
 		ret = l2x0_of_init(machine_desc->l2c_aux_val,
 				   machine_desc->l2c_aux_mask);
 		if (ret)
-- 
1.9.3


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

* [PATCH 3/5] ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers
  2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
  2014-06-11 15:30 ` [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback Tomasz Figa
  2014-06-11 15:30 ` [PATCH 2/5] ARM: Get outer cache .write_sec callback from mach_desc only if not NULL Tomasz Figa
@ 2014-06-11 15:30 ` Tomasz Figa
  2014-06-11 15:30 ` [PATCH 4/5] ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310 Tomasz Figa
  2014-06-11 15:30 ` [PATCH 5/5] ARM: dts: exynos4: Add nodes for L2 cache controller Tomasz Figa
  4 siblings, 0 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

According to the documentation, TAG_LATENCY_CTRL and DATA_LATENCY_CTRL
registers of L2C-310 can be written only in secure mode, so
l2c_write_sec() should be used to change them, instead of plain
writel_relaxed().

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/mm/cache-l2x0.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 1695eab..0180eb7 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1024,20 +1024,20 @@ static void __init l2c310_of_parse(const struct device_node *np,
 
 	of_property_read_u32_array(np, "arm,tag-latency", tag, ARRAY_SIZE(tag));
 	if (tag[0] && tag[1] && tag[2])
-		writel_relaxed(
+		l2c_write_sec(
 			L310_LATENCY_CTRL_RD(tag[0] - 1) |
 			L310_LATENCY_CTRL_WR(tag[1] - 1) |
 			L310_LATENCY_CTRL_SETUP(tag[2] - 1),
-			l2x0_base + L310_TAG_LATENCY_CTRL);
+			l2x0_base, L310_TAG_LATENCY_CTRL);
 
 	of_property_read_u32_array(np, "arm,data-latency",
 				   data, ARRAY_SIZE(data));
 	if (data[0] && data[1] && data[2])
-		writel_relaxed(
+		l2c_write_sec(
 			L310_LATENCY_CTRL_RD(data[0] - 1) |
 			L310_LATENCY_CTRL_WR(data[1] - 1) |
 			L310_LATENCY_CTRL_SETUP(data[2] - 1),
-			l2x0_base + L310_DATA_LATENCY_CTRL);
+			l2x0_base, L310_DATA_LATENCY_CTRL);
 
 	of_property_read_u32_array(np, "arm,filter-ranges",
 				   filter, ARRAY_SIZE(filter));
-- 
1.9.3


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

* [PATCH 4/5] ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310
  2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
                   ` (2 preceding siblings ...)
  2014-06-11 15:30 ` [PATCH 3/5] ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers Tomasz Figa
@ 2014-06-11 15:30 ` Tomasz Figa
  2014-06-11 15:30 ` [PATCH 5/5] ARM: dts: exynos4: Add nodes for L2 cache controller Tomasz Figa
  4 siblings, 0 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

Exynos4 SoCs equipped with an L2C-310 cache controller and running under
secure firmware require certain registers of aforementioned IP to be
accessed only from secure mode. This means that SMC calls are required
for certain register writes. To handle this, an implementation of
.write_sec callback is provided by this patch.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/mach-exynos/firmware.c | 61 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index eb91d23..34f7798 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -14,7 +14,9 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
+#include <asm/cputype.h>
 #include <asm/firmware.h>
+#include <asm/hardware/cache-l2x0.h>
 
 #include <mach/map.h>
 
@@ -70,6 +72,55 @@ static const struct firmware_ops exynos_firmware_ops = {
 	.cpu_boot		= exynos_cpu_boot,
 };
 
+static void exynos_l2_write_sec(void __iomem *base, unsigned long val,
+				unsigned reg)
+{
+	switch (reg) {
+	case L2X0_CTRL:
+		exynos_smc(SMC_CMD_L2X0CTRL, val, 0, 0);
+		break;
+
+	case L2X0_AUX_CTRL:
+		exynos_smc(SMC_CMD_L2X0SETUP2,
+				readl_relaxed(base + L310_POWER_CTRL),
+				val, 0);
+		break;
+
+	case L2X0_DEBUG_CTRL:
+		exynos_smc(SMC_CMD_L2X0DEBUG, val, 0, 0);
+		break;
+
+	case L310_TAG_LATENCY_CTRL:
+		exynos_smc(SMC_CMD_L2X0SETUP1,
+				val,
+				readl_relaxed(base + L310_DATA_LATENCY_CTRL),
+				readl_relaxed(base + L310_PREFETCH_CTRL));
+		break;
+
+	case L310_DATA_LATENCY_CTRL:
+		exynos_smc(SMC_CMD_L2X0SETUP1,
+				readl_relaxed(base + L310_TAG_LATENCY_CTRL),
+				val,
+				readl_relaxed(base + L310_PREFETCH_CTRL));
+		break;
+
+	case L310_PREFETCH_CTRL:
+		exynos_smc(SMC_CMD_L2X0SETUP1,
+				readl_relaxed(base + L310_TAG_LATENCY_CTRL),
+				readl_relaxed(base + L310_DATA_LATENCY_CTRL),
+				val);
+		break;
+
+	case L310_POWER_CTRL:
+		exynos_smc(SMC_CMD_L2X0SETUP2, val,
+				readl_relaxed(base + L2X0_AUX_CTRL), 0);
+		break;
+
+	default:
+		WARN_ONCE(1, "%s: ignoring write to reg 0x%x\n", __func__, reg);
+	}
+}
+
 void __init exynos_firmware_init(void)
 {
 	struct device_node *nd;
@@ -89,4 +140,14 @@ void __init exynos_firmware_init(void)
 	pr_info("Running under secure firmware.\n");
 
 	register_firmware_ops(&exynos_firmware_ops);
+
+	/*
+	 * Exynos 4 SoCs (based on Cortex A9 and equipped with L2C-310),
+	 * running under secure firmware, require certain registers of L2
+	 * cache controller to be written in secure mode. Here .write_sec
+	 * callback is provided to perform necessary SMC calls.
+	 */
+	if (IS_ENABLED(CONFIG_CACHE_L2X0)
+	    && read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
+		outer_cache.write_sec = exynos_l2_write_sec;
 }
-- 
1.9.3


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

* [PATCH 5/5] ARM: dts: exynos4: Add nodes for L2 cache controller
  2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
                   ` (3 preceding siblings ...)
  2014-06-11 15:30 ` [PATCH 4/5] ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310 Tomasz Figa
@ 2014-06-11 15:30 ` Tomasz Figa
  4 siblings, 0 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 15:30 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: linux-arm-kernel, linux-kernel, linux-omap, Kukjin Kim,
	Laura Abbott, Linus Walleij, Robin Holt, Russell King,
	Santosh Shilimkar, Tony Lindgren, Tomasz Figa, Tomasz Figa

This patch adds device tree nodes for L2 cache controller present on
Exynos4 SoCs.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi | 9 +++++++++
 arch/arm/boot/dts/exynos4x12.dtsi | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index ee3001f..99970ab 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -54,6 +54,15 @@
 		reg = <0x10023CA0 0x20>;
 	};
 
+	l2c: l2-cache-controller@10502000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x10502000 0x1000>;
+		cache-unified;
+		cache-level = <2>;
+		arm,tag-latency = <2 2 1>;
+		arm,data-latency = <2 2 1>;
+	};
+
 	gic: interrupt-controller@10490000 {
 		cpu-offset = <0x8000>;
 	};
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c5a943d..9487f9c 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -60,6 +60,15 @@
 		reg = <0x10023CA0 0x20>;
 	};
 
+	l2c: l2-cache-controller@10502000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x10502000 0x1000>;
+		cache-unified;
+		cache-level = <2>;
+		arm,tag-latency = <2 2 1>;
+		arm,data-latency = <3 2 1>;
+	};
+
 	clock: clock-controller@10030000 {
 		compatible = "samsung,exynos4412-clock";
 		reg = <0x10030000 0x20000>;
-- 
1.9.3


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

* Re: [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback
  2014-06-11 15:30 ` [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback Tomasz Figa
@ 2014-06-11 16:00   ` Jon Loeliger
  2014-06-11 16:07     ` Tomasz Figa
  0 siblings, 1 reply; 8+ messages in thread
From: Jon Loeliger @ 2014-06-11 16:00 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-samsung-soc, Kukjin Kim, Laura Abbott, Tony Lindgren,
	Linus Walleij, linux-kernel, Tomasz Figa, Santosh Shilimkar,
	Robin Holt, Russell King, linux-omap, linux-arm-kernel

> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> index 060a75e..ddaebcd 100644
> --- a/arch/arm/include/asm/mach/arch.h
> +++ b/arch/arm/include/asm/mach/arch.h
> @@ -46,7 +46,8 @@ struct machine_desc {
>         enum reboot_mode        reboot_mode;    /* default restart mode */
>         unsigned                l2c_aux_val;    /* L2 cache aux value   */
>         unsigned                l2c_aux_mask;   /* L2 cache aux mask    */
> -       void                    (*l2c_write_sec)(unsigned long, unsigned);
> +       void                    (*l2c_write_sec)(void __iomem *,
> +                                                unsigned long, unsigned);
>         struct smp_operations   *smp;           /* SMP operations       */
>         bool                    (*smp_init)(void);
>         void                    (*fixup)(struct tag *, char **);

> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index efc5cab..1695eab 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg)
>         if (val == readl_relaxed(base + reg))
>                 return;
>         if (outer_cache.write_sec)
> -               outer_cache.write_sec(val, reg);
> +               outer_cache.write_sec(base, val, reg);
>         else
>                 writel_relaxed(val, base + reg);
>  }

The parameter order (base, val, reg) seems very non-intuitive.
Are you matching some existing prototype or adhering to some
backwards compatibility issue?  If not wouldn't, say, (base, reg, val)
or (val, base, reg) be more intuitive?

Thanks,
jdl

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

* Re: [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback
  2014-06-11 16:00   ` Jon Loeliger
@ 2014-06-11 16:07     ` Tomasz Figa
  0 siblings, 0 replies; 8+ messages in thread
From: Tomasz Figa @ 2014-06-11 16:07 UTC (permalink / raw)
  To: Jon Loeliger
  Cc: linux-samsung-soc, Kukjin Kim, Laura Abbott, Tony Lindgren,
	Linus Walleij, linux-kernel, Tomasz Figa, Santosh Shilimkar,
	Robin Holt, Russell King, linux-omap, linux-arm-kernel

On 11.06.2014 18:00, Jon Loeliger wrote:
>> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
>> index 060a75e..ddaebcd 100644
>> --- a/arch/arm/include/asm/mach/arch.h
>> +++ b/arch/arm/include/asm/mach/arch.h
>> @@ -46,7 +46,8 @@ struct machine_desc {
>>         enum reboot_mode        reboot_mode;    /* default restart mode */
>>         unsigned                l2c_aux_val;    /* L2 cache aux value   */
>>         unsigned                l2c_aux_mask;   /* L2 cache aux mask    */
>> -       void                    (*l2c_write_sec)(unsigned long, unsigned);
>> +       void                    (*l2c_write_sec)(void __iomem *,
>> +                                                unsigned long, unsigned);
>>         struct smp_operations   *smp;           /* SMP operations       */
>>         bool                    (*smp_init)(void);
>>         void                    (*fixup)(struct tag *, char **);
> 
>> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
>> index efc5cab..1695eab 100644
>> --- a/arch/arm/mm/cache-l2x0.c
>> +++ b/arch/arm/mm/cache-l2x0.c
>> @@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg)
>>         if (val == readl_relaxed(base + reg))
>>                 return;
>>         if (outer_cache.write_sec)
>> -               outer_cache.write_sec(val, reg);
>> +               outer_cache.write_sec(base, val, reg);
>>         else
>>                 writel_relaxed(val, base + reg);
>>  }
> 
> The parameter order (base, val, reg) seems very non-intuitive.
> Are you matching some existing prototype or adhering to some
> backwards compatibility issue?  If not wouldn't, say, (base, reg, val)
> or (val, base, reg) be more intuitive?

Hmm, I didn't think too much about this, so this order is just whatever
first came to my mind, probably because I'm used to xxx_write(ctx, val,
reg) accessors found in many drivers.

Anyway, l2c_write_sec() in arm/mm/cache-l2x0.c, which calls
outer_cache.write_sec(), already uses (val, base, reg) convention, so
probably this one would be most suitable. I'll change in v2.

Best regards,
Tomasz

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

end of thread, other threads:[~2014-06-11 16:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-11 15:30 [PATCH 0/5] Handle non-secure L2C initialization on Exynos4 Tomasz Figa
2014-06-11 15:30 ` [PATCH 1/5] ARM: mm: cache-l2x0: Add base address argument to write_sec callback Tomasz Figa
2014-06-11 16:00   ` Jon Loeliger
2014-06-11 16:07     ` Tomasz Figa
2014-06-11 15:30 ` [PATCH 2/5] ARM: Get outer cache .write_sec callback from mach_desc only if not NULL Tomasz Figa
2014-06-11 15:30 ` [PATCH 3/5] ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers Tomasz Figa
2014-06-11 15:30 ` [PATCH 4/5] ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310 Tomasz Figa
2014-06-11 15:30 ` [PATCH 5/5] ARM: dts: exynos4: Add nodes for L2 cache controller Tomasz Figa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox