Re: [patch 2/4 v2] PS3: Add logical performance monitor repository routines

[Stephen Rothwell <sfr@canb.auug.org.au>]

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]

On Wed, 9 Jan 2008 11:01:38 +0100 (CET) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
>
> On Wed, 9 Jan 2008, Stephen Rothwell wrote:
> > On Tue, 08 Jan 2008 22:35:36 -0800 Geoff Levand <geoffrey.levand@am.sony.com> wrote:
> > >
> > > +int ps3_repository_read_num_pu(unsigned int *num_pu)
> > > +{
> > > +	int result;
> > > +	u64 v1;
> > > +
> > > +	v1 = 0;
> > > +	result = read_node(PS3_LPAR_ID_CURRENT,
> > > +			   make_first_field("bi", 0),
> > > +			   make_field("pun", 0),
> > > +			   0, 0,
> > > +			   &v1, NULL);
> > > +	*num_pu = v1;
> > 
> > Why not just pass "num_pu" instead of &v1 to read_node()?
> 
> We need a temporary because num_pu and v1 differ in size.
> The alternative is to make num_pu u64.

Ooops, sorry.  I read the second one first and didn't look hard enough
when I came back to this one.

> > > +int ps3_repository_read_pu_id(unsigned int pu_index, u64 *pu_id)
> > > +{
> > > +	int result;
> > > +	u64 v1;
> > > +
> > > +	v1 = 0;
> > > +	result = read_node(PS3_LPAR_ID_CURRENT,
> > > +		make_first_field("bi", 0),
> > > +		make_field("pu", pu_index),
> > > +		0, 0,
> > > +		&v1, NULL);
> > > +	*pu_id = v1;
> > 
> > Similarly with "pu_id"?
> 
> Yep, here we don't need the temporary.

Which, of course means that the "result" temporary isn't needed either.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]