Re: [PATCH] gdth: convert to PCI hotplug API
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 :-)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
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 - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
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. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
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 - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
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 - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
Comments noted for my next round of revisions (its a low priority, so definitely not this week). Good spotting, thanks! Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gdth: convert to PCI hotplug API
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;
-
Re: [PATCH] gdth: convert to PCI hotplug API
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 [EMAIL PROTECTED]
---
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
[PATCH] gdth: convert to PCI hotplug API
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);
