stallion, convert to pci probing
Convert stallion driver to pci probing instead of pci_dev_get iteration.
Signed-off-by: Jiri Slaby <[email protected]>
---
commit c4a0f4d15661fe74b8c67b0258d5dfbcff57071b
tree 5da405798c9d47c7a07b63868e9fec1748908b6b
parent fcf3d1f86671d8e01a238935d906356442c92749
author Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25 +0159
committer Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25 +0159
drivers/char/stallion.c | 165 ++++++++++++++++++++++++-----------------------
1 files changed, 83 insertions(+), 82 deletions(-)
diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
index d2cbdb7..592bd6e 100644
--- a/drivers/char/stallion.c
+++ b/drivers/char/stallion.c
@@ -381,8 +381,6 @@ #define STL_CLOSEDELAY (5 * HZ / 10)
/*****************************************************************************/
-#ifdef CONFIG_PCI
-
/*
* Define the Stallion PCI vendor and device IDs.
*/
@@ -402,22 +400,19 @@ #endif
/*
* Define structure to hold all Stallion PCI boards.
*/
-typedef struct stlpcibrd {
- unsigned short vendid;
- unsigned short devid;
- int brdtype;
-} stlpcibrd_t;
-
-static stlpcibrd_t stl_pcibrds[] = {
- { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864, BRD_ECH64PCI },
- { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI, BRD_EASYIOPCI },
- { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832, BRD_ECHPCI },
- { PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410, BRD_ECHPCI },
-};
-static int stl_nrpcibrds = ARRAY_SIZE(stl_pcibrds);
-
-#endif
+static struct pci_device_id stl_pcibrds[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864),
+ .driver_data = BRD_ECH64PCI },
+ { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI),
+ .driver_data = BRD_EASYIOPCI },
+ { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832),
+ .driver_data = BRD_ECHPCI },
+ { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410),
+ .driver_data = BRD_ECHPCI },
+ { }
+};
+MODULE_DEVICE_TABLE(pci, stl_pcibrds);
/*****************************************************************************/
@@ -2392,24 +2387,52 @@ static int __init stl_getbrdnr(void)
return(-1);
}
-/*****************************************************************************/
+static void stl_cleanup_panels(struct stlbrd *brdp)
+{
+ struct stlpanel *panelp;
+ struct stlport *portp;
+ unsigned int j, k;
-#ifdef CONFIG_PCI
+ for (j = 0; j < STL_MAXPANELS; j++) {
+ panelp = brdp->panels[j];
+ if (panelp == NULL)
+ continue;
+ for (k = 0; k < STL_PORTSPERPANEL; k++) {
+ portp = panelp->ports[k];
+ if (portp == NULL)
+ continue;
+ if (portp->tty != NULL)
+ stl_hangup(portp->tty);
+ kfree(portp->tx.buf);
+ kfree(portp);
+ }
+ kfree(panelp);
+ }
+}
+/*****************************************************************************/
/*
* We have a Stallion board. Allocate a board structure and
* initialize it. Read its IO and IRQ resources from PCI
* configuration space.
*/
-static int __init stl_initpcibrd(int brdtype, struct pci_dev *devp)
+static int __devinit stl_pciprobe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
{
- struct stlbrd *brdp;
+ struct stlbrd *brdp;
+ unsigned int brdtype = ent->driver_data;
pr_debug("stl_initpcibrd(brdtype=%d,busnr=%x,devnr=%x)\n", brdtype,
- devp->bus->number, devp->devfn);
+ pdev->bus->number, pdev->devfn);
+
+ if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE)
+ return -ENODEV;
+
+ dev_info(&pdev->dev, "please, report this to LKML: %x/%x/%x\n",
+ pdev->vendor, pdev->device, pdev->class);
- if (pci_enable_device(devp))
+ if (pci_enable_device(pdev))
return(-EIO);
if ((brdp = stl_allocbrd()) == NULL)
return(-ENOMEM);
@@ -2425,8 +2448,8 @@ static int __init stl_initpcibrd(int brd
* so set up io addresses based on board type.
*/
pr_debug("%s(%d): BAR[]=%Lx,%Lx,%Lx,%Lx IRQ=%x\n", __FILE__, __LINE__,
- pci_resource_start(devp, 0), pci_resource_start(devp, 1),
- pci_resource_start(devp, 2), pci_resource_start(devp, 3), devp->irq);
+ pci_resource_start(pdev, 0), pci_resource_start(pdev, 1),
+ pci_resource_start(pdev, 2), pci_resource_start(pdev, 3), pdev->irq);
/*
* We have all resources from the board, so let's setup the actual
@@ -2434,63 +2457,52 @@ static int __init stl_initpcibrd(int brd
*/
switch (brdtype) {
case BRD_ECHPCI:
- brdp->ioaddr2 = pci_resource_start(devp, 0);
- brdp->ioaddr1 = pci_resource_start(devp, 1);
+ brdp->ioaddr2 = pci_resource_start(pdev, 0);
+ brdp->ioaddr1 = pci_resource_start(pdev, 1);
break;
case BRD_ECH64PCI:
- brdp->ioaddr2 = pci_resource_start(devp, 2);
- brdp->ioaddr1 = pci_resource_start(devp, 1);
+ brdp->ioaddr2 = pci_resource_start(pdev, 2);
+ brdp->ioaddr1 = pci_resource_start(pdev, 1);
break;
case BRD_EASYIOPCI:
- brdp->ioaddr1 = pci_resource_start(devp, 2);
- brdp->ioaddr2 = pci_resource_start(devp, 1);
+ brdp->ioaddr1 = pci_resource_start(pdev, 2);
+ brdp->ioaddr2 = pci_resource_start(pdev, 1);
break;
default:
printk("STALLION: unknown PCI board type=%d\n", brdtype);
break;
}
- brdp->irq = devp->irq;
+ brdp->irq = pdev->irq;
stl_brdinit(brdp);
+ pci_set_drvdata(pdev, brdp);
+
return(0);
}
-/*****************************************************************************/
-
-/*
- * Find all Stallion PCI boards that might be installed. Initialize each
- * one as it is found.
- */
-
-
-static int __init stl_findpcibrds(void)
+static void __devexit stl_pciremove(struct pci_dev *pdev)
{
- struct pci_dev *dev = NULL;
- int i, rc;
-
- pr_debug("stl_findpcibrds()\n");
+ struct stlbrd *brdp = pci_get_drvdata(pdev);
- for (i = 0; (i < stl_nrpcibrds); i++)
- while ((dev = pci_get_device(stl_pcibrds[i].vendid,
- stl_pcibrds[i].devid, dev))) {
+ free_irq(brdp->irq, brdp);
-/*
- * Found a device on the PCI bus that has our vendor and
- * device ID. Need to check now that it is really us.
- */
- if ((dev->class >> 8) == PCI_CLASS_STORAGE_IDE)
- continue;
+ stl_cleanup_panels(brdp);
- rc = stl_initpcibrd(stl_pcibrds[i].brdtype, dev);
- if (rc)
- return(rc);
- }
+ release_region(brdp->ioaddr1, brdp->iosize1);
+ if (brdp->iosize2 > 0)
+ release_region(brdp->ioaddr2, brdp->iosize2);
- return(0);
+ stl_brds[brdp->brdnr] = NULL;
+ kfree(brdp);
}
-#endif
+static struct pci_driver stl_pcidriver = {
+ .name = "stallion",
+ .id_table = stl_pcibrds,
+ .probe = stl_pciprobe,
+ .remove = __devexit_p(stl_pciremove)
+};
/*****************************************************************************/
@@ -2537,9 +2549,6 @@ static int __init stl_initbrds(void)
* line options or auto-detected on the PCI bus.
*/
stl_argbrds();
-#ifdef CONFIG_PCI
- stl_findpcibrds();
-#endif
return(0);
}
@@ -4778,7 +4787,7 @@ static void stl_sc26198otherisr(struct s
*/
static int __init stallion_module_init(void)
{
- unsigned int i;
+ unsigned int i, retval;
printk(KERN_INFO "%s: version %s\n", stl_drvtitle, stl_drvversion);
@@ -4787,6 +4796,10 @@ static int __init stallion_module_init(v
stl_initbrds();
+ retval = pci_register_driver(&stl_pcidriver);
+ if (retval)
+ goto err;
+
stl_serial = alloc_tty_driver(STL_MAXBRDS * STL_MAXPORTS);
if (!stl_serial)
return -1;
@@ -4822,14 +4835,14 @@ static int __init stallion_module_init(v
}
return 0;
+err:
+ return retval;
}
static void __exit stallion_module_exit(void)
{
struct stlbrd *brdp;
- struct stlpanel *panelp;
- struct stlport *portp;
- int i, j, k;
+ int i;
pr_debug("cleanup_module()\n");
@@ -4856,27 +4869,15 @@ static void __exit stallion_module_exit(
"errno=%d\n", -i);
class_destroy(stallion_class);
+ pci_unregister_driver(&stl_pcidriver);
+
for (i = 0; (i < stl_nrbrds); i++) {
if ((brdp = stl_brds[i]) == NULL)
continue;
free_irq(brdp->irq, brdp);
- for (j = 0; (j < STL_MAXPANELS); j++) {
- panelp = brdp->panels[j];
- if (panelp == NULL)
- continue;
- for (k = 0; (k < STL_PORTSPERPANEL); k++) {
- portp = panelp->ports[k];
- if (portp == NULL)
- continue;
- if (portp->tty != NULL)
- stl_hangup(portp->tty);
- kfree(portp->tx.buf);
- kfree(portp);
- }
- kfree(panelp);
- }
+ stl_cleanup_panels(brdp);
release_region(brdp->ioaddr1, brdp->iosize1);
if (brdp->iosize2 > 0)
Am Sonntag, 22. Oktober 2006 17:48 schrieb Jiri Slaby:
> stallion, convert to pci probing
>
> Convert stallion driver to pci probing instead of pci_dev_get iteration.
>
> Signed-off-by: Jiri Slaby <[email protected]>
>
> ---
> commit c4a0f4d15661fe74b8c67b0258d5dfbcff57071b
> tree 5da405798c9d47c7a07b63868e9fec1748908b6b
> parent fcf3d1f86671d8e01a238935d906356442c92749
> author Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25 +0159
> committer Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25
> +0159
>
> drivers/char/stallion.c | 165
> ++++++++++++++++++++++++----------------------- 1 files changed, 83
> insertions(+), 82 deletions(-)
>
> diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
> index d2cbdb7..592bd6e 100644
> --- a/drivers/char/stallion.c
> +++ b/drivers/char/stallion.c
> @@ -381,8 +381,6 @@ #define STL_CLOSEDELAY (5 * HZ / 10)
>
>
> /**************************************************************************
>***/
>
> -#ifdef CONFIG_PCI
> -
> /*
> * Define the Stallion PCI vendor and device IDs.
> */
> @@ -402,22 +400,19 @@ #endif
> /*
> * Define structure to hold all Stallion PCI boards.
> */
> -typedef struct stlpcibrd {
> - unsigned short vendid;
> - unsigned short devid;
> - int brdtype;
> -} stlpcibrd_t;
> -
> -static stlpcibrd_t stl_pcibrds[] = {
> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864, BRD_ECH64PCI },
> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI, BRD_EASYIOPCI },
> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832, BRD_ECHPCI },
> - { PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410, BRD_ECHPCI },
> -};
>
> -static int stl_nrpcibrds = ARRAY_SIZE(stl_pcibrds);
> -
> -#endif
> +static struct pci_device_id stl_pcibrds[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864),
> + .driver_data = BRD_ECH64PCI },
> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI),
> + .driver_data = BRD_EASYIOPCI },
> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832),
> + .driver_data = BRD_ECHPCI },
> + { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410),
> + .driver_data = BRD_ECHPCI },
> + { }
> +};
> +MODULE_DEVICE_TABLE(pci, stl_pcibrds);
>
>
> /**************************************************************************
>***/
>
> @@ -2392,24 +2387,52 @@ static int __init stl_getbrdnr(void)
> return(-1);
> }
>
> -/*************************************************************************
>****/ +static void stl_cleanup_panels(struct stlbrd *brdp)
> +{
> + struct stlpanel *panelp;
> + struct stlport *portp;
> + unsigned int j, k;
>
> -#ifdef CONFIG_PCI
> + for (j = 0; j < STL_MAXPANELS; j++) {
> + panelp = brdp->panels[j];
> + if (panelp == NULL)
> + continue;
> + for (k = 0; k < STL_PORTSPERPANEL; k++) {
> + portp = panelp->ports[k];
> + if (portp == NULL)
> + continue;
> + if (portp->tty != NULL)
> + stl_hangup(portp->tty);
> + kfree(portp->tx.buf);
> + kfree(portp);
> + }
> + kfree(panelp);
> + }
> +}
>
> +/*************************************************************************
>****/ /*
> * We have a Stallion board. Allocate a board structure and
> * initialize it. Read its IO and IRQ resources from PCI
> * configuration space.
> */
>
> -static int __init stl_initpcibrd(int brdtype, struct pci_dev *devp)
> +static int __devinit stl_pciprobe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> {
> - struct stlbrd *brdp;
> + struct stlbrd *brdp;
> + unsigned int brdtype = ent->driver_data;
>
> pr_debug("stl_initpcibrd(brdtype=%d,busnr=%x,devnr=%x)\n", brdtype,
> - devp->bus->number, devp->devfn);
> + pdev->bus->number, pdev->devfn);
> +
> + if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE)
> + return -ENODEV;
> +
> + dev_info(&pdev->dev, "please, report this to LKML: %x/%x/%x\n",
> + pdev->vendor, pdev->device, pdev->class);
I guess you wanted the dev_info _before_ the return? And why could this ever
happen?
>
> - if (pci_enable_device(devp))
> + if (pci_enable_device(pdev))
> return(-EIO);
> if ((brdp = stl_allocbrd()) == NULL)
> return(-ENOMEM);
> @@ -2425,8 +2448,8 @@ static int __init stl_initpcibrd(int brd
> * so set up io addresses based on board type.
> */
> pr_debug("%s(%d): BAR[]=%Lx,%Lx,%Lx,%Lx IRQ=%x\n", __FILE__, __LINE__,
> - pci_resource_start(devp, 0), pci_resource_start(devp, 1),
> - pci_resource_start(devp, 2), pci_resource_start(devp, 3), devp->irq);
> + pci_resource_start(pdev, 0), pci_resource_start(pdev, 1),
> + pci_resource_start(pdev, 2), pci_resource_start(pdev, 3), pdev->irq);
> /*
> * We have all resources from the board, so let's setup the actual
I would vote for deleting this completely. If someone wants to know lspci and
sysfs can tell the whole story.
> @@ -2434,63 +2457,52 @@ static int __init stl_initpcibrd(int brd
> */
> switch (brdtype) {
> case BRD_ECHPCI:
> - brdp->ioaddr2 = pci_resource_start(devp, 0);
> - brdp->ioaddr1 = pci_resource_start(devp, 1);
> + brdp->ioaddr2 = pci_resource_start(pdev, 0);
> + brdp->ioaddr1 = pci_resource_start(pdev, 1);
> break;
> case BRD_ECH64PCI:
> - brdp->ioaddr2 = pci_resource_start(devp, 2);
> - brdp->ioaddr1 = pci_resource_start(devp, 1);
> + brdp->ioaddr2 = pci_resource_start(pdev, 2);
> + brdp->ioaddr1 = pci_resource_start(pdev, 1);
> break;
> case BRD_EASYIOPCI:
> - brdp->ioaddr1 = pci_resource_start(devp, 2);
> - brdp->ioaddr2 = pci_resource_start(devp, 1);
> + brdp->ioaddr1 = pci_resource_start(pdev, 2);
> + brdp->ioaddr2 = pci_resource_start(pdev, 1);
> break;
Is it really that important to name it pdev instead of devp? This causes tons
of noise in the patch.
> + release_region(brdp->ioaddr1, brdp->iosize1);
> + if (brdp->iosize2 > 0)
> + release_region(brdp->ioaddr2, brdp->iosize2);
pci_release_region() maybe? This would be a conversion that actually would
make the code more readable. Since this code path looks PCI only...
> - return(0);
> + stl_brds[brdp->brdnr] = NULL;
> + kfree(brdp);
> }
>
> -#endif
> +static struct pci_driver stl_pcidriver = {
> + .name = "stallion",
> + .id_table = stl_pcibrds,
> + .probe = stl_pciprobe,
> + .remove = __devexit_p(stl_pciremove)
> +};
>
>
> /**************************************************************************
>***/
>
> @@ -2537,9 +2549,6 @@ static int __init stl_initbrds(void)
> * line options or auto-detected on the PCI bus.
> */
> stl_argbrds();
> -#ifdef CONFIG_PCI
> - stl_findpcibrds();
> -#endif
>
> return(0);
> }
> @@ -4778,7 +4787,7 @@ static void stl_sc26198otherisr(struct s
> */
> static int __init stallion_module_init(void)
> {
> - unsigned int i;
> + unsigned int i, retval;
>
> printk(KERN_INFO "%s: version %s\n", stl_drvtitle, stl_drvversion);
>
> @@ -4787,6 +4796,10 @@ static int __init stallion_module_init(v
>
> stl_initbrds();
>
> + retval = pci_register_driver(&stl_pcidriver);
> + if (retval)
> + goto err;
> +
> stl_serial = alloc_tty_driver(STL_MAXBRDS * STL_MAXPORTS);
> if (!stl_serial)
pci_unregister_driver() here?
> return -1;
Eike
Rolf Eike Beer wrote:
> Am Sonntag, 22. Oktober 2006 17:48 schrieb Jiri Slaby:
>> stallion, convert to pci probing
>>
>> Convert stallion driver to pci probing instead of pci_dev_get iteration.
>>
>> Signed-off-by: Jiri Slaby <[email protected]>
>>
>> ---
>> commit c4a0f4d15661fe74b8c67b0258d5dfbcff57071b
>> tree 5da405798c9d47c7a07b63868e9fec1748908b6b
>> parent fcf3d1f86671d8e01a238935d906356442c92749
>> author Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25 +0159
>> committer Jiri Slaby <[email protected]> Sun, 22 Oct 2006 16:40:25
>> +0159
>>
>> drivers/char/stallion.c | 165
>> ++++++++++++++++++++++++----------------------- 1 files changed, 83
>> insertions(+), 82 deletions(-)
>>
>> diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
>> index d2cbdb7..592bd6e 100644
>> --- a/drivers/char/stallion.c
>> +++ b/drivers/char/stallion.c
>> @@ -381,8 +381,6 @@ #define STL_CLOSEDELAY (5 * HZ / 10)
>>
>>
>> /**************************************************************************
>> ***/
>>
>> -#ifdef CONFIG_PCI
>> -
>> /*
>> * Define the Stallion PCI vendor and device IDs.
>> */
>> @@ -402,22 +400,19 @@ #endif
>> /*
>> * Define structure to hold all Stallion PCI boards.
>> */
>> -typedef struct stlpcibrd {
>> - unsigned short vendid;
>> - unsigned short devid;
>> - int brdtype;
>> -} stlpcibrd_t;
>> -
>> -static stlpcibrd_t stl_pcibrds[] = {
>> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864, BRD_ECH64PCI },
>> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI, BRD_EASYIOPCI },
>> - { PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832, BRD_ECHPCI },
>> - { PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410, BRD_ECHPCI },
>> -};
>>
>> -static int stl_nrpcibrds = ARRAY_SIZE(stl_pcibrds);
>> -
>> -#endif
>> +static struct pci_device_id stl_pcibrds[] = {
>> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI864),
>> + .driver_data = BRD_ECH64PCI },
>> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_EIOPCI),
>> + .driver_data = BRD_EASYIOPCI },
>> + { PCI_DEVICE(PCI_VENDOR_ID_STALLION, PCI_DEVICE_ID_ECHPCI832),
>> + .driver_data = BRD_ECHPCI },
>> + { PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410),
>> + .driver_data = BRD_ECHPCI },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(pci, stl_pcibrds);
>>
>>
>> /**************************************************************************
>> ***/
>>
>> @@ -2392,24 +2387,52 @@ static int __init stl_getbrdnr(void)
>> return(-1);
>> }
>>
>> -/*************************************************************************
>> ****/ +static void stl_cleanup_panels(struct stlbrd *brdp)
>> +{
>> + struct stlpanel *panelp;
>> + struct stlport *portp;
>> + unsigned int j, k;
>>
>> -#ifdef CONFIG_PCI
>> + for (j = 0; j < STL_MAXPANELS; j++) {
>> + panelp = brdp->panels[j];
>> + if (panelp == NULL)
>> + continue;
>> + for (k = 0; k < STL_PORTSPERPANEL; k++) {
>> + portp = panelp->ports[k];
>> + if (portp == NULL)
>> + continue;
>> + if (portp->tty != NULL)
>> + stl_hangup(portp->tty);
>> + kfree(portp->tx.buf);
>> + kfree(portp);
>> + }
>> + kfree(panelp);
>> + }
>> +}
>>
>> +/*************************************************************************
>> ****/ /*
>> * We have a Stallion board. Allocate a board structure and
>> * initialize it. Read its IO and IRQ resources from PCI
>> * configuration space.
>> */
>>
>> -static int __init stl_initpcibrd(int brdtype, struct pci_dev *devp)
>> +static int __devinit stl_pciprobe(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> {
>> - struct stlbrd *brdp;
>> + struct stlbrd *brdp;
>> + unsigned int brdtype = ent->driver_data;
>>
>> pr_debug("stl_initpcibrd(brdtype=%d,busnr=%x,devnr=%x)\n", brdtype,
>> - devp->bus->number, devp->devfn);
>> + pdev->bus->number, pdev->devfn);
>> +
>> + if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE)
>> + return -ENODEV;
>> +
>> + dev_info(&pdev->dev, "please, report this to LKML: %x/%x/%x\n",
>> + pdev->vendor, pdev->device, pdev->class);
>
> I guess you wanted the dev_info _before_ the return? And why could this ever
> happen?
Nope, I want to know which CLASSes we want to control (to push it into the table
not to take care of it here...).
>> - if (pci_enable_device(devp))
>> + if (pci_enable_device(pdev))
>> return(-EIO);
>> if ((brdp = stl_allocbrd()) == NULL)
>> return(-ENOMEM);
>> @@ -2425,8 +2448,8 @@ static int __init stl_initpcibrd(int brd
>> * so set up io addresses based on board type.
>> */
>> pr_debug("%s(%d): BAR[]=%Lx,%Lx,%Lx,%Lx IRQ=%x\n", __FILE__, __LINE__,
>> - pci_resource_start(devp, 0), pci_resource_start(devp, 1),
>> - pci_resource_start(devp, 2), pci_resource_start(devp, 3), devp->irq);
>> + pci_resource_start(pdev, 0), pci_resource_start(pdev, 1),
>> + pci_resource_start(pdev, 2), pci_resource_start(pdev, 3), pdev->irq);
>> /*
>> * We have all resources from the board, so let's setup the actual
>
> I would vote for deleting this completely. If someone wants to know lspci and
> sysfs can tell the whole story.
Yup, this is done in patch named 'Char: stallion, prints cleanup'.
>> @@ -2434,63 +2457,52 @@ static int __init stl_initpcibrd(int brd
>> */
>> switch (brdtype) {
>> case BRD_ECHPCI:
>> - brdp->ioaddr2 = pci_resource_start(devp, 0);
>> - brdp->ioaddr1 = pci_resource_start(devp, 1);
>> + brdp->ioaddr2 = pci_resource_start(pdev, 0);
>> + brdp->ioaddr1 = pci_resource_start(pdev, 1);
>> break;
>> case BRD_ECH64PCI:
>> - brdp->ioaddr2 = pci_resource_start(devp, 2);
>> - brdp->ioaddr1 = pci_resource_start(devp, 1);
>> + brdp->ioaddr2 = pci_resource_start(pdev, 2);
>> + brdp->ioaddr1 = pci_resource_start(pdev, 1);
>> break;
>> case BRD_EASYIOPCI:
>> - brdp->ioaddr1 = pci_resource_start(devp, 2);
>> - brdp->ioaddr2 = pci_resource_start(devp, 1);
>> + brdp->ioaddr1 = pci_resource_start(pdev, 2);
>> + brdp->ioaddr2 = pci_resource_start(pdev, 1);
>> break;
>
> Is it really that important to name it pdev instead of devp? This causes tons
> of noise in the patch.
Hmm, that's correct note, but pci device is called pdev in most cases, I wanted
to keep it consistent (and thought that it doesn't want to be in separate patch).
>> + release_region(brdp->ioaddr1, brdp->iosize1);
>> + if (brdp->iosize2 > 0)
>> + release_region(brdp->ioaddr2, brdp->iosize2);
>
> pci_release_region() maybe? This would be a conversion that actually would
> make the code more readable. Since this code path looks PCI only...
I'm definitely going to do this in the next patch serie.
>> - return(0);
>> + stl_brds[brdp->brdnr] = NULL;
>> + kfree(brdp);
>> }
>>
>> -#endif
>> +static struct pci_driver stl_pcidriver = {
>> + .name = "stallion",
>> + .id_table = stl_pcibrds,
>> + .probe = stl_pciprobe,
>> + .remove = __devexit_p(stl_pciremove)
>> +};
>>
>>
>> /**************************************************************************
>> ***/
>>
>> @@ -2537,9 +2549,6 @@ static int __init stl_initbrds(void)
>> * line options or auto-detected on the PCI bus.
>> */
>> stl_argbrds();
>> -#ifdef CONFIG_PCI
>> - stl_findpcibrds();
>> -#endif
>>
>> return(0);
>> }
>> @@ -4778,7 +4787,7 @@ static void stl_sc26198otherisr(struct s
>> */
>> static int __init stallion_module_init(void)
>> {
>> - unsigned int i;
>> + unsigned int i, retval;
>>
>> printk(KERN_INFO "%s: version %s\n", stl_drvtitle, stl_drvversion);
>>
>> @@ -4787,6 +4796,10 @@ static int __init stallion_module_init(v
>>
>> stl_initbrds();
>>
>> + retval = pci_register_driver(&stl_pcidriver);
>> + if (retval)
>> + goto err;
>> +
>> stl_serial = alloc_tty_driver(STL_MAXBRDS * STL_MAXPORTS);
>> if (!stl_serial)
>
> pci_unregister_driver() here?
Good catch, I'll post a fix (As I see it now, stl_initbrds should be checked for
the returned value too.). I have no idea, why did I omit this changes in fail
paths patch :/.
thanks,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E