Re: [U-Boot] [PATCH] powerpc: Retain compatible property for L2 cache
On 2/12/2016 6:34 AM, "york sun" york.sun@nxp.com wrote:
On 11/30/2016 11:47 PM, Chris Packham wrote:
On Thu, Dec 1, 2016 at 6:18 AM, york sun york.sun@nxp.com wrote:
On 11/28/2016 07:10 PM, Chris Packham wrote:
Instead of setting the compatible property to "cache", append the desired value retaining what may already be set in the current
property.
Signed-off-by: Chris Packham judge.packham@gmail.com
<snip>
arch/powerpc/cpu/mpc85xx/fdt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
b/arch/powerpc/cpu/mpc85xx/fdt.c
index 047c972ac78e..f31df41836d5 100644 --- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -337,7 +337,8 @@ static inline void ft_fixup_l2cache(void *blob) fdt_setprop_cell(blob, l2_off, "cache-size",
size);
fdt_setprop_cell(blob, l2_off, "cache-sets",
num_sets);
fdt_setprop_cell(blob, l2_off, "cache-level",
2);
fdt_setprop(blob, l2_off, "compatible",
"cache", 6);
if (fdt_node_check_compatible(blob, l2_off,
"cache") == 1)
fdt_appendprop_string(blob, l2_off,
"compatible", "cache");
} if (l3_off < 0) {You drop fdt_setprop, check the compatible "cache" and append it with "cache" again? I thought you wanted
compatible = "fsl,t2080-l2-cache-controller", "cache";
I already have "fsl,t2080-l2-cache-controller" in my dts. Really I just
want
fdt_appendprop_string(blob, l2_off, "compatible", "cache");
I see.
But the check is necessary because we run through this block multiple times (once per CPU). My initial version was
struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr())); int len; char buf[40];
len = sprintf(buf, "fsl,%c%s-l2-cache-controller",tolower(cpu->name[0]), cpu->name + 1) + 1; len += sprintf(buf + len, "cache") + 1;
fdt_setprop(blob, l2_off, "compatible", buf, len);
But that's more code.
Ideally we don't have to fix up dts. Since if we have to do it, I like the long version better. If the dts doesn't have correct compatible, the kernel won't take it, right?
Ok. I'll post a v2 based on the long version. As it's a repeat of code in another block I'll see if i can extract it to a helper function.
When setting the compatible property for the L2 cache ensure that we follow the documented binding by setting both "<chip>-l2-cache-controller" and "cache" as values.
Signed-off-by: Chris Packham judge.packham@gmail.com ---
Changes in v2: - extract a helper function to set the compatible property and use it in both the CONFIG_L2_CACHE and CONFIG_BACKSIDE_L2_CACHE cases.
arch/powerpc/cpu/mpc85xx/fdt.c | 61 +++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c index 047c972ac78e..8aaf53a65458 100644 --- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -180,6 +180,39 @@ static inline void ft_fixup_l3cache(void *blob, int off) #define ft_fixup_l3cache(x, y) #endif
+#if defined(CONFIG_L2_CACHE) || \ + defined(CONFIG_BACKSIDE_L2_CACHE) || \ + defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) +static inline void ft_fixup_l2cache_compatible(void *blob, int off) +{ + int len; + struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr())); + + if (cpu) { + char buf[40]; + + if (isdigit(cpu->name[0])) { + /* MPCxxxx, where xxxx == 4-digit number */ + len = sprintf(buf, "fsl,mpc%s-l2-cache-controller", + cpu->name) + 1; + } else { + /* Pxxxx or Txxxx, where xxxx == 4-digit number */ + len = sprintf(buf, "fsl,%c%s-l2-cache-controller", + tolower(cpu->name[0]), cpu->name + 1) + 1; + } + + /* + * append "cache" after the NULL character that the previous + * sprintf wrote. This is how a device tree stores multiple + * strings in a property. + */ + len += sprintf(buf + len, "cache") + 1; + + fdt_setprop(blob, off, "compatible", buf, len); + } +} +#endif + #if defined(CONFIG_L2_CACHE) /* return size in kilobytes */ static inline u32 l2cache_size(void) @@ -215,9 +248,8 @@ static inline u32 l2cache_size(void)
static inline void ft_fixup_l2cache(void *blob) { - int len, off; + int off; u32 *ph; - struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr()));
const u32 line_size = 32; const u32 num_ways = 8; @@ -243,28 +275,7 @@ static inline void ft_fixup_l2cache(void *blob) return ; }
- if (cpu) { - char buf[40]; - - if (isdigit(cpu->name[0])) { - /* MPCxxxx, where xxxx == 4-digit number */ - len = sprintf(buf, "fsl,mpc%s-l2-cache-controller", - cpu->name) + 1; - } else { - /* Pxxxx or Txxxx, where xxxx == 4-digit number */ - len = sprintf(buf, "fsl,%c%s-l2-cache-controller", - tolower(cpu->name[0]), cpu->name + 1) + 1; - } - - /* - * append "cache" after the NULL character that the previous - * sprintf wrote. This is how a device tree stores multiple - * strings in a property. - */ - len += sprintf(buf + len, "cache") + 1; - - fdt_setprop(blob, off, "compatible", buf, len); - } + ft_fixup_l2cache_compatible(blob, off); fdt_setprop(blob, off, "cache-unified", NULL, 0); fdt_setprop_cell(blob, off, "cache-block-size", line_size); fdt_setprop_cell(blob, off, "cache-size", size); @@ -337,7 +348,7 @@ static inline void ft_fixup_l2cache(void *blob) fdt_setprop_cell(blob, l2_off, "cache-size", size); fdt_setprop_cell(blob, l2_off, "cache-sets", num_sets); fdt_setprop_cell(blob, l2_off, "cache-level", 2); - fdt_setprop(blob, l2_off, "compatible", "cache", 6); + ft_fixup_l2cache_compatible(blob, l2_off); }
if (l3_off < 0) {
On 12/02/2016 12:22 AM, Chris Packham wrote:
When setting the compatible property for the L2 cache ensure that we follow the documented binding by setting both "<chip>-l2-cache-controller" and "cache" as values.
Signed-off-by: Chris Packham judge.packham@gmail.com
Changes in v2:
- extract a helper function to set the compatible property and use it in both the CONFIG_L2_CACHE and CONFIG_BACKSIDE_L2_CACHE cases.
arch/powerpc/cpu/mpc85xx/fdt.c | 61 +++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 25 deletions(-)
Applied to u-boot-mpc85xx master, awaiting upstream. Thanks.
York
participants (2)
-
Chris Packham -
york sun