Re: [PATCH 5/7 v6] ARM: l2c: parse 'cache-size' and 'cache-sets' properties

On Monday 08 September 2014 14:36:48 Linus Walleij wrote:

On Mon, Sep 8, 2014 at 2:20 PM, Arnd Bergmann [off-list ref] wrote:

quoted

On Monday 08 September 2014 13:38:04 Linus Walleij wrote:

quoted

+       of_property_read_u32(np, "cache-size", &size);
+       of_property_read_u32(np, "cache-sets", &sets);
+
+       if (!size || !sets)
+               return;
+
+       way_size = size / sets;

Going back to this one: Isn't (size / sets) the set-size rather
than the way-size?

After we discussed this on IRC, I had expected a calculation like

        set_size = size / sets;
        ways = set_size / line_size;
        way_size = size / ways;

First: in this PB1176 case:

set_size = 128K/8 = 16K
ways = 16K/32 = 512 bytes
way_size = 128K/512 = 128 bytes

I guess we should first try to find the right units so we know what
we are talking about. I was under the impression that the set size
is the number of bytes in a set, while 'ways' is counting the
associativity and has no unit.

The last line also seems to incorrectly computed. Dividing 128KB
by 512 bytes should be 256 (no unit).

Well maybe it's the ARM reference manual internal lingo that
is actually causing the confusion here. It will say something
like:

[19:17] Way-size 3’b000 = Reserved, internally mapped to 16KB
3’b001 = 16KB, this is the default value
3’b010 = 32KB
3’b011 = 64KB
3’b100 = 128KB
3’b101 = 256KB
3’b110 to 3’b111 = Reserved, internally mapped to 256 KB

OK way-size ... is in the 16 thru 256KB range, which fits nicely
with set size incidentally. And also corresponds to current
comments in the code such as this from
arch/arm/mach-realview/realview_pb1176.c:

#ifdef CONFIG_CACHE_L2X0
        /*
         * The PL220 needs to be manually configured as the hardware
         * doesn't report the correct sizes.
         * 128kB (16kB/way), 8-way associativity, event monitor and
         * parity enabled, ignore share bit, no force write allocate
         * Bits:  .... ...0 0111 0011 0000 .... .... ....
         */
        l2x0_init(__io_address(REALVIEW_PB1176_L220_BASE), 0x00730000,
0xfe000fff);
#endif

16KB way-size seems realistic, yes.

I can add a comment explaining that ARMs terminology does
not match the academic terminology or something, and say that
the thing we poke into "way-size" is actually "set size", if we agree
that is what we're seeing here.

Let's see again: 16KB way-size * 8 ways = 128KB, that seems correct.
16KB way-size / 32B line-size = 512 sets, that also seems realistic.
128KB cache-size / 512 sets = 256B set-size. 256B / 32B = 8 ways,
so everything fits together.

Now in the code:

+      of_property_read_u32(np, "cache-size", &size);

131072

+       of_property_read_u32(np, "cache-sets", &sets);

512

+
+       if (!size || !sets)
+               return;
+
+       way_size = size / sets;

256 -> impossible.


         set_size = size / sets;

256

         ways = set_size / line_size;

8

         way_size = size / ways;

16KB -> ok


	Arnd