Re: [PATCH 6/7 v6] ARM: l2x0: calculate associativity from ePAPR cache props

[Arnd Bergmann <arnd@arndb.de>]

On Monday 08 September 2014 13:38:05 Linus Walleij wrote:
>  static void __init l2x0_cache_size_of_parse(const struct device_node *np,
>                                             u32 *aux_val, u32 *aux_mask,
> -                                           u32 max_way_size)
> +                                           u32 max_way_size,
> +                                           u32 max_associativity)
>  {
>         u32 mask = 0, val = 0;
>         u32 size = 0, sets = 0;
>         u32 way_size = 0, way_size_bits = 1;
> +       u32 linesize = 0;
> +       u32 assoc = 0;
>  
>         of_property_read_u32(np, "cache-size", &size);
>         of_property_read_u32(np, "cache-sets", &sets);
> +       of_property_read_u32(np, "cache-line-size", &linesize);
>  

If I read ePAPR correctly, you have to read "cache-block-size" as well, and
use that as a fallback if "cache-line-size" is not provided. The line size
is only required to be in the DT if it's different from the block size.

>         if (!size || !sets)
>                 return;
> @@ -962,10 +966,56 @@ static void __init l2x0_cache_size_of_parse(const struct device_node *np,
>         way_size = size / sets;
>  
>         if (way_size > max_way_size) {
> -               pr_warn("L2C: way size %dKB is too large\n", way_size >> 10);
> +               pr_warn("L2C OF: way size %dKB is too large\n", way_size >> 10);
>                 return;
>         }
>  
> +       /* All these l2 caches have the same line size actually */
> +       if (!linesize)
> +               linesize = CACHE_LINE_SIZE;
> +       if (linesize != CACHE_LINE_SIZE)
> +               pr_warn("L2C OF: DT supplied line size %d bytes does "
> +                       "not match hardware line size of %d bytes\n",
> +                       linesize,
> +                       CACHE_LINE_SIZE);

and CACHE_LINE_SIZE seems to be what ePAPR calls cache-block-size, i.e.
the fundamental unit of cache management as seen from the CPU, independent
of how it is physically organized.

> +       /*
> +        * This cache is set associative. By increasing associativity
> +        * we increase the number of blocks per set. Usually this
> +        * quickly hits the roof at 8 or 16 ways of associativity.
> +        */
> +       assoc = way_size / linesize;
> +       if (assoc > max_associativity)
> +               assoc = max_associativity;
> +

I don't see what this helps us. Doesn't this silently try to fix up
incorrect device trees?
If so, I'd rather see the cache get disabled and a warning printed
here.

> +       /*
> +        * Special checks for the PL310 that only has two settings and
> +        * cannot be set to fully associative.
> +        */
> +       if (max_associativity == 16) {
> +               if (assoc <= 8) {
> +                       assoc = 8;
> +                       /* Leave bit 16 in associativity set to 0 */
> +               }
> +               if (assoc > 8 && assoc <= 16) {
> +                       assoc = 16;
> +                       val |= L310_AUX_CTRL_ASSOCIATIVITY_16;
> +               }

Same thing here.

> +       } else {
> +               if (sets == 1)
> +                       /* Direct-mapped cache */
> +                       assoc = 1;
> +               val |= (assoc << L2X0_AUX_CTRL_ASSOC_SHIFT);
> +       }

This looks wrong: isn't "sets==1" fully associative rather than
direct-mapped?

	Arnd