Hi Rusty,
The attached patch makes depmod to output modules.pnpmap file
generated from the pnp device table.
The output format is not compatible with the old modules.isapnpmap.
The new format shows the pnp id string (e.g. CTL0301) while the old
format uses the hex numbers. I don't think it's worthy to keep the
compatibility for this (since the new one is more intuitive), but it'd
be easy to follow the old style.
ciao,
--
Takashi Iwai <[email protected]> ALSA Developer - http://www.alsa-project.org
diff -ru module-init-tools-0.9.15-pre3/depmod.c module-init-tools-0.9.15-pre3-pnp/depmod.c
--- module-init-tools-0.9.15-pre3/depmod.c 2003-09-15 03:44:02.000000000 +0200
+++ module-init-tools-0.9.15-pre3-pnp/depmod.c 2003-11-14 12:59:04.000000000 +0100
@@ -594,6 +594,7 @@
{ "modules.usbmap", output_usb_table },
{ "modules.ccwmap", output_ccw_table },
{ "modules.ieee1394map", output_ieee1394_table },
+ { "modules.pnpmap", output_pnp_table },
{ "modules.alias", output_aliases },
{ "modules.symbols", output_symbols },
};
diff -ru module-init-tools-0.9.15-pre3/depmod.h module-init-tools-0.9.15-pre3-pnp/depmod.h
--- module-init-tools-0.9.15-pre3/depmod.h 2003-08-15 16:21:02.000000000 +0200
+++ module-init-tools-0.9.15-pre3-pnp/depmod.h 2003-11-14 14:46:30.000000000 +0100
@@ -40,6 +40,10 @@
void *ieee1394_table;
unsigned int ccw_size;
void *ccw_table;
+ unsigned int pnp_size;
+ void *pnp_table;
+ unsigned int pnp_card_size;
+ void *pnp_card_table;
/* File contents and length. */
void *data;
diff -ru module-init-tools-0.9.15-pre3/moduleops_core.c module-init-tools-0.9.15-pre3-pnp/moduleops_core.c
--- module-init-tools-0.9.15-pre3/moduleops_core.c 2003-10-01 01:17:42.000000000 +0200
+++ module-init-tools-0.9.15-pre3-pnp/moduleops_core.c 2003-11-14 14:46:24.000000000 +0100
@@ -179,6 +179,14 @@
module->ieee1394_size = PERBIT(IEEE1394_DEVICE_SIZE);
module->ieee1394_table = PERBIT(deref_sym)(module->data,
"__mod_ieee1394_device_table");
+
+ module->pnp_size = PERBIT(PNP_DEVICE_SIZE);
+ module->pnp_table = PERBIT(deref_sym)(module->data,
+ "__mod_pnp_device_table");
+
+ module->pnp_card_size = PERBIT(PNP_CARD_DEVICE_SIZE);
+ module->pnp_card_table = PERBIT(deref_sym)(module->data,
+ "__mod_pnp_card_device_table");
}
struct module_ops PERBIT(mod_ops) = {
diff -ru module-init-tools-0.9.15-pre3/tables.c module-init-tools-0.9.15-pre3-pnp/tables.c
--- module-init-tools-0.9.15-pre3/tables.c 2003-08-19 12:07:46.000000000 +0200
+++ module-init-tools-0.9.15-pre3-pnp/tables.c 2003-11-14 14:52:43.000000000 +0100
@@ -158,3 +158,47 @@
output_ccw_entry(e, shortname, out);
}
}
+
+static void put_pnp_id(FILE *out, const char *id)
+{
+ int i;
+ putc(' ', out);
+ for (i = 0; i < 8 && *id; i++, id++)
+ putc(*id, out);
+}
+
+void output_pnp_table(struct module *modules, FILE *out)
+{
+ struct module *i;
+
+ fprintf(out, "# pnp module ");
+ fprintf(out, "id device ...\n");
+
+ for (i = modules; i; i = i->next) {
+ char shortname[strlen(i->pathname) + 1];
+
+ if (i->pnp_table) {
+ struct pnp_device_id *id;
+ make_shortname(shortname, i->pathname);
+ for (id = i->pnp_table; id->id[0]; id++) {
+ fprintf(out, "%-20s", shortname);
+ put_pnp_id(out, id->id);
+ fprintf(out, "\n");
+ }
+ } else if (i->pnp_card_table) {
+ struct pnp_card_device_id *id;
+ make_shortname(shortname, i->pathname);
+ for (id = i->pnp_card_table; id->id[0]; id++) {
+ int idx;
+ fprintf(out, "%-20s", shortname);
+ put_pnp_id(out, id->id);
+ for (idx = 0; idx < 8; idx++) {
+ if (! id->devid[idx][0])
+ break;
+ put_pnp_id(out, id->devid[idx]);
+ }
+ fprintf(out, "\n");
+ }
+ }
+ }
+}
diff -ru module-init-tools-0.9.15-pre3/tables.h module-init-tools-0.9.15-pre3-pnp/tables.h
--- module-init-tools-0.9.15-pre3/tables.h 2003-05-02 08:28:01.000000000 +0200
+++ module-init-tools-0.9.15-pre3-pnp/tables.h 2003-11-14 14:47:05.000000000 +0100
@@ -55,11 +55,26 @@
#define CCW_DEVICE_SIZE32 (3 * 2 + 2 * 1 + 4)
#define CCW_DEVICE_SIZE64 (3 * 2 + 2 * 1 + 8)
+struct pnp_device_id {
+ char id[8]; /* id string (always 8 letter) */
+};
+#define PNP_DEVICE_SIZE32 8
+#define PNP_DEVICE_SIZE64 8
+
+struct pnp_card_device_id {
+ char id[8]; /* id string (always 8 letter) */
+ unsigned long driver_data; /* grrr... */
+ char devid[8][8];
+};
+#define PNP_CARD_DEVICE_SIZE32 (8 + sizeof(long) + 8 * 8)
+#define PNP_CARD_DEVICE_SIZE64 (8 + sizeof(long) + 8 * 8)
+
/* Functions provided by tables.c */
struct module;
void output_usb_table(struct module *modules, FILE *out);
void output_ieee1394_table(struct module *modules, FILE *out);
void output_pci_table(struct module *modules, FILE *out);
void output_ccw_table(struct module *modules, FILE *out);
+void output_pnp_table(struct module *modules, FILE *out);
#endif /* MODINITTOOLS_TABLES_H */
In message <[email protected]> you write:
> Hi Rusty,
Hi Takashi!
> The attached patch makes depmod to output modules.pnpmap file
> generated from the pnp device table.
>
> The output format is not compatible with the old modules.isapnpmap.
> The new format shows the pnp id string (e.g. CTL0301) while the old
> format uses the hex numbers. I don't think it's worthy to keep the
> compatibility for this (since the new one is more intuitive), but it'd
> be easy to follow the old style.
That seems strange. If you don't worry about backwards compatibility,
then the new scripts/file2alias.c approach is better, which generates
aliases for each module (depmod then collects these into
/lib/modules/`uname -r`/modules.alias for speed).
The tables generated by depmod are purely for backwards compatibility,
although it does look like they will be required throughout 2.6 at
this stage.
Does that clarify?
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
At Mon, 17 Nov 2003 14:46:16 +1100,
Rusty Russell wrote:
>
> In message <[email protected]> you write:
> > Hi Rusty,
>
> Hi Takashi!
>
> > The attached patch makes depmod to output modules.pnpmap file
> > generated from the pnp device table.
> >
> > The output format is not compatible with the old modules.isapnpmap.
> > The new format shows the pnp id string (e.g. CTL0301) while the old
> > format uses the hex numbers. I don't think it's worthy to keep the
> > compatibility for this (since the new one is more intuitive), but it'd
> > be easy to follow the old style.
>
> That seems strange. If you don't worry about backwards compatibility,
> then the new scripts/file2alias.c approach is better, which generates
> aliases for each module (depmod then collects these into
> /lib/modules/`uname -r`/modules.alias for speed).
>
> The tables generated by depmod are purely for backwards compatibility,
> although it does look like they will be required throughout 2.6 at
> this stage.
>
> Does that clarify?
ah, ok, that makes sense.
but still, file2alias (as of test9) doesn't output the entries for pnp
devices...
Takashi
>> > The attached patch makes depmod to output modules.pnpmap file
>> > generated from the pnp device table.
>> >
thank you. I was about to do it for hotplug.
>> > The output format is not compatible with the old modules.isapnpmap.
>> > The new format shows the pnp id string (e.g. CTL0301) while the old
>> > format uses the hex numbers. I don't think it's worthy to keep the
>> > compatibility for this (since the new one is more intuitive), but it'd
>> > be easy to follow the old style.
>>
can you get full ID out of sysfs? This is required for
coldplugging to work (I think mainly of loading drivers
for on-board legacy devices detected by PNP BIOS like floppy,
parport, serial).
Oh, BTW, it reminds me - file2alias prints hex in upper
case while both sysfs and hotplug present them in lower case
(for sure for USB and PCI, and for PNP entries detected by
PNP BIOS). Should not we unify representation?
>> That seems strange. If you don't worry about backwards compatibility,
>> then the new scripts/file2alias.c approach is better,
welcome to the family :)
> which generates
>> aliases for each module (depmod then collects these into
>> /lib/modules/`uname -r`/modules.alias for speed).
>>
>> The tables generated by depmod are purely for backwards compatibility,
>> although it does look like they will be required throughout 2.6 at
>> this stage.
>>
>> Does that clarify?
>
>ah, ok, that makes sense.
>
> but still, file2alias (as of test9) doesn't output the entries for pnp
> devices...
Sure it does not, noone did it as yet. If you do it please let me
know, specifically about format for aliases.
Thank you
-andrey
At Mon, 17 Nov 2003 15:37:25 +0300,
Andrey Borzenkov wrote:
>
>
> >> > The attached patch makes depmod to output modules.pnpmap file
> >> > generated from the pnp device table.
> >> >
>
> thank you. I was about to do it for hotplug.
>
> >> > The output format is not compatible with the old modules.isapnpmap.
> >> > The new format shows the pnp id string (e.g. CTL0301) while the old
> >> > format uses the hex numbers. I don't think it's worthy to keep the
> >> > compatibility for this (since the new one is more intuitive), but it'd
> >> > be easy to follow the old style.
> >>
>
> can you get full ID out of sysfs?
yes.
> This is required for
> coldplugging to work (I think mainly of loading drivers
> for on-board legacy devices detected by PNP BIOS like floppy,
> parport, serial).
>
> Oh, BTW, it reminds me - file2alias prints hex in upper
> case while both sysfs and hotplug present them in lower case
> (for sure for USB and PCI, and for PNP entries detected by
> PNP BIOS). Should not we unify representation?
hmm, file2alias uses lower letters as the identifier (seprator?), so i
think simply using lower hex letter will be confusing. wouldn't it be
better to have an explicit delimter character like ':' (or '/' or
whatever) ?
> >> That seems strange. If you don't worry about backwards compatibility,
> >> then the new scripts/file2alias.c approach is better,
>
> welcome to the family :)
yeah brother :)
> > which generates
> >> aliases for each module (depmod then collects these into
> >> /lib/modules/`uname -r`/modules.alias for speed).
> >>
> >> The tables generated by depmod are purely for backwards compatibility,
> >> although it does look like they will be required throughout 2.6 at
> >> this stage.
> >>
> >> Does that clarify?
> >
> >ah, ok, that makes sense.
> >
> > but still, file2alias (as of test9) doesn't output the entries for pnp
> > devices...
>
> Sure it does not, noone did it as yet. If you do it please let me
> know, specifically about format for aliases.
at first i'll try to add the support of old isapnp format for
compatibility, so that old programs can work as they are.
the file2alias format of (isa) pnp devices will need variable number
of items, since a driver may require multiple ids.
for example, snd-cs4236 driver supports the cards with three ids like
CSCe825:CSC0100:CSC0110
and four ids like
CSCd937:CSC0000:CSC0010:CSC0003
in each case, a matching card must include all ids listed there.
well, i'm not sure which identifier (separator) letter in which style
should be used. something like
pnp:idXXXxxxxd0XXXxxxxd1XXXXxxxx
?? separators including a number might be a bad idea, though...
Takashi
At Mon, 17 Nov 2003 14:34:08 +0100,
I wrote:
>
> at first i'll try to add the support of old isapnp format for
> compatibility, so that old programs can work as they are.
done.
Rusty, could you check whether it's ok?
--
Takashi Iwai <[email protected]> ALSA Developer - http://www.alsa-project.org
> >
> > Oh, BTW, it reminds me - file2alias prints hex in upper
> > case while both sysfs and hotplug present them in lower case
> > (for sure for USB and PCI, and for PNP entries detected by
> > PNP BIOS). Should not we unify representation?
>
> hmm, file2alias uses lower letters as the identifier (seprator?), so i
> think simply using lower hex letter will be confusing. wouldn't it be
> better to have an explicit delimter character like ':' (or '/' or
> whatever) ?
>
those are not meant to be end-user visible anyway so no I do not
expect any confustion. Nor do I suggest making them lower case -
we could fix all occurences in kernel instead :)
if you want to use separator, = is probably better. v=XXXp=YYY
>
> at first i'll try to add the support of old isapnp format for
> compatibility, so that old programs can work as they are.
>
??? old programs do not know about modules.alias at all. Or I
do not understand what you mean.
> the file2alias format of (isa) pnp devices will need variable number
> of items, since a driver may require multiple ids.
> for example, snd-cs4236 driver supports the cards with three ids like
> CSCe825:CSC0100:CSC0110
> and four ids like
> CSCd937:CSC0000:CSC0010:CSC0003
> in each case, a matching card must include all ids listed there.
>
do you mean that card will have to have all of these IDs to match?
I can't get it reading sources. When driver matches card against
card driver it is apparently using only main IDs, not logical
device IDs:
driver/pnp/card.c:match_card()
static const struct pnp_card_device_id * match_card(struct pnp_card_driver * drv, struct pnp_card * card)
{
const struct pnp_card_device_id * drv_id = drv->id_table;
while (*drv_id->id){
if (compare_pnp_id(card->id,drv_id->id))
return drv_id;
drv_id++;
}
return NULL;
}
where are drv_id->devs used?
because the point of modules.alias is to follow the same logic (to
determine which driver to load), where do ->devs fit in?
> well, i'm not sure which identifier (separator) letter in which style
> should be used. something like
> pnp:idXXXxxxxd0XXXxxxxd1XXXXxxxx
leave it as is, it is just fine actually. Assuming you really need
those extra IDs at all.
What application do you have in mind? I aim at hotplug, i.e.
(isa|bios)pnp after getting list of devices would call hotplug to load
drivers. So i need the same logic as in driver matching and this
does not need extra IDs.
regards
-andrey
At Mon, 17 Nov 2003 18:07:04 +0300,
Andrey Borzenkov wrote:
>
> > >
> > > Oh, BTW, it reminds me - file2alias prints hex in upper
> > > case while both sysfs and hotplug present them in lower case
> > > (for sure for USB and PCI, and for PNP entries detected by
> > > PNP BIOS). Should not we unify representation?
> >
> > hmm, file2alias uses lower letters as the identifier (seprator?), so i
> > think simply using lower hex letter will be confusing. wouldn't it be
> > better to have an explicit delimter character like ':' (or '/' or
> > whatever) ?
> >
>
> those are not meant to be end-user visible anyway so no I do not
> expect any confustion. Nor do I suggest making them lower case -
> we could fix all occurences in kernel instead :)
>
> if you want to use separator, = is probably better. v=XXXp=YYY
well, parsing pnp would be easiser because the id is always 7
letters, so no need for that...
> > at first i'll try to add the support of old isapnp format for
> > compatibility, so that old programs can work as they are.
> >
>
> ??? old programs do not know about modules.alias at all. Or I
> do not understand what you mean.
no, not modules.alias but the old modules.isapnpmap, which depmod
should generate, too.
> > the file2alias format of (isa) pnp devices will need variable number
> > of items, since a driver may require multiple ids.
> > for example, snd-cs4236 driver supports the cards with three ids like
> > CSCe825:CSC0100:CSC0110
> > and four ids like
> > CSCd937:CSC0000:CSC0010:CSC0003
> > in each case, a matching card must include all ids listed there.
> >
>
> do you mean that card will have to have all of these IDs to match?
>
> I can't get it reading sources. When driver matches card against
> card driver it is apparently using only main IDs, not logical
> device IDs:
>
> driver/pnp/card.c:match_card()
>
> static const struct pnp_card_device_id * match_card(struct pnp_card_driver * drv, struct pnp_card * card)
> {
> const struct pnp_card_device_id * drv_id = drv->id_table;
> while (*drv_id->id){
> if (compare_pnp_id(card->id,drv_id->id))
> return drv_id;
> drv_id++;
> }
> return NULL;
> }
>
> where are drv_id->devs used?
hmm, i thought it checks the device ids but apparently it's not.
IMO, this is a bug, because there are cards with the same card id but
different device ids. (e.g. sound/isa/cs423x/cs4236.c)
in the logic above, only the first matching entry is checked and it
results in the failure of probing.
Adam, what do you think?
> because the point of modules.alias is to follow the same logic (to
> determine which driver to load), where do ->devs fit in?
> > well, i'm not sure which identifier (separator) letter in which style
> > should be used. something like
> > pnp:idXXXxxxxd0XXXxxxxd1XXXXxxxx
>
> leave it as is, it is just fine actually. Assuming you really need
> those extra IDs at all.
>
> What application do you have in mind? I aim at hotplug, i.e.
> (isa|bios)pnp after getting list of devices would call hotplug to load
> drivers. So i need the same logic as in driver matching and this
> does not need extra IDs.
yes, sure, the alias stuff should reflect to the driver's probing logic.
well, in the case of hotplug, the device ids aren't meaningful,
because (so far) there is no confliction between drivers with the same
card id. i mean, in theory, it's possible that the driver A supports
only ids X:Y while the driver B X:Z (where X is the card id, Y and Z
are device ids).
--
Takashi Iwai <[email protected]> ALSA Developer - http://www.alsa-project.org
In message <[email protected]> you write:
> but still, file2alias (as of test9) doesn't output the entries for pnp
> devices...
Patch welcome. 8)
Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
In message <[email protected]> you write:
>
> Oh, BTW, it reminds me - file2alias prints hex in upper
> case while both sysfs and hotplug present them in lower case
> (for sure for USB and PCI, and for PNP entries detected by
> PNP BIOS). Should not we unify representation?
Sure. I made it up. Patch welcome.
Andrey Borzenkov for file2alias maintainer!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
In message <[email protected]> you write:
> --Multipart_Mon_Nov_17_15:05:36_2003-1
> Content-Type: text/plain; charset=US-ASCII
>
> At Mon, 17 Nov 2003 14:34:08 +0100,
> I wrote:
> >
> > at first i'll try to add the support of old isapnp format for
> > compatibility, so that old programs can work as they are.
>
> done.
>
> Rusty, could you check whether it's ok?
Hmm, I had to modify it a little. You have to be careful for 32-bit
and 64-bit kernels, and I added a couple of tests to the testsuite. I
have uploaded -pre4 with these changes in it.
Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
At Thu, 20 Nov 2003 15:35:28 +1100,
Rusty Russell wrote:
>
> In message <[email protected]> you write:
> > --Multipart_Mon_Nov_17_15:05:36_2003-1
> > Content-Type: text/plain; charset=US-ASCII
> >
> > At Mon, 17 Nov 2003 14:34:08 +0100,
> > I wrote:
> > >
> > > at first i'll try to add the support of old isapnp format for
> > > compatibility, so that old programs can work as they are.
> >
> > done.
> >
> > Rusty, could you check whether it's ok?
>
> Hmm, I had to modify it a little. You have to be careful for 32-bit
> and 64-bit kernels, and I added a couple of tests to the testsuite. I
> have uploaded -pre4 with these changes in it.
thanks, i wasn't sure also about the 64bit cleanness.
now we need to modify file2alias... once when the format is defined,
the implementation should be quite easy.
Takashi
On Mon, Nov 17, 2003 at 04:37:50PM +0100, Takashi Iwai wrote:
> At Mon, 17 Nov 2003 18:07:04 +0300,
> Andrey Borzenkov wrote:
-->snip
> > > the file2alias format of (isa) pnp devices will need variable number
> > > of items, since a driver may require multiple ids.
> > > for example, snd-cs4236 driver supports the cards with three ids like
> > > CSCe825:CSC0100:CSC0110
> > > and four ids like
> > > CSCd937:CSC0000:CSC0010:CSC0003
> > > in each case, a matching card must include all ids listed there.
> > >
> >
> > do you mean that card will have to have all of these IDs to match?
> >
> > I can't get it reading sources. When driver matches card against
> > card driver it is apparently using only main IDs, not logical
> > device IDs:
> >
> > driver/pnp/card.c:match_card()
> >
> > static const struct pnp_card_device_id * match_card(struct pnp_card_driver * drv, struct pnp_card * card)
> > {
> > const struct pnp_card_device_id * drv_id = drv->id_table;
> > while (*drv_id->id){
> > if (compare_pnp_id(card->id,drv_id->id))
> > return drv_id;
> > drv_id++;
> > }
> > return NULL;
> > }
> >
> > where are drv_id->devs used?
>
> hmm, i thought it checks the device ids but apparently it's not.
> IMO, this is a bug, because there are cards with the same card id but
> different device ids. (e.g. sound/isa/cs423x/cs4236.c)
> in the logic above, only the first matching entry is checked and it
> results in the failure of probing.
>
> Adam, what do you think?
>
The device ID is checked, but this checking occurs during the driver's
probe function, when it calls pnp_request_card_device. This is needed
in order for us to properly deal with multidevice cards, especially in
ALSA. If possibly, I'd like to see the devices in these cards be
handled individually in 2.7 but note that doing so would require
changes to some drivers and subsystems. The current system works well
for 2.6.
Because of these factors, and the fact that pnp_device_id is also used,
I think that we have to include all of the IDs in the pnpidmap. This
would include both the card id and the individual device IDs on each
card. pnp_device_id can use the isapnp card device ids in addition to
the ids reported by the pnpbios.
Thanks,
Adam
At Thu, 20 Nov 2003 21:23:20 +0000,
Adam Belay wrote:
>
> On Mon, Nov 17, 2003 at 04:37:50PM +0100, Takashi Iwai wrote:
> > At Mon, 17 Nov 2003 18:07:04 +0300,
> > Andrey Borzenkov wrote:
>
> -->snip
>
> > > > the file2alias format of (isa) pnp devices will need variable number
> > > > of items, since a driver may require multiple ids.
> > > > for example, snd-cs4236 driver supports the cards with three ids like
> > > > CSCe825:CSC0100:CSC0110
> > > > and four ids like
> > > > CSCd937:CSC0000:CSC0010:CSC0003
> > > > in each case, a matching card must include all ids listed there.
> > > >
> > >
> > > do you mean that card will have to have all of these IDs to match?
> > >
> > > I can't get it reading sources. When driver matches card against
> > > card driver it is apparently using only main IDs, not logical
> > > device IDs:
> > >
> > > driver/pnp/card.c:match_card()
> > >
> > > static const struct pnp_card_device_id * match_card(struct pnp_card_driver * drv, struct pnp_card * card)
> > > {
> > > const struct pnp_card_device_id * drv_id = drv->id_table;
> > > while (*drv_id->id){
> > > if (compare_pnp_id(card->id,drv_id->id))
> > > return drv_id;
> > > drv_id++;
> > > }
> > > return NULL;
> > > }
> > >
> > > where are drv_id->devs used?
> >
> > hmm, i thought it checks the device ids but apparently it's not.
> > IMO, this is a bug, because there are cards with the same card id but
> > different device ids. (e.g. sound/isa/cs423x/cs4236.c)
> > in the logic above, only the first matching entry is checked and it
> > results in the failure of probing.
> >
> > Adam, what do you think?
> >
>
> The device ID is checked, but this checking occurs during the driver's
> probe function, when it calls pnp_request_card_device. This is needed
> in order for us to properly deal with multidevice cards, especially in
> ALSA.
well, the probe callback of all ALSA isapnp drivers doesn't look for
the matching device ids. that is, the callback trusts that the pnp
core passes the correct pnp_card_device_id, and it checks only the
devices listed on this id. so, as mentioned above, if there are
multiple entries with the same card id but different device ids,
probing the second entry will fail, because match_card() returns the
first matching id.
> If possibly, I'd like to see the devices in these cards be
> handled individually in 2.7 but note that doing so would require
> changes to some drivers and subsystems. The current system works well
> for 2.6.
>
> Because of these factors, and the fact that pnp_device_id is also used,
> I think that we have to include all of the IDs in the pnpidmap. This
> would include both the card id and the individual device IDs on each
> card. pnp_device_id can use the isapnp card device ids in addition to
> the ids reported by the pnpbios.
oh, can pnp_device_id be a card id, too, not only a device id?
Takashi
On Fri, Nov 21, 2003 at 12:44:45PM +0100, Takashi Iwai wrote:
> At Thu, 20 Nov 2003 21:23:20 +0000,
> Adam Belay wrote:
--> snip
> > The device ID is checked, but this checking occurs during the driver's
> > probe function, when it calls pnp_request_card_device. This is needed
> > in order for us to properly deal with multidevice cards, especially in
> > ALSA.
>
> well, the probe callback of all ALSA isapnp drivers doesn't look for
> the matching device ids. that is, the callback trusts that the pnp
> core passes the correct pnp_card_device_id, and it checks only the
> devices listed on this id. so, as mentioned above, if there are
> multiple entries with the same card id but different device ids,
> probing the second entry will fail, because match_card() returns the
> first matching id.
Hmm, I agree, if there are multiple entries with the same card ID then
only the first ID would be matched. I could add some code to match_card()
so that it checks to see if all of the devices in the list are present.
This would, however, break ALSA's current policy of enabling a card, even
if some of its device components are unavailabe. Could you provide any
specific examples of multiple entries with the same card ID? I'd like to
look at some closer.
>
> > If possibly, I'd like to see the devices in these cards be
> > handled individually in 2.7 but note that doing so would require
> > changes to some drivers and subsystems. The current system works well
> > for 2.6.
> >
> > Because of these factors, and the fact that pnp_device_id is also used,
> > I think that we have to include all of the IDs in the pnpidmap. This
> > would include both the card id and the individual device IDs on each
> > card. pnp_device_id can use the isapnp card device ids in addition to
> > the ids reported by the pnpbios.
>
> oh, can pnp_device_id be a card id, too, not only a device id?
Only a device ID, but those device IDs can reside on isapnp cards.
>
>
> Takashi
Thanks,
Adam
At Sun, 23 Nov 2003 22:07:12 +0000,
Adam Belay wrote:
>
> On Fri, Nov 21, 2003 at 12:44:45PM +0100, Takashi Iwai wrote:
> > At Thu, 20 Nov 2003 21:23:20 +0000,
> > Adam Belay wrote:
>
> --> snip
>
> > > The device ID is checked, but this checking occurs during the driver's
> > > probe function, when it calls pnp_request_card_device. This is needed
> > > in order for us to properly deal with multidevice cards, especially in
> > > ALSA.
> >
> > well, the probe callback of all ALSA isapnp drivers doesn't look for
> > the matching device ids. that is, the callback trusts that the pnp
> > core passes the correct pnp_card_device_id, and it checks only the
> > devices listed on this id. so, as mentioned above, if there are
> > multiple entries with the same card id but different device ids,
> > probing the second entry will fail, because match_card() returns the
> > first matching id.
>
> Hmm, I agree, if there are multiple entries with the same card ID then
> only the first ID would be matched. I could add some code to match_card()
> so that it checks to see if all of the devices in the list are present.
> This would, however, break ALSA's current policy of enabling a card, even
> if some of its device components are unavailabe.
this should be ok. we already have multiple definitions for such a
case.
> Could you provide any
> specific examples of multiple entries with the same card ID? I'd like to
> look at some closer.
sound/isa/cs423x/cs4236.c includes the following ids:
static struct pnp_card_device_id snd_cs423x_pnpids[] = {
/* Intel Marlin Spike Motherboard - CS4235 */
{ .id = "CSC0225", .devs = { { "CSC0000" }, { "CSC0010" }, { "CSC0003" } } },
/* Intel Marlin Spike Motherboard (#2) - CS4235 */
{ .id = "CSC0225", .devs = { { "CSC0100" }, { "CSC0110" }, { "CSC0103" } } },
also within the same list:
/* Dell Optiplex GX1 - CS4236B */
{ .id = "CSC6835", .devs = { { "CSC0000" }, { "CSC0010" }, { "CSC0003" } } },
/* Dell P410 motherboard - CS4236B */
{ .id = "CSC6835", .devs = { { "CSC0000" }, { "CSC0010" } } },
the latter is fallback for the board without MPU401.
> > > If possibly, I'd like to see the devices in these cards be
> > > handled individually in 2.7 but note that doing so would require
> > > changes to some drivers and subsystems. The current system works well
> > > for 2.6.
> > >
> > > Because of these factors, and the fact that pnp_device_id is also used,
> > > I think that we have to include all of the IDs in the pnpidmap. This
> > > would include both the card id and the individual device IDs on each
> > > card. pnp_device_id can use the isapnp card device ids in addition to
> > > the ids reported by the pnpbios.
> >
> > oh, can pnp_device_id be a card id, too, not only a device id?
>
> Only a device ID, but those device IDs can reside on isapnp cards.
ok, that what i expected.
--
Takashi Iwai <[email protected]> ALSA Developer - http://www.alsa-project.org
--- linux-2.6.0-test10/scripts/file2alias.c-dist 2003-11-26 17:10:34.000000000 +0100
+++ linux-2.6.0-test10/scripts/file2alias.c 2003-11-26 17:33:54.000000000 +0100
@@ -169,6 +169,28 @@
return 1;
}
+/* looks like: "pnp:dD" */
+static int do_pnp_entry(const char *filename,
+ struct pnp_device_id *id, char *alias)
+{
+ sprintf(alias, "pnp:d%s", id->id);
+ return 1;
+}
+
+/* looks like: "pnp:cCdD..." */
+static int do_pnp_card_entry(const char *filename,
+ struct pnp_card_device_id *id, char *alias)
+{
+ int i;
+ sprintf(alias, "pnp:c%s", id->id);
+ for (i = 0; i < PNP_MAX_DEVICES; i++) {
+ if (! *id->devs[i].id)
+ break;
+ sprintf(alias + strlen(alias), "d%s", id->devs[i].id);
+ }
+ return 1;
+}
+
/* Ignore any prefix, eg. v850 prepends _ */
static inline int sym_is(const char *symbol, const char *name)
{
@@ -235,6 +257,12 @@
else if (sym_is(symname, "__mod_ccw_device_table"))
do_table(symval, sym->st_size, sizeof(struct ccw_device_id),
do_ccw_entry, mod);
+ else if (sym_is(symname, "__mod_pnp_device_table"))
+ do_table(symval, sym->st_size, sizeof(struct pnp_device_id),
+ do_pnp_entry, mod);
+ else if (sym_is(symname, "__mod_pnp_card_device_table"))
+ do_table(symval, sym->st_size, sizeof(struct pnp_card_device_id),
+ do_pnp_card_entry, mod);
}
/* Now add out buffered information to the generated C source */
--- linux-2.6.0-test10/include/linux/pnp.h-dist 2003-11-26 17:23:00.000000000 +0100
+++ linux-2.6.0-test10/include/linux/pnp.h 2003-11-26 17:27:35.000000000 +0100
@@ -12,13 +12,12 @@
#include <linux/device.h>
#include <linux/list.h>
#include <linux/errno.h>
+#include <linux/mod_devicetable.h>
#define PNP_MAX_PORT 8
#define PNP_MAX_MEM 4
#define PNP_MAX_IRQ 2
#define PNP_MAX_DMA 2
-#define PNP_MAX_DEVICES 8
-#define PNP_ID_LEN 8
#define PNP_NAME_LEN 50
struct pnp_protocol;
@@ -279,19 +278,6 @@
struct pnp_id * next;
};
-struct pnp_device_id {
- char id[PNP_ID_LEN];
- unsigned long driver_data; /* data private to the driver */
-};
-
-struct pnp_card_device_id {
- char id[PNP_ID_LEN];
- unsigned long driver_data; /* data private to the driver */
- struct {
- char id[PNP_ID_LEN];
- } devs[PNP_MAX_DEVICES]; /* logical devices */
-};
-
struct pnp_driver {
char * name;
const struct pnp_device_id *id_table;
--- linux-2.6.0-test10/include/linux/mod_devicetable.h-dist 2003-11-26 17:22:57.000000000 +0100
+++ linux-2.6.0-test10/include/linux/mod_devicetable.h 2003-11-26 17:27:34.000000000 +0100
@@ -148,4 +148,20 @@
#define CCW_DEVICE_ID_MATCH_DEVICE_MODEL 0x08
+#define PNP_ID_LEN 8
+#define PNP_MAX_DEVICES 8
+
+struct pnp_device_id {
+ __u8 id[PNP_ID_LEN];
+ kernel_ulong_t driver_data; /* data private to the driver */
+};
+
+struct pnp_card_device_id {
+ __u8 id[PNP_ID_LEN];
+ kernel_ulong_t driver_data; /* data private to the driver */
+ struct {
+ __u8 id[PNP_ID_LEN];
+ } devs[PNP_MAX_DEVICES]; /* logical devices */
+};
+
#endif /* LINUX_MOD_DEVICETABLE_H */
On Thursday 27 November 2003 17:41, Takashi Iwai wrote:
> Hi,
>
> the attached is the patch to add pnp entries to file2alias.c.
> i moved the definitions of pnp_device_id and pnp_card_device_id into
> mod_devicetable.h as other devices do. if you don't like it, i'll try
> to revert them and put definitions inside file2alias.c to keep the
> changes minimum.
>
> the format of pnp alias is:
> pnp:dXXXYYYY
> or
> pnp:cXXXYYYYdXXXYYYY[dXXXYYYY...]
> where XXXYYYY is the pnp id with 7 letters (e.g. CTL0031), c shows the
> card id, and d means the device id. multiple device ids will be
> listed depending on the driver.
>
> for example,
>
> alias pnp:dYMH0021* opl3sa2
> alias pnp:cALS0001d@@@0001d@X@0001d@H@0001* snd_als100
>
> Andrey, would it be feasible for hotplug stuff?
>
Will then every d be passed as separate parameter to hotplug? It means agent
has to deal with unknown number of parameters or are there always fixed
number of devs? apparently not as max is 8 and in your example only 3 are
defined.
If number is variable I guess better would be
alias pnp:cXXXXXXdYYYYYYY[:YYYYYY...]
i.e. put all devs IDs in one field; actually may be even separator is
redundant as IDs have strict format to my knowledge.
Then hotplug agent gets two parameters - PNPID and PNPDEVS - and it is quite
easy to build alias.
can you give example how entries in sysfs look like (I do not have any ISA
card). Is it possible to list them in the same order as in pnp_card_device_id
table? Otherwise coldplugging becomes quite complicated. coldplugging script
has to build the same string as hotplug invocation gets.
Oh, BTW, what are those `@' in your example? If they mean single char
wildcard, you should build them using `?' because modprobe is using normal
fnmatch to match module name against aliases.
thank you
-andrey
At Thu, 27 Nov 2003 21:58:58 +0300,
Andrey Borzenkov wrote:
>
> On Thursday 27 November 2003 17:41, Takashi Iwai wrote:
> > Hi,
> >
> > the attached is the patch to add pnp entries to file2alias.c.
> > i moved the definitions of pnp_device_id and pnp_card_device_id into
> > mod_devicetable.h as other devices do. if you don't like it, i'll try
> > to revert them and put definitions inside file2alias.c to keep the
> > changes minimum.
> >
> > the format of pnp alias is:
> > pnp:dXXXYYYY
> > or
> > pnp:cXXXYYYYdXXXYYYY[dXXXYYYY...]
> > where XXXYYYY is the pnp id with 7 letters (e.g. CTL0031), c shows the
> > card id, and d means the device id. multiple device ids will be
> > listed depending on the driver.
> >
> > for example,
> >
> > alias pnp:dYMH0021* opl3sa2
> > alias pnp:cALS0001d@@@0001d@X@0001d@H@0001* snd_als100
> >
> > Andrey, would it be feasible for hotplug stuff?
> >
>
> Will then every d be passed as separate parameter to hotplug? It means agent
> has to deal with unknown number of parameters or are there always fixed
> number of devs? apparently not as max is 8 and in your example only 3 are
> defined.
yes. the number of probed devices is variable.
note that, as seen above, there are two cases: with a card id and
without a card id. in the latter case, the card id is not checked but
only the given device id is checked, while the former case will need
checks all ids.
>
> If number is variable I guess better would be
>
> alias pnp:cXXXXXXdYYYYYYY[:YYYYYY...]
>
> i.e. put all devs IDs in one field; actually may be even separator is
> redundant as IDs have strict format to my knowledge.
it looks fine to me.
> Then hotplug agent gets two parameters - PNPID and PNPDEVS - and it is quite
> easy to build alias.
>
> can you give example how entries in sysfs look like (I do not have any ISA
> card). Is it possible to list them in the same order as in pnp_card_device_id
> table? Otherwise coldplugging becomes quite complicated. coldplugging script
> has to build the same string as hotplug invocation gets.
% ls -RF /sys/bus/pnp/
/sys/bus/pnp/:
devices/ drivers/
/sys/bus/pnp/devices:
00:01.00@ 00:01.01@ 00:01.02@ 00:01.03@ 00:01.04@ 00:01.05@
...
% ls -RF /sys/devices/pnp0/
/sys/devices/pnp0/:
00:01/ detach_state power/
/sys/devices/pnp0/00:01:
00:01.00/ 00:01.02/ 00:01.04/ detach_state
00:01.01/ 00:01.03/ 00:01.05/ power/
/sys/devices/pnp0/00:01/00:01.00:
detach_state id options power/ resources
...
...hmm, i don't see the card id on sysfs.
i think it's not implemented yet!
Adam, any plan to give the card id to sysfs ?
> Oh, BTW, what are those `@' in your example? If they mean single char
> wildcard, you should build them using `?' because modprobe is using normal
> fnmatch to match module name against aliases.
@ is a valid latter as the ISA pnp id string. it's not a wildcard.
(that's one of the reasons i showed it :)
the id consists of three letters [A-Z@] and four hex numbers
[0-9a-f].
one exception is 'XXXX' used as numbers, which means that any numbers
match. for example, CTLXXXX matches CTL0305, CTLa12f, etc.
ciao,
Takashi