Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-12-10 Thread Grant Likely 
On Wed, Nov 25, 2009 at 10:27 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
 On Wed, 2009-11-25 at 21:05 -0700, Grant Likely wrote:

 You're right, it's not, but makes merging less complex, and then I can
 refactor properly.

 Still, make them __be32 at least

Okay.  Done.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-26 Thread Mitch Bradley 


Right, that's the only sane way to do it, I just didn't remember off
hand what was said in the OF spec  :-) 



3.2.2.1.2 Property values

The property-encoding format is independent of hardware byte order and 
alignment characteristics.  The encoded byte order is well-defined (in 
particular, it is big endian). ...


...

-- 32-bit integer.  A 32-bit integer is encoded into a property value 
byte array by storing the most significant byte at the next available 
address, followed (at address+1) by the high middle byte, the low middle 
byte, and (at address+3) the least significant byte.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-26 Thread Segher Boessenkool 
You're right, it's not, but makes merging less complex, and then I  
can

refactor properly.


Still, make them __be32 at least


There is no alignment guarantee at all either, better make it all u8
and use accessor functions everywhere.


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-26 Thread Benjamin Herrenschmidt 
On Thu, 2009-11-26 at 22:36 +0100, Segher Boessenkool wrote:
  You're right, it's not, but makes merging less complex, and then I  
  can
  refactor properly.
 
  Still, make them __be32 at least
 
 There is no alignment guarantee at all either, better make it all u8
 and use accessor functions everywhere.

Well... if you want to force using an accessor, then make it an opaque
type. But __be32 is fine. It doesn't necessarily convey alignment and
besides, there happens to -be- aligned in almost all cases so far :-)
The flat tree format guarantees 32-bit alignment for the start of a
property, so we are good here I think.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-26 Thread David Miller 
From: Segher Boessenkool seg...@kernel.crashing.org
Date: Thu, 26 Nov 2009 22:36:41 +0100

 You're right, it's not, but makes merging less complex, and then I can
 refactor properly.

 Still, make them __be32 at least
 
 There is no alignment guarantee at all either, better make it all u8
 and use accessor functions everywhere.

I think that might be overkill.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-25 Thread Benjamin Herrenschmidt 
On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
 A cell is firmly established as a u32.  No need to do an ugly typedef
 to redefine it to cell_t.  Eliminate the unnecessary typedef so that
 it doesn't have to be added to the of_fdt header file
 
 Signed-off-by: Grant Likely grant.lik...@secretlab.ca
 ---

I'm not sure about that one. Yes, we do use u32 a lot and cell_t rarely,
so it would seem logical to switch On the other hand, we have that
pesky endianness issue we have never fully solved. So we need accessors
to sort that out, which means directly tapping things as u32 * is not a
good idea if we're going to enforce the use of such accessors.

I believe we should probably just enforce that properties are big endian
for flat device-trees. In which case we could just use __be32 or on of
thoes sparse-friendly types. I know x86 people won't like that much and
to be honest I don't know what 1295 specifies for real OFs but there
aren't enough real OFs around on LE machines for us to care much about
it, is there ?

The reason I prefer a fixed endianness is that allowing LE trees
becomes really nasty when a number is expressed using multiple cells.
That brings the question as to whether the two cells need to be flipped
as well or only the bytes within each cell. And that's the easy bit
(probably flip the whole thing). What about something like a PCI reg
property which is made of 3 cells, two of them forming a 64-bit address
and one containing additional data  attributes ? What is flipped and
where ?

So yes, cell_t might not be the right approach and by far to generic a
name, but u32 isn't the answer neither.

