[PATCH] ARM: prima2: remove L2 cache size override

[PATCH] ARM: prima2: remove L2 cache size override

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

L2 cache size has been read by l2x0_init(), it is useless to set
WAY_SIZE and ASSOCIATIVITY in prima2.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 -Derived from Russell's "ARM: l2c: prima2: remove cache size override"

 arch/arm/boot/dts/marco.dtsi  |    2 +-
 arch/arm/boot/dts/prima2.dtsi |    2 +-
 arch/arm/mach-prima2/common.c |   14 +++++++++--
 arch/arm/mach-prima2/l2x0.c   |   49 -----------------------------------------
 4 files changed, 13 insertions(+), 54 deletions(-)
 delete mode 100644 arch/arm/mach-prima2/l2x0.c

diff --git a/arch/arm/boot/dts/marco.dtsi b/arch/arm/boot/dts/marco.dtsi
index 0c9647d..fb35422 100644
--- a/arch/arm/boot/dts/marco.dtsi
+++ b/arch/arm/boot/dts/marco.dtsi
@@ -36,7 +36,7 @@
 		ranges = <0x40000000 0x40000000 0xa0000000>;
 
 		l2-cache-controller at c0030000 {
-			compatible = "sirf,marco-pl310-cache", "arm,pl310-cache";
+			compatible = "arm,pl310-cache";
 			reg = <0xc0030000 0x1000>;
 			interrupts = <0 59 0>;
 			arm,tag-latency = <1 1 1>;
diff --git a/arch/arm/boot/dts/prima2.dtsi b/arch/arm/boot/dts/prima2.dtsi
index 1e82571..0d6588d 100644
--- a/arch/arm/boot/dts/prima2.dtsi
+++ b/arch/arm/boot/dts/prima2.dtsi
@@ -48,7 +48,7 @@
 		ranges = <0x40000000 0x40000000 0x80000000>;
 
 		l2-cache-controller at 80040000 {
-			compatible = "arm,pl310-cache", "sirf,prima2-pl310-cache";
+			compatible = "arm,pl310-cache";
 			reg = <0x80040000 0x1000>;
 			interrupts = <59>;
 			arm,tag-latency = <1 1 1>;
diff --git a/arch/arm/mach-prima2/common.c b/arch/arm/mach-prima2/common.c
index 47c7819..23fcade 100644
--- a/arch/arm/mach-prima2/common.c
+++ b/arch/arm/mach-prima2/common.c
@@ -8,11 +8,10 @@
 
 #include <linux/init.h>
 #include <linux/kernel.h>
-#include <asm/sizes.h>
-#include <asm/mach-types.h>
-#include <asm/mach/arch.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/mach/arch.h>
 #include "common.h"
 
 static void __init sirfsoc_init_late(void)
@@ -26,6 +25,12 @@ static __init void sirfsoc_map_io(void)
 	sirfsoc_map_scu();
 }
 
+static __init void sirfsoc_init_mach(void)
+{
+	l2x0_of_init(0, 0xFDFFFFFFUL);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
+
 #ifdef CONFIG_ARCH_ATLAS6
 static const char *atlas6_dt_match[] __initconst = {
 	"sirf,atlas6",
@@ -35,6 +40,7 @@ static const char *atlas6_dt_match[] __initconst = {
 DT_MACHINE_START(ATLAS6_DT, "Generic ATLAS6 (Flattened Device Tree)")
 	/* Maintainer: Barry Song <baohua.song@csr.com> */
 	.map_io         = sirfsoc_map_io,
+	.init_machine   = sirfsoc_init_mach,
 	.init_late	= sirfsoc_init_late,
 	.dt_compat      = atlas6_dt_match,
 MACHINE_END
@@ -50,6 +56,7 @@ DT_MACHINE_START(PRIMA2_DT, "Generic PRIMA2 (Flattened Device Tree)")
 	/* Maintainer: Barry Song <baohua.song@csr.com> */
 	.map_io         = sirfsoc_map_io,
 	.dma_zone_size	= SZ_256M,
+	.init_machine   = sirfsoc_init_mach,
 	.init_late	= sirfsoc_init_late,
 	.dt_compat      = prima2_dt_match,
 MACHINE_END
@@ -65,6 +72,7 @@ DT_MACHINE_START(MARCO_DT, "Generic MARCO (Flattened Device Tree)")
 	/* Maintainer: Barry Song <baohua.song@csr.com> */
 	.smp            = smp_ops(sirfsoc_smp_ops),
 	.map_io         = sirfsoc_map_io,
+	.init_machine   = sirfsoc_init_mach,
 	.init_late	= sirfsoc_init_late,
 	.dt_compat      = marco_dt_match,
 MACHINE_END
diff --git a/arch/arm/mach-prima2/l2x0.c b/arch/arm/mach-prima2/l2x0.c
deleted file mode 100644
index c710253..0000000
--- a/arch/arm/mach-prima2/l2x0.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * l2 cache initialization for CSR SiRFprimaII
- *
- * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
- *
- * Licensed under GPLv2 or later.
- */
-
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/of.h>
-#include <asm/hardware/cache-l2x0.h>
-
-struct l2x0_aux {
-	u32 val;
-	u32 mask;
-};
-
-static const struct l2x0_aux prima2_l2x0_aux __initconst = {
-	.val = 2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT,
-	.mask =	0,
-};
-
-static const struct l2x0_aux marco_l2x0_aux __initconst = {
-	.val = (2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT) |
-		(1 << L2X0_AUX_CTRL_ASSOCIATIVITY_SHIFT),
-	.mask = L2X0_AUX_CTRL_MASK,
-};
-
-static const struct of_device_id sirf_l2x0_ids[] __initconst = {
-	{ .compatible = "sirf,prima2-pl310-cache", .data = &prima2_l2x0_aux, },
-	{ .compatible = "sirf,marco-pl310-cache", .data = &marco_l2x0_aux, },
-	{},
-};
-
-static int __init sirfsoc_l2x0_init(void)
-{
-	struct device_node *np;
-	const struct l2x0_aux *aux;
-
-	np = of_find_matching_node(NULL, sirf_l2x0_ids);
-	if (np) {
-		aux = of_match_node(sirf_l2x0_ids, np)->data;
-		return l2x0_of_init(aux->val, aux->mask);
-	}
-
-	return 0;
-}
-early_initcall(sirfsoc_l2x0_init);
-- 
1.7.5.4

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
> From: Barry Song <Baohua.Song@csr.com>
> 
> L2 cache size has been read by l2x0_init(), it is useless to set
> WAY_SIZE and ASSOCIATIVITY in prima2.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  -Derived from Russell's "ARM: l2c: prima2: remove cache size override"

You're a SMP architecture... it would be much better to initialise the
L2 cache before the secondary CPUs are brought online.

In any case, across the board, it's preferable that the L2 cache is
enabled before the delay loop calibration as it can have an effect on
the calibrated value.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Barry Song]

2014-04-15 18:00 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> L2 cache size has been read by l2x0_init(), it is useless to set
>> WAY_SIZE and ASSOCIATIVITY in prima2.
>>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  -Derived from Russell's "ARM: l2c: prima2: remove cache size override"
>
> You're a SMP architecture... it would be much better to initialise the
> L2 cache before the secondary CPUs are brought online.

ok, it seems we still need to hook this into the callback of this
specific machine instead of using a global early_initcall(). since now
the dt compatible field has been generic enough, it will cause other
platforms to execute l2x0_of_init() too.

>
> In any case, across the board, it's preferable that the L2 cache is
> enabled before the delay loop calibration as it can have an effect on
> the calibrated value.

what if the real case is that l2x0 has been enabled in bootloader?

>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.

-barry

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
> 2014-04-15 18:00 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
> >> From: Barry Song <Baohua.Song@csr.com>
> >>
> >> L2 cache size has been read by l2x0_init(), it is useless to set
> >> WAY_SIZE and ASSOCIATIVITY in prima2.
> >>
> >> Cc: Russell King <linux@arm.linux.org.uk>
> >> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> >> ---
> >>  -Derived from Russell's "ARM: l2c: prima2: remove cache size override"
> >
> > You're a SMP architecture... it would be much better to initialise the
> > L2 cache before the secondary CPUs are brought online.
> 
> ok, it seems we still need to hook this into the callback of this
> specific machine instead of using a global early_initcall(). since now
> the dt compatible field has been generic enough, it will cause other
> platforms to execute l2x0_of_init() too.

What I did for Versatile Express was to move it to the init_irq stage.
I'm not condoning using init_irq() for non-IRQ stuff, but that was
just to see whether it was possible (the .init_early is too early for
it.)  Let me put it another way: I'm not thrilled by the idea of
having it in init_irq() but that's a better location to call L2 cache
initialisation than half-way through the kernel's initialisation.

> > In any case, across the board, it's preferable that the L2 cache is
> > enabled before the delay loop calibration as it can have an effect on
> > the calibrated value.
> 
> what if the real case is that l2x0 has been enabled in bootloader?

If the L2 cache has already been enabled, l2x0_of_init() handles it -
in that situation, we can't re-do the enable (because writing to the
aux ctrl register is not allowed) so we merely setup the outer cache
function pointers and don't try to do any configuration or enabling.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Barry Song]

2014-04-16 5:56 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
>> 2014-04-15 18:00 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
>> > On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
>> >> From: Barry Song <Baohua.Song@csr.com>
>> >>
>> >> L2 cache size has been read by l2x0_init(), it is useless to set
>> >> WAY_SIZE and ASSOCIATIVITY in prima2.
>> >>
>> >> Cc: Russell King <linux@arm.linux.org.uk>
>> >> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> >> ---
>> >>  -Derived from Russell's "ARM: l2c: prima2: remove cache size override"
>> >
>> > You're a SMP architecture... it would be much better to initialise the
>> > L2 cache before the secondary CPUs are brought online.
>>
>> ok, it seems we still need to hook this into the callback of this
>> specific machine instead of using a global early_initcall(). since now
>> the dt compatible field has been generic enough, it will cause other
>> platforms to execute l2x0_of_init() too.
>
> What I did for Versatile Express was to move it to the init_irq stage.
> I'm not condoning using init_irq() for non-IRQ stuff, but that was
> just to see whether it was possible (the .init_early is too early for
> it.)  Let me put it another way: I'm not thrilled by the idea of
> having it in init_irq() but that's a better location to call L2 cache
> initialisation than half-way through the kernel's initialisation.

i would like to believe you are right since you have dug into the
callbacks and finally thinks init_irq() is the suitable stage for it
even though it looks misnamed.

>
>> > In any case, across the board, it's preferable that the L2 cache is
>> > enabled before the delay loop calibration as it can have an effect on
>> > the calibrated value.
>>
>> what if the real case is that l2x0 has been enabled in bootloader?
>
> If the L2 cache has already been enabled, l2x0_of_init() handles it -
> in that situation, we can't re-do the enable (because writing to the
> aux ctrl register is not allowed) so we merely setup the outer cache
> function pointers and don't try to do any configuration or enabling.

yes. i meant if that has been done in bootloader, it turns out we
don't care about which callback is better any more. and for linux
running in non-security, same.

>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.

- barry

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 04:24:34PM +0800, Barry Song wrote:
> yes. i meant if that has been done in bootloader, it turns out we
> don't care about which callback is better any more. and for linux
> running in non-security, same.

However, I don't think it's quite as simple as that, because things
like the Cortex A9 Full-Line-Of-Zeros feature need a specific enable
sequence which (I think) can't be done in the boot loader.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Arnd Bergmann]

On Tuesday 15 April 2014 22:56:15 Russell King - ARM Linux wrote:
> On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
> > 
> > ok, it seems we still need to hook this into the callback of this
> > specific machine instead of using a global early_initcall(). since now
> > the dt compatible field has been generic enough, it will cause other
> > platforms to execute l2x0_of_init() too.
> 
> What I did for Versatile Express was to move it to the init_irq stage.
> I'm not condoning using init_irq() for non-IRQ stuff, but that was
> just to see whether it was possible (the .init_early is too early for
> it.)  Let me put it another way: I'm not thrilled by the idea of
> having it in init_irq() but that's a better location to call L2 cache
> initialisation than half-way through the kernel's initialisation.

Would it help to introduce a new .init_l2_cache() callback? That
way, the architecture code can default to initializing any l2x0
that it finds in DT, and have give platforms the option of
overriding the call if they need to, and we can do it at the
same point during early boot for everyone.

A number of platforms currently have the same

static void __init this_platform_init(void)
{
      l2x0_of_init(0, ~0UL);
      of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}

Where the second line is already the default for .init_machine,
and it would be nice to get rid of the callback entirely, especially
for the machines that don't do anything else in their platform
code.

	Arnd

[PATCH] ARM: prima2: remove L2 cache size override

[Barry Song]

2014-04-16 16:52 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 15 April 2014 22:56:15 Russell King - ARM Linux wrote:
>> On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
>> >
>> > ok, it seems we still need to hook this into the callback of this
>> > specific machine instead of using a global early_initcall(). since now
>> > the dt compatible field has been generic enough, it will cause other
>> > platforms to execute l2x0_of_init() too.
>>
>> What I did for Versatile Express was to move it to the init_irq stage.
>> I'm not condoning using init_irq() for non-IRQ stuff, but that was
>> just to see whether it was possible (the .init_early is too early for
>> it.)  Let me put it another way: I'm not thrilled by the idea of
>> having it in init_irq() but that's a better location to call L2 cache
>> initialisation than half-way through the kernel's initialisation.
>
> Would it help to introduce a new .init_l2_cache() callback? That
> way, the architecture code can default to initializing any l2x0
> that it finds in DT, and have give platforms the option of
> overriding the call if they need to, and we can do it at the
> same point during early boot for everyone.
>
> A number of platforms currently have the same
>
> static void __init this_platform_init(void)
> {
>       l2x0_of_init(0, ~0UL);
>       of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
>
> Where the second line is already the default for .init_machine,
> and it would be nice to get rid of the callback entirely, especially
> for the machines that don't do anything else in their platform
> code.

it seems you mean put a general customize_l2x0_init() like customize_machine().

what is the reason stopping l2x0 from becoming a standalone driver
which can initialize itself?

>
>         Arnd

-barry

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 10:52:27AM +0200, Arnd Bergmann wrote:
> On Tuesday 15 April 2014 22:56:15 Russell King - ARM Linux wrote:
> > On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
> > > 
> > > ok, it seems we still need to hook this into the callback of this
> > > specific machine instead of using a global early_initcall(). since now
> > > the dt compatible field has been generic enough, it will cause other
> > > platforms to execute l2x0_of_init() too.
> > 
> > What I did for Versatile Express was to move it to the init_irq stage.
> > I'm not condoning using init_irq() for non-IRQ stuff, but that was
> > just to see whether it was possible (the .init_early is too early for
> > it.)  Let me put it another way: I'm not thrilled by the idea of
> > having it in init_irq() but that's a better location to call L2 cache
> > initialisation than half-way through the kernel's initialisation.
> 
> Would it help to introduce a new .init_l2_cache() callback? That
> way, the architecture code can default to initializing any l2x0
> that it finds in DT, and have give platforms the option of
> overriding the call if they need to, and we can do it at the
> same point during early boot for everyone.

As my ideal target is to remove l2x0_*init from platforms, I'd rather
not do that unless we have a clear idea that we are going to keep this
around.

Another potential solution would be to have the two auxctl values
specified, and do nothing if they're both zero.

First though, we really need to work out what each platform requires
from these, and turn everything else into either something the core
code handles, or if there is no good reason for it, it is simply
removed.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 05:13:18PM +0800, Barry Song wrote:
> 2014-04-16 16:52 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> > On Tuesday 15 April 2014 22:56:15 Russell King - ARM Linux wrote:
> >> On Tue, Apr 15, 2014 at 06:13:27PM +0800, Barry Song wrote:
> >> >
> >> > ok, it seems we still need to hook this into the callback of this
> >> > specific machine instead of using a global early_initcall(). since now
> >> > the dt compatible field has been generic enough, it will cause other
> >> > platforms to execute l2x0_of_init() too.
> >>
> >> What I did for Versatile Express was to move it to the init_irq stage.
> >> I'm not condoning using init_irq() for non-IRQ stuff, but that was
> >> just to see whether it was possible (the .init_early is too early for
> >> it.)  Let me put it another way: I'm not thrilled by the idea of
> >> having it in init_irq() but that's a better location to call L2 cache
> >> initialisation than half-way through the kernel's initialisation.
> >
> > Would it help to introduce a new .init_l2_cache() callback? That
> > way, the architecture code can default to initializing any l2x0
> > that it finds in DT, and have give platforms the option of
> > overriding the call if they need to, and we can do it at the
> > same point during early boot for everyone.
> >
> > A number of platforms currently have the same
> >
> > static void __init this_platform_init(void)
> > {
> >       l2x0_of_init(0, ~0UL);
> >       of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > }
> >
> > Where the second line is already the default for .init_machine,
> > and it would be nice to get rid of the callback entirely, especially
> > for the machines that don't do anything else in their platform
> > code.
> 
> it seems you mean put a general customize_l2x0_init() like customize_machine().
> 
> what is the reason stopping l2x0 from becoming a standalone driver
> which can initialize itself?

Well... if you didn't need to give this:

	l2x0_of_init(0, 0xFDFFFFFFUL);

but instead could cope with:

	l2x0_of_init(0, ~0);

then that would be one less reason for it /not/ to be a standalone driver.
It's every "tweak" that the platform does that stands in the way.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
> +static __init void sirfsoc_init_mach(void)
> +{
> +	l2x0_of_init(0, 0xFDFFFFFFUL);

BTW, this should be just the plain: 0xfdffffff - no need for UL on the
end.  Also lower case is preferred for hex.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Wed, Apr 16, 2014 at 10:24:33AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
> > +static __init void sirfsoc_init_mach(void)
> > +{
> > +	l2x0_of_init(0, 0xFDFFFFFFUL);
> 
> BTW, this should be just the plain: 0xfdffffff - no need for UL on the
> end.  Also lower case is preferred for hex.

So... I provide feedback, and nothing happens.

FYI, tonight, I'm pushing out my L2C branch into linux-next with the
original patch for prima2 incorporated, as my concerns against your
patch have not been addressed.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] ARM: prima2: remove L2 cache size override

[Barry Song]

2014-04-24 2:21 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Wed, Apr 16, 2014 at 10:24:33AM +0100, Russell King - ARM Linux wrote:
>> On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
>> > +static __init void sirfsoc_init_mach(void)
>> > +{
>> > +   l2x0_of_init(0, 0xFDFFFFFFUL);
>>
>> BTW, this should be just the plain: 0xfdffffff - no need for UL on the
>> end.  Also lower case is preferred for hex.
>
> So... I provide feedback, and nothing happens.
>
> FYI, tonight, I'm pushing out my L2C branch into linux-next with the
> original patch for prima2 incorporated, as my concerns against your
> patch have not been addressed.

Russell, i do agree with your feedback.
but, in your original patch,  moving to early_initcall will make other
non-csr platforms call l2x0_of_init() and finish initialization by (0,
~0) directly since other platforms also put same general compatible
prop in dts. in your patch, you have dropped "sirf,prima2-pl310-cache"
and "sirf,marco-pl310-cache", so it happens by matching general
"arm,pl310-cache".
i am still on the way to send v2. if it is not too urgent to you, will
you wait for me to send v2 by fixing issues in your feedback?

>

-barry

[PATCH] ARM: prima2: remove L2 cache size override

[Russell King - ARM Linux]

On Thu, Apr 24, 2014 at 07:11:46AM +0800, Barry Song wrote:
> 2014-04-24 2:21 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Wed, Apr 16, 2014 at 10:24:33AM +0100, Russell King - ARM Linux wrote:
> >> On Tue, Apr 15, 2014 at 05:49:02PM +0800, Barry Song wrote:
> >> > +static __init void sirfsoc_init_mach(void)
> >> > +{
> >> > +   l2x0_of_init(0, 0xFDFFFFFFUL);
> >>
> >> BTW, this should be just the plain: 0xfdffffff - no need for UL on the
> >> end.  Also lower case is preferred for hex.
> >
> > So... I provide feedback, and nothing happens.
> >
> > FYI, tonight, I'm pushing out my L2C branch into linux-next with the
> > original patch for prima2 incorporated, as my concerns against your
> > patch have not been addressed.
> 
> Russell, i do agree with your feedback.
> but, in your original patch,  moving to early_initcall will make other
> non-csr platforms call l2x0_of_init() and finish initialization by (0,
> ~0) directly since other platforms also put same general compatible
> prop in dts. in your patch, you have dropped "sirf,prima2-pl310-cache"
> and "sirf,marco-pl310-cache", so it happens by matching general
> "arm,pl310-cache".
> i am still on the way to send v2. if it is not too urgent to you, will
> you wait for me to send v2 by fixing issues in your feedback?

I have no problem adding a patch, or replacing my patch in the series.
All I ask is that it isn't forgotten, and that it's sent before -rc6.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.