Re: [PATCH] Power5,Power6 BSR driver

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:

quoted

quoted

+		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.

quoted

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.