Re: [PATCH] [PNP] 'modalias' sysfs export

From: Kay Sievers
Date: Thu Mar 02 2006 - 11:56:02 EST


On Thu, Mar 02, 2006 at 09:39:03AM +0100, Pierre Ossman wrote:
> Kay Sievers wrote:
> > On Mon, Feb 27, 2006 at 10:40:19PM +0100, Pierre Ossman wrote:
> >> User space hardware detection need the 'modalias' attributes in the
> >> sysfs tree. This patch adds support to the PNP bus.
> >
> >> +
> >> + /* FIXME: modalias can only do one alias */
> >> + return sprintf(buf, "pnp:c%s\n", pos->id);
> >
> > Without the FIXME actually "fixed", this does not make sense. You want to
> > match always on _all_ aliases. In most cases where you have more than
> > one, the first one is the vendor specific one which isn't interesting at
> > all on Linux. If you have more than one entry usually the second one is the
> > one you are looking for.
> >
> > So eighter we find a way to encode _all_ id's in one modalias string which can
> > be matched by a wildcard or keep the current solution which iterates over the
> > sysfs "id" file and calls modprobe for every entry.
> >
>
> That's a bit harsh. Although the FIXME is a downer, this patch is a
> strict addition of functionality, not removal. It solves a real problem
> for me, and it does so without removing any functionality for anyone
> else. The fact is that most PNP devices do not have multiple id:s (at
> least the ACPI variant which is the most common in todays machines), so
> the problem is not near as big as you make it out to be.

Sorry, but your patch is just incomplete and in some cases just wrong.
On my new ThinkPad, 3 of 12 devices would not work with your patch, so this
is far from "most common" and not acceptable. So eighter we get a fully
working modalias or we better stay without one for PNP and handle that
with the custom script we already use today.

Kay
-
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/