Cheers,
Ben. 

  arch/microblaze/kernel/prom.c |   10 --
  arch/powerpc/kernel/prom.c|   14 ++
  2 files changed, 10 insertions(+), 14 deletions(-)
 
 diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
 index e0f4c34..7760186 100644
 --- a/arch/microblaze/kernel/prom.c
 +++ b/arch/microblaze/kernel/prom.c
 @@ -42,8 +42,6 @@
  #include asm/sections.h
  #include asm/pci-bridge.h
  
 -typedef u32 cell_t;
 -
  /* export that to outside world */
  struct device_node *of_chosen;
  
 @@ -159,7 +157,7 @@ static int __init early_init_dt_scan_memory(unsigned long 
 node,
   const char *uname, int depth, void *data)
  {
   char *type = of_get_flat_dt_prop(node, device_type, NULL);
 - cell_t *reg, *endp;
 + u32 *reg, *endp;
   unsigned long l;
  
   /* Look for the ibm,dynamic-reconfiguration-memory node */
 @@ -178,13 +176,13 @@ static int __init early_init_dt_scan_memory(unsigned 
 long node,
   } else if (strcmp(type, memory) != 0)
   return 0;
  
 - reg = (cell_t *)of_get_flat_dt_prop(node, linux,usable-memory, l);
 + reg = (u32 *)of_get_flat_dt_prop(node, linux,usable-memory, l);
   if (reg == NULL)
 - reg = (cell_t *)of_get_flat_dt_prop(node, reg, l);
 + reg = (u32 *)of_get_flat_dt_prop(node, reg, l);
   if (reg == NULL)
   return 0;
  
 - endp = reg + (l / sizeof(cell_t));
 + endp = reg + (l / sizeof(u32));
  
   pr_debug(memory scan node %s, reg size %ld, data: %x %x %x %x,\n,
   uname, l, reg[0], reg[1], reg[2], reg[3]);
 diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
 index 048e3a3..43cdba2 100644
 --- a/arch/powerpc/kernel/prom.c
 +++ b/arch/powerpc/kernel/prom.c
 @@ -67,8 +67,6 @@ int __initdata iommu_force_on;
  unsigned long tce_alloc_start, tce_alloc_end;
  #endif
  
 -typedef u32 cell_t;
 -
  extern rwlock_t devtree_lock;/* temporary while merging */
  
  /* export that to outside world */
 @@ -441,22 +439,22 @@ static int __init early_init_dt_scan_chosen(unsigned 
 long node,
   */
  static int __init early_init_dt_scan_drconf_memory(unsigned long node)
  {
 - cell_t *dm, *ls, *usm;
 + u32 *dm, *ls, *usm;
   unsigned long l, n, flags;
   u64 base, size, lmb_size;
   unsigned int is_kexec_kdump = 0, rngs;
  
   ls = of_get_flat_dt_prop(node, ibm,lmb-size, l);
 - if (ls == NULL || l  dt_root_size_cells * sizeof(cell_t))
 + if (ls == NULL || l  dt_root_size_cells * sizeof(u32))
   return 0;
   lmb_size = dt_mem_next_cell(dt_root_size_cells, ls);
  
   dm = of_get_flat_dt_prop(node, ibm,dynamic-memory, l);
 - if (dm == NULL || l  sizeof(cell_t))
 + if (dm == NULL || l  sizeof(u32))
   return 0;
  
   n = *dm++;  /* number of entries */
 - if (l  (n * (dt_root_addr_cells + 4) + 1) * sizeof(cell_t))
 + if (l  (n * (dt_root_addr_cells + 4) + 1) * sizeof(u32))
   return 0;
  
   /* check if this is a kexec/kdump kernel. */
 @@ -515,7 +513,7 @@ static int __init early_init_dt_scan_memory(unsigned long 
 node,
   const char *uname, int depth, void 
 *data)
  {
   char *type = of_get_flat_dt_prop(node, device_type, NULL);
 - cell_t

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-25 Thread Grant Likely 
On Wed, Nov 25, 2009 at 8:59 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
 On Tue, 2009-11-24 at 01:18 -0700, Grant Likely wrote:
 A cell is firmly established as a u32.  No need to do an ugly typedef
 to redefine it to cell_t.  Eliminate the unnecessary typedef so that
 it doesn't have to be added to the of_fdt header file

 Signed-off-by: Grant Likely grant.lik...@secretlab.ca
 ---

 I'm not sure about that one. Yes, we do use u32 a lot and cell_t rarely,
 so it would seem logical to switch On the other hand, we have that
 pesky endianness issue we have never fully solved. So we need accessors
 to sort that out, which means directly tapping things as u32 * is not a
 good idea if we're going to enforce the use of such accessors.

 I believe we should probably just enforce that properties are big endian
 for flat device-trees. In which case we could just use __be32 or on of
 thoes sparse-friendly types. I know x86 people won't like that much and
 to be honest I don't know what 1295 specifies for real OFs but there
 aren't enough real OFs around on LE machines for us to care much about
 it, is there ?

Word from Mitch is the device tree is network byte order.  period.

 The reason I prefer a fixed endianness is that allowing LE trees
 becomes really nasty when a number is expressed using multiple cells.
 That brings the question as to whether the two cells need to be flipped
 as well or only the bytes within each cell. And that's the easy bit
 (probably flip the whole thing). What about something like a PCI reg
 property which is made of 3 cells, two of them forming a 64-bit address
 and one containing additional data  attributes ? What is flipped and
 where ?

exactly.

 So yes, cell_t might not be the right approach and by far to generic a
 name, but u32 isn't the answer neither.

You're right, it's not, but makes merging less complex, and then I can
refactor properly.

g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-25 Thread Benjamin Herrenschmidt 
On Wed, 2009-11-25 at 21:05 -0700, Grant Likely wrote:
 
 You're right, it's not, but makes merging less complex, and then I can
 refactor properly. 

Still, make them __be32 at least

Cheers,
Ben.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-25 Thread M. Warner Losh 
In message: fa686aa40911252005o2db85dfk3d9acc61c12ca...@mail.gmail.com
Grant Likely grant.lik...@secretlab.ca writes:
: Word from Mitch is the device tree is network byte order.  period.

OpenFirmware defines the order to be big endian always, even on little
endian processors.

Warner
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-25 Thread Benjamin Herrenschmidt 
On Wed, 2009-11-25 at 23:28 -0700, M. Warner Losh wrote:
 In message:
 fa686aa40911252005o2db85dfk3d9acc61c12ca...@mail.gmail.com
 Grant Likely grant.lik...@secretlab.ca writes:
 : Word from Mitch is the device tree is network byte order.  period.
 
 OpenFirmware defines the order to be big endian always, even on little
 endian processors.

Right, that's the only sane way to do it, I just didn't remember off
hand what was said in the OF spec :-)

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 04/11] of/flattree: eliminate cell_t typedef

2009-11-24 Thread Grant Likely 
A cell is firmly established as a u32.  No need to do an ugly typedef
to redefine it to cell_t.  Eliminate the unnecessary typedef so that
it doesn't have to be added to the of_fdt header file

Signed-off-by: Grant Likely grant.lik...@secretlab.ca
---

 arch/microblaze/kernel/prom.c |   10 --
 arch/powerpc/kernel/prom.c|   14 ++
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index e0f4c34..7760186 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -42,8 +42,6 @@
 #include asm/sections.h
 #include asm/pci-bridge.h
 
-typedef u32 cell_t;
-
 /* export that to outside world */
 struct device_node *of_chosen;
 
@@ -159,7 +157,7 @@ static int __init early_init_dt_scan_memory(unsigned long 
node,
const char *uname, int depth, void *data)
 {
char *type = of_get_flat_dt_prop(node, device_type, NULL);
-   cell_t *reg, *endp;
+   u32 *reg, *endp;
unsigned long l;
 
/* Look for the ibm,dynamic-reconfiguration-memory node */
@@ -178,13 +176,13 @@ static int __init early_init_dt_scan_memory(unsigned long 
node,
} else if (strcmp(type, memory) != 0)
return 0;
 
-   reg = (cell_t *)of_get_flat_dt_prop(node, linux,usable-memory, l);
+   reg = (u32 *)of_get_flat_dt_prop(node, linux,usable-memory, l);
if (reg == NULL)
-   reg = (cell_t *)of_get_flat_dt_prop(node, reg, l);
+   reg = (u32 *)of_get_flat_dt_prop(node, reg, l);
if (reg == NULL)
return 0;
 
-   endp = reg + (l / sizeof(cell_t));
+   endp = reg + (l / sizeof(u32));
 
pr_debug(memory scan node %s, reg size %ld, data: %x %x %x %x,\n,
uname, l, reg[0], reg[1], reg[2], reg[3]);
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 048e3a3..43cdba2 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -67,8 +67,6 @@ int __initdata iommu_force_on;
 unsigned long tce_alloc_start, tce_alloc_end;
 #endif
 
-typedef u32 cell_t;
-
 extern rwlock_t devtree_lock;  /* temporary while merging */
 
 /* export that to outside world */
@@ -441,22 +439,22 @@ static int __init early_init_dt_scan_chosen(unsigned long 
node,
  */
 static int __init early_init_dt_scan_drconf_memory(unsigned long node)
 {
-   cell_t *dm, *ls, *usm;
+   u32 *dm, *ls, *usm;
unsigned long l, n, flags;
u64 base, size, lmb_size;
unsigned int is_kexec_kdump = 0, rngs;
 
ls = of_get_flat_dt_prop(node, ibm,lmb-size, l);
-   if (ls == NULL || l  dt_root_size_cells * sizeof(cell_t))
+   if (ls == NULL || l  dt_root_size_cells * sizeof(u32))
return 0;
lmb_size = dt_mem_next_cell(dt_root_size_cells, ls);
 
dm = of_get_flat_dt_prop(node, ibm,dynamic-memory, l);
-   if (dm == NULL || l  sizeof(cell_t))
+   if (dm == NULL || l  sizeof(u32))
return 0;
 
n = *dm++;  /* number of entries */
-   if (l  (n * (dt_root_addr_cells + 4) + 1) * sizeof(cell_t))
+   if (l  (n * (dt_root_addr_cells + 4) + 1) * sizeof(u32))
return 0;
 
/* check if this is a kexec/kdump kernel. */
@@ -515,7 +513,7 @@ static int __init early_init_dt_scan_memory(unsigned long 
node,
const char *uname, int depth, void 
*data)
 {
char *type = of_get_flat_dt_prop(node, device_type, NULL);
-   cell_t *reg, *endp;
+   u32 *reg, *endp;
unsigned long l;
 
/* Look for the ibm,dynamic-reconfiguration-memory node */
@@ -540,7 +538,7 @@ static int __init early_init_dt_scan_memory(unsigned long 
node,
if (reg == NULL)
return 0;
 
-   endp = reg + (l / sizeof(cell_t));
+   endp = reg + (l / sizeof(u32));
 
DBG(memory scan node %s, reg size %ld, data: %x %x %x %x,\n,
uname, l, reg[0], reg[1], reg[2], reg[3]);

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev