* [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
@ 2017-02-06 8:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2017-02-06 8:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi Chris,
CC linux-arm-kernel
On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
>
> The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
> because the sideband signals between the CA9 and PL310 are not connected.
> Since there is no option to disable this feature in the cache-l2x0 driver,
> our only option is to specify a secure write function which will then cause
> the cache-l2x0 driver to not enable this feature.
What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
"arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-l2x0.c
instead?
> If you do not override a l2c_write_sec function which causes the line of
> zeros mode to be enabled, then the system will crash pretty quickly after
> the L2C is enabled.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> arch/arm/boot/dts/r7s72100.dtsi | 9 +++++++++
> arch/arm/mach-shmobile/setup-r7s72100.c | 21 +++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index 74e684f..08aaaff 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -177,6 +177,7 @@
> compatible = "arm,cortex-a9";
> reg = <0>;
> clock-frequency = <400000000>;
> + next-level-cache = <&L2>;
> };
> };
>
> @@ -368,6 +369,14 @@
> <0xe8202000 0x1000>;
> };
>
> + L2: cache-controller at 3ffff000 {
> + compatible = "arm,pl310-cache";
> + reg = <0x3ffff000 0x1000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + cache-unified;
> + cache-level = <2>;
> + };
> +
> i2c0: i2c at fcfee000 {
> #address-cells = <1>;
> #size-cells = <0>;
> diff --git a/arch/arm/mach-shmobile/setup-r7s72100.c b/arch/arm/mach-shmobile/setup-r7s72100.c
> index d46639f..655deba 100644
> --- a/arch/arm/mach-shmobile/setup-r7s72100.c
> +++ b/arch/arm/mach-shmobile/setup-r7s72100.c
> @@ -15,6 +15,7 @@
> */
>
> #include <linux/kernel.h>
> +#include <linux/io.h>
>
> #include <asm/mach/arch.h>
>
> @@ -25,7 +26,27 @@ static const char *const r7s72100_boards_compat_dt[] __initconst = {
> NULL,
> };
>
> +/*
> + * The 'Write full line of zeros mode' of this Cortex-A9 cannot be used because
> + * the sideband signals between the CA9 and PL310 are not connected. Since there
> + * is no option to disable this feature in the cache-l2x0 driver, our only
> + * option is to specify a secure write function which will then cause the
> + * cache-l2x0 driver to not enable this feature.
> + */
> +static void r7s72100_l2c_write_sec(unsigned long val, unsigned int reg)
> +{
> + static void __iomem *base;
> +
> + if (!base)
> + base = ioremap_nocache(0x3ffff000, SZ_4K);
FWIW, plain ioremap() is fine.
> +
> + writel_relaxed(val, base + reg);
> +}
> +
> DT_MACHINE_START(R7S72100_DT, "Generic R7S72100 (Flattened Device Tree)")
> + .l2c_aux_val = 0,
> + .l2c_aux_mask = ~0,
> + .l2c_write_sec = r7s72100_l2c_write_sec,
> .init_early = shmobile_init_delay,
> .init_late = shmobile_init_late,
> .dt_compat = r7s72100_boards_compat_dt,
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
@ 2017-02-06 8:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2017-02-06 8:50 UTC (permalink / raw)
To: Chris Brandt
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Russell King, devicetree, Linux-Renesas, linux-arm-kernel
Hi Chris,
CC linux-arm-kernel
On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
>
> The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
> because the sideband signals between the CA9 and PL310 are not connected.
> Since there is no option to disable this feature in the cache-l2x0 driver,
> our only option is to specify a secure write function which will then cause
> the cache-l2x0 driver to not enable this feature.
What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
"arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-l2x0.c
instead?
> If you do not override a l2c_write_sec function which causes the line of
> zeros mode to be enabled, then the system will crash pretty quickly after
> the L2C is enabled.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> arch/arm/boot/dts/r7s72100.dtsi | 9 +++++++++
> arch/arm/mach-shmobile/setup-r7s72100.c | 21 +++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index 74e684f..08aaaff 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -177,6 +177,7 @@
> compatible = "arm,cortex-a9";
> reg = <0>;
> clock-frequency = <400000000>;
> + next-level-cache = <&L2>;
> };
> };
>
> @@ -368,6 +369,14 @@
> <0xe8202000 0x1000>;
> };
>
> + L2: cache-controller@3ffff000 {
> + compatible = "arm,pl310-cache";
> + reg = <0x3ffff000 0x1000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + cache-unified;
> + cache-level = <2>;
> + };
> +
> i2c0: i2c@fcfee000 {
> #address-cells = <1>;
> #size-cells = <0>;
> diff --git a/arch/arm/mach-shmobile/setup-r7s72100.c b/arch/arm/mach-shmobile/setup-r7s72100.c
> index d46639f..655deba 100644
> --- a/arch/arm/mach-shmobile/setup-r7s72100.c
> +++ b/arch/arm/mach-shmobile/setup-r7s72100.c
> @@ -15,6 +15,7 @@
> */
>
> #include <linux/kernel.h>
> +#include <linux/io.h>
>
> #include <asm/mach/arch.h>
>
> @@ -25,7 +26,27 @@ static const char *const r7s72100_boards_compat_dt[] __initconst = {
> NULL,
> };
>
> +/*
> + * The 'Write full line of zeros mode' of this Cortex-A9 cannot be used because
> + * the sideband signals between the CA9 and PL310 are not connected. Since there
> + * is no option to disable this feature in the cache-l2x0 driver, our only
> + * option is to specify a secure write function which will then cause the
> + * cache-l2x0 driver to not enable this feature.
> + */
> +static void r7s72100_l2c_write_sec(unsigned long val, unsigned int reg)
> +{
> + static void __iomem *base;
> +
> + if (!base)
> + base = ioremap_nocache(0x3ffff000, SZ_4K);
FWIW, plain ioremap() is fine.
> +
> + writel_relaxed(val, base + reg);
> +}
> +
> DT_MACHINE_START(R7S72100_DT, "Generic R7S72100 (Flattened Device Tree)")
> + .l2c_aux_val = 0,
> + .l2c_aux_mask = ~0,
> + .l2c_write_sec = r7s72100_l2c_write_sec,
> .init_early = shmobile_init_delay,
> .init_late = shmobile_init_late,
> .dt_compat = r7s72100_boards_compat_dt,
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
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
2017-02-06 8:50 ` Geert Uytterhoeven
@ 2017-02-06 14:58 ` Chris Brandt
-1 siblings, 0 replies; 12+ messages in thread
From: Chris Brandt @ 2017-02-06 14:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Russell King, devicetree, Linux-Renesas, linux-arm-kernel
Hi Geert,
On Monday, February 06, 2017, Geert Uytterhoeven wrote:
> CC linux-arm-kernel
>
> On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com>
> wrote:
> > This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
> >
> > The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
> > because the sideband signals between the CA9 and PL310 are not connected.
> > Since there is no option to disable this feature in the cache-l2x0
> > driver, our only option is to specify a secure write function which
> > will then cause the cache-l2x0 driver to not enable this feature.
>
> What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
> "arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-
> l2x0.c instead?
Well, first I have to say that 'broken-sideband' is not actually "accurate"
in this case.
From the RZ/A1H Hardware Manual:
4. Secondary Cache
4.1 Features
* Sideband signal from CA9: No
So the chip designers knew the sideband signals were not connected.
If you have a look at the next chapter "5. LSI Internal Bus", you'll notice
that the CA9 is on the North bus (fig 5.2) but the PL310 is on the south
bus (fig 5.3) in between the AXI and the SDRAM/QSPI controller. So in this
in SoC, maybe the PL310 looks more like a L3 than an L2 cache???
So, I would say "arm,pl310-no-sideband" is a better name.
I agree that faking out a secure write function just so the fill-zeros
sideband feature is not enabled is a bit of a hack, but I'm not sure if
modifying the cache-l2x0.c was an option.
If you think so, I can try the "arm,pl310-no-sideband" path first,
and if that doesn't get in I can fall back to what I'm doing now.
Thoughts???
> > +static void r7s72100_l2c_write_sec(unsigned long val, unsigned int
> > +reg) {
> > + static void __iomem *base;
> > +
> > + if (!base)
> > + base = ioremap_nocache(0x3ffff000, SZ_4K);
>
> FWIW, plain ioremap() is fine.
Of course they both go to the same thing for ARM, but I thought the "_nocache"
version should be used for sanity purposes (as ie, "yes, I know what I'm
doing").
Thanks,
Chris
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
@ 2017-02-06 14:58 ` Chris Brandt
0 siblings, 0 replies; 12+ messages in thread
From: Chris Brandt @ 2017-02-06 14:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi Geert,
On Monday, February 06, 2017, Geert Uytterhoeven wrote:
> CC linux-arm-kernel
>
> On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com>
> wrote:
> > This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
> >
> > The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
> > because the sideband signals between the CA9 and PL310 are not connected.
> > Since there is no option to disable this feature in the cache-l2x0
> > driver, our only option is to specify a secure write function which
> > will then cause the cache-l2x0 driver to not enable this feature.
>
> What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
> "arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-
> l2x0.c instead?
Well, first I have to say that 'broken-sideband' is not actually "accurate"
in this case.
>From the RZ/A1H Hardware Manual:
4. Secondary Cache
4.1 Features
* Sideband signal from CA9: No
So the chip designers knew the sideband signals were not connected.
If you have a look at the next chapter "5. LSI Internal Bus", you'll notice
that the CA9 is on the North bus (fig 5.2) but the PL310 is on the south
bus (fig 5.3) in between the AXI and the SDRAM/QSPI controller. So in this
in SoC, maybe the PL310 looks more like a L3 than an L2 cache???
So, I would say "arm,pl310-no-sideband" is a better name.
I agree that faking out a secure write function just so the fill-zeros
sideband feature is not enabled is a bit of a hack, but I'm not sure if
modifying the cache-l2x0.c was an option.
If you think so, I can try the "arm,pl310-no-sideband" path first,
and if that doesn't get in I can fall back to what I'm doing now.
Thoughts???
> > +static void r7s72100_l2c_write_sec(unsigned long val, unsigned int
> > +reg) {
> > + static void __iomem *base;
> > +
> > + if (!base)
> > + base = ioremap_nocache(0x3ffff000, SZ_4K);
>
> FWIW, plain ioremap() is fine.
Of course they both go to the same thing for ARM, but I thought the "_nocache"
version should be used for sanity purposes (as ie, "yes, I know what I'm
doing").
Thanks,
Chris
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
2017-02-06 14:58 ` Chris Brandt
@ 2017-02-06 15:30 ` Geert Uytterhoeven
-1 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2017-02-06 15:30 UTC (permalink / raw)
To: Chris Brandt
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Russell King, devicetree, Linux-Renesas, linux-arm-kernel
Hi Chris,
On Mon, Feb 6, 2017 at 3:58 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, February 06, 2017, Geert Uytterhoeven wrote:
>> CC linux-arm-kernel
>>
>> On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com>
>> wrote:
>> > This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
>> >
>> > The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
>> > because the sideband signals between the CA9 and PL310 are not connected.
>> > Since there is no option to disable this feature in the cache-l2x0
>> > driver, our only option is to specify a secure write function which
>> > will then cause the cache-l2x0 driver to not enable this feature.
>>
>> What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
>> "arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-
>> l2x0.c instead?
>
> Well, first I have to say that 'broken-sideband' is not actually "accurate"
> in this case.
>
> From the RZ/A1H Hardware Manual:
>
> 4. Secondary Cache
> 4.1 Features
>
> * Sideband signal from CA9: No
OK.
> So the chip designers knew the sideband signals were not connected.
> If you have a look at the next chapter "5. LSI Internal Bus", you'll notice
> that the CA9 is on the North bus (fig 5.2) but the PL310 is on the south
> bus (fig 5.3) in between the AXI and the SDRAM/QSPI controller. So in this
> in SoC, maybe the PL310 looks more like a L3 than an L2 cache???
No, according to Figures 5.1 and 5.3, the CA9 is connected to both the
North and South main buses.
> So, I would say "arm,pl310-no-sideband" is a better name.
OK.
> I agree that faking out a secure write function just so the fill-zeros
> sideband feature is not enabled is a bit of a hack, but I'm not sure if
> modifying the cache-l2x0.c was an option.
Given I've added "arm,shared-override" in the past, I'd say yes ;-)
> If you think so, I can try the "arm,pl310-no-sideband" path first,
> and if that doesn't get in I can fall back to what I'm doing now.
>
> Thoughts???
According to the "CoreLink Level 2 Cache Controller L2C-310" TRM,
"no sideband signals" is even the default configuration?
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
@ 2017-02-06 15:30 ` Geert Uytterhoeven
0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2017-02-06 15:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Chris,
On Mon, Feb 6, 2017 at 3:58 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, February 06, 2017, Geert Uytterhoeven wrote:
>> CC linux-arm-kernel
>>
>> On Thu, Feb 2, 2017 at 10:20 PM, Chris Brandt <chris.brandt@renesas.com>
>> wrote:
>> > This enables the 128KB L2 cache in the RZ/A1 (R7S72100).
>> >
>> > The 'Write full line of zeros mode' of this Cortex-A9 cannot be used
>> > because the sideband signals between the CA9 and PL310 are not connected.
>> > Since there is no option to disable this feature in the cache-l2x0
>> > driver, our only option is to specify a secure write function which
>> > will then cause the cache-l2x0 driver to not enable this feature.
>>
>> What about adding a DT property (e.g. "arm,pl310-broken-sideband", cfr.
>> "arm,pl330-broken-no-flushp"), and handling this in arch/arm/mm/cache-
>> l2x0.c instead?
>
> Well, first I have to say that 'broken-sideband' is not actually "accurate"
> in this case.
>
> From the RZ/A1H Hardware Manual:
>
> 4. Secondary Cache
> 4.1 Features
>
> * Sideband signal from CA9: No
OK.
> So the chip designers knew the sideband signals were not connected.
> If you have a look at the next chapter "5. LSI Internal Bus", you'll notice
> that the CA9 is on the North bus (fig 5.2) but the PL310 is on the south
> bus (fig 5.3) in between the AXI and the SDRAM/QSPI controller. So in this
> in SoC, maybe the PL310 looks more like a L3 than an L2 cache???
No, according to Figures 5.1 and 5.3, the CA9 is connected to both the
North and South main buses.
> So, I would say "arm,pl310-no-sideband" is a better name.
OK.
> I agree that faking out a secure write function just so the fill-zeros
> sideband feature is not enabled is a bit of a hack, but I'm not sure if
> modifying the cache-l2x0.c was an option.
Given I've added "arm,shared-override" in the past, I'd say yes ;-)
> If you think so, I can try the "arm,pl310-no-sideband" path first,
> and if that doesn't get in I can fall back to what I'm doing now.
>
> Thoughts???
According to the "CoreLink Level 2 Cache Controller L2C-310" TRM,
"no sideband signals" is even the default configuration?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
2017-02-06 15:30 ` Geert Uytterhoeven
@ 2017-02-06 16:02 ` Chris Brandt
-1 siblings, 0 replies; 12+ messages in thread
From: Chris Brandt @ 2017-02-06 16:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Russell King, devicetree, Linux-Renesas, linux-arm-kernel
Hi Geert,
On Monday, February 06, 2017, Geert Uytterhoeven wrote:
> > I agree that faking out a secure write function just so the fill-zeros
> > sideband feature is not enabled is a bit of a hack, but I'm not sure
> > if modifying the cache-l2x0.c was an option.
>
> Given I've added "arm,shared-override" in the past, I'd say yes ;-)
>
> > If you think so, I can try the "arm,pl310-no-sideband" path first, and
> > if that doesn't get in I can fall back to what I'm doing now.
> >
> > Thoughts???
>
> According to the "CoreLink Level 2 Cache Controller L2C-310" TRM, "no
> sideband signals" is even the default configuration?
Good point. So technically there is nothing "illegal" about hooking up
only the AXI interface and not the sideband signals.
I guess that also means things like "Early BRESP Enable" are not really
going to work either.
Of course you'll still see "L2C-310 enabling early BRESP for Cortex-A9"
printed out on boot, which is true because it was enabled in the PL310,
but in reality it will never get used.
False hope :(
At first I'll put together a patch just for Full line of zero write since
that one changes the behavior of the CA9....and will break a system.
Thanks,
Chris
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] ARM: shmobile: r7s72100: Enable L2 cache
@ 2017-02-06 16:02 ` Chris Brandt
0 siblings, 0 replies; 12+ messages in thread
From: Chris Brandt @ 2017-02-06 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi Geert,
On Monday, February 06, 2017, Geert Uytterhoeven wrote:
> > I agree that faking out a secure write function just so the fill-zeros
> > sideband feature is not enabled is a bit of a hack, but I'm not sure
> > if modifying the cache-l2x0.c was an option.
>
> Given I've added "arm,shared-override" in the past, I'd say yes ;-)
>
> > If you think so, I can try the "arm,pl310-no-sideband" path first, and
> > if that doesn't get in I can fall back to what I'm doing now.
> >
> > Thoughts???
>
> According to the "CoreLink Level 2 Cache Controller L2C-310" TRM, "no
> sideband signals" is even the default configuration?
Good point. So technically there is nothing "illegal" about hooking up
only the AXI interface and not the sideband signals.
I guess that also means things like "Early BRESP Enable" are not really
going to work either.
Of course you'll still see "L2C-310 enabling early BRESP for Cortex-A9"
printed out on boot, which is true because it was enabled in the PL310,
but in reality it will never get used.
False hope :(
At first I'll put together a patch just for Full line of zero write since
that one changes the behavior of the CA9....and will break a system.
Thanks,
Chris
^ permalink raw reply [flat|nested] 12+ messages in thread