Re: sunhme.c patch for new PCI interface (UNTESTED)
Albert D. Cahalan <[EMAIL PROTECTED]> writes: >PCI is certainly hot-plug hardware, but not on common desktop PCs. >Since PCI is so popular and so often not hot-plug, users should >not be forced to have hot-plug PCI support when they only need >hot-plug SCSI, etc. >Obvious hack: __pciinit, __pciexit, __pciinitdata... Yes, as I mentioned in a previous discussion, sometime after 2.4.0, I would like to see CONFIG_HOTPLUG replaced with CONFIG_PCI_HOTPLUG with __pci{init,exit}{,data}, and CONFIG_USB_HOTPLUG with __usb{init,exit}{,data} and likewise for other busses, since these facilities are completely independent, and there are reasons for wanting to compile in one facility compiled in and not the others, and it would make drivers self-document which hotplug facility is the reason why something should be marked as __dev{init,exit}{,data}. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Adam J. Richter writes: > Can I have a hot plug PCI bridge card that connects to > a regular PCI backplane (perhaps as some kind of CardBus docking > station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. PCI is certainly hot-plug hardware, but not on common desktop PCs. Since PCI is so popular and so often not hot-plug, users should not be forced to have hot-plug PCI support when they only need hot-plug SCSI, etc. Obvious hack: __pciinit, __pciexit, __pciinitdata... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Adam J. Richter writes: Can I have a hot plug PCI bridge card that connects to a regular PCI backplane (perhaps as some kind of CardBus docking station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. PCI is certainly hot-plug hardware, but not on common desktop PCs. Since PCI is so popular and so often not hot-plug, users should not be forced to have hot-plug PCI support when they only need hot-plug SCSI, etc. Obvious hack: __pciinit, __pciexit, __pciinitdata... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
I wrote: >[...] the cost of incorrectly >using __initdata when __devinitdata was correct is that the user's >KERNEL WILL CRASH when the notebook is inserted or removed from such a >docking station, even when the kernel is built with CONFIG_HOTPLUG. My statement above, without some missing qualification, is wrong. I should have qualified this statement more carefully. (I'm sure the flames are already in the mail about this.) The kernel will crash in any case if the relevant driver does not support hot plugging.and the notebook is being removed with the PCI driver still loaded. For drivers that do not support hot plugging, we could use __initdata instead of __devinitdata, since they will crash in any case. However, violating the instructions in Documentation/pci.txt ("The ID table array should be marked __devinitdata") in this way will provoke a slew of driver bugs as the over one hundred remaining PCI drivers are converted to the new PCI interface and some authors overlook the need to change the MODULE_DEVICE_TABLE storage class. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
>I am willing to consider adding __devxxx only when other __devxxx >entries already exist. >These conversions to _devxxx are too late in the freeze, and only have >value for isolated cases --which you admit you don't even know exist--. >Linus Rule 1: Don't overdesign. Even ignoring CardBus, there apparently are docking stations that have PCI busses, such as shown in http://www.pangolin.com/LD2000/dock/gateway.htm ("Each supports hot docking, so you can attach or detach your system without turning it off"), and a web search turns up plenty of hits, at least for mobile chipsets apparently designed for this. It looks like this is a pretty standard capability. Even if we were unsure, the more conservative approach would be to use __devinitdata. The only cost of incorrectly using __devinitdata instead of __initdata, would be to make the effected kernel module use ~100 bytes more unswappable kernel memory when CONFIG_HOTPLUG is defined and the module is loaded (28 bytes per entry, including a null entry). On the other side that risk comparison, the cost of incorrectly using __initdata when __devinitdata was correct is that the user's KERNEL WILL CRASH when the notebook is inserted or removed from such a docking station, even when the kernel is built with CONFIG_HOTPLUG. Although I'm not into quoting any programmer like some religious figure, I will say that I think you're misinterpreting the meaning of an admonition like "Don't overdesign." We are not talking about designing some new abstraction or making the code more complex, but rather selecting rather a choice between two words, one of which is the more conservatively crash resistant __devinitdata, the other of which would save 28 bytes in CONFIG_HOTPLUG kernels only, and at the expense of probably causing kernel crashes. >Even if such cases do exist, and this >is a need, it should be addressed some other way. 1. Why? 2. What other way did you have in mind? By the way, although I do not have the PCI standard here, I do see from _PCI System Architecture_, 4th edition, chapter 19, "Configuration Registers", in the "Device ID Register" section that devices "designed around the same third party core logic" are allowed to have the same device ID (and they are even complaint to PCI 2.2 if they have different Subsystem ID values). So, my approach also has the minor advantage that if a vendor wants to ship a hot pluggable version of their PCI card in the future with the same device ID (a likely decision), then there may not need to coordinate a new release of the Linux driver. (This is a really small benefit; the kernel crashes that you want to change my existing patches to produce is the big issue.) 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
"Adam J. Richter" wrote: > Jeff Garzik writes: > >Are you aware of any hotplug sunhme hardware? If no, don't change it to > >__devinit... > > Can I have a hot plug PCI bridge card that connects to > a regular PCI backplane (perhaps as some kind of CardBus docking > station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. I am willing to consider adding __devxxx only when other __devxxx entries already exist. These conversions to _devxxx are too late in the freeze, and only have value for isolated cases --which you admit you don't even know exist--. Linus Rule 1: Don't overdesign. Even if such cases do exist, and this is a need, it should be addressed some other way. Your suggestion bloats drivers needlessly for the majority of cases and I will not be applying any such patches... Jeff -- Jeff Garzik | Building 1024 | The chief enemy of creativity is "good" sense MandrakeSoft| -- Picasso - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Jeff Garzik writes: >Are you aware of any hotplug sunhme hardware? If no, don't change it to >__devinit... Can I have a hot plug PCI bridge card that connects to a regular PCI backplane (perhaps as some kind of CardBus docking station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. The other excellent points that you raise apply equally to the driver before and after my patch (although my patch made it more clear that the struct netdevice parameters previously being passed around were always null). It is important to realize this becase, as of yesterday, Dave had said that he was not going to apply the new style PCI changes at this point, but had integrated the MODULE_DEVICE_TABLE changes. So, Dave: you should look at the points that Jeff raised, even if you are not integrating the rest of my new style PCI patch. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
"Adam J. Richter" wrote: > -static struct happy_meal *root_happy_dev = NULL; > - > #ifdef CONFIG_SBUS > +static struct happy_meal *root_happy_dev = NULL; > static struct quattro *qfe_sbus_list = NULL; > #endif don't initialize static to zero/null explicitly.. > - if (dev == NULL) { > - dev = init_etherdev(0, sizeof(struct happy_meal)); > - } else { > - dev->priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); > - if (dev->priv == NULL) > - return -ENOMEM; > - } > + dev = init_etherdev(0, sizeof(struct happy_meal)); allocation failure not checked > -static int __init happy_meal_pci_init(struct net_device *dev, struct pci_dev *pdev) > +static int __devinit happy_meal_pci_probe(struct pci_dev *pdev, > + const struct pci_device_id *id) Are you aware of any hotplug sunhme hardware? If no, don't change it to __devinit... > - if (dev == NULL) { > - dev = init_etherdev(0, sizeof(struct happy_meal)); > - } else { > - dev->priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); > - if (dev->priv == NULL) > - return -ENOMEM; > - } > + dev = init_etherdev(0, sizeof(struct happy_meal)); check for failure. also, ether_setup() call is not needed if you always call init_etherdev(NULL, ...). > +static void __devexit happy_meal_pci_remove (struct pci_dev *pdev) > +{ > + struct happy_meal *hp = pdev->driver_data; > + > + pci_free_consistent(hp->happy_dev, > + PAGE_SIZE, > + hp->happy_block, > + hp->hblock_dvma); > + unregister_netdev(hp->dev); > + kfree(hp->dev); > +} zero pdev->driver_data. If this driver has to be portable, use pci_{get,set}_drvdata() instead of directly accessing ::driver_data. > +static struct pci_device_id happymeal_pci_ids[] __devinitdata = { again, no __devxxx if not hotplug. > #ifdef CONFIG_PCI > - cards += happy_meal_pci_probe(dev); > + return pci_module_init(_meal_pci_driver); > +#else > + return (cards > 0) ? 0 : -ENODEV; > #endif ifdef not needed > +#ifdef CONFIG_PCI > + pci_unregister_driver (_meal_pci_driver); > +#endif ifdef not needed. -- Jeff Garzik | Building 1024 | The chief enemy of creativity is "good" sense MandrakeSoft| -- Picasso - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Jeff Garzik writes: Are you aware of any hotplug sunhme hardware? If no, don't change it to __devinit... Can I have a hot plug PCI bridge card that connects to a regular PCI backplane (perhaps as some kind of CardBus docking station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. The other excellent points that you raise apply equally to the driver before and after my patch (although my patch made it more clear that the struct netdevice parameters previously being passed around were always null). It is important to realize this becase, as of yesterday, Dave had said that he was not going to apply the new style PCI changes at this point, but had integrated the MODULE_DEVICE_TABLE changes. So, Dave: you should look at the points that Jeff raised, even if you are not integrating the rest of my new style PCI patch. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
I am willing to consider adding __devxxx only when other __devxxx entries already exist. These conversions to _devxxx are too late in the freeze, and only have value for isolated cases --which you admit you don't even know exist--. Linus Rule 1: Don't overdesign. Even ignoring CardBus, there apparently are docking stations that have PCI busses, such as shown in http://www.pangolin.com/LD2000/dock/gateway.htm ("Each supports hot docking, so you can attach or detach your system without turning it off"), and a web search turns up plenty of hits, at least for mobile chipsets apparently designed for this. It looks like this is a pretty standard capability. Even if we were unsure, the more conservative approach would be to use __devinitdata. The only cost of incorrectly using __devinitdata instead of __initdata, would be to make the effected kernel module use ~100 bytes more unswappable kernel memory when CONFIG_HOTPLUG is defined and the module is loaded (28 bytes per entry, including a null entry). On the other side that risk comparison, the cost of incorrectly using __initdata when __devinitdata was correct is that the user's KERNEL WILL CRASH when the notebook is inserted or removed from such a docking station, even when the kernel is built with CONFIG_HOTPLUG. Although I'm not into quoting any programmer like some religious figure, I will say that I think you're misinterpreting the meaning of an admonition like "Don't overdesign." We are not talking about designing some new abstraction or making the code more complex, but rather selecting rather a choice between two words, one of which is the more conservatively crash resistant __devinitdata, the other of which would save 28 bytes in CONFIG_HOTPLUG kernels only, and at the expense of probably causing kernel crashes. Even if such cases do exist, and this is a need, it should be addressed some other way. 1. Why? 2. What other way did you have in mind? By the way, although I do not have the PCI standard here, I do see from _PCI System Architecture_, 4th edition, chapter 19, "Configuration Registers", in the "Device ID Register" section that devices "designed around the same third party core logic" are allowed to have the same device ID (and they are even complaint to PCI 2.2 if they have different Subsystem ID values). So, my approach also has the minor advantage that if a vendor wants to ship a hot pluggable version of their PCI card in the future with the same device ID (a likely decision), then there may not need to coordinate a new release of the Linux driver. (This is a really small benefit; the kernel crashes that you want to change my existing patches to produce is the big issue.) 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
I wrote: [...] the cost of incorrectly using __initdata when __devinitdata was correct is that the user's KERNEL WILL CRASH when the notebook is inserted or removed from such a docking station, even when the kernel is built with CONFIG_HOTPLUG. My statement above, without some missing qualification, is wrong. I should have qualified this statement more carefully. (I'm sure the flames are already in the mail about this.) The kernel will crash in any case if the relevant driver does not support hot plugging.and the notebook is being removed with the PCI driver still loaded. For drivers that do not support hot plugging, we could use __initdata instead of __devinitdata, since they will crash in any case. However, violating the instructions in Documentation/pci.txt ("The ID table array should be marked __devinitdata") in this way will provoke a slew of driver bugs as the over one hundred remaining PCI drivers are converted to the new PCI interface and some authors overlook the need to change the MODULE_DEVICE_TABLE storage class. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
"Adam J. Richter" wrote: -static struct happy_meal *root_happy_dev = NULL; - #ifdef CONFIG_SBUS +static struct happy_meal *root_happy_dev = NULL; static struct quattro *qfe_sbus_list = NULL; #endif don't initialize static to zero/null explicitly.. - if (dev == NULL) { - dev = init_etherdev(0, sizeof(struct happy_meal)); - } else { - dev-priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); - if (dev-priv == NULL) - return -ENOMEM; - } + dev = init_etherdev(0, sizeof(struct happy_meal)); allocation failure not checked -static int __init happy_meal_pci_init(struct net_device *dev, struct pci_dev *pdev) +static int __devinit happy_meal_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *id) Are you aware of any hotplug sunhme hardware? If no, don't change it to __devinit... - if (dev == NULL) { - dev = init_etherdev(0, sizeof(struct happy_meal)); - } else { - dev-priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); - if (dev-priv == NULL) - return -ENOMEM; - } + dev = init_etherdev(0, sizeof(struct happy_meal)); check for failure. also, ether_setup() call is not needed if you always call init_etherdev(NULL, ...). +static void __devexit happy_meal_pci_remove (struct pci_dev *pdev) +{ + struct happy_meal *hp = pdev-driver_data; + + pci_free_consistent(hp-happy_dev, + PAGE_SIZE, + hp-happy_block, + hp-hblock_dvma); + unregister_netdev(hp-dev); + kfree(hp-dev); +} zero pdev-driver_data. If this driver has to be portable, use pci_{get,set}_drvdata() instead of directly accessing ::driver_data. +static struct pci_device_id happymeal_pci_ids[] __devinitdata = { again, no __devxxx if not hotplug. #ifdef CONFIG_PCI - cards += happy_meal_pci_probe(dev); + return pci_module_init(happy_meal_pci_driver); +#else + return (cards 0) ? 0 : -ENODEV; #endif ifdef not needed +#ifdef CONFIG_PCI + pci_unregister_driver (happy_meal_pci_driver); +#endif ifdef not needed. -- Jeff Garzik | Building 1024 | The chief enemy of creativity is "good" sense MandrakeSoft| -- Picasso - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
"Adam J. Richter" wrote: Jeff Garzik writes: Are you aware of any hotplug sunhme hardware? If no, don't change it to __devinit... Can I have a hot plug PCI bridge card that connects to a regular PCI backplane (perhaps as some kind of CardBus docking station card)? If so, all PCI drivers should use __dev{init,exit}{,data}. I am willing to consider adding __devxxx only when other __devxxx entries already exist. These conversions to _devxxx are too late in the freeze, and only have value for isolated cases --which you admit you don't even know exist--. Linus Rule 1: Don't overdesign. Even if such cases do exist, and this is a need, it should be addressed some other way. Your suggestion bloats drivers needlessly for the majority of cases and I will not be applying any such patches... Jeff -- Jeff Garzik | Building 1024 | The chief enemy of creativity is "good" sense MandrakeSoft| -- Picasso - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
On Thu, Nov 16, 2000 at 04:22:36AM -0800, David S. Miller wrote: > Sure, that sounds nice. > > Actually, one of the possible "grand plans" for 2.5 is a unified > "struct device". I don't know what will actually happen here. apologies for pointing out the potentially obvoius, but people might want to investigate 'newbus' while brainstorming about this: http://www.daemonnews.org/27/newbus-intro.html -- zach - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Date: Thu, 16 Nov 2000 13:13:37 +0100 From: Christoph Hellwig <[EMAIL PROTECTED]> Would you accept such a change for 2.5? Sure, that sounds nice. Actually, one of the possible "grand plans" for 2.5 is a unified "struct device". I don't know what will actually happen here. Later, David S. Miller [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
In article <[EMAIL PROTECTED]> you wrote: > I never ported it to the new PCI interfaces strictly because when > combined with SBUS it makes the driver initialization look really > sloppy. BTW, what do you think of a new PCI style probing for SBUS? When I hacked on a small sbus driver, I thought it might be a good idea to have tables like the new PCI interface for all busses. The interface for SBUS would be: struct sbus_device_id { char * promname; unsigned long driver_data; }; struct sbus_driver { struct list_headnode; char * name; struct sbus_device_id * id_table; int (* probe)(struct sbus_dev * dev, const struct sbus_device_id * id); void (* remove)(struct sbus_dev * dev); }; Would you accept such a change for 2.5? Christoph -- Always remember that you are unique. Just like everyone else. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
From: "Adam J. Richter" <[EMAIL PROTECTED]> Date: Thu, 16 Nov 2000 03:10:14 -0800 >Sorry, I don't like this change. Can you at least add the MODULE_DEVICE_TABLE declaration and the pci_device_id table that it refers to, even if the code does not directly reference it? (You can make it as __initdata rather than __devinitdata, since it can safely be thrown away.) That was automatic PCI ID recognition will work. Feel free to send me a patch which does this. No problem. Later, David S. Miller [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
>Sorry, I don't like this change. Can you at least add the MODULE_DEVICE_TABLE declaration and the pci_device_id table that it refers to, even if the code does not directly reference it? (You can make it as __initdata rather than __devinitdata, since it can safely be thrown away.) That was automatic PCI ID recognition will work. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
I never ported it to the new PCI interfaces strictly because when combined with SBUS it makes the driver initialization look really sloppy. Sorry, I don't like this change. Later, David S. Miller [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
sunhme.c patch for new PCI interface (UNTESTED)
I don't have access to a Sun HME card, but, for some neurotic reason, I decided to try porting sunhme.c to the new PCI interface. I believe it has simplified the code slightly. More importantly, it causes the module to export a table with the PCI ID's that it cares about, which is used by depmod to generate /lib/modules/.../modules.pcimap, which can be used to support automatic loading (I wrote a small program for this purpose, "pcimodules", which I have submitted hopefully for inclusion in pciutils, and I'd be glad to provide that patch if anyone is interested). The patch also eliminates some places where a struct netdevice that was *always* null was being passed around. Also note that, under this patch, root_happy_dev has become a list of only the sbus interfaces. Anyhow, I know that this compiles without any complaint other than Dave Miller's built in warning message ("This needs to be corrected ... -DaveM"). I would appreciate it if you or anyone else interested would test it and, if it works and is generally approved of, then have Dave bless it and send it to Linus. It looks like sunhme was one of only seven drivers remaining in the drivers/net (excluding subdirectories) that used the old PCI probing interface. This patch is against 2.4.0-test11-pre5, and includes Dave's changes to make it compile. 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." --- linux-2.4.0-test11-pre5/drivers/net/sunhme.cWed Nov 15 22:07:59 2000 +++ linux/drivers/net/sunhme.c Thu Nov 16 00:07:15 2000 @@ -38,6 +38,7 @@ #include #include #include +#include #ifdef __sparc__ #include @@ -48,7 +49,6 @@ #ifndef __sparc_v9__ #include #endif -#include #endif #include @@ -73,9 +73,8 @@ /* accept MAC address of the form macaddr=0x08,0x00,0x20,0x30,0x40,0x50 */ MODULE_PARM(macaddr, "6i"); -static struct happy_meal *root_happy_dev = NULL; - #ifdef CONFIG_SBUS +static struct happy_meal *root_happy_dev = NULL; static struct quattro *qfe_sbus_list = NULL; #endif @@ -101,6 +100,7 @@ unsigned int status; }; #define TX_LOG_LEN 128 + static struct hme_tx_logent tx_log[TX_LOG_LEN]; static int txlog_cur_entry = 0; static __inline__ void tx_add_log(struct happy_meal *hp, unsigned int a, unsigned int s) @@ -1600,6 +1600,10 @@ HMD(("happy_meal_init: old[%08x] bursts<", hme_read32(hp, gregs + GREG_CFG))); +#ifndef __sparc__ + /* It is always PCI and can handle 64byte bursts. */ + hme_write32(hp, gregs + GREG_CFG, GREG_CFG_BURST64); +#else if ((hp->happy_bursts & DMA_BURST64) && ((hp->happy_flags & HFLAG_PCI) != 0 #ifdef CONFIG_SBUS @@ -1633,6 +1637,7 @@ HMD(("XXX>")); hme_write32(hp, gregs + GREG_CFG, 0); } +#endif /* __sparc__ */ /* Turn off interrupts we do not want to hear. */ HMD((", enable global interrupts, ")); @@ -2553,10 +2558,9 @@ #endif /* CONFIG_PCI */ #ifdef CONFIG_SBUS -static int __init happy_meal_sbus_init(struct net_device *dev, - struct sbus_dev *sdev, - int is_qfe) +static int __init happy_meal_sbus_init(struct sbus_dev *sdev, int is_qfe) { + struct net_device *dev; struct quattro *qp = NULL; struct happy_meal *hp; int i, qfe_slot = -1; @@ -2571,13 +2575,7 @@ if (qfe_slot == 4) return -ENODEV; } - if (dev == NULL) { - dev = init_etherdev(0, sizeof(struct happy_meal)); - } else { - dev->priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); - if (dev->priv == NULL) - return -ENOMEM; - } + dev = init_etherdev(0, sizeof(struct happy_meal)); if (hme_version_printed++ == 0) printk(KERN_INFO "%s", version); @@ -2741,7 +2739,8 @@ #endif #ifdef CONFIG_PCI -static int __init happy_meal_pci_init(struct net_device *dev, struct pci_dev *pdev) +static int __devinit happy_meal_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *id) { struct quattro *qp = NULL; #ifdef __sparc__ @@ -2752,6 +2751,10 @@ unsigned long hpreg_base; int i, qfe_slot = -1; char prom_name[64]; + struct net_device *dev; + + if (pci_enable_device(pdev)) + return -EIO; /* Now make sure pci_dev cookie is there. */ #ifdef __sparc__ @@ -2778,13 +2781,7 @@ if (qfe_slot == 4) return -ENODEV; } - if (dev == NULL) { - dev = init_etherdev(0, sizeof(struct happy_meal)); - }
sunhme.c patch for new PCI interface (UNTESTED)
I don't have access to a Sun HME card, but, for some neurotic reason, I decided to try porting sunhme.c to the new PCI interface. I believe it has simplified the code slightly. More importantly, it causes the module to export a table with the PCI ID's that it cares about, which is used by depmod to generate /lib/modules/.../modules.pcimap, which can be used to support automatic loading (I wrote a small program for this purpose, "pcimodules", which I have submitted hopefully for inclusion in pciutils, and I'd be glad to provide that patch if anyone is interested). The patch also eliminates some places where a struct netdevice that was *always* null was being passed around. Also note that, under this patch, root_happy_dev has become a list of only the sbus interfaces. Anyhow, I know that this compiles without any complaint other than Dave Miller's built in warning message ("This needs to be corrected ... -DaveM"). I would appreciate it if you or anyone else interested would test it and, if it works and is generally approved of, then have Dave bless it and send it to Linus. It looks like sunhme was one of only seven drivers remaining in the drivers/net (excluding subdirectories) that used the old PCI probing interface. This patch is against 2.4.0-test11-pre5, and includes Dave's changes to make it compile. 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." --- linux-2.4.0-test11-pre5/drivers/net/sunhme.cWed Nov 15 22:07:59 2000 +++ linux/drivers/net/sunhme.c Thu Nov 16 00:07:15 2000 @@ -38,6 +38,7 @@ #include asm/dma.h #include linux/errno.h #include asm/byteorder.h +#include asm/uaccess.h #ifdef __sparc__ #include asm/idprom.h @@ -48,7 +49,6 @@ #ifndef __sparc_v9__ #include asm/io-unit.h #endif -#include asm/uaccess.h #endif #include asm/pgtable.h @@ -73,9 +73,8 @@ /* accept MAC address of the form macaddr=0x08,0x00,0x20,0x30,0x40,0x50 */ MODULE_PARM(macaddr, "6i"); -static struct happy_meal *root_happy_dev = NULL; - #ifdef CONFIG_SBUS +static struct happy_meal *root_happy_dev = NULL; static struct quattro *qfe_sbus_list = NULL; #endif @@ -101,6 +100,7 @@ unsigned int status; }; #define TX_LOG_LEN 128 + static struct hme_tx_logent tx_log[TX_LOG_LEN]; static int txlog_cur_entry = 0; static __inline__ void tx_add_log(struct happy_meal *hp, unsigned int a, unsigned int s) @@ -1600,6 +1600,10 @@ HMD(("happy_meal_init: old[%08x] bursts", hme_read32(hp, gregs + GREG_CFG))); +#ifndef __sparc__ + /* It is always PCI and can handle 64byte bursts. */ + hme_write32(hp, gregs + GREG_CFG, GREG_CFG_BURST64); +#else if ((hp-happy_bursts DMA_BURST64) ((hp-happy_flags HFLAG_PCI) != 0 #ifdef CONFIG_SBUS @@ -1633,6 +1637,7 @@ HMD(("XXX")); hme_write32(hp, gregs + GREG_CFG, 0); } +#endif /* __sparc__ */ /* Turn off interrupts we do not want to hear. */ HMD((", enable global interrupts, ")); @@ -2553,10 +2558,9 @@ #endif /* CONFIG_PCI */ #ifdef CONFIG_SBUS -static int __init happy_meal_sbus_init(struct net_device *dev, - struct sbus_dev *sdev, - int is_qfe) +static int __init happy_meal_sbus_init(struct sbus_dev *sdev, int is_qfe) { + struct net_device *dev; struct quattro *qp = NULL; struct happy_meal *hp; int i, qfe_slot = -1; @@ -2571,13 +2575,7 @@ if (qfe_slot == 4) return -ENODEV; } - if (dev == NULL) { - dev = init_etherdev(0, sizeof(struct happy_meal)); - } else { - dev-priv = kmalloc(sizeof(struct happy_meal), GFP_KERNEL); - if (dev-priv == NULL) - return -ENOMEM; - } + dev = init_etherdev(0, sizeof(struct happy_meal)); if (hme_version_printed++ == 0) printk(KERN_INFO "%s", version); @@ -2741,7 +2739,8 @@ #endif #ifdef CONFIG_PCI -static int __init happy_meal_pci_init(struct net_device *dev, struct pci_dev *pdev) +static int __devinit happy_meal_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *id) { struct quattro *qp = NULL; #ifdef __sparc__ @@ -2752,6 +2751,10 @@ unsigned long hpreg_base; int i, qfe_slot = -1; char prom_name[64]; + struct net_device *dev; + + if (pci_enable_device(pdev)) + return -EIO; /* Now make sure pci_dev cookie is there. */ #ifdef __sparc__ @@ -2778,13 +2781,7 @@ if (qfe_slot == 4) return -ENODEV; } - if (dev
Re: sunhme.c patch for new PCI interface (UNTESTED)
Sorry, I don't like this change. Can you at least add the MODULE_DEVICE_TABLE declaration and the pci_device_id table that it refers to, even if the code does not directly reference it? (You can make it as __initdata rather than __devinitdata, since it can safely be thrown away.) That was automatic PCI ID recognition will work. 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." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
From: "Adam J. Richter" [EMAIL PROTECTED] Date: Thu, 16 Nov 2000 03:10:14 -0800 Sorry, I don't like this change. Can you at least add the MODULE_DEVICE_TABLE declaration and the pci_device_id table that it refers to, even if the code does not directly reference it? (You can make it as __initdata rather than __devinitdata, since it can safely be thrown away.) That was automatic PCI ID recognition will work. Feel free to send me a patch which does this. No problem. Later, David S. Miller [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
In article [EMAIL PROTECTED] you wrote: I never ported it to the new PCI interfaces strictly because when combined with SBUS it makes the driver initialization look really sloppy. BTW, what do you think of a new PCI style probing for SBUS? When I hacked on a small sbus driver, I thought it might be a good idea to have tables like the new PCI interface for all busses. The interface for SBUS would be: struct sbus_device_id { char * promname; unsigned long driver_data; }; struct sbus_driver { struct list_headnode; char * name; struct sbus_device_id * id_table; int (* probe)(struct sbus_dev * dev, const struct sbus_device_id * id); void (* remove)(struct sbus_dev * dev); }; Would you accept such a change for 2.5? Christoph -- Always remember that you are unique. Just like everyone else. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
Date: Thu, 16 Nov 2000 13:13:37 +0100 From: Christoph Hellwig [EMAIL PROTECTED] Would you accept such a change for 2.5? Sure, that sounds nice. Actually, one of the possible "grand plans" for 2.5 is a unified "struct device". I don't know what will actually happen here. Later, David S. Miller [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: sunhme.c patch for new PCI interface (UNTESTED)
On Thu, Nov 16, 2000 at 04:22:36AM -0800, David S. Miller wrote: Sure, that sounds nice. Actually, one of the possible "grand plans" for 2.5 is a unified "struct device". I don't know what will actually happen here. apologies for pointing out the potentially obvoius, but people might want to investigate 'newbus' while brainstorming about this: http://www.daemonnews.org/27/newbus-intro.html -- zach - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/