Re: [PATCH 05/16] viafb: Determine type of 2D engine and store itin chip_info

From: Jonathan Corbet
Date: Sun Apr 18 2010 - 13:34:41 EST


[Getting back to the older stuff...]

On Fri, 09 Apr 2010 22:34:16 +0200
Florian Tobias Schandinat <FlorianSchandinat@xxxxxx> wrote:

> >> Just a minor nit:
> >> Could we change the default so that if someone adds support for a new
> >> IGP (and misses this function) we default to either the newest or
> >> preferably to none? I've just seen too much poorly maintained code in
> >> this driver and defaulting to the oldest is hence a bad idea.
> >> Otherwise it's fine.
> >
> > That would require making an exhaustive list of older chipset types.
> > It could probably be inferred through inspection of the code, but I
> > worry about making assumptions in this area...
>
> Such list already exists. gfx_chip_name = pdi->driver_data in hw.c (and
> only there) so what is needed is the list viafb_pci_table in viafbdev.c
> (relatively at the end) of all chips:

I've spent a bit of time looking at this. What's really needed is a
better way of abstracting the chip types so that we can maybe get rid
of all those switch statements throughout the driver. For the purposes
of getting this work in, I'm not quite prepared to make that change,
though I could certainly consider doing it in the future.

In the absence of that, the only course of action which makes sense is
to simply fail the initialization if an unknown chip type shows up
there. That's easy, and I can do it. But, given that this was a
"minor nit," can we leave it as-is for now? There's a *lot* of things
to clean up in this driver, I'd like to make it better a step at a time
rather than trying to do the whole thing at once.

Thanks,

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/