Signed-off-by: Jeff Garzik <[email protected]>
---
drivers/scsi/gdth.c | 143 +++++++++++++++++++++++++++++++---------------------
1 file changed, 86 insertions(+), 57 deletions(-)
06196f50915da97bb897495863f9f084d785c1e4
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index c825239..1b53e92 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -595,85 +595,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
#endif /* CONFIG_ISA */
#ifdef CONFIG_PCI
-static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
- ushort vendor, ushort dev);
+static gdth_pci_str gdth_pcistr[MAXHA];
+static int gdth_pci_cnt;
+static bool gdth_pci_registered;
-static int __init gdth_search_pci(gdth_pci_str *pcistr)
+static bool __init gdth_search_vortex(ushort device)
{
- ushort device, cnt;
-
- TRACE(("gdth_search_pci()\n"));
-
- cnt = 0;
- for (device = 0; device <= PCI_DEVICE_ID_VORTEX_GDT6555; ++device)
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
- for (device = PCI_DEVICE_ID_VORTEX_GDT6x17RP;
- device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP; ++device)
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
- PCI_DEVICE_ID_VORTEX_GDTNEWRX);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
- PCI_DEVICE_ID_VORTEX_GDTNEWRX2);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_SRC);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_SRC_XSCALE);
- return cnt;
+ if (device <= PCI_DEVICE_ID_VORTEX_GDT6555)
+ return true;
+ if (device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP &&
+ device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP)
+ return true;
+ if (device == PCI_DEVICE_ID_VORTEX_GDTNEWRX ||
+ device == PCI_DEVICE_ID_VORTEX_GDTNEWRX2)
+ return true;
+ return false;
}
+static int gdth_pci_init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent);
+static void gdth_pci_remove_one(struct pci_dev *pdev);
+static void gdth_remove_one(gdth_ha_str *ha);
+
/* Vortex only makes RAID controllers.
* We do not really want to specify all 550 ids here, so wildcard match.
*/
-static struct pci_device_id gdthtable[] __maybe_unused = {
- {PCI_VENDOR_ID_VORTEX,PCI_ANY_ID,PCI_ANY_ID, PCI_ANY_ID},
- {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC,PCI_ANY_ID,PCI_ANY_ID},
- {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC_XSCALE,PCI_ANY_ID,PCI_ANY_ID},
- {0}
+static struct pci_device_id gdthtable[] __devinitdata = {
+ { PCI_VDEVICE(VORTEX, PCI_ANY_ID) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC_XSCALE) },
+ { } /* terminate list */
+};
+MODULE_DEVICE_TABLE(pci, gdthtable);
+
+static struct pci_driver gdth_pci_driver = {
+ .name = "gdth",
+ .id_table = gdthtable,
+ .probe = gdth_pci_init_one,
+ .remove = gdth_pci_remove_one,
};
-MODULE_DEVICE_TABLE(pci,gdthtable);
-static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
- ushort vendor, ushort device)
+static void gdth_pci_remove_one(struct pci_dev *pdev)
+{
+ gdth_ha_str *ha = pci_get_drvdata(pdev);
+
+ pci_set_drvdata(pdev, NULL);
+
+ list_del(&ha->list);
+ gdth_remove_one(ha);
+
+ pci_disable_device(pdev);
+}
+
+static int __devinit gdth_pci_init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
{
- ulong base0, base1, base2;
- struct pci_dev *pdev;
+ ushort vendor = pdev->vendor;
+ ushort device = pdev->device;
+ ulong base0, base1, base2;
+ int rc;
- TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
- *cnt, vendor, device));
+ TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
+ gdth_pci_cnt, vendor, device));
+
+ if (vendor == PCI_VENDOR_ID_VORTEX && !gdth_search_vortex(device))
+ return -ENODEV;
+
+ rc = pci_enable_device(pdev);
+ if (rc)
+ return rc;
+
+ if (gdth_pci_cnt >= MAXHA)
+ return -EBUSY;
- pdev = NULL;
- while ((pdev = pci_find_device(vendor, device, pdev))
- != NULL) {
- if (pci_enable_device(pdev))
- continue;
- if (*cnt >= MAXHA)
- return;
/* GDT PCI controller found, resources are already in pdev */
- pcistr[*cnt].pdev = pdev;
- pcistr[*cnt].irq = pdev->irq;
+ gdth_pcistr[gdth_pci_cnt].pdev = pdev;
+ gdth_pcistr[gdth_pci_cnt].irq = pdev->irq;
base0 = pci_resource_flags(pdev, 0);
base1 = pci_resource_flags(pdev, 1);
base2 = pci_resource_flags(pdev, 2);
if (device <= PCI_DEVICE_ID_VORTEX_GDT6000B || /* GDT6000/B */
device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP) { /* MPR */
if (!(base0 & IORESOURCE_MEM))
- continue;
- pcistr[*cnt].dpmem = pci_resource_start(pdev, 0);
+ return -ENODEV;
+ gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 0);
} else { /* GDT6110, GDT6120, .. */
if (!(base0 & IORESOURCE_MEM) ||
!(base2 & IORESOURCE_MEM) ||
!(base1 & IORESOURCE_IO))
- continue;
- pcistr[*cnt].dpmem = pci_resource_start(pdev, 2);
- pcistr[*cnt].io_mm = pci_resource_start(pdev, 0);
- pcistr[*cnt].io = pci_resource_start(pdev, 1);
+ return -ENODEV;
+ gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 2);
+ gdth_pcistr[gdth_pci_cnt].io_mm = pci_resource_start(pdev, 0);
+ gdth_pcistr[gdth_pci_cnt].io = pci_resource_start(pdev, 1);
}
TRACE2(("Controller found at %d/%d, irq %d, dpmem 0x%lx\n",
- pcistr[*cnt].pdev->bus->number,
- PCI_SLOT(pcistr[*cnt].pdev->devfn),
- pcistr[*cnt].irq, pcistr[*cnt].dpmem));
- (*cnt)++;
- }
+ gdth_pcistr[gdth_pci_cnt].pdev->bus->number,
+ PCI_SLOT(gdth_pcistr[gdth_pci_cnt].pdev->devfn),
+ gdth_pcistr[gdth_pci_cnt].irq,
+ gdth_pcistr[gdth_pci_cnt].dpmem));
+ gdth_pci_cnt++;
+
+ return 0;
}
static void __init gdth_sort_pci(gdth_pci_str *pcistr, int cnt)
@@ -5100,6 +5122,7 @@ static int __init gdth_pci_probe_one(gdth_pci_str *pcistr, int ctr)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+ pci_set_drvdata(ha->pdev, ha);
return 0;
out_free_coal_stat:
@@ -5198,15 +5221,16 @@ static int __init gdth_init(void)
#ifdef CONFIG_PCI
/* scanning for PCI controllers */
- {
- gdth_pci_str pcistr[MAXHA];
+ if (pci_register_driver(&gdth_pci_driver) == 0) {
int cnt,ctr;
+ gdth_pci_str *pcistr = gdth_pcistr;
- cnt = gdth_search_pci(pcistr);
+ cnt = gdth_pci_cnt;
printk("GDT-HA: Found %d PCI Storage RAID Controllers\n", cnt);
gdth_sort_pci(pcistr,cnt);
for (ctr = 0; ctr < cnt; ++ctr)
gdth_pci_probe_one(pcistr, ctr);
+ gdth_pci_registered = true;
}
#endif /* CONFIG_PCI */
@@ -5234,6 +5258,11 @@ static void __exit gdth_exit(void)
{
gdth_ha_str *ha;
+#ifdef CONFIG_PCI
+ if (gdth_pci_registered)
+ pci_unregister_driver(&gdth_pci_driver);
+#endif
+
list_for_each_entry(ha, &gdth_instances, list)
gdth_remove_one(ha);
On 02/13/2008 12:49 AM, Jeff Garzik wrote:
> Signed-off-by: Jeff Garzik <[email protected]>
> ---
> drivers/scsi/gdth.c | 143 +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 86 insertions(+), 57 deletions(-)
>
> 06196f50915da97bb897495863f9f084d785c1e4
> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> index c825239..1b53e92 100644
> --- a/drivers/scsi/gdth.c
> +++ b/drivers/scsi/gdth.c
> @@ -595,85 +595,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
> #endif /* CONFIG_ISA */
>
> #ifdef CONFIG_PCI
> -static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
> - ushort vendor, ushort dev);
> +static gdth_pci_str gdth_pcistr[MAXHA];
> +static int gdth_pci_cnt;
> +static bool gdth_pci_registered;
>
> -static int __init gdth_search_pci(gdth_pci_str *pcistr)
> +static bool __init gdth_search_vortex(ushort device)
> {
> - ushort device, cnt;
> -
> - TRACE(("gdth_search_pci()\n"));
> -
> - cnt = 0;
> - for (device = 0; device <= PCI_DEVICE_ID_VORTEX_GDT6555; ++device)
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
> - for (device = PCI_DEVICE_ID_VORTEX_GDT6x17RP;
> - device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP; ++device)
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
> - PCI_DEVICE_ID_VORTEX_GDTNEWRX);
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
> - PCI_DEVICE_ID_VORTEX_GDTNEWRX2);
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
> - PCI_DEVICE_ID_INTEL_SRC);
> - gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
> - PCI_DEVICE_ID_INTEL_SRC_XSCALE);
> - return cnt;
> + if (device <= PCI_DEVICE_ID_VORTEX_GDT6555)
> + return true;
> + if (device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP &&
> + device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP)
> + return true;
> + if (device == PCI_DEVICE_ID_VORTEX_GDTNEWRX ||
> + device == PCI_DEVICE_ID_VORTEX_GDTNEWRX2)
> + return true;
> + return false;
> }
>
> +static int gdth_pci_init_one(struct pci_dev *pdev,
> + const struct pci_device_id *ent);
> +static void gdth_pci_remove_one(struct pci_dev *pdev);
> +static void gdth_remove_one(gdth_ha_str *ha);
> +
> /* Vortex only makes RAID controllers.
> * We do not really want to specify all 550 ids here, so wildcard match.
> */
> -static struct pci_device_id gdthtable[] __maybe_unused = {
> - {PCI_VENDOR_ID_VORTEX,PCI_ANY_ID,PCI_ANY_ID, PCI_ANY_ID},
> - {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC,PCI_ANY_ID,PCI_ANY_ID},
> - {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC_XSCALE,PCI_ANY_ID,PCI_ANY_ID},
> - {0}
> +static struct pci_device_id gdthtable[] __devinitdata = {
> + { PCI_VDEVICE(VORTEX, PCI_ANY_ID) },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC) },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC_XSCALE) },
> + { } /* terminate list */
> +};
> +MODULE_DEVICE_TABLE(pci, gdthtable);
> +
> +static struct pci_driver gdth_pci_driver = {
> + .name = "gdth",
> + .id_table = gdthtable,
> + .probe = gdth_pci_init_one,
> + .remove = gdth_pci_remove_one,
__devexit_p()
> };
> -MODULE_DEVICE_TABLE(pci,gdthtable);
>
> -static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
> - ushort vendor, ushort device)
> +static void gdth_pci_remove_one(struct pci_dev *pdev)
> +{
> + gdth_ha_str *ha = pci_get_drvdata(pdev);
> +
> + pci_set_drvdata(pdev, NULL);
> +
> + list_del(&ha->list);
If you remove the card before the sort and gdth_pci_probe_one function, this
will oops. Also the adds/removes to the list are racy and may break the
gdth_instances list.
> + gdth_remove_one(ha);
And so this...
> + pci_disable_device(pdev);
> +}
> +
> +static int __devinit gdth_pci_init_one(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> {
> - ulong base0, base1, base2;
> - struct pci_dev *pdev;
> + ushort vendor = pdev->vendor;
> + ushort device = pdev->device;
> + ulong base0, base1, base2;
> + int rc;
>
> - TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
> - *cnt, vendor, device));
> + TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
> + gdth_pci_cnt, vendor, device));
> +
> + if (vendor == PCI_VENDOR_ID_VORTEX && !gdth_search_vortex(device))
> + return -ENODEV;
> +
> + rc = pci_enable_device(pdev);
> + if (rc)
> + return rc;
> +
> + if (gdth_pci_cnt >= MAXHA)
> + return -EBUSY;
> - pdev = NULL;
> - while ((pdev = pci_find_device(vendor, device, pdev))
> - != NULL) {
> - if (pci_enable_device(pdev))
> - continue;
> - if (*cnt >= MAXHA)
> - return;
> /* GDT PCI controller found, resources are already in pdev */
> - pcistr[*cnt].pdev = pdev;
> - pcistr[*cnt].irq = pdev->irq;
> + gdth_pcistr[gdth_pci_cnt].pdev = pdev;
> + gdth_pcistr[gdth_pci_cnt].irq = pdev->irq;
> base0 = pci_resource_flags(pdev, 0);
> base1 = pci_resource_flags(pdev, 1);
> base2 = pci_resource_flags(pdev, 2);
> if (device <= PCI_DEVICE_ID_VORTEX_GDT6000B || /* GDT6000/B */
> device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP) { /* MPR */
> if (!(base0 & IORESOURCE_MEM))
> - continue;
> - pcistr[*cnt].dpmem = pci_resource_start(pdev, 0);
> + return -ENODEV;
> + gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 0);
> } else { /* GDT6110, GDT6120, .. */
> if (!(base0 & IORESOURCE_MEM) ||
> !(base2 & IORESOURCE_MEM) ||
> !(base1 & IORESOURCE_IO))
> - continue;
> - pcistr[*cnt].dpmem = pci_resource_start(pdev, 2);
> - pcistr[*cnt].io_mm = pci_resource_start(pdev, 0);
> - pcistr[*cnt].io = pci_resource_start(pdev, 1);
> + return -ENODEV;
> + gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 2);
> + gdth_pcistr[gdth_pci_cnt].io_mm = pci_resource_start(pdev, 0);
> + gdth_pcistr[gdth_pci_cnt].io = pci_resource_start(pdev, 1);
> }
> TRACE2(("Controller found at %d/%d, irq %d, dpmem 0x%lx\n",
> - pcistr[*cnt].pdev->bus->number,
> - PCI_SLOT(pcistr[*cnt].pdev->devfn),
> - pcistr[*cnt].irq, pcistr[*cnt].dpmem));
> - (*cnt)++;
> - }
> + gdth_pcistr[gdth_pci_cnt].pdev->bus->number,
> + PCI_SLOT(gdth_pcistr[gdth_pci_cnt].pdev->devfn),
> + gdth_pcistr[gdth_pci_cnt].irq,
> + gdth_pcistr[gdth_pci_cnt].dpmem));
> + gdth_pci_cnt++;
> +
> + return 0;
> }
>
> static void __init gdth_sort_pci(gdth_pci_str *pcistr, int cnt)
> @@ -5100,6 +5122,7 @@ static int __init gdth_pci_probe_one(gdth_pci_str *pcistr, int ctr)
> if (error)
> goto out_free_coal_stat;
> list_add_tail(&ha->list, &gdth_instances);
> + pci_set_drvdata(ha->pdev, ha);
> return 0;
>
> out_free_coal_stat:
> @@ -5198,15 +5221,16 @@ static int __init gdth_init(void)
>
> #ifdef CONFIG_PCI
> /* scanning for PCI controllers */
> - {
> - gdth_pci_str pcistr[MAXHA];
> + if (pci_register_driver(&gdth_pci_driver) == 0) {
> int cnt,ctr;
> + gdth_pci_str *pcistr = gdth_pcistr;
>
> - cnt = gdth_search_pci(pcistr);
> + cnt = gdth_pci_cnt;
> printk("GDT-HA: Found %d PCI Storage RAID Controllers\n", cnt);
> gdth_sort_pci(pcistr,cnt);
> for (ctr = 0; ctr < cnt; ++ctr)
> gdth_pci_probe_one(pcistr, ctr);
This is not true hotplug... this should go into the probe function, right? But
it can't due to the sort? Then what this change brings to the driver? Isn't
conversion to pci_get_device better in this case (As who will gaurantee that the
register returns after calling probes for all devices in the future)?
> + gdth_pci_registered = true;
> }
> #endif /* CONFIG_PCI */
>
> @@ -5234,6 +5258,11 @@ static void __exit gdth_exit(void)
> {
> gdth_ha_str *ha;
>
> +#ifdef CONFIG_PCI
> + if (gdth_pci_registered)
> + pci_unregister_driver(&gdth_pci_driver);
> +#endif
> +
> list_for_each_entry(ha, &gdth_instances, list)
> gdth_remove_one(ha);
>
On Wed, Feb 13 2008 at 1:49 +0200, Jeff Garzik <[email protected]> wrote:
> Signed-off-by: Jeff Garzik <[email protected]>
> ---
> drivers/scsi/gdth.c | 143 +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 86 insertions(+), 57 deletions(-)
>
<snip>
below is the same exact patch rebased on my last two bugfixes.
(Already in scsi-rc-fixes)
do you intend this to be pushed into 2.6.25-rcx or this is already
for 2.6.26? Should we put this in -mm tree for testing?
Boaz
THIS IS NOT YET TESTED
---
From: Jeff Garzik <[email protected]>
Date: Wed, 13 Feb 2008 13:01:16 +0200
Subject: [PATCH] gdth: convert to PCI hotplug API
Signed-off-by: Jeff Garzik <[email protected]>
Signed-off-by: Boaz Harrosh <bharrosh@panasas>
---
drivers/scsi/gdth.c | 143 ++++++++++++++++++++++++++++++--------------------
1 files changed, 86 insertions(+), 57 deletions(-)
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 103280e..107d2c7 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -596,85 +596,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
#endif /* CONFIG_ISA */
#ifdef CONFIG_PCI
-static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
- ushort vendor, ushort dev);
+static gdth_pci_str gdth_pcistr[MAXHA];
+static int gdth_pci_cnt;
+static bool gdth_pci_registered;
-static int __init gdth_search_pci(gdth_pci_str *pcistr)
+static bool __init gdth_search_vortex(ushort device)
{
- ushort device, cnt;
-
- TRACE(("gdth_search_pci()\n"));
-
- cnt = 0;
- for (device = 0; device <= PCI_DEVICE_ID_VORTEX_GDT6555; ++device)
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
- for (device = PCI_DEVICE_ID_VORTEX_GDT6x17RP;
- device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP; ++device)
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
- PCI_DEVICE_ID_VORTEX_GDTNEWRX);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX,
- PCI_DEVICE_ID_VORTEX_GDTNEWRX2);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_SRC);
- gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_SRC_XSCALE);
- return cnt;
+ if (device <= PCI_DEVICE_ID_VORTEX_GDT6555)
+ return true;
+ if (device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP &&
+ device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP)
+ return true;
+ if (device == PCI_DEVICE_ID_VORTEX_GDTNEWRX ||
+ device == PCI_DEVICE_ID_VORTEX_GDTNEWRX2)
+ return true;
+ return false;
}
+static int gdth_pci_init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent);
+static void gdth_pci_remove_one(struct pci_dev *pdev);
+static void gdth_remove_one(gdth_ha_str *ha);
+
/* Vortex only makes RAID controllers.
* We do not really want to specify all 550 ids here, so wildcard match.
*/
-static struct pci_device_id gdthtable[] __maybe_unused = {
- {PCI_VENDOR_ID_VORTEX,PCI_ANY_ID,PCI_ANY_ID, PCI_ANY_ID},
- {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC,PCI_ANY_ID,PCI_ANY_ID},
- {PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC_XSCALE,PCI_ANY_ID,PCI_ANY_ID},
- {0}
+static struct pci_device_id gdthtable[] __devinitdata = {
+ { PCI_VDEVICE(VORTEX, PCI_ANY_ID) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC_XSCALE) },
+ { } /* terminate list */
+};
+MODULE_DEVICE_TABLE(pci, gdthtable);
+
+static struct pci_driver gdth_pci_driver = {
+ .name = "gdth",
+ .id_table = gdthtable,
+ .probe = gdth_pci_init_one,
+ .remove = gdth_pci_remove_one,
};
-MODULE_DEVICE_TABLE(pci,gdthtable);
-static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
- ushort vendor, ushort device)
+static void gdth_pci_remove_one(struct pci_dev *pdev)
+{
+ gdth_ha_str *ha = pci_get_drvdata(pdev);
+
+ pci_set_drvdata(pdev, NULL);
+
+ list_del(&ha->list);
+ gdth_remove_one(ha);
+
+ pci_disable_device(pdev);
+}
+
+static int __devinit gdth_pci_init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
{
- ulong base0, base1, base2;
- struct pci_dev *pdev;
+ ushort vendor = pdev->vendor;
+ ushort device = pdev->device;
+ ulong base0, base1, base2;
+ int rc;
- TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
- *cnt, vendor, device));
+ TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
+ gdth_pci_cnt, vendor, device));
+
+ if (vendor == PCI_VENDOR_ID_VORTEX && !gdth_search_vortex(device))
+ return -ENODEV;
+
+ rc = pci_enable_device(pdev);
+ if (rc)
+ return rc;
+
+ if (gdth_pci_cnt >= MAXHA)
+ return -EBUSY;
- pdev = NULL;
- while ((pdev = pci_find_device(vendor, device, pdev))
- != NULL) {
- if (pci_enable_device(pdev))
- continue;
- if (*cnt >= MAXHA)
- return;
/* GDT PCI controller found, resources are already in pdev */
- pcistr[*cnt].pdev = pdev;
- pcistr[*cnt].irq = pdev->irq;
+ gdth_pcistr[gdth_pci_cnt].pdev = pdev;
+ gdth_pcistr[gdth_pci_cnt].irq = pdev->irq;
base0 = pci_resource_flags(pdev, 0);
base1 = pci_resource_flags(pdev, 1);
base2 = pci_resource_flags(pdev, 2);
if (device <= PCI_DEVICE_ID_VORTEX_GDT6000B || /* GDT6000/B */
device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP) { /* MPR */
if (!(base0 & IORESOURCE_MEM))
- continue;
- pcistr[*cnt].dpmem = pci_resource_start(pdev, 0);
+ return -ENODEV;
+ gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 0);
} else { /* GDT6110, GDT6120, .. */
if (!(base0 & IORESOURCE_MEM) ||
!(base2 & IORESOURCE_MEM) ||
!(base1 & IORESOURCE_IO))
- continue;
- pcistr[*cnt].dpmem = pci_resource_start(pdev, 2);
- pcistr[*cnt].io_mm = pci_resource_start(pdev, 0);
- pcistr[*cnt].io = pci_resource_start(pdev, 1);
+ return -ENODEV;
+ gdth_pcistr[gdth_pci_cnt].dpmem = pci_resource_start(pdev, 2);
+ gdth_pcistr[gdth_pci_cnt].io_mm = pci_resource_start(pdev, 0);
+ gdth_pcistr[gdth_pci_cnt].io = pci_resource_start(pdev, 1);
}
TRACE2(("Controller found at %d/%d, irq %d, dpmem 0x%lx\n",
- pcistr[*cnt].pdev->bus->number,
- PCI_SLOT(pcistr[*cnt].pdev->devfn),
- pcistr[*cnt].irq, pcistr[*cnt].dpmem));
- (*cnt)++;
- }
+ gdth_pcistr[gdth_pci_cnt].pdev->bus->number,
+ PCI_SLOT(gdth_pcistr[gdth_pci_cnt].pdev->devfn),
+ gdth_pcistr[gdth_pci_cnt].irq,
+ gdth_pcistr[gdth_pci_cnt].dpmem));
+ gdth_pci_cnt++;
+
+ return 0;
}
static void __init gdth_sort_pci(gdth_pci_str *pcistr, int cnt)
@@ -5110,6 +5132,7 @@ static int __init gdth_pci_probe_one(gdth_pci_str *pcistr, int ctr)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+ pci_set_drvdata(ha->pdev, ha);
scsi_scan_host(shp);
@@ -5209,15 +5232,16 @@ static int __init gdth_init(void)
#ifdef CONFIG_PCI
/* scanning for PCI controllers */
- {
- gdth_pci_str pcistr[MAXHA];
+ if (pci_register_driver(&gdth_pci_driver) == 0) {
int cnt,ctr;
+ gdth_pci_str *pcistr = gdth_pcistr;
- cnt = gdth_search_pci(pcistr);
+ cnt = gdth_pci_cnt;
printk("GDT-HA: Found %d PCI Storage RAID Controllers\n", cnt);
gdth_sort_pci(pcistr,cnt);
for (ctr = 0; ctr < cnt; ++ctr)
gdth_pci_probe_one(pcistr, ctr);
+ gdth_pci_registered = true;
}
#endif /* CONFIG_PCI */
@@ -5252,6 +5276,11 @@ static void __exit gdth_exit(void)
del_timer_sync(&gdth_timer);
#endif
+#ifdef CONFIG_PCI
+ if (gdth_pci_registered)
+ pci_unregister_driver(&gdth_pci_driver);
+#endif
+
list_for_each_entry(ha, &gdth_instances, list)
gdth_remove_one(ha);
}
--
1.5.3.3
On Tue, 2008-02-12 at 18:49 -0500, Jeff Garzik wrote:
> Signed-off-by: Jeff Garzik <[email protected]>
Nice work, thanks. This is PCI only I take it. ISA and EISA look like
they'll be a tad more troublesome, so PCI only is fine.
> ---
> drivers/scsi/gdth.c | 143 +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 86 insertions(+), 57 deletions(-)
>
> 06196f50915da97bb897495863f9f084d785c1e4
> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> index c825239..1b53e92 100644
> --- a/drivers/scsi/gdth.c
> +++ b/drivers/scsi/gdth.c
> @@ -595,85 +595,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
> #endif /* CONFIG_ISA */
>
> #ifdef CONFIG_PCI
> -static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
> - ushort vendor, ushort dev);
> +static gdth_pci_str gdth_pcistr[MAXHA];
> +static int gdth_pci_cnt;
> +static bool gdth_pci_registered;
Could we get rid of these static arrays and MAXHA entirely? It should
be possible just to bung the parameters in pci_str into gdth_ha_str and
dump the arrays.
Thanks,
James
Boaz Harrosh wrote:
> do you intend this to be pushed into 2.6.25-rcx or this is already
> for 2.6.26? Should we put this in -mm tree for testing?
Not intended for 2.6.25. I just wanted to get this "in process"
somewhere, and keep this issue moving. I would definitely prefer to
have this tested before it goes to Linus.
Its a long term goal to kill pci_find_device(), and this conversion was
a side effect of that.
Jeff
Comments noted for my next round of revisions (its a low priority, so
definitely not this week). Good spotting, thanks!
Jeff
On Feb 13 2008 13:07, Boaz Harrosh wrote:
>+static struct pci_device_id gdthtable[] __devinitdata = {
>+ { PCI_VDEVICE(VORTEX, PCI_ANY_ID) },
>+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC) },
>+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC_XSCALE) },
>+ { } /* terminate list */
>+};
As usual, +const and +__devinitconst :-)
James Bottomley wrote:
>> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
>> index c825239..1b53e92 100644
>> --- a/drivers/scsi/gdth.c
>> +++ b/drivers/scsi/gdth.c
>> @@ -595,85 +595,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
>> #endif /* CONFIG_ISA */
>>
>> #ifdef CONFIG_PCI
>> -static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
>> - ushort vendor, ushort dev);
>> +static gdth_pci_str gdth_pcistr[MAXHA];
>> +static int gdth_pci_cnt;
>> +static bool gdth_pci_registered;
>
> Could we get rid of these static arrays and MAXHA entirely? It should
> be possible just to bung the parameters in pci_str into gdth_ha_str and
> dump the arrays.
I kept those array for one reason: you need it to preserve the existing
in-driver PCI device sort.
If we can eliminate the sorting, then the array can easily disappear.
I /think/ the sort can be eliminated now because we have pci=reverse,
but I have not verified that guess.
Jeff
On Fri, Feb 15, 2008 at 10:44:52AM -0500, Jeff Garzik wrote:
> I kept those array for one reason: you need it to preserve the existing
> in-driver PCI device sort.
Just get rid of it. I got rid of it for sym2 during 2.5 and very few
people have complained.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."