From: Arnd Bergmann <arnd@arndb.de> To: Linus Walleij <linus.walleij@linaro.org> Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-leds@vger.kernel.org, linux-pm@vger.kernel.org, Pawel Moll <pawel.moll@arm.com>, Mark Rutland <mark.rutland@arm.com>, Marc Zyngier <marc.zyngier@arm.com>, Will Deacon <will.deacon@arm.com>, Rob Herring <robh@kernel.org> Subject: Re: [PATCH 6/7 v6] ARM: l2x0: calculate associativity from ePAPR cache props Date: Mon, 08 Sep 2014 14:15:35 +0200 [thread overview] Message-ID: <65434961.9Ji0STQpff@wuerfel> (raw) In-Reply-To: <1410176286-32533-7-git-send-email-linus.walleij@linaro.org> 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
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 6/7 v6] ARM: l2x0: calculate associativity from ePAPR cache props Date: Mon, 08 Sep 2014 14:15:35 +0200 [thread overview] Message-ID: <65434961.9Ji0STQpff@wuerfel> (raw) In-Reply-To: <1410176286-32533-7-git-send-email-linus.walleij@linaro.org> 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
next prev parent reply other threads:[~2014-09-08 12:16 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-09-08 11:37 [PATCH 0/7] ARM RealView DeviceTree support v6 Linus Walleij 2014-09-08 11:37 ` Linus Walleij 2014-09-08 11:38 ` [PATCH 1/7 v6] leds: add a driver for syscon-based LEDs Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-12 8:56 ` Linus Walleij 2014-09-12 8:56 ` Linus Walleij 2014-09-18 22:39 ` Linus Walleij 2014-09-18 22:39 ` Linus Walleij 2015-01-13 20:19 ` Bryan Wu 2015-01-13 20:19 ` Bryan Wu 2014-09-08 11:38 ` [PATCH 2/7 v6] leds: add device tree bindings for register bit LEDs Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-08 11:38 ` [PATCH 3/7 v6] power: reset: driver for the Versatile syscon reboot Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-12 8:55 ` Linus Walleij 2014-09-12 8:55 ` Linus Walleij 2014-09-15 16:06 ` Sebastian Reichel 2014-09-15 16:06 ` Sebastian Reichel [not found] ` <1410176286-32533-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-09-08 11:38 ` [PATCH 4/7 v6] soc: add driver for the ARM RealView Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-08 11:38 ` [PATCH 5/7 v6] ARM: l2c: parse 'cache-size' and 'cache-sets' properties Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-08 12:20 ` Arnd Bergmann 2014-09-08 12:20 ` Arnd Bergmann 2014-09-08 12:36 ` Linus Walleij 2014-09-08 12:36 ` Linus Walleij 2014-09-08 13:16 ` Arnd Bergmann 2014-09-08 13:16 ` Arnd Bergmann 2014-09-08 20:33 ` Florian Fainelli 2014-09-08 20:33 ` Florian Fainelli 2014-09-08 20:41 ` Arnd Bergmann 2014-09-08 20:41 ` Arnd Bergmann 2014-09-09 7:14 ` Linus Walleij 2014-09-09 7:14 ` Linus Walleij [not found] ` <CACRpkdb_bS0P-fGxogiyCzG9CRPhJK1Gro=H3X1Ks_-UoXh52w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-09-08 19:57 ` Florian Fainelli 2014-09-08 19:57 ` Florian Fainelli 2014-09-08 11:38 ` [PATCH 6/7 v6] ARM: l2x0: calculate associativity from ePAPR cache props Linus Walleij 2014-09-08 11:38 ` Linus Walleij 2014-09-08 12:15 ` Arnd Bergmann [this message] 2014-09-08 12:15 ` Arnd Bergmann 2014-09-08 12:43 ` Linus Walleij 2014-09-08 12:43 ` Linus Walleij 2014-09-08 13:18 ` Arnd Bergmann 2014-09-08 13:18 ` Arnd Bergmann 2014-09-08 11:38 ` [PATCH 7/7 v6] ARM: realview: basic device tree implementation Linus Walleij 2014-09-08 11:38 ` Linus Walleij
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=65434961.9Ji0STQpff@wuerfel \ --to=arnd@arndb.de \ --cc=devicetree@vger.kernel.org \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-leds@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=mark.rutland@arm.com \ --cc=pawel.moll@arm.com \ --cc=robh@kernel.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.