[PATCH] Make of_read_number() handle unaligned data

[David VomLehn]

On Fri, Mar 04, 2011 at 10:20:45PM -0700, Grant Likely wrote:
> On Tue, Feb 22, 2011 at 02:36:47PM -0800, David VomLehn wrote:
> > Numeric values in properties can be unaligned, so introduce get_unaligned()
> > in of_read_number() to make this work correctly on all processors.
> > 
> > Signed-off-by: David VomLehn <dvomlehn at cisco.com>
> 
> Hi David,
> 
> I've thought a lot about this change, (which is a big part of why it
> has taken me so long to reply) and while the change itself doesn't
> look problematic, I'm really bothered by the implications.  I really
> don't like the binding that makes this change necessary.  Mixing cell
> and string values is already troublesome.  Placing the cell after the
> string so that alignment is wonky is doubly so.  (however, I am
> working from memory here about the binding that requires this change.
> Can you send it to me again, and I'll have a fresh look).
> 
> g.
> 
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index cad7cf0..a0a6925 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -22,6 +22,7 @@
> >  #include <linux/spinlock.h>
> >  
> >  #include <asm/byteorder.h>
> > +#include <asm/unaligned.h>
> >  
> >  #ifdef CONFIG_OF
> >  
> > @@ -110,8 +111,11 @@ extern void of_node_put(struct device_node *node);
> >  static inline u64 of_read_number(const __be32 *cell, int size)
> >  {
> >  	u64 r = 0;
> > -	while (size--)
> > -		r = (r << 32) | be32_to_cpu(*(cell++));
> > +	while (size--) {
> > +		r = (r << 32) | be32_to_cpu(get_unaligned(cell));
> > +		cell++;
> > +	}
> > +
> >  	return r;
> >  }
> >  

Several devices on our system need to use very large, physically
contiguous, buffers. These buffers can be up to 80 MB long. Hardware
constrains some of the buffer to be at statically determined addresses
and the others have addresses dynamically assigned within a specific
range of memory. Both the size of buffers and the requirement to use
specific addresses imply that the buffers must be set up before or
during the time the bootmem allocator is active.

A given device can have buffers with both statically and dynamically
assigned addresses and may have more than one of each type. Drivers
get the address via tag, which is a string.

The specific syntax for a buffer with a dynamically assigned address
(ignoring optional whitespace) is:

	<dbuf-property> ::= "cisco,dynamic-bufs" "=" <dbuf-specs> ";"
	<dbufs-specs> ::= <dbuf-spec> | <dbuf-spec> "," <dbuf-specs>
	<dbuf-spec> ::= <name> "," "<" <size> <alignment> <range-start>
		<range-size> ">"
	<name> ::= '"' <non-quote-chars> '"'
	<size>, <alignment>, <range-start>, and <range-size> are all hex
		constants

Example: cisco,dynamic-bufs =
		"DisplayBins0",<0x00101000 0x1000 0x10000000 0x0fc00000>,
		"DisplayBins1",<0x00101000 0x1000 0x60000000 0x10000000>;

Buffers with statically assigned addresses have the following syntax
(again, ignoring optional whitespace):

	<sbuf-property> ::= "cisco,static-bufs" "=" <sbuf-spec> ";"
	<sbuf-specs ::= <sbuf-spec> | <sbuf-spec> "," <sbuf-specs>
	<sbuf-spec> ::= <name> "," "<" <start-address> <size> ">"
	<start-address> and <size> are hex constants

Example: cisco,static-bufs = "AvfsDmaMem",<0x67c00000 0x00ea0000>;

Since I may have a string sandwiched betweeen cells, and property values
have no padding for alignment, cells are generally not aligned in this
property.

This syntax gives me something pretty readable from the human standpoint
and easy to find in the device tree. Any other option I tried looked
less desirable, but I'm open to suggestions.

-- 
David VL