Re: [patch] 16-way x440 breakage

[Dave Hansen <haveblue@us.ibm.com>]

On Fri, 2003-08-08 at 16:06, Matthew Dobson wrote:
> +	if ((rth->num_scal_dev > MAX_NUMNODES) || 
> +	    (rth->num_rio_dev > MAX_NUMNODES * 2)){
> +		printk("%s ERROR!  MAX_NUMNODES incorrectly defined as %d!!!\n", __FUNCTION__, MAX_NUMNODES);
> +		return 1;
> +	}

Why don't you actually warn for the real condition?  MAX_NUMNODES isn't
incorrect, it's just too low.  Could you mention that it needs to be
raised, and maybe the value it should be raised to?

>  	ptr = (unsigned long)rth + 3;
> -	for(i = 0; i < rth->num_scal_dev; i++)
> -		sd[i] = (struct scal_detail *)(ptr + (scal_detail_size * i));
> +	for(i = 0; i < rth->num_scal_dev; i++, ptr += scal_detail_size)
> +		sd[i] = (struct scal_detail *)ptr;
> +
> +	for(i = 0; i < rth->num_rio_dev; i++, ptr += rio_detail_size)
> +		rd[i] = (struct rio_detail *)ptr;

All the casting here scares me.  If you're doing this: 
(struct scal_detail *)(ptr + (scal_detail_size * i)
and this:
ptr += scal_detail_size
just make it a struct scal_detail * and be done with it.  If it walks
like a duck...

-- 
Dave Hansen
haveblue@us.ibm.com