diff mbox

[v2,2/5] ARM: shmobile: r8a7740: Migrate to generic l2c OF initialization

Message ID 1423059315-28519-3-git-send-email-geert+renesas@glider.be (mailing list archive)
State Superseded
Headers show

Commit Message

Geert Uytterhoeven Feb. 4, 2015, 2:15 p.m. UTC
Migrate the generic r8a7740 platform from calling l2x0_of_init() to the
generic l2c OF initialization.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2
  - Keep only {,~}L2C_AUX_CTRL_SHARED_OVERRIDE
---
 arch/arm/mach-shmobile/setup-r8a7740.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Arnd Bergmann Feb. 4, 2015, 2:34 p.m. UTC | #1
On Wednesday 04 February 2015 15:15:12 Geert Uytterhoeven wrote:
>  DT_MACHINE_START(R8A7740_DT, "Generic R8A7740 (Flattened Device Tree)")
> +       .l2c_aux_val    = L2C_AUX_CTRL_SHARED_OVERRIDE,
> +       .l2c_aux_mask   = ~L2C_AUX_CTRL_SHARED_OVERRIDE,
>         .map_io         = r8a7740_map_io,
>         .init_early     = shmobile_init_delay,
>         .init_irq       = r8a7740_init_irq_of,

+Russell

I'd hope we could avoid having any overrides in here that are not
specified in DT. I can never remember what we discussed about particular
bits in the past though. Is this bit something we could add a binding
for, or could we make it enabled by default?

I assume you have to add it because the boot loader sets the wrong
default, right?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Feb. 4, 2015, 2:43 p.m. UTC | #2
Hi Arnd,

On Wed, Feb 4, 2015 at 3:34 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 04 February 2015 15:15:12 Geert Uytterhoeven wrote:
>>  DT_MACHINE_START(R8A7740_DT, "Generic R8A7740 (Flattened Device Tree)")
>> +       .l2c_aux_val    = L2C_AUX_CTRL_SHARED_OVERRIDE,
>> +       .l2c_aux_mask   = ~L2C_AUX_CTRL_SHARED_OVERRIDE,
>>         .map_io         = r8a7740_map_io,
>>         .init_early     = shmobile_init_delay,
>>         .init_irq       = r8a7740_init_irq_of,
>
> +Russell
>
> I'd hope we could avoid having any overrides in here that are not
> specified in DT. I can never remember what we discussed about particular
> bits in the past though. Is this bit something we could add a binding
> for, or could we make it enabled by default?
>
> I assume you have to add it because the boot loader sets the wrong
> default, right?

Indeed. Note that I only try to preserve the register value before and after
migrating to generic l2c OF initialization. I did read (part of) the pl310
documentation, but it's still not clear to me if we really need
L2C_AUX_CTRL_SHARED_OVERRIDE. Perhaps this was just copied from
somewhere else without much afterthought?

The previous version was in thread
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/318328.html

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Feb. 4, 2015, 4:36 p.m. UTC | #3
On Wednesday 04 February 2015 15:43:43 Geert Uytterhoeven wrote:
> 
> Indeed. Note that I only try to preserve the register value before and after
> migrating to generic l2c OF initialization. I did read (part of) the pl310
> documentation, but it's still not clear to me if we really need
> L2C_AUX_CTRL_SHARED_OVERRIDE. Perhaps this was just copied from
> somewhere else without much afterthought?

Have you checked what the hardware/bootloader actually sets in that bit?

Maybe you can remove that override completely and the initial Linux
default was just set to the same that is already set on powerup?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Feb. 4, 2015, 5:04 p.m. UTC | #4
Hi Arnd,

On Wed, Feb 4, 2015 at 5:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 04 February 2015 15:43:43 Geert Uytterhoeven wrote:
>>
>> Indeed. Note that I only try to preserve the register value before and after
>> migrating to generic l2c OF initialization. I did read (part of) the pl310
>> documentation, but it's still not clear to me if we really need
>> L2C_AUX_CTRL_SHARED_OVERRIDE. Perhaps this was just copied from
>> somewhere else without much afterthought?
>
> Have you checked what the hardware/bootloader actually sets in that bit?

