Re: [PATCH 2.6.11-rc2 3/3] altix: tioca chip driver (agp)

From: Mark Maule <maule_at_sgi.com>
Date: 2005-02-08 02:49:14
On Mon, 7 Feb 2005, Christoph Hellwig wrote:

> Private headers after <asm/*> headers, please.  If tioca_provider.h needs
> xwidgetdev.h the latter should move to <asm/sn/>

ok.

> All the exports aren't nice because they expose internals.  In cases
> where AGP and PCI iommu code are similar interwinded (e.g. alpha and amd64)
> we made building the agpgart code builin mandatory which sounds like the
> way to go for altix aswell.

Will investigate this further.

> 
> > +	ca_base = (struct tioca *)tioca_common->ca_common.bs_base;
> 

Will remove unneeded casts and cleanup your style-related comments.

> >
> > +	if (!tioca_kern) {
> > +		return -1;
> > +	}
> 
> And you're leaking the previous allocation here.

yep.

> 
> list_for_each_entry() please

ok.

> 
> > +int
> > +tioca_init_provider(void)
> > +{
> > +	if (sn_sal_rev_major() < 4 || sn_sal_rev_minor() < 0x06) {
> > +		printk
> > +		    (KERN_ERR "%s:  SGI prom rev 4.06 or greater required "
> > +		     "for tioca support\n", __FUNCTION__);
> > +		return 0;
> > +	}
> 
> isn't that a little noisy for people who have systems without the hardware
> this driver actually supports?  Also the hex notation for the SAL minor
> is pure obsfucation.
> 

Agreed.  This check should move to tioca_bus_fixup().

Mark

-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Mon Feb 7 10:51:01 2005

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:35 EST