[Fwd: Fw: [PATCH] PURR data on iSeries Linux]

[Stephen Rothwell]

Hi Jeff,

Looks good in general.  I cannot, of course, comment on the semantics due
to lack of documentation of the HV interfaces :-)

Just a couple of comments:

On Tue, 09 Nov 2004 15:27:01 -0600 Jeff Scheel <scheel at vnet.ibm.com> wrote:
>
>  #ifdef CONFIG_PPC_ISERIES
>  
> +static unsigned long get_purr(void);

This prototype is unnecessary.

> +
> +/*
> + * For iSeries legacy systems, the PPA purr function is available from the
> + * xEmulatedTimeBase field in the paca.
> + */
> +static unsigned long get_purr()

Need "void" in paramter list.

> +{
> +	unsigned long sum_purr=0;
                              ^
Use spaces around = ----------|

> +	int cpu;
> +	struct paca_struct *lpaca;
> +
> +	for_each_online_cpu(cpu) {

Rusty suggests using for_each_cpu in all cricumstances when collecting stats.
If the machine is not capable of cpu hotplug, there is no penalty.

> +		lpaca = paca + cpu;
> +		sum_purr += lpaca->xLpPaca.xEmulatedTimeBase;
> +
> +#ifdef PURR_DEBUG
> +		printk(KERN_INFO "get_purr for cpu (%x) has value (%lx) \n",

Maybe decimal would be more meaningful for these values than hex.

> +			cpu,lpaca->xLpPaca.xEmulatedTimeBase);

Spaces after commas.

You caught me in "picky" mode :-)
-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://ozlabs.org/pipermail/linuxppc64-dev/attachments/20041110/ea0774b5/attachment.pgp