On Sat, Oct 25, 2014 at 1:56 AM, Simon Glass <[email protected]> wrote: > +Tom > > Hi Rob, > > On 15 October 2014 03:21, Rob Herring <[email protected]> wrote: >> From: Rob Herring <[email protected]> >> >> Commit f18295d3837c282f (fdt_support: fix an endian bug of >> fdt_fixup_memory_banks) changed fdt_fixup_memory_banks cell writing from a >> byte at a time to casting the buffer pointer to a 64-bit pointer. This can >> result in unaligned accesses when there is a mixture of cell sizes of 1 >> and 2. > > Should that be 739a01ed8e0?
Uhh, yes. > >> >> Cc: Masahiro Yamada <[email protected]> >> Signed-off-by: Rob Herring <[email protected]> >> --- >> common/fdt_support.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index 3f64156..9c41f3a 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -388,19 +388,22 @@ static int fdt_pack_reg(const void *fdt, void *buf, >> uint64_t *address, >> int i; >> int address_len = get_cells_len(fdt, "#address-cells"); >> int size_len = get_cells_len(fdt, "#size-cells"); >> + u64 cell; >> char *p = buf; >> >> for (i = 0; i < n; i++) { >> if (address_len == 8) >> - *(fdt64_t *)p = cpu_to_fdt64(address[i]); >> + cell = cpu_to_fdt64(address[i]); >> else >> - *(fdt32_t *)p = cpu_to_fdt32(address[i]); >> + cell = cpu_to_fdt32(address[i]); >> + memcpy(p, &cell, address_len); >> p += address_len; >> >> if (size_len == 8) >> - *(fdt64_t *)p = cpu_to_fdt64(size[i]); >> + cell = cpu_to_fdt64(size[i]); >> else >> - *(fdt32_t *)p = cpu_to_fdt32(size[i]); >> + cell = cpu_to_fdt32(size[i]); >> + memcpy(p, &cell, size_len); > > What will this do with 32-bit values? Aren't use assuming that the > first 32-bit word of 'cell' will contain the least significant 32 > bits? Is that always true? I'm not quite sure. We've already done the endian conversion, so we're just copying a string of bytes only changing the alignment potentially. > > Also I really thing this needs a test. It's a pretty simple function > so a unit test would be easy to write. I'll look into that. Rob > >> p += size_len; >> } > > Regards, > Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
