From: Russell King - ARM Linux admin <linux@armlinux.org.uk> To: Guillaume Tucker <guillaume.tucker@collabora.com> Cc: Kukjin Kim <kgene@kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>, Rob Herring <robh+dt@kernel.org>, kernel@collabora.com, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] ARM: l2c: update prefetch bits in L2X0_AUX_CTRL using DT value Date: Wed, 29 Jul 2020 15:18:02 +0100 [thread overview] Message-ID: <20200729141801.GB1551@shell.armlinux.org.uk> (raw) In-Reply-To: <79a628daef56c2d542e379f550de21da4fe3c901.1596028601.git.guillaume.tucker@collabora.com> On Wed, Jul 29, 2020 at 02:47:32PM +0100, Guillaume Tucker wrote: > The L310_PREFETCH_CTRL register bits 28 and 29 to enable data and > instruction prefetch respectively can also be accessed via the > L2X0_AUX_CTRL register. They appear to be actually wired together in > hardware between the registers. Changing them in the prefetch > register only will get undone when restoring the aux control register > later on. For this reason, set these bits in both registers during > initialisation according to the DT attributes. How will that happen? We write the auxiliary control register before the prefetch control register, so the prefetch control register will take precedence. See l2c310_configure() - l2c_configure() writes the auxiliary control register, and the function writes the prefetch control register later. I think the real issue is that Exynos has been modifying the prefetch settings via its machine .aux_mask / .aux_val configuration, and the opposite is actually true: the prefetch control register values will overwrite the attempt to modify the auxiliary control values set through the machine .aux_mask/.aux_val. > > Fixes: ec3bd0e68a67 ("ARM: 8391/1: l2c: add options to overwrite prefetching behavior") > Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com> > --- > arch/arm/mm/cache-l2x0.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > index 12c26eb88afb..43d91bfd2360 100644 > --- a/arch/arm/mm/cache-l2x0.c > +++ b/arch/arm/mm/cache-l2x0.c > @@ -1249,20 +1249,28 @@ static void __init l2c310_of_parse(const struct device_node *np, > > ret = of_property_read_u32(np, "prefetch-data", &val); > if (ret == 0) { > - if (val) > + if (val) { > prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH; > - else > + *aux_val |= L310_PREFETCH_CTRL_DATA_PREFETCH; > + } else { > prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > + *aux_val &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > + } > + *aux_mask &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > } else if (ret != -EINVAL) { > pr_err("L2C-310 OF prefetch-data property value is missing\n"); > } > > ret = of_property_read_u32(np, "prefetch-instr", &val); > if (ret == 0) { > - if (val) > + if (val) { > prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH; > - else > + *aux_val |= L310_PREFETCH_CTRL_INSTR_PREFETCH; > + } else { > prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > + *aux_val &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > + } > + *aux_mask &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > } else if (ret != -EINVAL) { > pr_err("L2C-310 OF prefetch-instr property value is missing\n"); > } > -- > 2.20.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux admin <linux@armlinux.org.uk> To: Guillaume Tucker <guillaume.tucker@collabora.com> Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Krzysztof Kozlowski <krzk@kernel.org>, Rob Herring <robh+dt@kernel.org>, Kukjin Kim <kgene@kernel.org>, kernel@collabora.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] ARM: l2c: update prefetch bits in L2X0_AUX_CTRL using DT value Date: Wed, 29 Jul 2020 15:18:02 +0100 [thread overview] Message-ID: <20200729141801.GB1551@shell.armlinux.org.uk> (raw) In-Reply-To: <79a628daef56c2d542e379f550de21da4fe3c901.1596028601.git.guillaume.tucker@collabora.com> On Wed, Jul 29, 2020 at 02:47:32PM +0100, Guillaume Tucker wrote: > The L310_PREFETCH_CTRL register bits 28 and 29 to enable data and > instruction prefetch respectively can also be accessed via the > L2X0_AUX_CTRL register. They appear to be actually wired together in > hardware between the registers. Changing them in the prefetch > register only will get undone when restoring the aux control register > later on. For this reason, set these bits in both registers during > initialisation according to the DT attributes. How will that happen? We write the auxiliary control register before the prefetch control register, so the prefetch control register will take precedence. See l2c310_configure() - l2c_configure() writes the auxiliary control register, and the function writes the prefetch control register later. I think the real issue is that Exynos has been modifying the prefetch settings via its machine .aux_mask / .aux_val configuration, and the opposite is actually true: the prefetch control register values will overwrite the attempt to modify the auxiliary control values set through the machine .aux_mask/.aux_val. > > Fixes: ec3bd0e68a67 ("ARM: 8391/1: l2c: add options to overwrite prefetching behavior") > Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com> > --- > arch/arm/mm/cache-l2x0.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > index 12c26eb88afb..43d91bfd2360 100644 > --- a/arch/arm/mm/cache-l2x0.c > +++ b/arch/arm/mm/cache-l2x0.c > @@ -1249,20 +1249,28 @@ static void __init l2c310_of_parse(const struct device_node *np, > > ret = of_property_read_u32(np, "prefetch-data", &val); > if (ret == 0) { > - if (val) > + if (val) { > prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH; > - else > + *aux_val |= L310_PREFETCH_CTRL_DATA_PREFETCH; > + } else { > prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > + *aux_val &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > + } > + *aux_mask &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > } else if (ret != -EINVAL) { > pr_err("L2C-310 OF prefetch-data property value is missing\n"); > } > > ret = of_property_read_u32(np, "prefetch-instr", &val); > if (ret == 0) { > - if (val) > + if (val) { > prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH; > - else > + *aux_val |= L310_PREFETCH_CTRL_INSTR_PREFETCH; > + } else { > prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > + *aux_val &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > + } > + *aux_mask &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > } else if (ret != -EINVAL) { > pr_err("L2C-310 OF prefetch-instr property value is missing\n"); > } > -- > 2.20.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-07-29 14:18 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-29 13:47 [PATCH 1/3] ARM: exynos: clear L220_AUX_CTRL_NS_LOCKDOWN in default l2c_aux_val Guillaume Tucker 2020-07-29 13:47 ` Guillaume Tucker 2020-07-29 13:47 ` [PATCH 2/3] ARM: l2c: update prefetch bits in L2X0_AUX_CTRL using DT value Guillaume Tucker 2020-07-29 13:47 ` Guillaume Tucker 2020-07-29 14:18 ` Russell King - ARM Linux admin [this message] 2020-07-29 14:18 ` Russell King - ARM Linux admin 2020-07-29 16:22 ` Guillaume Tucker 2020-07-29 16:22 ` Guillaume Tucker 2020-08-10 12:34 ` Guillaume Tucker 2020-08-10 12:34 ` Guillaume Tucker 2020-07-29 13:47 ` [PATCH 3/3] ARM: exynos: use DT prefetch attributes rather than l2c_aux_val Guillaume Tucker 2020-07-29 13:47 ` Guillaume Tucker 2020-08-03 13:13 ` Krzysztof Kozlowski 2020-08-03 13:13 ` Krzysztof Kozlowski 2020-08-10 12:29 ` Guillaume Tucker 2020-08-10 12:29 ` Guillaume Tucker 2020-08-03 13:34 ` [PATCH 1/3] ARM: exynos: clear L220_AUX_CTRL_NS_LOCKDOWN in default l2c_aux_val Krzysztof Kozlowski 2020-08-03 13:34 ` Krzysztof Kozlowski 2020-08-03 14:22 ` Russell King - ARM Linux admin 2020-08-03 14:22 ` Russell King - ARM Linux admin 2020-08-10 12:25 ` Guillaume Tucker 2020-08-10 12:25 ` Guillaume Tucker
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=20200729141801.GB1551@shell.armlinux.org.uk \ --to=linux@armlinux.org.uk \ --cc=devicetree@vger.kernel.org \ --cc=guillaume.tucker@collabora.com \ --cc=kernel@collabora.com \ --cc=kgene@kernel.org \ --cc=krzk@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=robh+dt@kernel.org \ /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.