* [PATCH 00/16] Another 16 L2C patches
@ 2014-04-28 16:56 Russell King - ARM Linux
2014-04-28 16:58 ` [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation Russell King
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2014-04-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Andrew Lunn, kernel, Linus Walleij, Gregory Clement,
Thierry Reding, Heiko Stuebner, Rob Herring, linux-samsung-soc,
Michal Simek, bcm-kernel-feedback-list, Dinh Nguyen,
Sebastian Hesselbarth, Jason Cooper, Stephen Warren,
Hauke Mehrtens, Matt Porter, Kukjin Kim, linux-tegra, Shawn Guo,
Maxime Coquelin, Barry Song, Srinivas Kandagatla, Christian Daudt,
Patrice
So, in response to Matt Porter's complaint about breaking prima2, here's
another 16 patches which changes the way the L2 cache is initialised on
many platforms. This series moves towards a situation where the generic
code initialises the L2 cache itself, with as little help as possible
from board specific code.
A number of platforms are left alone because they're more complex -
these should still eventually be converted.
At some point in the near future, I will see about sorting out their
ordering wrt the previous patch set. For the time being, they apply
on top of the existing l2c changes.
arch/arm/include/asm/mach/arch.h | 3 +++
arch/arm/kernel/irq.c | 12 ++++++++++++
arch/arm/mach-bcm/bcm_5301x.c | 9 ++-------
arch/arm/mach-berlin/berlin.c | 17 ++++++-----------
arch/arm/mach-exynos/exynos.c | 8 ++------
arch/arm/mach-highbank/highbank.c | 9 +++------
arch/arm/mach-imx/mach-vf610.c | 9 ++-------
arch/arm/mach-mvebu/board-v7.c | 9 ++++++---
arch/arm/mach-nomadik/cpu-8815.c | 13 +++----------
arch/arm/mach-prima2/Makefile | 1 -
arch/arm/mach-prima2/common.c | 6 ++++++
arch/arm/mach-prima2/l2x0.c | 17 -----------------
arch/arm/mach-rockchip/rockchip.c | 9 ++-------
arch/arm/mach-socfpga/socfpga.c | 9 ++-------
arch/arm/mach-sti/board-dt.c | 20 +++++---------------
arch/arm/mach-tegra/tegra.c | 12 +++---------
arch/arm/mach-vexpress/v2m.c | 3 ++-
arch/arm/mach-zynq/common.c | 8 +++-----
18 files changed, 62 insertions(+), 112 deletions(-)
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation 2014-04-28 16:56 [PATCH 00/16] Another 16 L2C patches Russell King - ARM Linux @ 2014-04-28 16:58 ` Russell King 2014-04-28 17:28 ` Stephen Warren 2014-04-28 17:12 ` [PATCH 00/16] Another 16 L2C patches Stephen Warren 2014-04-28 19:00 ` Heiko Stübner 2 siblings, 1 reply; 12+ messages in thread From: Russell King @ 2014-04-28 16:58 UTC (permalink / raw) To: linux-arm-kernel; +Cc: linux-tegra, Thierry Reding, Stephen Warren Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/mach-tegra/tegra.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c index 1bc49f9db015..d90065e2552a 100644 --- a/arch/arm/mach-tegra/tegra.c +++ b/arch/arm/mach-tegra/tegra.c @@ -70,20 +70,12 @@ u32 tegra_uart_config[3] = { 0, }; -static void __init tegra_init_cache(void) -{ -#ifdef CONFIG_CACHE_L2X0 - l2x0_of_init(0x3c400001, 0xc20fc3fe); -#endif -} - static void __init tegra_init_early(void) { of_register_trusted_foundations(); tegra_apb_io_init(); tegra_init_fuse(); tegra_cpu_reset_handler_init(); - tegra_init_cache(); tegra_powergate_init(); tegra_hotplug_init(); } @@ -171,8 +163,10 @@ static const char * const tegra_dt_board_compat[] = { }; DT_MACHINE_START(TEGRA_DT, "NVIDIA Tegra SoC (Flattened Device Tree)") - .map_io = tegra_map_common_io, .smp = smp_ops(tegra_smp_ops), + .l2c_aux_val = 0x3c400001, + .l2c_aux_mask = 0xc20fc3fe, + .map_io = tegra_map_common_io, .init_early = tegra_init_early, .init_irq = tegra_dt_init_irq, .init_machine = tegra_dt_init, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation 2014-04-28 16:58 ` [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation Russell King @ 2014-04-28 17:28 ` Stephen Warren 2014-04-28 17:41 ` Russell King - ARM Linux 0 siblings, 1 reply; 12+ messages in thread From: Stephen Warren @ 2014-04-28 17:28 UTC (permalink / raw) To: Russell King, linux-arm-kernel; +Cc: linux-tegra, Thierry Reding On 04/28/2014 10:58 AM, Russell King wrote: > diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c > DT_MACHINE_START(TEGRA_DT, "NVIDIA Tegra SoC (Flattened Device Tree)") > - .map_io = tegra_map_common_io, > .smp = smp_ops(tegra_smp_ops), > + .l2c_aux_val = 0x3c400001, > + .l2c_aux_mask = 0xc20fc3fe, > + .map_io = tegra_map_common_io, I'm not sure why .map_io was moved. Was it to sort the entries in order of execution? If so, I assume that .smp should be moved too? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation 2014-04-28 17:28 ` Stephen Warren @ 2014-04-28 17:41 ` Russell King - ARM Linux 0 siblings, 0 replies; 12+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 17:41 UTC (permalink / raw) To: Stephen Warren; +Cc: linux-tegra, Thierry Reding, linux-arm-kernel On Mon, Apr 28, 2014 at 11:28:00AM -0600, Stephen Warren wrote: > On 04/28/2014 10:58 AM, Russell King wrote: > > > diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c > > > DT_MACHINE_START(TEGRA_DT, "NVIDIA Tegra SoC (Flattened Device Tree)") > > - .map_io = tegra_map_common_io, > > .smp = smp_ops(tegra_smp_ops), > > + .l2c_aux_val = 0x3c400001, > > + .l2c_aux_mask = 0xc20fc3fe, > > + .map_io = tegra_map_common_io, > > I'm not sure why .map_io was moved. Was it to sort the entries in order > of execution? If so, I assume that .smp should be moved too? I moved it quite simply because I keep entries in order: unsigned l2c_aux_val; /* L2 cache aux value */ unsigned l2c_aux_mask; /* L2 cache aux mask */ void (*l2c_write_sec)(unsigned long, unsigned); struct smp_operations *smp; /* SMP operations */ bool (*smp_init)(void); void (*fixup)(struct tag *, char **, struct meminfo *); void (*init_meminfo)(void); void (*reserve)(void);/* reserve mem blocks */ void (*map_io)(void);/* IO mapping function */ void (*init_early)(void); And the order of declaration in there (for the functions after "smp") is the order in which they are called during kernel initialisation. Platforms randomising their declarations is... silly. And yes, I'll move those l2c ones before .smp. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 16:56 [PATCH 00/16] Another 16 L2C patches Russell King - ARM Linux 2014-04-28 16:58 ` [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation Russell King @ 2014-04-28 17:12 ` Stephen Warren 2014-04-28 17:27 ` Stephen Warren 2014-04-28 19:00 ` Heiko Stübner 2 siblings, 1 reply; 12+ messages in thread From: Stephen Warren @ 2014-04-28 17:12 UTC (permalink / raw) To: Russell King - ARM Linux, linux-arm-kernel Cc: Andrew Lunn, kernel, Linus Walleij, Gregory Clement, Thierry Reding, Heiko Stuebner, Rob Herring, linux-samsung-soc, Michal Simek, bcm-kernel-feedback-list, Dinh Nguyen, Sebastian Hesselbarth, Jason Cooper, Hauke Mehrtens, Matt Porter, Kukjin Kim, linux-tegra, Shawn Guo, Maxime Coquelin, Barry Song, Srinivas Kandagatla, Christian Daudt, Patrice Chotard, Sascha On 04/28/2014 10:56 AM, Russell King - ARM Linux wrote: > So, in response to Matt Porter's complaint about breaking prima2, here's > another 16 patches which changes the way the L2 cache is initialised on > many platforms. This series moves towards a situation where the generic > code initialises the L2 cache itself, with as little help as possible > from board specific code. > > A number of platforms are left alone because they're more complex - > these should still eventually be converted. > > At some point in the near future, I will see about sorting out their > ordering wrt the previous patch set. For the time being, they apply > on top of the existing l2c changes. Are "the existing l2c changes" in next-20140428? If not, is there a git branch I can pull to test the whole thing, rather than tracking down and applying "the existing l2c changes" first? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 17:12 ` [PATCH 00/16] Another 16 L2C patches Stephen Warren @ 2014-04-28 17:27 ` Stephen Warren 2014-04-28 17:39 ` Russell King - ARM Linux 0 siblings, 1 reply; 12+ messages in thread From: Stephen Warren @ 2014-04-28 17:27 UTC (permalink / raw) To: Russell King - ARM Linux, linux-arm-kernel Cc: Andrew Lunn, kernel, Linus Walleij, Gregory Clement, Thierry Reding, Heiko Stuebner, Rob Herring, linux-samsung-soc, Michal Simek, bcm-kernel-feedback-list, Dinh Nguyen, Sebastian Hesselbarth, Jason Cooper, Hauke Mehrtens, Matt Porter, Kukjin Kim, linux-tegra, Shawn Guo, Maxime Coquelin, Barry Song, Srinivas Kandagatla, Christian Daudt, Patrice Chotard, Sascha On 04/28/2014 11:12 AM, Stephen Warren wrote: > On 04/28/2014 10:56 AM, Russell King - ARM Linux wrote: >> So, in response to Matt Porter's complaint about breaking prima2, here's >> another 16 patches which changes the way the L2 cache is initialised on >> many platforms. This series moves towards a situation where the generic >> code initialises the L2 cache itself, with as little help as possible >> from board specific code. >> >> A number of platforms are left alone because they're more complex - >> these should still eventually be converted. >> >> At some point in the near future, I will see about sorting out their >> ordering wrt the previous patch set. For the time being, they apply >> on top of the existing l2c changes. > > Are "the existing l2c changes" in next-20140428? If not, is there a git > branch I can pull to test the whole thing, rather than tracking down and > applying "the existing l2c changes" first? I guess they must be in linux-next, since this series applies cleanly on top of it. So, patches 2/16 ("ARM: l2c: add platform independent core L2 cache initialisation") and 7/16 ("ARM: l2c: convert tegra to generic l2c initialisation"), Tested-by: Stephen Warren <swarren@nvidia.com> (On an NVIDIA Tegra20 Seaboard/Springbank board, on top of next-20140428) I do see one error in dmesg during boot, but it doesn't appear to negatively affect operation in brief testing, and is present in linux-next without this series anyway. Is this message a problem? > [ 0.000000] L2C: platform modifies aux control register: 0x02080000 -> 0x3e480001 > [ 0.000000] L2C: DT/platform modifies aux control register: 0x02080000 -> 0x3e480001 > [ 0.000000] L2C-310 errata 727915 769419 enabled > [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9 > [ 0.000000] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 ^^^^^^ this is logged at error level > [ 0.000000] L2C-310 ID prefetch enabled, offset 1 lines > [ 0.000000] L2C-310 dynamic clock gating disabled, standby mode disabled > [ 0.000000] L2C-310 cache controller enabled, 8 ways, 1024 kB > [ 0.000000] L2C-310: CACHE_ID 0x410000c4, AUX_CTRL 0x7e480001 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 17:27 ` Stephen Warren @ 2014-04-28 17:39 ` Russell King - ARM Linux 2014-04-28 18:08 ` Stephen Warren 0 siblings, 1 reply; 12+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 17:39 UTC (permalink / raw) To: Stephen Warren Cc: Andrew Lunn, kernel, Linus Walleij, Gregory Clement, Thierry Reding, Heiko Stuebner, Rob Herring, linux-samsung-soc, Michal Simek, bcm-kernel-feedback-list, Shawn Guo, Dinh Nguyen, Alessandro Rubini, Jason Cooper, Hauke Mehrtens, Matt Porter, Kukjin Kim, linux-tegra, linux-arm-kernel, Maxime Coquelin, Barry Song, Srinivas Kandagatla, Christian Daudt, Patrice Chotard <patrice.chotard@ On Mon, Apr 28, 2014 at 11:27:09AM -0600, Stephen Warren wrote: > On 04/28/2014 11:12 AM, Stephen Warren wrote: > > On 04/28/2014 10:56 AM, Russell King - ARM Linux wrote: > >> So, in response to Matt Porter's complaint about breaking prima2, here's > >> another 16 patches which changes the way the L2 cache is initialised on > >> many platforms. This series moves towards a situation where the generic > >> code initialises the L2 cache itself, with as little help as possible > >> from board specific code. > >> > >> A number of platforms are left alone because they're more complex - > >> these should still eventually be converted. > >> > >> At some point in the near future, I will see about sorting out their > >> ordering wrt the previous patch set. For the time being, they apply > >> on top of the existing l2c changes. > > > > Are "the existing l2c changes" in next-20140428? If not, is there a git > > branch I can pull to test the whole thing, rather than tracking down and > > applying "the existing l2c changes" first? > > I guess they must be in linux-next, since this series applies cleanly on > top of it. > > So, patches 2/16 ("ARM: l2c: add platform independent core L2 cache > initialisation") and 7/16 ("ARM: l2c: convert tegra to generic l2c > initialisation"), > > Tested-by: Stephen Warren <swarren@nvidia.com> > > (On an NVIDIA Tegra20 Seaboard/Springbank board, on top of next-20140428) > > I do see one error in dmesg during boot, but it doesn't appear to > negatively affect operation in brief testing, and is present in > linux-next without this series anyway. Is this message a problem? > > > [ 0.000000] L2C: platform modifies aux control register: 0x02080000 -> 0x3e480001 > > [ 0.000000] L2C: DT/platform modifies aux control register: 0x02080000 -> 0x3e480001 > > [ 0.000000] L2C-310 errata 727915 769419 enabled > > [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9 > > [ 0.000000] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 > ^^^^^^ this is logged at error level Correct, it's an error because on Tegra you explicitly set bit 0 in the auxiliary control register, which is pointless unless the feature is also enabled in the Cortex-A9 control register as well. Rather than trying to track down everyone who does this, and then end up in a long discussion about it, I'm just going to make the kernel print an error message as a result, it's just wrong to set random bits in device control registers without first properly understanding what they're doing. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 17:39 ` Russell King - ARM Linux @ 2014-04-28 18:08 ` Stephen Warren 2014-04-28 18:19 ` Russell King - ARM Linux 0 siblings, 1 reply; 12+ messages in thread From: Stephen Warren @ 2014-04-28 18:08 UTC (permalink / raw) To: Russell King - ARM Linux, Peter De Schrijver Cc: linux-tegra, Thierry Reding, linux-arm-kernel (Dropping most people from CC since this sub-thread is a Tegra-specific discussion) On 04/28/2014 11:39 AM, Russell King - ARM Linux wrote: > On Mon, Apr 28, 2014 at 11:27:09AM -0600, Stephen Warren wrote: ... >> I do see one error in dmesg during boot, but it doesn't appear to >> negatively affect operation in brief testing, and is present in >> linux-next without this series anyway. Is this message a problem? >> >>> [ 0.000000] L2C: platform modifies aux control register: 0x02080000 -> 0x3e480001 >>> [ 0.000000] L2C: DT/platform modifies aux control register: 0x02080000 -> 0x3e480001 >>> [ 0.000000] L2C-310 errata 727915 769419 enabled >>> [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9 >>> [ 0.000000] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 >> ^^^^^^ this is logged at error level > > Correct, it's an error because on Tegra you explicitly set bit 0 in the > auxiliary control register, which is pointless unless the feature is > also enabled in the Cortex-A9 control register as well. Please forgive my almost complete lack of knowledge re: cache controllers. Is the correct fix for this: a) To remove bit 0 from the aux_val passed to l2x0_of_init() b) To set some BIT(3) in the Cortex-A9 auxcr, so this feature is enabled there too. And if (b), I assume that's something that the bootloader should be doing, not the kernel? Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 18:08 ` Stephen Warren @ 2014-04-28 18:19 ` Russell King - ARM Linux 2014-04-28 18:40 ` Stephen Warren 0 siblings, 1 reply; 12+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 18:19 UTC (permalink / raw) To: Stephen Warren Cc: linux-tegra, Peter De Schrijver, Thierry Reding, linux-arm-kernel On Mon, Apr 28, 2014 at 12:08:11PM -0600, Stephen Warren wrote: > (Dropping most people from CC since this sub-thread is a Tegra-specific > discussion) > > On 04/28/2014 11:39 AM, Russell King - ARM Linux wrote: > > On Mon, Apr 28, 2014 at 11:27:09AM -0600, Stephen Warren wrote: > ... > >> I do see one error in dmesg during boot, but it doesn't appear to > >> negatively affect operation in brief testing, and is present in > >> linux-next without this series anyway. Is this message a problem? > >> > >>> [ 0.000000] L2C: platform modifies aux control register: 0x02080000 -> 0x3e480001 > >>> [ 0.000000] L2C: DT/platform modifies aux control register: 0x02080000 -> 0x3e480001 > >>> [ 0.000000] L2C-310 errata 727915 769419 enabled > >>> [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9 > >>> [ 0.000000] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 > >> ^^^^^^ this is logged at error level > > > > Correct, it's an error because on Tegra you explicitly set bit 0 in the > > auxiliary control register, which is pointless unless the feature is > > also enabled in the Cortex-A9 control register as well. > > Please forgive my almost complete lack of knowledge re: cache > controllers. Is the correct fix for this: > > a) To remove bit 0 from the aux_val passed to l2x0_of_init() > > b) To set some BIT(3) in the Cortex-A9 auxcr, so this feature is enabled > there too. > > And if (b), I assume that's something that the bootloader should be > doing, not the kernel? See "Full line of zero write." http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246h/CJACBHHB.html#CJADBCJJ which makes it (a). The reverse steps are required when disabling the L2 cache controller. If we decide that we encounter a platform which needs this feature disabled, the correct way to deal with that is to add a L2C-310 specific property to DT, and not to try to crowbar it in via the L2C aux control register masks (which should never have been exposed to platforms using DT.) -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 18:19 ` Russell King - ARM Linux @ 2014-04-28 18:40 ` Stephen Warren [not found] ` <535EA090.6030903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Stephen Warren @ 2014-04-28 18:40 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-tegra, Peter De Schrijver, Thierry Reding, linux-arm-kernel On 04/28/2014 12:19 PM, Russell King - ARM Linux wrote: > On Mon, Apr 28, 2014 at 12:08:11PM -0600, Stephen Warren wrote: >> (Dropping most people from CC since this sub-thread is a Tegra-specific >> discussion) >> >> On 04/28/2014 11:39 AM, Russell King - ARM Linux wrote: >>> On Mon, Apr 28, 2014 at 11:27:09AM -0600, Stephen Warren wrote: >> ... >>>> I do see one error in dmesg during boot, but it doesn't appear to >>>> negatively affect operation in brief testing, and is present in >>>> linux-next without this series anyway. Is this message a problem? >>>> >>>>> [ 0.000000] L2C: platform modifies aux control register: 0x02080000 -> 0x3e480001 >>>>> [ 0.000000] L2C: DT/platform modifies aux control register: 0x02080000 -> 0x3e480001 >>>>> [ 0.000000] L2C-310 errata 727915 769419 enabled >>>>> [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9 >>>>> [ 0.000000] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 >>>> ^^^^^^ this is logged at error level >>> >>> Correct, it's an error because on Tegra you explicitly set bit 0 in the >>> auxiliary control register, which is pointless unless the feature is >>> also enabled in the Cortex-A9 control register as well. >> >> Please forgive my almost complete lack of knowledge re: cache >> controllers. Is the correct fix for this: >> >> a) To remove bit 0 from the aux_val passed to l2x0_of_init() >> >> b) To set some BIT(3) in the Cortex-A9 auxcr, so this feature is enabled >> there too. >> >> And if (b), I assume that's something that the bootloader should be >> doing, not the kernel? > > See "Full line of zero write." > > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246h/CJACBHHB.html#CJADBCJJ > > which makes it (a). The reverse steps are required when disabling the L2 > cache controller. Do you say that simply because to make use of this feature, it also needs to be enabled in the A9 but no code in the kernel is currently doing that? Or, because enabling the feature stops the cache controller from supporting strongly ordered writes? It seems like the condition this error message detects is benign; the cache controller is prepared to receive these transactions, but the A9 will never send them. Nothing can go wrong in this case, I believe, although admittedly it's a pointless configuration. Equally, given the required enable sequence in that document, won't this error always get printed if BIT(0) is sent in aux_val; before the bit is enabled in the cache controller, the associated bit in the A9 /should/ be disabled. It seems like the test in the kernel should simply check if BIT(0) is set in aux_val and ignore the value in the A9 completely, if you want to ban people from enabling this feature via aux_val. Or, are there some platforms where something outside (before) the kernel enables the feature in the A9 even before the kernel cache init code runs, and you want to avoid printing the error in that case? > If we decide that we encounter a platform which needs this feature > disabled, the correct way to deal with that is to add a L2C-310 I assume /disabled/enabled/ there? > specific property to DT, and not to try to crowbar it in via the L2C > aux control register masks (which should never have been exposed to > platforms using DT.) ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <535EA090.6030903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH 00/16] Another 16 L2C patches [not found] ` <535EA090.6030903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2014-04-28 19:10 ` Russell King - ARM Linux 0 siblings, 0 replies; 12+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 19:10 UTC (permalink / raw) To: Stephen Warren Cc: Peter De Schrijver, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-tegra-u79uwXL29TY76Z2rM5mHXA, Thierry Reding On Mon, Apr 28, 2014 at 12:40:16PM -0600, Stephen Warren wrote: > Do you say that simply because to make use of this feature, it also > needs to be enabled in the A9 but no code in the kernel is currently > doing that? Or, because enabling the feature stops the cache controller > from supporting strongly ordered writes? > > It seems like the condition this error message detects is benign; the > cache controller is prepared to receive these transactions, but the A9 > will never send them. Nothing can go wrong in this case, I believe, > although admittedly it's a pointless configuration. Yes, it's benign, but it's an unnecessary difference between platforms which stands in the way of trying to clean this crap up. This is what I've been moaning about for a while: totally pointless differences between platforms which then stand in the way of future cleanups, because it's impossible to tell whether a difference is there because it's required or because (speaking plainly) the maintainer didn't have any clue what they were doing when the wrote the code. The L2 cache code has suffered a lot from cargo cult programming, and other sloppiness - so yes, while it's benign, we can now also have a benign warning to encourage people not to do this kind of stuff. > Equally, given the required enable sequence in that document, won't this > error always get printed if BIT(0) is sent in aux_val; before the bit is > enabled in the cache controller, the associated bit in the A9 /should/ > be disabled. The point here is that if we are going to enable various Cortex-A9 optimisations, the way it _should_ be done is that the L2 cache code makes that decision based on the revision of the parts found, and the only control which platforms should ever have is one to say "this feature is broken on platform X, please don't enable it" which that done via DT, and not via random values passed via an initialisation interface. So, what this error is all about is: - the platform maintainer decided to set bit 0 which enables a cortex-a9 specific feature, which is useless given that the enable bit in cortex-a9 is disabled. - the platform maintainer is setting bits without thinking. - platform maintainers copy code from other platforms without thinking (just look at all the crap uses of L2X0_AUX_CTRL_MASK which is only suitable for L2C-210, but gets used on everything.) So once one person does it, it ends up propagating to other platforms, and the problem just spreads. > It seems like the test in the kernel should simply check if > BIT(0) is set in aux_val and ignore the value in the A9 completely, if > you want to ban people from enabling this feature via aux_val. Or, are > there some platforms where something outside (before) the kernel enables > the feature in the A9 even before the kernel cache init code runs, and > you want to avoid printing the error in that case? You're asking questions I have no answer to - and this is the whole problem here. Platform maintainers go off in their own tiny worlds, with blinkers on, doing their own crap without really knowing what the hell they're doing. I've *no* clue whether there's a platform which gets this wrong and enables the FLZ bit in the CA9 control register. Given the kind of crap that we have in the kernel - with platforms just deciding to set bit 0 of the L2 cache auxiliary control register without taking any time to think, I have no reason to doubt that someone hasn't tried crap like that, and I have no doubt that it _could_ have worked for someone sufficiently to get into the kernel. This is the whole problem: I have no idea what kind of bollocks platform maintainers do in their spare time. Frankly, this is another in the long list of arm-soc failures: this is something that should have been fixed a /long/ time ago - it's far more important than getting rid of silly timex.h header files, and the problem is more pervasive. Instead, it was left, and left and left, and the problem has grown bigger and bigger. So no, I'm not offering any sympathy here to anyone, partly because of the shere amount of effort that the L2 cache series has taken to get this far - and it's now soo big that it in itself is starting to become unmanageable. And with many platform maintainers not really taking an interest in it when it has been posted, I'm at the point where I just don't give a that much of a damn about their platform code unless they take an interest and help do some testing. > > If we decide that we encounter a platform which needs this feature > > disabled, the correct way to deal with that is to add a L2C-310 > > I assume /disabled/enabled/ there? No, I meant what I said. Enable the optimistions by default. If someone whinges, we'll fix it, but *only* if it causes a failure. Any other solution is total madness, and doesn't get much in the way of interest. It's easy to ignore a plea for help. It's impossible to ignore if it causes a problem. > > specific property to DT, and not to try to crowbar it in via the L2C > > aux control register masks (which should never have been exposed to > > platforms using DT.) We're only at this point _because_ I've forced people to take an interest in this code. I know most platform maintainers would rather not think about it, as evidenced by the wide-spread cargo-cult programming that has been going on here. The only way to get this sorted is to ram this kind of thing down people's throats. Yes, it's not a nice way to do it, but it's the only way. Also, remember that I don't have these huge board farms that Olof and Kevin have - in comparison I'm a pauper when it comes to platforms I can directly test. This makes my job that much harder, especially when platform maintainers are uncooperative, and is why I resort to pushing such stuff into linux-next to _force_ the issue, after I've completed what testing I can do here, and given it a run through Olof's build system. Olof's system is useless for remote debugging though, so it is of little value other than seeing how many platforms succeed or fail. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 00/16] Another 16 L2C patches 2014-04-28 16:56 [PATCH 00/16] Another 16 L2C patches Russell King - ARM Linux 2014-04-28 16:58 ` [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation Russell King 2014-04-28 17:12 ` [PATCH 00/16] Another 16 L2C patches Stephen Warren @ 2014-04-28 19:00 ` Heiko Stübner 2 siblings, 0 replies; 12+ messages in thread From: Heiko Stübner @ 2014-04-28 19:00 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Andrew Lunn, kernel, Linus Walleij, Gregory Clement, Thierry Reding, Rob Herring, linux-samsung-soc, Michal Simek, bcm-kernel-feedback-list, Shawn Guo, Dinh Nguyen, Sebastian Hesselbarth, Jason Cooper, Stephen Warren, Hauke Mehrtens, Matt Porter, Kukjin Kim, linux-tegra, linux-arm-kernel, Maxime Coquelin, Barry Song, Srinivas Kandagatla, Christian Daudt, Patric Am Montag, 28. April 2014, 17:56:31 schrieb Russell King - ARM Linux: > So, in response to Matt Porter's complaint about breaking prima2, here's > another 16 patches which changes the way the L2 cache is initialised on > many platforms. This series moves towards a situation where the generic > code initialises the L2 cache itself, with as little help as possible > from board specific code. Patches 2/16 ("ARM: l2c: add platform independent core L2 cache initialisation") and 3/16 ("ARM: l2c: convert rockchip to generic l2c initialisation") applied on a linux-next from 20140428, Tested-by: Heiko Stuebner <heiko@sntech.de> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-04-28 19:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-28 16:56 [PATCH 00/16] Another 16 L2C patches Russell King - ARM Linux
2014-04-28 16:58 ` [PATCH 07/16] ARM: l2c: convert tegra to generic l2c initialisation Russell King
2014-04-28 17:28 ` Stephen Warren
2014-04-28 17:41 ` Russell King - ARM Linux
2014-04-28 17:12 ` [PATCH 00/16] Another 16 L2C patches Stephen Warren
2014-04-28 17:27 ` Stephen Warren
2014-04-28 17:39 ` Russell King - ARM Linux
2014-04-28 18:08 ` Stephen Warren
2014-04-28 18:19 ` Russell King - ARM Linux
2014-04-28 18:40 ` Stephen Warren
[not found] ` <535EA090.6030903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-04-28 19:10 ` Russell King - ARM Linux
2014-04-28 19:00 ` Heiko Stübner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox