2000-11-25 00:07:58

by Adam J. Richter

[permalink] [raw]
Subject: Patch(?): isapnp_card_id tables for all isapnp drivers in 2.4.0-test11

I have made added isapnp_card_id table to all isapnp drivers.
This is the isapnp equivalent of the pci MODULE_DEVICE_TABLE declarations.
They allow a user level software agent to know which modules correspond
to your hardware configuration and to load them. One such implementation
(GPL'ed) is available from
ftp://ftp.yggdrasil.com/pub/dist/device_control/isapnpmodules/.

There previously were no isapnp_card_id tables in the kernel
drivers. I believe this patch adds isapnp_card_id tables to all of
them, completing the coverage of Keith Owens's
/lib/modules/<version>/modules.{pci,usb,isapnp}map files. I have
attached a patch below that covers just the files that have the
isapnp changes. Note that it includes a couple of pci_device_id
table declarations that happened to flow into the same "diff" sections
as the isapnp_card_id declarations. A complete set of patches
with both pci and isapnp declarations is available from

ftp://ftp.yggdrasil.com/pub/dist/device_control/kernel/pci_id_tables-2.4.0-test11.patch2.gz

Note that this is not a "final" version. I plan to go
through all of the changes and bracket all of these new tables
with #ifdef MODULE...#endif so they do not result in complaints
about the table being defined static and never used in cases where
the driver is compiled directly into the kernel. After 2.4.0, I
imainge most of those #ifdef MODULE conditions will go away
as the tables come to be actually used by the driver code.

Any comments are welcome.

--
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."


Attachments:
(No filename) (1.74 kB)
diffs.isapnp (28.94 kB)
Download all attachments

2000-11-25 02:12:10

by Keith Owens

[permalink] [raw]
Subject: Re: Patch(?): isapnp_card_id tables for all isapnp drivers in 2.4.0-test11

On Fri, 24 Nov 2000 15:37:33 -0800,
"Adam J. Richter" <[email protected]> wrote:
> Note that this is not a "final" version. I plan to go
>through all of the changes and bracket all of these new tables
>with #ifdef MODULE...#endif so they do not result in complaints
>about the table being defined static and never used in cases where
>the driver is compiled directly into the kernel.

This is cleaner. Append MODULE_ONLY after __initdata and remove the
ifdef. It increases the size of initdata in the kernel, compared to
ifdef, but since initdata is promptly reused as scratch space it should
not be a problem.

Index: 0-test11.1/include/linux/module.h
--- 0-test11.1/include/linux/module.h Sun, 12 Nov 2000 14:59:01 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.3 644)
+++ 0-test11.1(w)/include/linux/module.h Sat, 25 Nov 2000 12:40:10 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.3 644)
@@ -261,6 +261,7 @@ extern struct module __this_module;
#define MOD_INC_USE_COUNT __MOD_INC_USE_COUNT(THIS_MODULE)
#define MOD_DEC_USE_COUNT __MOD_DEC_USE_COUNT(THIS_MODULE)
#define MOD_IN_USE __MOD_IN_USE(THIS_MODULE)
+#define MODULE_ONLY __attribute__ ((unused))

#include <linux/version.h>
static const char __module_kernel_version[] __attribute__((section(".modinfo"))) =
@@ -286,6 +287,7 @@ static const char __module_using_checksu
#define MOD_INC_USE_COUNT do { } while (0)
#define MOD_DEC_USE_COUNT do { } while (0)
#define MOD_IN_USE 1
+#define MODULE_ONLY

extern struct module *module_list;


2000-11-25 05:15:37

by Adam J. Richter

[permalink] [raw]
Subject: Re: Patch(?): isapnp_card_id tables for all isapnp drivers in 2.4.0-test11

Keith Owens <[email protected]> wrote:
>"Adam J. Richter" <[email protected]> wrote:
>> Note that this is not a "final" version. I plan to go
>>through all of the changes and bracket all of these new tables
>>with #ifdef MODULE...#endif so they do not result in complaints
>>about the table being defined static and never used in cases where
>>the driver is compiled directly into the kernel.

>This is cleaner. Append MODULE_ONLY after __initdata and remove the
>ifdef. It increases the size of initdata in the kernel, compared to
>ifdef, but since initdata is promptly reused as scratch space it should
>not be a problem.
[patch elided]

Thanks for the patch, but I think I'll stick with the ifdefs
for now, for the following reasons.

1. I think ifdef MODULE is more understandable to the casual observer.
2. There is often some other condition that I need to combine
with (CONFIG_PCI, CONFIG_ISAPNP, CONFIG_ISAPNP_MODULE).
3. There is often an existing ifdef in the right place that I
can just tuck the code into.
4. I would prefer that this change not have even a file size cost
to people who want to build minimal monolithic kernels
for applicance applications.
5. My feeling is that just the few kilobytes of file size cost
associated with #4 and knowing that absolutely nothing is
added for non-module usage will psychologically make
maintainers feel better about it and have even fewer misgivings
about integrating it.
6. We can expect the lines bracketing these table declarations
to be changed in the near future as the drivers are changed
to use the new PCI and isapnp interfaces or to use the ID
tables just to eliminate the old custom data structures that
hold the same information.

Thanks for the patch anyhow, though. It's a clever idea that
may be useful in other situations.

Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."

2000-11-25 16:03:04

by Kai Germaschewski

[permalink] [raw]
Subject: Re: Patch(?): isapnp_card_id tables for all isapnp drivers in 2.4.0-test11

On Sat, 25 Nov 2000, Keith Owens wrote:

> On Fri, 24 Nov 2000 15:37:33 -0800,
> "Adam J. Richter" <[email protected]> wrote:
> > Note that this is not a "final" version. I plan to go
> >through all of the changes and bracket all of these new tables
> >with #ifdef MODULE...#endif so they do not result in complaints
> >about the table being defined static and never used in cases where
> >the driver is compiled directly into the kernel.
>
> This is cleaner. Append MODULE_ONLY after __initdata and remove the
> ifdef. It increases the size of initdata in the kernel, compared to
> ifdef, but since initdata is promptly reused as scratch space it should
> not be a problem.
>
> Index: 0-test11.1/include/linux/module.h
> --- 0-test11.1/include/linux/module.h Sun, 12 Nov 2000 14:59:01 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.3 644)
> +++ 0-test11.1(w)/include/linux/module.h Sat, 25 Nov 2000 12:40:10 +1100 kaos (linux-2.4/W/33_module.h 1.1.2.1.2.1.2.1.2.1.1.3 644)
> @@ -261,6 +261,7 @@ extern struct module __this_module;
> #define MOD_INC_USE_COUNT __MOD_INC_USE_COUNT(THIS_MODULE)
> #define MOD_DEC_USE_COUNT __MOD_DEC_USE_COUNT(THIS_MODULE)
> #define MOD_IN_USE __MOD_IN_USE(THIS_MODULE)
> +#define MODULE_ONLY __attribute__ ((unused))

What about the making MODULE_DEVICE_TABLE reference this table? This has
the same disadvantage (i.e. having a little unneeded __initdata in the
kernel image), but it wouldn't need the rather ugly MODULE_ONLY macro.

I'ld suggest something like this in module.h, #ifndef MODULE part:

#define MODULE_DEVICE_TABLE(type,name) \
static struct type##_device_id *__dummy_##name \
__attribute__ ((unused, __section__(".text.exit"))) \
= name;

This references the table (the variable holding the reference is in
text.exit and thus discarded at link time), maybe there's an easier way to
accomplish this. This of course smells like a hack, but at least it's
hidden away from the drivers ;-)

--Kai


2000-11-26 03:29:10

by Adam J. Richter

[permalink] [raw]
Subject: Re: Patch(?): isapnp_card_id tables for all isapnp drivers in 2.4.0-test11

> == Kai Germaschewski
>> == Keith Owens
>>> == Adam Richter

>>> [...] I plan to go
>>>through all of the changes and bracket all of these new tables
>>>with #ifdef MODULE...#endif so they do not result in complaints
>>>about the table being defined static and never used in cases where
>>>the driver is compiled directly into the kernel.

(I have now done this and release the patch at
ftp://ftp.yggdrasil.com/pub/dist/device_control/kernel/.)


>> This is cleaner. Append MODULE_ONLY after __initdata and remove the
>> ifdef. It increases the size of initdata in the kernel, compared to
>> ifdef, but since initdata is promptly reused as scratch space it should
>> not be a problem.
[...]
>> +#define MODULE_ONLY __attribute__ ((unused))

>What about the making MODULE_DEVICE_TABLE reference this table? This has
>the same disadvantage (i.e. having a little unneeded __initdata in the
>kernel image), but it wouldn't need the rather ugly MODULE_ONLY macro.

>I'ld suggest something like this in module.h, #ifndef MODULE part:

>#define MODULE_DEVICE_TABLE(type,name) \
>static struct type##_device_id *__dummy_##name \
> __attribute__ ((unused, __section__(".text.exit"))) \
> = name;

I did not realize that this thread had been posted to
linux-kernel. Here is a response that I emailed to Keith Owens
and Kai Germaschewski that explains my reasons for sticking with
#ifndef MODULE...#endif rather than creating a new kernel facility
for something that, by the way, should become completely unused in
the next couple of months after 2.4.0 is released and the device
drivers are converted to the new PCI and isapnp interfaces:

|From: "Adam J. Richter" <[email protected]>
|To: [email protected]
|
| Thanks for the patch, but I think I'll stick with the ifdefs
|for now, for the following reasons.
|
| 1. I think ifdef MODULE is more understandable to the casual observer.
| 2. There is often some other condition that I need to combine
| with (CONFIG_PCI, CONFIG_ISAPNP, CONFIG_ISAPNP_MODULE).
| 3. There is often an existing ifdef in the right place that I
| can just tuck the code into.
| 4. I would prefer that this change not have even a file size cost
| to people who want to build minimal monolithic kernels
| for applicance applications.
| 5. My feeling is that just the few kilobytes of file size cost
| associated with #4 and knowing that absolutely nothing is
| added for non-module usage will psychologically make
| maintainers feel better about it and have even fewer misgivings
| about integrating it.
| 6. We can expect the lines bracketing these table declarations
| to be changed in the near future as the drivers are changed
| to use the new PCI and isapnp interfaces or to use the ID
| tables just to eliminate the old custom data structures that
| hold the same information.
|
| Thanks for the patch anyhow, though. It's a clever idea that
|may be useful in other situations.

Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."