On Tue, Mar 08, 2011 at 09:55:09AM -0800, David VomLehn wrote:
> 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 <[email protected]>
> >
> > 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.
Right, and it is this bit, plus the mixed string+cell values, that
bothers me. I'd be much happier with a binding that uses the natural
structure to provide the delineation. Something like:
cisco,dynamic-buffers {
display-bins-0 {
cisco,size = <size>;
cisco,alignment = <alignment>;
cisco,range = <range-start range-size>;
};
display-bins-1 {
cisco,size = <size>;
cisco,alignment = <alignment>;
cisco,range = <start size>;
};
};
cisco,static-buffers {
avfs-dma-mem {
cisco,size = <size>;
cisco,range = <start size>;
};
};
Which uses the existing dt functions for parsing the data, and is
easy for a driver to find the specific data it needs. Plus it stays
out of the territory of mixed data types in properties which raises
eyebrows.
g.
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