Yes, it boots with 0x02040000, i.e. L2C_AUX_CTRL_SHARED_OVERRIDE
(bit 22) is not set.

After the l2c driver is finished with it, it contains 0x46440001.

> Maybe you can remove that override completely and the initial Linux
> default was just set to the same that is already set on powerup?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Feb. 4, 2015, 6:33 p.m. UTC | #5
On Wednesday 04 February 2015 18:04:57 Geert Uytterhoeven wrote:
> On Wed, Feb 4, 2015 at 5:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 04 February 2015 15:43:43 Geert Uytterhoeven wrote:
> >>
> >> Indeed. Note that I only try to preserve the register value before and after
> >> migrating to generic l2c OF initialization. I did read (part of) the pl310
> >> documentation, but it's still not clear to me if we really need
> >> L2C_AUX_CTRL_SHARED_OVERRIDE. Perhaps this was just copied from
> >> somewhere else without much afterthought?
> >
> > Have you checked what the hardware/bootloader actually sets in that bit?
> 
> Yes, it boots with 0x02040000, i.e. L2C_AUX_CTRL_SHARED_OVERRIDE
> (bit 22) is not set.
> 
> After the l2c driver is finished with it, it contains 0x46440001.

I see. I guess someone understand the effect of this flag.
Does it make a difference on the machine you are looking at?

I've read the spec at http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246a/DDI0246A_l2cc_pl310_r0p0_trm.pdf
on the topic but couldn't make sense of it to understand whether a
particular setting might be required for specific hardware or might
be a performance optimization.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Feb. 4, 2015, 6:40 p.m. UTC | #6
Hi Arnd,

On Wed, Feb 4, 2015 at 7:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 04 February 2015 18:04:57 Geert Uytterhoeven wrote:
>> On Wed, Feb 4, 2015 at 5:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 04 February 2015 15:43:43 Geert Uytterhoeven wrote:
>> >>
>> >> Indeed. Note that I only try to preserve the register value before and after
>> >> migrating to generic l2c OF initialization. I did read (part of) the pl310
>> >> documentation, but it's still not clear to me if we really need
>> >> L2C_AUX_CTRL_SHARED_OVERRIDE. Perhaps this was just copied from
>> >> somewhere else without much afterthought?
>> >
>> > Have you checked what the hardware/bootloader actually sets in that bit?
>>
>> Yes, it boots with 0x02040000, i.e. L2C_AUX_CTRL_SHARED_OVERRIDE
>> (bit 22) is not set.
>>
>> After the l2c driver is finished with it, it contains 0x46440001.
>
> I see. I guess someone understand the effect of this flag.
> Does it make a difference on the machine you are looking at?

Nope. I booted it without, too. No obvious differences.

> I've read the spec at http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246a/DDI0246A_l2cc_pl310_r0p0_trm.pdf
> on the topic but couldn't make sense of it to understand whether a
> particular setting might be required for specific hardware or might
> be a performance optimization.

Thanks! Ah, so I'm not alone...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/setup-r8a7740.c b/arch/arm/mach-shmobile/setup-r8a7740.c
index d191cf4197313482..3ebd4dfc8e853a9b 100644
--- a/arch/arm/mach-shmobile/setup-r8a7740.c
+++ b/arch/arm/mach-shmobile/setup-r8a7740.c
@@ -835,10 +835,6 @@  static void __init r8a7740_generic_init(void)
 {
 	r8a7740_meram_workaround();
 
-#ifdef CONFIG_CACHE_L2X0
-	/* Shared attribute override enable, 32K*8way */
-	l2x0_init(IOMEM(0xf0002000), 0x00400000, 0xc20f0fff);
-#endif
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
@@ -855,6 +851,8 @@  static const char *r8a7740_boards_compat_dt[] __initdata = {
 };
 
 DT_MACHINE_START(R8A7740_DT, "Generic R8A7740 (Flattened Device Tree)")
+	.l2c_aux_val	= L2C_AUX_CTRL_SHARED_OVERRIDE,
+	.l2c_aux_mask	= ~L2C_AUX_CTRL_SHARED_OVERRIDE,
 	.map_io		= r8a7740_map_io,
 	.init_early	= shmobile_init_delay,
 	.init_irq	= r8a7740_init_irq_of,