[U-Boot] [PATCH] [v4] powerpc/85xx: fix compatible property for the L2 cache node
The compatible property for the L2 cache node (on 85xx systems that don't have a CPC) was using a value for the property length that did not match the actual length of the property.
Signed-off-by: Timur Tabi timur@freescale.com --- arch/powerpc/cpu/mpc85xx/fdt.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c index 642f6c5..121db5f 100644 --- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -165,7 +165,6 @@ static inline void ft_fixup_l2cache(void *blob) int len, off; u32 *ph; struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr())); - char compat_buf[38];
const u32 line_size = 32; const u32 num_ways = 8; @@ -192,22 +191,27 @@ static inline void ft_fixup_l2cache(void *blob) }
if (cpu) { + char buf[40]; + if (isdigit(cpu->name[0])) - len = sprintf(compat_buf, - "fsl,mpc%s-l2-cache-controller", cpu->name); + /* MPCxxxx, where xxxx == 4-digit number */ + len = sprintf(buf, "fsl,mpc%s-l2-cache-controller", + cpu->name) + 1; else - len = sprintf(compat_buf, - "fsl,%c%s-l2-cache-controller", - tolower(cpu->name[0]), cpu->name + 1); + /* 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" to the string */ + len += sprintf(buf + len, "cache") + 1;
- sprintf(&compat_buf[len + 1], "cache"); + fdt_setprop(blob, off, "compatible", buf, len); } 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); fdt_setprop_cell(blob, off, "cache-sets", num_sets); fdt_setprop_cell(blob, off, "cache-level", 2); - fdt_setprop(blob, off, "compatible", compat_buf, sizeof(compat_buf));
/* we dont bother w/L3 since no platform of this type has one */ }
Dear Timur Tabi,
In message 1304113875-6749-1-git-send-email-timur@freescale.com you wrote:
The compatible property for the L2 cache node (on 85xx systems that don't have a CPC) was using a value for the property length that did not match the actual length of the property.
Signed-off-by: Timur Tabi timur@freescale.com
arch/powerpc/cpu/mpc85xx/fdt.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c index 642f6c5..121db5f 100644 --- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -165,7 +165,6 @@ static inline void ft_fixup_l2cache(void *blob) int len, off; u32 *ph; struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr()));
char compat_buf[38];
const u32 line_size = 32; const u32 num_ways = 8;
@@ -192,22 +191,27 @@ static inline void ft_fixup_l2cache(void *blob) }
if (cpu) {
char buf[40];- if (isdigit(cpu->name[0]))
len = sprintf(compat_buf,"fsl,mpc%s-l2-cache-controller", cpu->name);
/* MPCxxxx, where xxxx == 4-digit number */len = sprintf(buf, "fsl,mpc%s-l2-cache-controller",cpu->name) + 1;
len = sprintf(compat_buf,"fsl,%c%s-l2-cache-controller",tolower(cpu->name[0]), cpu->name + 1);
/* 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;
Why do we need this "if" at all? tolower() on a digit is a nop, so you can omit the first branch.
/* append "cache" to the string */len += sprintf(buf + len, "cache") + 1;
This is wrong and misleading. This is not an operation on a C string. You do not "append" (or concatenate) the string cache. You build a specifically structured data set, which is not a C string. So please don't call it a string.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Why do we need this "if" at all? tolower() on a digit is a nop, so you can omit the first branch.
Because cpu->name looks like one of two ways:
8578
or P4080
In the case of "8578", we want to convert that to "mpc8578". In the case of "P4080", we want to convert that to "p4080".
The "if" is need to determine whether to prepend the "mpc".
/* append "cache" to the string */len += sprintf(buf + len, "cache") + 1;This is wrong and misleading. This is not an operation on a C string. You do not "append" (or concatenate) the string cache. You build a specifically structured data set, which is not a C string. So please don't call it a string.
Ok.
Dear Tabi Timur-B04825,
In message 4DBB3E01.7000704@freescale.com you wrote:
Wolfgang Denk wrote:
Why do we need this "if" at all? tolower() on a digit is a nop, so you can omit the first branch.
Because cpu->name looks like one of two ways:
8578
or
P4080
In the case of "8578", we want to convert that to "mpc8578". In the case o f "P4080", we want to convert that to "p4080".
The "if" is need to determine whether to prepend the "mpc".
This gets lost in the code.
If you keep the if / else, you have to add braces for the multiline statements.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
This gets lost in the code.
That's why I added the comments:
/* MPCxxxx, where xxxx == 4-digit number */
If you keep the if / else, you have to add braces for the multiline statements.
Ok.
participants (3)
-
Tabi Timur-B04825 -
Timur Tabi -
Wolfgang Denk