Re: [U-Boot] [PATCH] [v4] powerpc/85xx: fix compatible property for the L2 cache node

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;
  

  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;
  

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