| Message ID | 1377701263-3319-28-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, 2013-08-28 at 15:47 +0100, Julien Grall wrote: > Device tree cells are 32-bit big endian value. Use __be32 to avoid confusion > later. These are only used by sparse, right? So we wouldn't expect this to actually save us from any mistakes unless we make Xen support the use of sparse? (IOW I shouldn't rely on this and not think about this too hard during patch review because the compiler will get it right for me) > Also replace get_val call by dt_next_cell. Would prefer this separately. > Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Ian Campbell <ian.campbell@citrix.com> (on both patches if you split) > --- > xen/common/device_tree.c | 28 +++++++--------------------- > 1 file changed, 7 insertions(+), 21 deletions(-) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index b120585..4bc1ce4 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -154,25 +154,11 @@ static bool_t __init device_tree_node_compatible(const void *fdt, int node, > return 0; > } > > -static void __init get_val(const u32 **cell, u32 cells, u64 *val) > -{ > - *val = 0; > - > - if ( cells > 2 ) > - early_panic("dtb value contains > 2 cells\n"); > - > - while ( cells-- ) > - { > - *val <<= 32; > - *val |= fdt32_to_cpu(*(*cell)++); > - } > -} > - > -static void __init device_tree_get_reg(const u32 **cell, u32 address_cells, > +static void __init device_tree_get_reg(const __be32 **cell, u32 address_cells, > u32 size_cells, u64 *start, u64 *size) > { > - get_val(cell, address_cells, start); > - get_val(cell, size_cells, size); > + *start = dt_next_cell(address_cells, cell); > + *size = dt_next_cell(size_cells, cell); > } > > void dt_get_range(const __be32 **cell, const struct dt_device_node *np, > @@ -327,7 +313,7 @@ static void __init process_memory_node(const void *fdt, int node, > const struct fdt_property *prop; > int i; > int banks; > - const u32 *cell; > + const __be32 *cell; > paddr_t start, size; > u32 reg_cells = address_cells + size_cells; > > @@ -345,7 +331,7 @@ static void __init process_memory_node(const void *fdt, int node, > return; > } > > - cell = (const u32 *)prop->data; > + cell = (const __be32 *)prop->data; > banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); > > for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ ) > @@ -396,7 +382,7 @@ static void __init process_multiboot_node(const void *fdt, int node, > u32 address_cells, u32 size_cells) > { > const struct fdt_property *prop; > - const u32 *cell; > + const __be32 *cell; > int nr; > struct dt_mb_module *mod; > int len; > @@ -418,7 +404,7 @@ static void __init process_multiboot_node(const void *fdt, int node, > early_panic("fdt: node `%s': `reg` property length is too short\n", > name); > > - cell = (const u32 *)prop->data; > + cell = (const __be32 *)prop->data; > device_tree_get_reg(&cell, address_cells, size_cells, > &mod->start, &mod->size); >
On 09/09/2013 12:57 PM, Ian Campbell wrote: > On Wed, 2013-08-28 at 15:47 +0100, Julien Grall wrote: >> Device tree cells are 32-bit big endian value. Use __be32 to avoid confusion >> later. > > These are only used by sparse, right? So we wouldn't expect this to > actually save us from any mistakes unless we make Xen support the use of > sparse? (IOW I shouldn't rely on this and not think about this too hard > during patch review because the compiler will get it right for me) Rigth. I think it also helps the developer to know that value may not have the same endianess as the processor. >> Also replace get_val call by dt_next_cell. > > Would prefer this separately. > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > (on both patches if you split) I will split the patch in 2 parts. > >> --- >> xen/common/device_tree.c | 28 +++++++--------------------- >> 1 file changed, 7 insertions(+), 21 deletions(-) >> >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index b120585..4bc1ce4 100644 >> --- a/xen/common/device_tree.c >> +++ b/xen/common/device_tree.c >> @@ -154,25 +154,11 @@ static bool_t __init device_tree_node_compatible(const void *fdt, int node, >> return 0; >> } >> >> -static void __init get_val(const u32 **cell, u32 cells, u64 *val) >> -{ >> - *val = 0; >> - >> - if ( cells > 2 ) >> - early_panic("dtb value contains > 2 cells\n"); >> - >> - while ( cells-- ) >> - { >> - *val <<= 32; >> - *val |= fdt32_to_cpu(*(*cell)++); >> - } >> -} >> - >> -static void __init device_tree_get_reg(const u32 **cell, u32 address_cells, >> +static void __init device_tree_get_reg(const __be32 **cell, u32 address_cells, >> u32 size_cells, u64 *start, u64 *size) >> { >> - get_val(cell, address_cells, start); >> - get_val(cell, size_cells, size); >> + *start = dt_next_cell(address_cells, cell); >> + *size = dt_next_cell(size_cells, cell); >> } >> >> void dt_get_range(const __be32 **cell, const struct dt_device_node *np, >> @@ -327,7 +313,7 @@ static void __init process_memory_node(const void *fdt, int node, >> const struct fdt_property *prop; >> int i; >> int banks; >> - const u32 *cell; >> + const __be32 *cell; >> paddr_t start, size; >> u32 reg_cells = address_cells + size_cells; >> >> @@ -345,7 +331,7 @@ static void __init process_memory_node(const void *fdt, int node, >> return; >> } >> >> - cell = (const u32 *)prop->data; >> + cell = (const __be32 *)prop->data; >> banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); >> >> for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ ) >> @@ -396,7 +382,7 @@ static void __init process_multiboot_node(const void *fdt, int node, >> u32 address_cells, u32 size_cells) >> { >> const struct fdt_property *prop; >> - const u32 *cell; >> + const __be32 *cell; >> int nr; >> struct dt_mb_module *mod; >> int len; >> @@ -418,7 +404,7 @@ static void __init process_multiboot_node(const void *fdt, int node, >> early_panic("fdt: node `%s': `reg` property length is too short\n", >> name); >> >> - cell = (const u32 *)prop->data; >> + cell = (const __be32 *)prop->data; >> device_tree_get_reg(&cell, address_cells, size_cells, >> &mod->start, &mod->size); >> > >
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index b120585..4bc1ce4 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -154,25 +154,11 @@ static bool_t __init device_tree_node_compatible(const void *fdt, int node, return 0; } -static void __init get_val(const u32 **cell, u32 cells, u64 *val) -{ - *val = 0; - - if ( cells > 2 ) - early_panic("dtb value contains > 2 cells\n"); - - while ( cells-- ) - { - *val <<= 32; - *val |= fdt32_to_cpu(*(*cell)++); - } -} - -static void __init device_tree_get_reg(const u32 **cell, u32 address_cells, +static void __init device_tree_get_reg(const __be32 **cell, u32 address_cells, u32 size_cells, u64 *start, u64 *size) { - get_val(cell, address_cells, start); - get_val(cell, size_cells, size); + *start = dt_next_cell(address_cells, cell); + *size = dt_next_cell(size_cells, cell); } void dt_get_range(const __be32 **cell, const struct dt_device_node *np, @@ -327,7 +313,7 @@ static void __init process_memory_node(const void *fdt, int node, const struct fdt_property *prop; int i; int banks; - const u32 *cell; + const __be32 *cell; paddr_t start, size; u32 reg_cells = address_cells + size_cells; @@ -345,7 +331,7 @@ static void __init process_memory_node(const void *fdt, int node, return; } - cell = (const u32 *)prop->data; + cell = (const __be32 *)prop->data; banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ ) @@ -396,7 +382,7 @@ static void __init process_multiboot_node(const void *fdt, int node, u32 address_cells, u32 size_cells) { const struct fdt_property *prop; - const u32 *cell; + const __be32 *cell; int nr; struct dt_mb_module *mod; int len; @@ -418,7 +404,7 @@ static void __init process_multiboot_node(const void *fdt, int node, early_panic("fdt: node `%s': `reg` property length is too short\n", name); - cell = (const u32 *)prop->data; + cell = (const __be32 *)prop->data; device_tree_get_reg(&cell, address_cells, size_cells, &mod->start, &mod->size);
Device tree cells are 32-bit big endian value. Use __be32 to avoid confusion later. Also replace get_val call by dt_next_cell. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/common/device_tree.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-)