2003-09-09 19:50:05

by Russell King

[permalink] [raw]
Subject: Buggy PCI drivers - do not mark pci_device_id as discardable data

Guys,

All these drivers are buggy wrt PCI hotplugging/Cardbus/PCI new_id
support and are all potential sources of oops dumps. You don't need
to have hotplugging enabled to cause your machine to oops with any
of these drivers loaded.

The authors of future drivers doing this will be sent home to write
1000 lines, using pen and paper, of: "I will not use __initdata with
pci_device_id tables." Printed copies, trick photography, and emails
will not be accepted.

(I'm sending Linus a separate patch since I've already sent him the
AGP fixes.)

--- orig/drivers/char/agp/ati-agp.c Mon Sep 8 23:36:52 2003
+++ linux/drivers/char/agp/ati-agp.c Tue Sep 9 20:11:19 2003
@@ -491,7 +491,7 @@
agp_put_bridge(bridge);
}

-static struct pci_device_id agp_ati_pci_table[] __initdata = {
+static struct pci_device_id agp_ati_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
.class_mask = ~0,
--- orig/drivers/char/agp/sis-agp.c Mon Sep 8 23:36:53 2003
+++ linux/drivers/char/agp/sis-agp.c Tue Sep 9 20:12:38 2003
@@ -215,7 +215,7 @@
agp_put_bridge(bridge);
}

-static struct pci_device_id agp_sis_pci_table[] __initdata = {
+static struct pci_device_id agp_sis_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
.class_mask = ~0,
--- orig/drivers/char/agp/uninorth-agp.c Thu Sep 4 16:36:58 2003
+++ linux/drivers/char/agp/uninorth-agp.c Tue Sep 9 20:12:38 2003
@@ -350,7 +350,7 @@
agp_put_bridge(bridge);
}

-static struct pci_device_id agp_uninorth_pci_table[] __initdata = {
+static struct pci_device_id agp_uninorth_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
.class_mask = ~0,
--- orig/drivers/char/agp/via-agp.c Mon Sep 8 23:36:53 2003
+++ linux/drivers/char/agp/via-agp.c Tue Sep 9 20:12:38 2003
@@ -432,7 +432,7 @@
agp_put_bridge(bridge);
}

