All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sonny Rao <sonnyrao@us.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: sonnyrao@linux.vnet.ibm.com, paulus@samba.org,
	Nathan Lynch <ntl@pobox.com>,
	linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] Power5,Power6 BSR driver
Date: Mon, 7 Jul 2008 16:17:40 -0500	[thread overview]
Message-ID: <20080707211740.GA15821@us.ibm.com> (raw)
In-Reply-To: <1215406775.8970.57.camel@pasglop>

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?

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

Thanks for the comments

Sonny

  reply	other threads:[~2008-07-07 21:17 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-16 18:53 [PATCH] Power5,Power6 BSR driver jschopp
2008-06-17 22:39 ` Nathan Lynch
2008-06-17 22:44   ` Sonny Rao
2008-06-18  6:51     ` Sonny Rao
2008-07-07  4:59       ` Benjamin Herrenschmidt
2008-07-07 21:17         ` Sonny Rao [this message]
2008-07-07 22:26           ` Benjamin Herrenschmidt
2008-07-08  2:58             ` [PATCHv3] " Sonny Rao
2008-07-08  4:52               ` Stephen Rothwell
2008-07-08  5:45                 ` [PATCHv4] " Sonny Rao
2008-06-18  6:53 ` [PATCH] " Sonny Rao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080707211740.GA15821@us.ibm.com \
    --to=sonnyrao@us.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=ntl@pobox.com \
    --cc=paulus@samba.org \
    --cc=sonnyrao@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.