Re: [PATCH] of: i2c: improve last resort compatible entry selection

[Jean Delvare <khali@linux-fr.org>]

On Tue, 15 Jul 2008 09:40:08 -0400, Jon Smirl wrote:
> On 7/15/08, Jochen Friedrich <jochen@scram.de> wrote:
> > Hi Anton,
> >
> >
> >  > Since no sane driver will ever match specific devices, what we want is
> >  > to select most generic option (last). Then driver may call
> >  > of_device_is_compatible() if it is really interested in details.
> >
> >
> > My original intention was to have alias entries for specific devices in the
> >  i2c device drivers. Later, Jean decided to only have the most generic names
> >  in there (like in http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;f=drivers/media/video/tvaudio.c;h=c77914d99d15c5972c94e9763a08b5789098e90a;hp=6f9945b04e1f2bcd676f0ed8dc910994b29ed300;hb=ae429083efe996ca2c569c44fd6fea440676dc33;hpb=60b129d7bfa3e20450816983bd52c49bb0bc1c21)
> >  So your patch is correct.

Eeeek. The patch you mention here is only the conversion of ONE driver.
It is absolutely not relevant as to what the general rule is.

Background:
Some media drivers have been relying on autodetection for years. We
will convert them to the new model as much as possible, however the
knowledge of what chip is present on each board has sometimes been
lost, so switching to the new i2c model isn't that easy. So, as a
transition step, for a few media drivers, we decided to not enumerate
all the supported chips (nobody would be able to instantiate any of
them specifically anyway). This might change in the future, as
developers gain knowledge about the hardware again.

Jochen, I am very surprised that you dare drawing conclusions based on
one random patch of mine. And I am unhappy that you even claim that I
took some decision when I definitely did not.

> Why aren't we listing the chip names in the driver's id section? Large
> chip id tables are not unusual in the kernel, the e1000 driver has
> about 70 entries.
> 
> Taking the chip id table out of the driver just means we have to build
> it somewhere else. It doesn't save space, the tables just get moved.
> Device firmware has the chip names in it so there has to be code in
> the kernel somewhere to do the mapping from chip id to linux driver
> name.
> 
> Pushing the linux driver name down into the firmware is even crazier.
> +  /* 2. search for linux,<i2c-type> entry */
> Now if the linux driver gets renamed everyone on the planet needs a
> firmware update.

I can't comment on the specific issue at hand as I am not familiar with
it, but overall Jon appears to be right. Listing individual chips in
id_table is the standard way to go. That's even the very reason why we
decided to add this id_table to i2c_driver, instead of matching on the
driver name as we were doing before.

-- 
Jean Delvare