-static struct pci_device_id agp_via_pci_table[] __initdata = {
+static struct pci_device_id agp_via_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
.class_mask = ~0,
--- orig/drivers/char/watchdog/alim1535_wdt.c Mon Sep 8 23:37:03 2003
+++ linux/drivers/char/watchdog/alim1535_wdt.c Tue Sep 9 20:45:16 2003
@@ -317,7 +317,7 @@
* want to register another driver on the same PCI id.
*/

-static struct pci_device_id ali_pci_tbl[] __initdata = {
+static struct pci_device_id ali_pci_tbl[] = {
{ PCI_VENDOR_ID_AL, 1535, PCI_ANY_ID, PCI_ANY_ID,},
{ 0, },
};
--- orig/drivers/char/watchdog/amd7xx_tco.c Sat Jun 14 22:33:48 2003
+++ linux/drivers/char/watchdog/amd7xx_tco.c Tue Sep 9 20:45:16 2003
@@ -294,7 +294,7 @@
.fops = &amdtco_fops
};

-static struct pci_device_id amdtco_pci_tbl[] __initdata = {
+static struct pci_device_id amdtco_pci_tbl[] = {
/* AMD 766 PCI_IDs here */
{ PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_OPUS_7443, PCI_ANY_ID, PCI_ANY_ID, },
{ 0, }
--- orig/drivers/char/watchdog/i810-tco.c Sun Aug 3 11:21:11 2003
+++ linux/drivers/char/watchdog/i810-tco.c Tue Sep 9 20:45:16 2003
@@ -301,7 +301,7 @@
* register a pci_driver, because someone else might one day
* want to register another driver on the same PCI id.
*/
-static struct pci_device_id i810tco_pci_tbl[] __initdata = {
+static struct pci_device_id i810tco_pci_tbl[] = {
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, PCI_ANY_ID, PCI_ANY_ID, },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, PCI_ANY_ID, PCI_ANY_ID, },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0, PCI_ANY_ID, PCI_ANY_ID, },
--- orig/drivers/char/hw_random.c Sat Jun 14 22:33:46 2003
+++ linux/drivers/char/hw_random.c Tue Sep 9 20:45:16 2003
@@ -149,7 +149,7 @@
* register a pci_driver, because someone else might one day
* want to register another driver on the same PCI id.
*/
-static struct pci_device_id rng_pci_tbl[] __initdata = {
+static struct pci_device_id rng_pci_tbl[] = {
{ 0x1022, 0x7443, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
{ 0x1022, 0x746b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },

--- orig/drivers/isdn/hisax/config.c Sun Aug 3 11:21:16 2003
+++ linux/drivers/isdn/hisax/config.c Tue Sep 9 20:45:16 2003
@@ -2113,7 +2113,7 @@

#include <linux/pci.h>

-static struct pci_device_id hisax_pci_tbl[] __initdata = {
+static struct pci_device_id hisax_pci_tbl[] = {
#ifdef CONFIG_HISAX_FRITZPCI
{PCI_VENDOR_ID_AVM, PCI_DEVICE_ID_AVM_A1, PCI_ANY_ID, PCI_ANY_ID},
#endif
--- orig/drivers/isdn/hysdn/hysdn_init.c Sat Jun 14 22:33:52 2003
+++ linux/drivers/isdn/hysdn/hysdn_init.c Tue Sep 9 20:45:16 2003
@@ -21,7 +21,7 @@

#include "hysdn_defs.h"

-static struct pci_device_id hysdn_pci_tbl[] __initdata = {
+static struct pci_device_id hysdn_pci_tbl[] = {
{PCI_VENDOR_ID_HYPERCOPE, PCI_DEVICE_ID_HYPERCOPE_PLX, PCI_ANY_ID, PCI_SUBDEVICE_ID_HYPERCOPE_METRO},
{PCI_VENDOR_ID_HYPERCOPE, PCI_DEVICE_ID_HYPERCOPE_PLX, PCI_ANY_ID, PCI_SUBDEVICE_ID_HYPERCOPE_CHAMP2},
{PCI_VENDOR_ID_HYPERCOPE, PCI_DEVICE_ID_HYPERCOPE_PLX, PCI_ANY_ID, PCI_SUBDEVICE_ID_HYPERCOPE_ERGO},
--- orig/drivers/net/fc/iph5526.c Thu Sep 4 16:37:13 2003
+++ linux/drivers/net/fc/iph5526.c Tue Sep 9 20:45:17 2003
@@ -110,7 +110,7 @@
#define ALIGNED_ADDR(addr, len) ((((unsigned long)(addr) + (len - 1)) & ~(len - 1)) - (unsigned long)(addr))


-static struct pci_device_id iph5526_pci_tbl[] __initdata = {
+static struct pci_device_id iph5526_pci_tbl[] = {
{ PCI_VENDOR_ID_INTERPHASE, PCI_DEVICE_ID_INTERPHASE_5526, PCI_ANY_ID, PCI_ANY_ID, },
{ PCI_VENDOR_ID_INTERPHASE, PCI_DEVICE_ID_INTERPHASE_55x6, PCI_ANY_ID, PCI_ANY_ID, },
{ } /* Terminating entry */
--- orig/drivers/net/irda/via-ircc.c Mon Sep 8 23:37:31 2003
+++ linux/drivers/net/irda/via-ircc.c Tue Sep 9 20:45:17 2003
@@ -121,7 +121,7 @@
}
}

-static struct pci_device_id via_pci_tbl[] __initdata = {
+static struct pci_device_id via_pci_tbl[] = {
{ PCI_VENDOR_ID_VIA, DeviceID1, PCI_ANY_ID, PCI_ANY_ID,0,0,0 },
{ PCI_VENDOR_ID_VIA, DeviceID2, PCI_ANY_ID, PCI_ANY_ID,0,0,1 },
{ PCI_VENDOR_ID_VIA, DeviceID3, PCI_ANY_ID, PCI_ANY_ID,0,0,2 },
--- orig/drivers/net/skfp/skfddi.c Mon Sep 8 23:37:35 2003
+++ linux/drivers/net/skfp/skfddi.c Tue Sep 9 20:45:17 2003
@@ -182,7 +182,7 @@
extern void enable_tx_irq(struct s_smc *smc, u_short queue);
extern void mac_drv_clear_txd(struct s_smc *smc);

-static struct pci_device_id skfddi_pci_tbl[] __initdata = {
+static struct pci_device_id skfddi_pci_tbl[] = {
{ PCI_VENDOR_ID_SK, PCI_DEVICE_ID_SK_FP, PCI_ANY_ID, PCI_ANY_ID, },
{ } /* Terminating entry */
};
--- orig/drivers/net/wan/sdladrv.c Wed Aug 13 10:33:18 2003
+++ linux/drivers/net/wan/sdladrv.c Tue Sep 9 20:45:17 2003
@@ -201,7 +201,7 @@
* Note: All data must be explicitly initialized!!!
*/

-static struct pci_device_id sdladrv_pci_tbl[] __initdata = {
+static struct pci_device_id sdladrv_pci_tbl[] = {
{ V3_VENDOR_ID, V3_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, },
{ } /* Terminating entry */
};
--- orig/drivers/net/dgrs.c Mon Sep 8 23:37:29 2003
+++ linux/drivers/net/dgrs.c Tue Sep 9 20:45:16 2003
@@ -120,7 +120,7 @@
#include "dgrs_asstruct.h"
#include "dgrs_bcomm.h"

-static struct pci_device_id dgrs_pci_tbl[] __initdata = {
+static struct pci_device_id dgrs_pci_tbl[] = {
{ SE6_PCI_VENDOR_ID, SE6_PCI_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, },
{ } /* Terminating entry */
};
--- orig/drivers/net/hp100.c Thu Sep 4 16:37:16 2003
+++ linux/drivers/net/hp100.c Tue Sep 9 20:45:16 2003
@@ -284,7 +284,7 @@

#define HP100_PCI_IDS_SIZE (sizeof(hp100_pci_ids)/sizeof(struct hp100_pci_id))

-static struct pci_device_id hp100_pci_tbl[] __initdata = {
+static struct pci_device_id hp100_pci_tbl[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585A, PCI_ANY_ID, PCI_ANY_ID,},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585B, PCI_ANY_ID, PCI_ANY_ID,},
{PCI_VENDOR_ID_COMPEX, PCI_DEVICE_ID_COMPEX_ENET100VG4, PCI_ANY_ID, PCI_ANY_ID,},
--- orig/drivers/net/sunhme.c Mon Sep 8 23:37:36 2003
+++ linux/drivers/net/sunhme.c Tue Sep 9 20:45:17 2003
@@ -179,7 +179,7 @@
where it could be referenced at any time due to hot plugging,
the __initdata reference should be removed. */

-struct pci_device_id happymeal_pci_ids[] __initdata = {
+struct pci_device_id happymeal_pci_ids[] = {
{
.vendor = PCI_VENDOR_ID_SUN,
.device = PCI_DEVICE_ID_SUN_HAPPYMEAL,
--- orig/drivers/video/i810/i810_main.c Wed Aug 13 10:33:34 2003
+++ linux/drivers/video/i810/i810_main.c Tue Sep 9 20:45:16 2003
@@ -66,7 +66,7 @@
"Intel(R) 815 (Internal Graphics with AGP) Framebuffer Device"
};

-static struct pci_device_id i810fb_pci_tbl[] __initdata = {
+static struct pci_device_id i810fb_pci_tbl[] = {
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82810_IG1,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82810_IG3,

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core


2003-09-09 20:04:18

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, 9 Sep 2003, Russell King wrote:

> --- orig/drivers/char/watchdog/amd7xx_tco.c Sat Jun 14 22:33:48 2003
> +++ linux/drivers/char/watchdog/amd7xx_tco.c Tue Sep 9 20:45:16 2003
> @@ -294,7 +294,7 @@
> .fops = &amdtco_fops
> };
>
> -static struct pci_device_id amdtco_pci_tbl[] __initdata = {
> +static struct pci_device_id amdtco_pci_tbl[] = {
> /* AMD 766 PCI_IDs here */
> { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_OPUS_7443, PCI_ANY_ID, PCI_ANY_ID, },
> { 0, }

That's not a bug.

> --- orig/drivers/char/watchdog/i810-tco.c Sun Aug 3 11:21:11 2003
> +++ linux/drivers/char/watchdog/i810-tco.c Tue Sep 9 20:45:16 2003
> @@ -301,7 +301,7 @@
> * register a pci_driver, because someone else might one day
> * want to register another driver on the same PCI id.
> */
> -static struct pci_device_id i810tco_pci_tbl[] __initdata = {
> +static struct pci_device_id i810tco_pci_tbl[] = {
> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, PCI_ANY_ID, PCI_ANY_ID, },
> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, PCI_ANY_ID, PCI_ANY_ID, },
> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0, PCI_ANY_ID, PCI_ANY_ID, },

Neither is that.

> --- orig/drivers/char/hw_random.c Sat Jun 14 22:33:46 2003
> +++ linux/drivers/char/hw_random.c Tue Sep 9 20:45:16 2003
> @@ -149,7 +149,7 @@
> * register a pci_driver, because someone else might one day
> * want to register another driver on the same PCI id.
> */
> -static struct pci_device_id rng_pci_tbl[] __initdata = {
> +static struct pci_device_id rng_pci_tbl[] = {
> { 0x1022, 0x7443, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
> { 0x1022, 0x746b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },

This too

2003-09-09 20:16:04

by Dave Jones

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, Sep 09, 2003 at 08:48:03PM +0100, Russell King wrote:

> -static struct pci_device_id agp_ati_pci_table[] __initdata = {
> +static struct pci_device_id agp_ati_pci_table[] = {

Wierd. I could swear akpm had these patches in his tree no so long back.

Dave

--
Dave Jones http://www.codemonkey.org.uk

2003-09-09 20:58:05

by Russell King

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, Sep 09, 2003 at 09:14:37PM +0100, Dave Jones wrote:
> On Tue, Sep 09, 2003 at 08:48:03PM +0100, Russell King wrote:
> > -static struct pci_device_id agp_ati_pci_table[] __initdata = {
> > +static struct pci_device_id agp_ati_pci_table[] = {
>
> Wierd. I could swear akpm had these patches in his tree no so long back.

So did I, so I checked the history in bk, and it seems it never made Linus'
tree.

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2003-09-09 21:05:07

by Russell King

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, Sep 09, 2003 at 04:02:58PM -0400, Zwane Mwaikambo wrote:
> On Tue, 9 Sep 2003, Russell King wrote:
>
> > --- orig/drivers/char/watchdog/amd7xx_tco.c Sat Jun 14 22:33:48 2003
> > +++ linux/drivers/char/watchdog/amd7xx_tco.c Tue Sep 9 20:45:16 2003
> > @@ -294,7 +294,7 @@
> > .fops = &amdtco_fops
> > };
> >
> > -static struct pci_device_id amdtco_pci_tbl[] __initdata = {
> > +static struct pci_device_id amdtco_pci_tbl[] = {
> > /* AMD 766 PCI_IDs here */
> > { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_OPUS_7443, PCI_ANY_ID, PCI_ANY_ID, },
> > { 0, }
>
> That's not a bug.
>
> > --- orig/drivers/char/watchdog/i810-tco.c Sun Aug 3 11:21:11 2003
> > +++ linux/drivers/char/watchdog/i810-tco.c Tue Sep 9 20:45:16 2003
> > @@ -301,7 +301,7 @@
> > * register a pci_driver, because someone else might one day
> > * want to register another driver on the same PCI id.
> > */
> > -static struct pci_device_id i810tco_pci_tbl[] __initdata = {
> > +static struct pci_device_id i810tco_pci_tbl[] = {
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, PCI_ANY_ID, PCI_ANY_ID, },
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, PCI_ANY_ID, PCI_ANY_ID, },
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0, PCI_ANY_ID, PCI_ANY_ID, },
>
> Neither is that.
>
> > --- orig/drivers/char/hw_random.c Sat Jun 14 22:33:46 2003
> > +++ linux/drivers/char/hw_random.c Tue Sep 9 20:45:16 2003
> > @@ -149,7 +149,7 @@
> > * register a pci_driver, because someone else might one day
> > * want to register another driver on the same PCI id.
> > */
> > -static struct pci_device_id rng_pci_tbl[] __initdata = {
> > +static struct pci_device_id rng_pci_tbl[] = {
> > { 0x1022, 0x7443, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
> > { 0x1022, 0x746b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
>
> This too

Ok, I'm happy that this aren't (after reading someone elses explaination).

Having these different makes it hard to ensure that no further bad cases
exist in the tree though.

I want this to be foolproof, because its me people bug when their cardbus
cards oops when they insert the damned things. If people are happy to
ignore this issue, I'm happy to ignore the bug reports.

It basically isn't something I want to deal with, and we need to find a
way to stop these stupidities appearing in the first place.

Any ideas?

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2003-09-09 20:59:35

by Russell King

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, Sep 09, 2003 at 04:02:58PM -0400, Zwane Mwaikambo wrote:
> On Tue, 9 Sep 2003, Russell King wrote:
>
> > --- orig/drivers/char/watchdog/amd7xx_tco.c Sat Jun 14 22:33:48 2003
> > +++ linux/drivers/char/watchdog/amd7xx_tco.c Tue Sep 9 20:45:16 2003
> > @@ -294,7 +294,7 @@
> > .fops = &amdtco_fops
> > };
> >
> > -static struct pci_device_id amdtco_pci_tbl[] __initdata = {
> > +static struct pci_device_id amdtco_pci_tbl[] = {
> > /* AMD 766 PCI_IDs here */
> > { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_OPUS_7443, PCI_ANY_ID, PCI_ANY_ID, },
> > { 0, }
>
> That's not a bug.
>
> > --- orig/drivers/char/watchdog/i810-tco.c Sun Aug 3 11:21:11 2003
> > +++ linux/drivers/char/watchdog/i810-tco.c Tue Sep 9 20:45:16 2003
> > @@ -301,7 +301,7 @@
> > * register a pci_driver, because someone else might one day
> > * want to register another driver on the same PCI id.
> > */
> > -static struct pci_device_id i810tco_pci_tbl[] __initdata = {
> > +static struct pci_device_id i810tco_pci_tbl[] = {
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, PCI_ANY_ID, PCI_ANY_ID, },
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, PCI_ANY_ID, PCI_ANY_ID, },
> > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0, PCI_ANY_ID, PCI_ANY_ID, },
>
> Neither is that.
>
> > --- orig/drivers/char/hw_random.c Sat Jun 14 22:33:46 2003
> > +++ linux/drivers/char/hw_random.c Tue Sep 9 20:45:16 2003
> > @@ -149,7 +149,7 @@
> > * register a pci_driver, because someone else might one day
> > * want to register another driver on the same PCI id.
> > */
> > -static struct pci_device_id rng_pci_tbl[] __initdata = {
> > +static struct pci_device_id rng_pci_tbl[] = {
> > { 0x1022, 0x7443, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
> > { 0x1022, 0x746b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rng_hw_amd },
>
> This too

They're all bugs, plain and simple. The pci device id tables are scanned
*whenever* a new pci device is inserted into the system, or the appropriate
numbers are echoed to the "new_id" driver model entry for the driver.

It doesn't matter if the driver doesn't care or not.

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2003-09-09 21:26:59

by Alan

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Maw, 2003-09-09 at 22:04, Russell King wrote:
> I want this to be foolproof, because its me people bug when their cardbus
> cards oops when they insert the damned things. If people are happy to
> ignore this issue, I'm happy to ignore the bug reports.
>
> It basically isn't something I want to deal with, and we need to find a
> way to stop these stupidities appearing in the first place.
>
> Any ideas?

You've already got symbols for initdata start and end, just check the
pointers in the pci_register code. I guess you want a per platform

BUG_IF_INIT(x)


2003-09-09 21:29:53

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, 9 Sep 2003, Alan Cox wrote:

> On Maw, 2003-09-09 at 22:04, Russell King wrote:
> > I want this to be foolproof, because its me people bug when their cardbus
> > cards oops when they insert the damned things. If people are happy to
> > ignore this issue, I'm happy to ignore the bug reports.
> >
> > It basically isn't something I want to deal with, and we need to find a
> > way to stop these stupidities appearing in the first place.
> >
> > Any ideas?
>
> You've already got symbols for initdata start and end, just check the
> pointers in the pci_register code. I guess you want a per platform
>
> BUG_IF_INIT(x)

I hacked up something similar, but i'm not sure what to do about modules.

2003-09-09 21:34:06

by Russell King

[permalink] [raw]
Subject: Re: Buggy PCI drivers - do not mark pci_device_id as discardable data

On Tue, Sep 09, 2003 at 10:22:59PM +0100, Alan Cox wrote:
> On Maw, 2003-09-09 at 22:04, Russell King wrote:
> > I want this to be foolproof, because its me people bug when their cardbus
> > cards oops when they insert the damned things. If people are happy to
> > ignore this issue, I'm happy to ignore the bug reports.
> >
> > It basically isn't something I want to deal with, and we need to find a
> > way to stop these stupidities appearing in the first place.
> >
> > Any ideas?
>
> You've already got symbols for initdata start and end, just check the
> pointers in the pci_register code. I guess you want a per platform
>
> BUG_IF_INIT(x)

That would work for built-in drivers. We could couple that with an idea
Kai came up with (in private mail) to catch them in modpost. However,
the problem with modpost is that it gets false positives for these
drivers which explicitly want to discard their module device id tables.

As we currently stand, there seem to be only four drivers which want to
discard their driver id tables. Is it really worth adding extra code
to the kernel to try to trap these, or just not mark the device id
tables with __initdata or __devinitdata and detect the bad guys with
a grep?

--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core