Re: [PATCH] Power5,Power6 BSR driver

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Mon, 2008-07-07 at 16:17 -0500, Sonny Rao wrote:
> On Mon, Jul 07, 2008 at 02:59:35PM +1000, Benjamin Herrenschmidt wrote:
> > 
> > > +		cur->bsr_addr   = reg[i * 2];
> > > +		cur->bsr_len    = reg[i * 2 + 1];
> > 
> > That's fishy... hand-reading of "reg" property without taking
> > into account the parent's #size-cells/#address-cells... can't you
> > use of_address_to_resource or something similar and carry a struct
> > resource around instead ?
> 
> So, with this suggestion I looked at the resource API... not very well
> documented, and I get the feeling like it's more for carving up a PCI
> memory address range.  In the case of the BSR, everything is already
> partitioned (by hardware) so I don't see the point of using this API
> here.  Or am I missing something about it?

It's about properly parsing a "reg" property in OF. The format of the
reg property depends on the parent #address-cells and #size-cells, and
the addresses in there may need remapping (or not) depending on parent
"ranges" properties. The special cases in prom_parse.c for PCI and ISA
are purely to deal with the additional address space flags those add to
addresses, but you shouldn't hit that with BSR.

> > In fact, same goes with the way you do num_bsr_devs = reg_len / 16.
> > 
> > You should rather use -another- property of well known lenght, or
> > get the #address/#size-cells of the parent and use those appropriately.
> 
> Well, I check to make sure the lengths are consistent with each other
> right above there so we shouldn't walk off the end of anything, but I
> will take a look at using #size-cells / #address-cells instead.

The code in prom_parse.c will do it for you :-)

Cheers,
Ben.