Re: [PATCH 1/2] i2c: Add support for device alias names

[Kay Sievers <kay.sievers-tD+1rO4QERM@public.gmane.org>]

On Thu, 2008-05-01 at 10:04 +0200, Jean Delvare wrote:
> On Mon, 28 Apr 2008 18:16:17 +0200, Kay Sievers wrote:
> > On Mon, 2008-04-28 at 17:40 +0200, Jean Delvare wrote:
> > > Why would i2c device modaliases ever contain multiple strings? A device
> > > can't have multiple names, can it?
> > 
> > Like ACPI/PNP devices, which can have several compat id's, which means
> > that a single device can have "multiple names":
> >   $ cat /sys/bus/pnp/devices/00:09/id
> >   IBM0057
> >   PNP0f13
> 
> Ah, I didn't know about this. Now I'm curious how it can work. Does it
> mean that several drivers attempt to bind to this device? 

They are usually all id's for the same type of device, and don't describe
multiple functions at the same time. In most cases the vendor id's, like
"IBM0057" here, do not match anything.

> > > Can't we just stop handle_moddevtable() from adding a tailing "*"
> > > automatically, and just let the device types which need it, add it on
> > > their own?
> > 
> > For a lot subsystems it's fine to have it appended, as there is a
> > defined list of identifiers, which must appear in the same order, and
> > new identifiers are appended to the end. So the "*" still matches
> > modules with possibly extended modalias strings.
> 
> I understand the logic, however I am skeptical how useful it is in
> practice. If we add an identifier to the device aliases, then we also
> update the corresponding modalias, so no in-tree driver can break. The
> only case where it makes a difference, as far as I can see, is for
> out-of-tree drivers. Am I correct?

That sounds correct, yes.

> On top of that, I doubt that we
> actually add new identifiers that frequently, do we?

Not that I know.

> > We would also need to review all buses which export modalias, if they
> > need the "*" or not, and add them by hand, if needed.
> > 
> > I guess, it's easier to introduce an additional parameter to
> > file2alias::do_table() and suppress the trailing "*" for i2c?
> 
> That's one possibility, but I had a slightly different approach, which
> is to just let the type-specific handlers add the trailing "*" by
> themselves if they need it. This allows for optimization in a few
> cases.
> 
> Subject: modpost: i2c aliases need no trailing wildcard
> 
> Not all device type aliases need a trailing wildcard, in particular
> the i2c aliases don't. Don't add a wildcard by default in do_table(),
> instead let each device type handler add it if needed.

...

> The patch only changes the i2c aliases, all the rest is the same as
> before (unless I messed up somewhere, that is.) Do you think this would
> be acceptable for upstream? If you think it's better to add a parameter
> to do_table() and let it add the "*" as it did so far, that's also fine
> with me, I can update the patch to do that.

Looks fine to me.

If the content of:
  /lib/modules/$(uname -r)/modules.alias
looks correct after the patch, this should go in, I think.

Thanks,
Kay


_______________________________________________
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c