public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sergey Vlasov <vsu@altlinux.ru>
To: Bill Nottingham <notting@redhat.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>,
	Pierre Ossman <drzeus-list@drzeus.cx>,
	Andrew Morton <akpm@osdl.org>,
	ambx1@neo.rr.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [PNP] 'modalias' sysfs export
Date: Tue, 14 Mar 2006 15:29:44 +0300	[thread overview]
Message-ID: <20060314152944.797390cd.vsu@altlinux.ru> (raw)
In-Reply-To: <20060313222644.GD1311@devserv.devel.redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4477 bytes --]

On Mon, 13 Mar 2006 17:26:44 -0500 Bill Nottingham wrote:

> Kay Sievers (kay.sievers@vrfy.org) said: 
> > > See:
> > >   https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=178998

I have written a comment in that bugzilla entry, but it is probably too
terse, so I'll try to explain the problem and my proposed solution
better.

> > > This doesn't work for everything.
> > 
> > Sure not, and I don't think "everything" in PnP will ever work. :) But
> > it does the same as the modalias patch to the kernel we are talking about.
> > There are device table entries completely missing and some other don't
> > match. Some of them can be fixed by adding the aliases as modprobe.conf
> > entries.
> 
> Well, it's just that if this is the solution proposed, I'd like it if
> it worked for any of the people who are seeing problems - in our bugs,
> it hasn't helped any of them yet.

There are two kinds of PNP drivers:

1) PNP device drivers, which use struct pnp_driver.  These drivers use
   struct pnp_device_id in id_table entries; struct pnp_device_id
   contains a single device ID field which is used for matching.
   Aliases for these drivers have the form:

	pnp:dXXXYYYY*

   where XXXYYYY is the PNP ID which is matched.

2) PNP card drivers, which use struct pnp_card_driver.  These drivers
   use struct pnp_card_device_id in id_table entries; struct
   pnp_card_device_id contains ID for the card itself and a variable
   number of logical device IDs.  drivers/pnp/card.c:match_card() uses
   these rules for matching struct pnp_card_device_id to a device:

    a) the card IDs must match;
    b) all device IDs mentioned in struct pnp_card_device_id must be
       present in the card, but can be in any order (and there may be
       more devices than listed in the ID table).

   Aliases for card drivers currently have the form:

	pnp:cXXXYYYYdXXXYYYYdXXXYYYY*

   The first "cXXXYYYY" part is the card ID, and "dXXXYYYY" parts are
   device IDs (there may be up to PNP_MAX_DEVICES == 8 of them).

Now, for the drivers of the first type the only problem is that the
devices can have several compatible IDs in addition to the primary ID,
and this requires either a multiline "modalias" attribute, or a helper
script to call modprobe multiple times with the pnp:dXXXYYYY alias for
all available IDs.  

Drivers of the second type - PNP card drivers - are only used for isapnp
(pnpbios and pnpacpi have only plain devices).  Cards itself have only a
single ID (there are no compatible IDs for cards), but every logical
device on a card can have up to DEVICE_COUNT_COMPATIBLE == 4 compatible
IDs in addition to the primary ID.

(BTW, #define DEVICE_COUNT_COMPATIBLE 4 is duplicated in <linux/pci.h>
and <linux/isapnp.h> - if these values were different, it would be a
nasty bug.)

For the card drivers, in addition to the problem with compatible IDs, we
have another problem - the alias format for them is wrong!  The problem
is that if device IDs in the alias happen to be in a different order
than the same IDs in the actual device (or even in the same order, but
some devices are not mentioned in the ID table), fnmatch() used by
modprobe will not match this alias.

To solve this problem, I suggest to do this:

1) Change the alias format for PNP card drivers to require logical
   device IDs to be sorted, and add an "*" before every device ID part.
   The alias format becomes:

	pnp:cXXXYYYY*dXXXYYYY*dXXXYYYY*

2) Update scripts/mod/file2alias.c:do_pnp_card_entry() to write the new
   alias format (it should sort device IDs - no need to change all
   drivers to list device IDs in sorted order and create potential for
   bugs when someone adds a non-sorted entry).

3) Update /etc/udev/isapnp script mentioned in bugzilla entry to sort
   device ID values before concatenating them.

After dust settles, we can then add "modalias" attribute generation for
PNP card devices to the kernel - note that this attribute would have
only a single value which would list the card ID and all (primary and
compatible) IDs of all logical devices in sorted order.

BTW, we can change the alias format for PNP device drivers to

	pnp:*dXXXYYYY*

(note the additional "*" before the device ID).  This would allow us to
have a single-value "modalias" attribute for PNP logical devices too -
it would have the form

	pnp:dXXXYYYYdXXXYYYYdXXXYYYY

(listing all IDs, in this case sorting is not required, because each
driver will match at most only a single dXXXYYYY entry).

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2006-03-14 12:30 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-27 21:40 [PATCH] [PNP] 'modalias' sysfs export Pierre Ossman
2006-03-01 19:45 ` Kay Sievers
2006-03-02  8:39   ` Pierre Ossman
2006-03-02 16:58     ` Kay Sievers
2006-03-03 11:52       ` Pierre Ossman
2006-03-11 16:05         ` Pierre Ossman
2006-03-11 16:15           ` Arjan van de Ven
2006-03-11 16:21             ` Pierre Ossman
2006-03-12  1:38           ` Andrew Morton
2006-03-12  4:05             ` Kay Sievers
2006-03-12  4:29             ` Adam Belay
2006-03-12  5:09               ` Kay Sievers
2006-03-12  6:01                 ` Adam Belay
2006-03-12 11:17             ` Pierre Ossman
2006-03-12 11:33               ` Matthieu CASTET
2006-03-12 17:23               ` Kay Sievers
2006-03-12 22:55                 ` Andrew Morton
2006-03-13  4:14                   ` Kay Sievers
2006-03-13  6:02                     ` Adam Belay
2006-03-13  6:21                       ` Kay Sievers
2006-03-13  7:04                         ` Adam Belay
2006-03-13  7:26                         ` Adam Belay
2006-03-13  7:36                           ` Kay Sievers
2006-03-14  1:25                             ` Adam Belay
2006-03-13 16:57                 ` Bill Nottingham
2006-03-13 19:24                   ` Kay Sievers
2006-03-13 22:26                     ` Bill Nottingham
2006-03-14 12:29                       ` Sergey Vlasov [this message]
2006-03-14 12:47                         ` Pierre Ossman
2006-03-14 15:00                           ` Sergey Vlasov
2006-05-09 17:41                         ` Pozsar Balazs
2006-05-12 11:09                           ` Kay Sievers
  -- strict thread matches above, loose matches on Subject: below --
2006-03-11 17:07 Andrey Borzenkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060314152944.797390cd.vsu@altlinux.ru \
    --to=vsu@altlinux.ru \
    --cc=akpm@osdl.org \
    --cc=ambx1@neo.rr.com \
    --cc=drzeus-list@drzeus.cx \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=notting@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox