ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Subsystems (2)
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Parsed auxiliary control properties for PL310 cache controller.
---
These properties are set for Exynos4 platform. If we can pass these properties through device tree for Exynos4, then we can remove the hard-coded L2_AUX_VAL.
Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++++++ arch/arm/include/asm/hardware/cache-l2x0.h | 1 + arch/arm/mm/cache-l2x0.c | 25 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+)
Documentation/devicetree/bindings/arm/l2cc.txt +10 -0
@@ -44,6 +44,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,early-write: If present then BRSEP mode (early write response) is enabled. +- arm,data-prefetch: If present then data prefetching is enabled. +- arm,instruction-prefetch: If present then instruction prefetching is enabled. +- arm,ns-interrupt-access: If present then interrupt mask and interrupt clear + registers can be read or modified in both secure or non-secure accesses. +- arm,ns-lockdown: If present then non-secure accesses can write to lockdown + register. +- arm,share-override: If present then shared attribute is ignored internally. +- arm,full-line-of-zero: If present then 'full line of write zero' behaviour is + enabled. Example:
arch/arm/include/asm/hardware/cache-l2x0.h +1 -0
@@ -78,6 +78,7 @@#define L2X0_CACHE_ID_RTL_R3P2 0x8#define L2X0_AUX_CTRL_MASK 0xc0000fff +#define L2X0_AUX_CTRL_FULL_LINE_OF_ZERO_SHIFT 0#define L2X0_AUX_CTRL_DATA_RD_LATENCY_SHIFT 0#define L2X0_AUX_CTRL_DATA_RD_LATENCY_MASK 0x7#define L2X0_AUX_CTRL_DATA_WR_LATENCY_SHIFT 3
arch/arm/mm/cache-l2x0.c +25 -0
@@ -705,6 +705,7 @@ static void __init pl310_of_setup(const struct device_node *np,u32 data[3 ] = {0 ,0 ,0 };u32 tag[3 ] = {0 ,0 ,0 };u32 filter[2 ] = {0 ,0 }; +u32 val =0 ;of_property_read_u32_array (np,"arm,tag-latency" , tag,ARRAY_SIZE (tag));if (tag[0 ] && tag[1 ] && tag[2 ])
@@ -731,6 +732,30 @@ static void __init pl310_of_setup(const struct device_node *np,writel_relaxed ((filter[0 ] & ~(SZ_1M -1 )) |L2X0_ADDR_FILTER_EN , l2x0_base +L2X0_ADDR_FILTER_START ); } + +if (of_find_property (np,"arm,early-write" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_EARLY_BRESP_SHIFT ); + +if (of_find_property (np,"arm,instruction-prefetch" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_INSTR_PREFETCH_SHIFT ); + +if (of_find_property (np,"arm,data-prefetch" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_DATA_PREFETCH_SHIFT ); + +if (of_find_property (np,"arm,ns-interrupt-access" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_NS_INT_CTRL_SHIFT ); + +if (of_find_property (np,"arm,ns-lockdown" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_NS_LOCKDOWN_SHIFT ); + +if (of_find_property (np,"arm,share-override" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_SHARE_OVERRIDE_SHIFT ); + +if (of_find_property (np,"arm,full-line-of-zero" ,NULL )) + val |=BIT (L2X0_AUX_CTRL_FULL_LINE_OF_ZERO_SHIFT ); + + *aux_val |= val; + *aux_mask &= ~val; }static void __init pl310_save (void )
Signature · 1 line
1.7.9.5
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 6 lines · On Tuesday 07 January 2014 17:11:28 Tushar Behera wrote:
Parsed auxiliary control properties for PL310 cache controller.
Signed-off-by Tushar Behera <[email protected]>---
These properties are set for Exynos4 platform. If we can pass these properties through device tree for Exynos4, then we can remove the hard-codedL2_AUX_VAL.
The explanation would be good to have in the actual changeset comment above the '---' line. Are all of them actually needed? You shouldn't havet o set the ones that are already turned on by the boot loader.
Quoted 9 lines
Documentation/devicetree/bindings/arm/l2cc.txt +0
-0@@ -44,6 +44,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode
Documentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 56
Quoted 10 lines
Documentation/devicetree/bindings/arm/l2cc.txt +10
-0+- arm,early-write: If present then BRSEP mode (early write response) is enabled. +- arm,data-prefetch: If present then data prefetching is enabled. +- arm,instruction-prefetch: If present then instruction prefetching is enabled. +- arm,ns-interrupt-access: If present then interrupt mask and interrupt clear + registers can be read or modified in both secure or non-secure accesses. +- arm,ns-lockdown: If present then non-secure accesses can write to lockdown + register. +- arm,share-override: If present then shared attribute is ignored internally. +- arm,full-line-of-zero: If present then 'full line of write zero' behaviour is + enabled.
Is it intentional that you have the "arm," prefix for the new ones, while it's not there for the existing ones? Should we drop it for consistency?
Arnd
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · On Tuesday 07 January 2014 17:11:28 Tushar Behera wrote:
Parsed auxiliary control properties for PL310 cache controller.
Signed-off-by Tushar Behera <[email protected]>---
These properties are set for Exynos4 platform. If we can pass these properties through device tree for Exynos4, then we can remove the hard-codedL2_AUX_VAL.The explanation would be good to have in the actual changeset
Ok, I will add these explanation to comment section.
Quoted 3 lines
comment above the '---' line. Are all of them actually needed? You shouldn't havet o set the ones that are already turned on by the boot loader.
These are the extra options that we used to pass in the kernel. For normal operation, I couldn't perceive any difference without these properties. The only reason for adding these properties was to be in sync with existing auxiliary control value.
Quoted 23 lines
Quoted 9 lines
Documentation/devicetree/bindings/arm/l2cc.txt +0
-0@@ -44,6 +44,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through modeDocumentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 56
Quoted 10 lines
Documentation/devicetree/bindings/arm/l2cc.txt +10
-0+- arm,early-write: If present then BRSEP mode (early write response) is enabled. +- arm,data-prefetch: If present then data prefetching is enabled. +- arm,instruction-prefetch: If present then instruction prefetching is enabled. +- arm,ns-interrupt-access: If present then interrupt mask and interrupt clear + registers can be read or modified in both secure or non-secure accesses. +- arm,ns-lockdown: If present then non-secure accesses can write to lockdown + register. +- arm,share-override: If present then shared attribute is ignored internally. +- arm,full-line-of-zero: If present then 'full line of write zero' behaviour is + enabled.Is it intentional that you have the "arm," prefix for the new ones, while it's not there for the existing ones? Should we drop it for consistency?
Quoted 1 line
Arnd
No specific reasons for this. I was following "arm,data-latency" format. We can drop the "arm," prefix.
Thanks for the review.
Signature · 1 line
Tushar Behera
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
In reply to Tushar BeheraQuoted 21 lines · Tushar Behera
Parsed auxiliary control properties for PL310 cache controller.
Signed-off-by Tushar Behera <[email protected]>---
These properties are set for Exynos4 platform. If we can pass these properties through device tree for Exynos4, then we can remove the hard-codedL2_AUX_VAL.Documentation/devicetree/bindings/arm/l2cc.txt | 10 ++++++++++ arch/arm/include/asm/hardware/cache-l2x0.h | 1 + arch/arm/mm/cache-l2x0.c | 25 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+)Documentation/devicetree/bindings/arm/l2cc.txt +1
-0@@ -44,6 +44,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,early-write: If present then BRSEP mode (early write response) is enabled.
Documentation/devicetree/bindings/arm/l2cc.txt Optional properties: · line 57
Quoted 10 lines
Documentation/devicetree/bindings/arm/l2cc.txt +9
-0+- arm,data-prefetch: If present then data prefetching is enabled. +- arm,instruction-prefetch: If present then instruction prefetching is enabled. +- arm,ns-interrupt-access: If present then interrupt mask and interrupt clear + registers can be read or modified in both secure or non-secure accesses. +- arm,ns-lockdown: If present then non-secure accesses can write to lockdown + register. +- arm,share-override: If present then shared attribute is ignored internally. +- arm,full-line-of-zero: If present then 'full line of write zero' behaviour is + enabled.
Hi Tushar,
This has been discussed couple of times in past[1][2], and the opinion was not to have these in DT as they are more configuration data than the actual hardware description.
Regards,
Sudeep
[1] https://lkml.org/lkml/2013/11/7/8
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/185610.html
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 5 lines · On Tuesday 07 January 2014 12:41:42 Sudeep Holla wrote:
Hi Tushar,
This has been discussed couple of times in past[1][2], and the opinion was not to have these in DT as they are more configuration data than the actual hardware description.
How do you suggest we get rid of the magic constants in platform code then? I definitely don't want to keep the current state, and having configuration data in DT seems the lesser evil.
Are there some reasonable defaults that Linux could use independent of the platform and of what the boot loader defaults to?
Arnd
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 11 lines · Arnd Bergmann
Quoted 5 lines · On Tuesday 07 January 2014 12:41:42 Sudeep Holla wrote:
Hi Tushar,
This has been discussed couple of times in past[1][2], and the opinion was not to have these in DT as they are more configuration data than the actual hardware description.
How do you suggest we get rid of the magic constants in platform code then? I definitely don't want to keep the current state, and having configuration data in DT seems the lesser evil.
I agree, but since these are more L2CC configuration than hardware description, IMO chosen node is one option. However it's good to get opinion from DT guys.
Quoted 3 lines
Are there some reasonable defaults that Linux could use independent of the platform and of what the boot loader defaults to?
Most of these registers can't be programmed in Non-secure mode. So as mentioned already in previous discussions it is better to avoid these settings in kernel. It would be better if bootloader programs these settings even if Linux runs in secure mode for simplicity.
Regards,
Sudeep
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 8 lines · On Tuesday 07 January 2014 15:55:45 Sudeep Holla wrote:
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · On Tuesday 07 January 2014 12:41:42 Sudeep Holla wrote:
Hi Tushar,
This has been discussed couple of times in past[1][2], and the opinion was not to have these in DT as they are more configuration data than the actual hardware description.
Quoted 7 lines
Quoted 4 lines
How do you suggest we get rid of the magic constants in platform code then? I definitely don't want to keep the current state, and having configuration data in DT seems the lesser evil.
I agree, but since these are more L2CC configuration than hardware description, IMO chosen node is one option. However it's good to get opinion from DT guys.
I think one of the discussions we had during the Edinburgh kernel summit resulted in being more relaxed towards configuration data in device nodes.
Quoted 8 lines
Quoted 3 lines
Are there some reasonable defaults that Linux could use independent of the platform and of what the boot loader defaults to?
Most of these registers can't be programmed in Non-secure mode. So as mentioned already in previous discussions it is better to avoid these settings in kernel. It would be better if bootloader programs these settings even if Linux runs in secure mode for simplicity.
Yes, that would be ideal, but I fear we have to live with the boot loaders that are in existence already. Whether or not the registers can be programmed in non-secure mode is certainly a piece of information that belongs into the DT node, so we don't try to write them when we shouldn't. We could also have a property that identifies whether the boot loader has in fact set up the cache controller correctly or whether Linux has to do it. There should be no argument about this being out of scope, since it describes a platform property (presence of a sane boot loader) rather than the actual configuration.
If we decide to have such properties and we boot on a system where we can and should change the settings, is there a way for Linux to know what the settings are supposed to be other than reading them from a DT file or from a per-platform default?
Arnd
PATCH 1/1 ARM: cache-l2x0: Parse properties from DT for PL310 cache controller
Quoted 14 lines · Arnd Bergmann
Quoted 13 lines · On Tuesday 07 January 2014 15:55:45 Sudeep Holla wrote:
Quoted 11 lines · Arnd Bergmann
Quoted 5 lines · On Tuesday 07 January 2014 12:41:42 Sudeep Holla wrote:
Hi Tushar,
This has been discussed couple of times in past[1][2], and the opinion was not to have these in DT as they are more configuration data than the actual hardware description.
How do you suggest we get rid of the magic constants in platform code then? I definitely don't want to keep the current state, and having configuration data in DT seems the lesser evil.
Quoted 6 lines
Quoted 2 lines
I agree, but since these are more L2CC configuration than hardware description, IMO chosen node is one option. However it's good to get opinion from DT guys.
I think one of the discussions we had during the Edinburgh kernel summit resulted in being more relaxed towards configuration data in device nodes.
Ok.
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
Are there some reasonable defaults that Linux could use independent of the platform and of what the boot loader defaults to?
Most of these registers can't be programmed in Non-secure mode. So as mentioned already in previous discussions it is better to avoid these settings in kernel. It would be better if bootloader programs these settings even if Linux runs in secure mode for simplicity.
Quoted 4 lines
Yes, that would be ideal, but I fear we have to live with the boot loaders that are in existence already. Whether or not the registers can be programmed in non-secure mode is certainly a piece of information that belongs into the DT node, so we don't try to write them when we shouldn't. We could also have a
But the main problem is that there is no way to detect the current mode (i.e. secure or non-secure). So it won't be of any help even if we can get these information from DT. It's better to assume Linux runs in non-secure mode and avoid using any registers with secure only access. Any attempt to such restricted access in non-secure mode results in error.
Quoted 10 lines
property that identifies whether the boot loader has in fact set up the cache controller correctly or whether Linux has to do it. There should be no argument about this being out of scope, since it describes a platform property (presence of a sane boot loader) rather than the actual configuration.
If we decide to have such properties and we boot on a system where we can and should change the settings, is there a way for Linux to know what the settings are supposed to be other than reading them from a DT file or from a per-platform default?
Yes I understand, but for above mentioned reason it's difficult.
Regards,
Sudeep