Re: pci_get_device_reverse(), why does Calgary need this?
On Fri, Feb 15, 2008 at 10:28:22AM -0800, Greg KH wrote: > That would be nice. Muli, want to make a patch for this? Sure, I'll put something together. Cheers, Muli - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Fri, Feb 15, 2008 at 04:31:40PM +0100, Sam Ravnborg wrote: > On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote: > > On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote: > > > In conclusion, our usage doesn't seem lika a good fit for the probe > > > approach, although it could probably converted provided we got the > > > ordering right with regards to regular PCI device > > > initialization. Doesn't seem to be worth the effort. > > > > After reading this, and looking at the code again, I agree. Thanks for > > the great explaination, I'll leave the code alone now :) > > Copy the nice explanation from the mail into the code as a comment > somewhere? That would be nice. Muli, want to make a patch for this? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
2008/2/15, Sam Ravnborg <[EMAIL PROTECTED]>: > On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote: > > On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote: > > > In conclusion, our usage doesn't seem lika a good fit for the probe > > > approach, although it could probably converted provided we got the > > > ordering right with regards to regular PCI device > > > initialization. Doesn't seem to be worth the effort. > > > > After reading this, and looking at the code again, I agree. Thanks for > > the great explaination, I'll leave the code alone now :) > > > Copy the nice explanation from the mail into the code as a comment > somewhere? > Sam I think it is good to do so. xueyong > > - > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote: > On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote: > > In conclusion, our usage doesn't seem lika a good fit for the probe > > approach, although it could probably converted provided we got the > > ordering right with regards to regular PCI device > > initialization. Doesn't seem to be worth the effort. > > After reading this, and looking at the code again, I agree. Thanks for > the great explaination, I'll leave the code alone now :) Copy the nice explanation from the mail into the code as a comment somewhere? Sam - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote: > In conclusion, our usage doesn't seem lika a good fit for the probe > approach, although it could probably converted provided we got the > ordering right with regards to regular PCI device > initialization. Doesn't seem to be worth the effort. After reading this, and looking at the code again, I agree. Thanks for the great explaination, I'll leave the code alone now :) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Thu, Feb 14, 2008 at 11:17:03PM -0800, Greg KH wrote: > Hm, that's wierd. I thought I got something, until I realized that > you are doing a lot of logic before you ever even determine that > your hardware is present in the system. Why are you calling > calgary_locate_bbars() and doing all of that work? Or am I mising > something in the code flow here? Ok, it's all starting to come back to me. See below. > Also, it looks like you use the pci_get_device() to find the pci > device, then you do somethings, and then drop the device, never to > use it again. > > So, a traditional "probe" might not work as well, but it could be > used if you want to. We have a two stage initialization process. First, we need to be initialized very early in order to be able to allocate relatively large contiguous chunks of RAM. That requires detecting whether we're on a Calgary/CalIOC2 system very early (detect_calgary(), called from pci_iommu_alloc()). Then we have "regular" PCI initialization at a later stage (calgary_iommu_init(), called from pci_iommu_init()), which also calls calgary_locate_bbars(). Unlike a regular PCI device which has its BARs in the configuration space, we need to probe BIOS provided tables to find where our BARs are before we do anything else with the "device representation" of the IOMMU. Note that the same two-stage initialization scheme is used by all other x86 IOMMUs, for similar reasons. Now, Calgary is an isolation-capable IOMMU which has per-root-bus (as opposed to global or per-BDF) IO address space. The reason we want to access the pci_dev of each Calgary bus is to hang off of it the IOMMU tables for all devices under that bus so that we end up using the right translation table when a device asks for a DMA mapping. Hence the loop in calgary_init, which loops over all pci_dev's of Calgary busses, raises the ref count for each pci_dev to protect against hot-unplug, and hooks up the IOMMU table for everything under that bus to the pci_dev for the bus. Unlike a regular driver, our main entry points are via the DMA-API, which takes a device's pci_dev, not Calgary's. We then walk up the bus hierarchy and find the Calgary pci_dev that is an ancestor of this device, and that gives us the right IOMMU table to us. Note that this has to occur *before* PCI device initialization, since we want to turn translation on before any device will try to DMA. In conclusion, our usage doesn't seem lika a good fit for the probe approach, although it could probably converted provided we got the ordering right with regards to regular PCI device initialization. Doesn't seem to be worth the effort. Cheers, Muli - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 10:14:59AM -0800, Greg KH wrote: > On Wed, Feb 13, 2008 at 07:47:11PM +0200, Muli Ben-Yehuda wrote: > > On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote: > > > > > Is there some reason you aren't using the "real" PCI driver api here > > > and registering a pci driver for these devices? That would take the > > > whole "loop over all pci devices" logic out of the code entirely. > > > > I recall we had a reason, but I no longer recall what it was. Some > > reason the "real" PCI driver API didn't fit at the time. If someone > > were to whip up a working patch, I'd happily apply it. > > "at the time"? It's been in place since the 2.2 days :) > > Is the problem that other drivers also want access to this PCI device > for some reason? > > I'll whip up a patch for you to test with in a bit... Hm, that's wierd. I thought I got something, until I realized that you are doing a lot of logic before you ever even determine that your hardware is present in the system. Why are you calling calgary_locate_bbars() and doing all of that work? Or am I mising something in the code flow here? Also, it looks like you use the pci_get_device() to find the pci device, then you do somethings, and then drop the device, never to use it again. So, a traditional "probe" might not work as well, but it could be used if you want to. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
From: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> Subject: [PATCH] ide: mark "ide=reverse" option as obsolete - it is valid only if "Probe IDE PCI devices in the PCI bus order (DEPRECATED)" config option is used - Greg needs to remove pci_get_device_reverse() for PCI core changes Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Cc: Alan Cox <[EMAIL PROTECTED]> Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> Acked-by: Sergei Shtylyov <[EMAIL PROTECTED]> MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Thu, Feb 14, 2008 at 01:02:55AM +0100, Bartlomiej Zolnierkiewicz wrote: > On Thursday 14 February 2008, Greg KH wrote: > > On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > On Wednesday 13 February 2008, Greg KH wrote: > > > > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote: > > > > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz > > > > > wrote: > > > > > > On Wednesday 13 February 2008, Greg KH wrote: > > > > > > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > > > > > > > > Why does the calgary driver need this? Can we just use > > > > > > > > > pci_get_device() > > > > > > > > > instead? Why do you need to walk the device list backwards? > > > > > > > > > Do you get > > > > > > > > > false positives going forward? > > > > > > > > > > > > > > > > It doesn't look to be performance critical so the driver can > > > > > > > > pci_get_device until the end and use the final hit anyway. > > > > > > > > > > > > > > That would make more sense. > > > > > > > > > > > > > > > IDE reverse is more problematic but nobody seems to use it. > > > > > > > > > > > > > > I've seen two posters say they use it. I'm wondering what it is > > > > > > > really > > > > > > > solving if they use it, and why if it's really needed, scsi never > > > > > > > had to > > > > > > > implement such a hack... > > > > > > > > > > > > It is no longer solving anything, just adds more pain. ;) > > > > > > > > > > > > [ The option comes from 2.2.x (so long before LABEL=/ and > > > > > > /dev/disk/by-id/ > > > > > > became popular). Some "off-board" controllers integrated on > > > > > > motherboards > > > > > > used to appear before "on-board" IDE on PCI bus so this option > > > > > > was meant > > > > > > to preserve the legacy ordering. ] > > > > > > > > > > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus > > > > > > order > > > > > > (DEPRECATED)" config option is used it is already on its way out > > > > > > (though > > > > > > marking it as obsoleted would make it more explicit). > > > > > > > > > > > > I think that removing "ide=reverse" in 2.6.26 would be OK... > > > > > > > > > > Great, thanks for your blessing. I'll make up a patch and send it to > > > > > you for approval. > > > > > > > > How does the patch below look? I didn't want to remove the whole config > > > > option, as there is more to the logic than just the "reverse order" > > > > stuff there. > > > > > > looks fine, > > > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> > > > > Thanks. > > > > > > If you don't mind, can I take this through the PCI tree so as to allow > > > > the removal of this pci function afterwards? > > > > > > [...] > > > > > > great, could you also: > > > - rebase it on top of the patch below > > > - forward the patch below to Linus for 2.6.25 > > > > Sure, you want this to go in for .25, but not the one I just posted > > removing this option, correct? That should wait for .26? > > Yes, lets give people the final call before actually removing this option > (unless of course there is some urgent reason for removing it in .25). No, no rush from me, I'll queue this up and send it to Linus in my next batch. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Thursday 14 February 2008, Greg KH wrote: > On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote: > > On Wednesday 13 February 2008, Greg KH wrote: > > > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote: > > > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz > > > > wrote: > > > > > On Wednesday 13 February 2008, Greg KH wrote: > > > > > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > > > > > > > Why does the calgary driver need this? Can we just use > > > > > > > > pci_get_device() > > > > > > > > instead? Why do you need to walk the device list backwards? > > > > > > > > Do you get > > > > > > > > false positives going forward? > > > > > > > > > > > > > > It doesn't look to be performance critical so the driver can > > > > > > > pci_get_device until the end and use the final hit anyway. > > > > > > > > > > > > That would make more sense. > > > > > > > > > > > > > IDE reverse is more problematic but nobody seems to use it. > > > > > > > > > > > > I've seen two posters say they use it. I'm wondering what it is > > > > > > really > > > > > > solving if they use it, and why if it's really needed, scsi never > > > > > > had to > > > > > > implement such a hack... > > > > > > > > > > It is no longer solving anything, just adds more pain. ;) > > > > > > > > > > [ The option comes from 2.2.x (so long before LABEL=/ and > > > > > /dev/disk/by-id/ > > > > > became popular). Some "off-board" controllers integrated on > > > > > motherboards > > > > > used to appear before "on-board" IDE on PCI bus so this option was > > > > > meant > > > > > to preserve the legacy ordering. ] > > > > > > > > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus > > > > > order > > > > > (DEPRECATED)" config option is used it is already on its way out > > > > > (though > > > > > marking it as obsoleted would make it more explicit). > > > > > > > > > > I think that removing "ide=reverse" in 2.6.26 would be OK... > > > > > > > > Great, thanks for your blessing. I'll make up a patch and send it to > > > > you for approval. > > > > > > How does the patch below look? I didn't want to remove the whole config > > > option, as there is more to the logic than just the "reverse order" > > > stuff there. > > > > looks fine, > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> > > Thanks. > > > > If you don't mind, can I take this through the PCI tree so as to allow > > > the removal of this pci function afterwards? > > > > [...] > > > > great, could you also: > > - rebase it on top of the patch below > > - forward the patch below to Linus for 2.6.25 > > Sure, you want this to go in for .25, but not the one I just posted > removing this option, correct? That should wait for .26? Yes, lets give people the final call before actually removing this option (unless of course there is some urgent reason for removing it in .25). Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote: > On Wednesday 13 February 2008, Greg KH wrote: > > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote: > > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > > On Wednesday 13 February 2008, Greg KH wrote: > > > > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > > > > > > Why does the calgary driver need this? Can we just use > > > > > > > pci_get_device() > > > > > > > instead? Why do you need to walk the device list backwards? Do > > > > > > > you get > > > > > > > false positives going forward? > > > > > > > > > > > > It doesn't look to be performance critical so the driver can > > > > > > pci_get_device until the end and use the final hit anyway. > > > > > > > > > > That would make more sense. > > > > > > > > > > > IDE reverse is more problematic but nobody seems to use it. > > > > > > > > > > I've seen two posters say they use it. I'm wondering what it is > > > > > really > > > > > solving if they use it, and why if it's really needed, scsi never had > > > > > to > > > > > implement such a hack... > > > > > > > > It is no longer solving anything, just adds more pain. ;) > > > > > > > > [ The option comes from 2.2.x (so long before LABEL=/ and > > > > /dev/disk/by-id/ > > > > became popular). Some "off-board" controllers integrated on > > > > motherboards > > > > used to appear before "on-board" IDE on PCI bus so this option was > > > > meant > > > > to preserve the legacy ordering. ] > > > > > > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order > > > > (DEPRECATED)" config option is used it is already on its way out (though > > > > marking it as obsoleted would make it more explicit). > > > > > > > > I think that removing "ide=reverse" in 2.6.26 would be OK... > > > > > > Great, thanks for your blessing. I'll make up a patch and send it to > > > you for approval. > > > > How does the patch below look? I didn't want to remove the whole config > > option, as there is more to the logic than just the "reverse order" > > stuff there. > > looks fine, > > Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> Thanks. > > If you don't mind, can I take this through the PCI tree so as to allow > > the removal of this pci function afterwards? > > [...] > > great, could you also: > - rebase it on top of the patch below > - forward the patch below to Linus for 2.6.25 Sure, you want this to go in for .25, but not the one I just posted removing this option, correct? That should wait for .26? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wednesday 13 February 2008, Greg KH wrote:
> On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote:
> > > > > > Why does the calgary driver need this? Can we just use
> > > > > > pci_get_device()
> > > > > > instead? Why do you need to walk the device list backwards? Do
> > > > > > you get
> > > > > > false positives going forward?
> > > > >
> > > > > It doesn't look to be performance critical so the driver can
> > > > > pci_get_device until the end and use the final hit anyway.
> > > >
> > > > That would make more sense.
> > > >
> > > > > IDE reverse is more problematic but nobody seems to use it.
> > > >
> > > > I've seen two posters say they use it. I'm wondering what it is really
> > > > solving if they use it, and why if it's really needed, scsi never had to
> > > > implement such a hack...
> > >
> > > It is no longer solving anything, just adds more pain. ;)
> > >
> > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > > became popular). Some "off-board" controllers integrated on
> > > motherboards
> > > used to appear before "on-board" IDE on PCI bus so this option was meant
> > > to preserve the legacy ordering. ]
> > >
> > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > > (DEPRECATED)" config option is used it is already on its way out (though
> > > marking it as obsoleted would make it more explicit).
> > >
> > > I think that removing "ide=reverse" in 2.6.26 would be OK...
> >
> > Great, thanks for your blessing. I'll make up a patch and send it to
> > you for approval.
>
> How does the patch below look? I didn't want to remove the whole config
> option, as there is more to the logic than just the "reverse order"
> stuff there.
looks fine,
Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
> If you don't mind, can I take this through the PCI tree so as to allow
> the removal of this pci function afterwards?
[...]
great, could you also:
- rebase it on top of the patch below
- forward the patch below to Linus for 2.6.25
?
From: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Subject: [PATCH] ide: mark "ide=reverse" option as obsolete
- it is valid only if "Probe IDE PCI devices in the PCI bus order
(DEPRECATED)" config option is used
- Greg needs to remove pci_get_device_reverse() for PCI core changes
Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]>
Cc: Alan Cox <[EMAIL PROTECTED]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
---
drivers/ide/ide.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: b/drivers/ide/ide.c
===
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -1038,7 +1038,7 @@ static int __init ide_setup(char *s)
if (!strcmp(s, "ide=reverse")) {
ide_scan_direction = 1;
printk(" : Enabled support for IDE inverse scan order.\n");
- return 1;
+ goto obsolete_option;
}
#endif
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Wednesday 13 February 2008, Greg KH wrote:
> > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote:
> > > > > Why does the calgary driver need this? Can we just use
> > > > > pci_get_device()
> > > > > instead? Why do you need to walk the device list backwards? Do you
> > > > > get
> > > > > false positives going forward?
> > > >
> > > > It doesn't look to be performance critical so the driver can
> > > > pci_get_device until the end and use the final hit anyway.
> > >
> > > That would make more sense.
> > >
> > > > IDE reverse is more problematic but nobody seems to use it.
> > >
> > > I've seen two posters say they use it. I'm wondering what it is really
> > > solving if they use it, and why if it's really needed, scsi never had to
> > > implement such a hack...
> >
> > It is no longer solving anything, just adds more pain. ;)
> >
> > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > became popular). Some "off-board" controllers integrated on motherboards
> > used to appear before "on-board" IDE on PCI bus so this option was meant
> > to preserve the legacy ordering. ]
> >
> > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > (DEPRECATED)" config option is used it is already on its way out (though
> > marking it as obsoleted would make it more explicit).
> >
> > I think that removing "ide=reverse" in 2.6.26 would be OK...
>
> Great, thanks for your blessing. I'll make up a patch and send it to
> you for approval.
How does the patch below look? I didn't want to remove the whole config
option, as there is more to the logic than just the "reverse order"
stuff there.
If you don't mind, can I take this through the PCI tree so as to allow
the removal of this pci function afterwards?
thanks,
greg k-h
From: Greg Kroah-Hartman <[EMAIL PROTECTED]>
Subject: IDE: remove ide=reverse IDE core
This option is obsolete and can be removed safely.
It allows us to remove the pci_get_device_reverse() function from the
PCI core.
Cc: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>
---
drivers/ide/ide-scan-pci.c |9 ++---
drivers/ide/ide.c | 12
include/linux/ide.h|1 -
3 files changed, 2 insertions(+), 20 deletions(-)
--- a/drivers/ide/ide-scan-pci.c
+++ b/drivers/ide/ide-scan-pci.c
@@ -88,13 +88,8 @@ static int __init ide_scan_pcibus(void)
struct list_head *l, *n;
pre_init = 0;
- if (!ide_scan_direction)
- while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)))
- ide_scan_pcidev(dev);
- else
- while ((dev = pci_get_device_reverse(PCI_ANY_ID, PCI_ANY_ID,
-dev)))
- ide_scan_pcidev(dev);
+ while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)))
+ ide_scan_pcidev(dev);
/*
* Hand the drivers over to the PCI layer now we
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -92,10 +92,6 @@ static int system_bus_speed; /* holds wh
DEFINE_MUTEX(ide_cfg_mtx);
__cacheline_aligned_in_smp DEFINE_SPINLOCK(ide_lock);
-#ifdef CONFIG_IDEPCI_PCIBUS_ORDER
-int ide_scan_direction; /* THIS was formerly 2.2.x pci=reverse */
-#endif
-
int noautodma = 0;
#ifdef CONFIG_BLK_DEV_IDEACPI
@@ -1227,14 +1223,6 @@ static int __init ide_setup(char *s)
goto obsolete_option;
}
-#ifdef CONFIG_IDEPCI_PCIBUS_ORDER
- if (!strcmp(s, "ide=reverse")) {
- ide_scan_direction = 1;
- printk(" : Enabled support for IDE inverse scan order.\n");
- return 1;
- }
-#endif
-
#ifdef CONFIG_BLK_DEV_IDEACPI
if (!strcmp(s, "ide=noacpi")) {
//printk(" : Disable IDE ACPI support.\n");
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -988,7 +988,6 @@ extern void do_ide_request(struct reques
void ide_init_disk(struct gendisk *, ide_drive_t *);
#ifdef CONFIG_IDEPCI_PCIBUS_ORDER
-extern int ide_scan_direction;
extern int __ide_pci_register_driver(struct pci_driver *driver, struct module
*owner, const char *mod_name);
#define ide_pci_register_driver(d) __ide_pci_register_driver(d, THIS_MODULE,
KBUILD_MODNAME)
#else
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 07:47:11PM +0200, Muli Ben-Yehuda wrote: > On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote: > > > Is there some reason you aren't using the "real" PCI driver api here > > and registering a pci driver for these devices? That would take the > > whole "loop over all pci devices" logic out of the code entirely. > > I recall we had a reason, but I no longer recall what it was. Some > reason the "real" PCI driver API didn't fit at the time. If someone > were to whip up a working patch, I'd happily apply it. "at the time"? It's been in place since the 2.2 days :) Is the problem that other drivers also want access to this PCI device for some reason? I'll whip up a patch for you to test with in a bit... > > > Feel free to nuke it and walk the list forward. > > > > So something like the patch below would be fine? > > Yep, looks good. > > Acked-by: Muli Ben-Yehuda <[EMAIL PROTECTED]> thanks for reviewing this. greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote: > Is there some reason you aren't using the "real" PCI driver api here > and registering a pci driver for these devices? That would take the > whole "loop over all pci devices" logic out of the code entirely. I recall we had a reason, but I no longer recall what it was. Some reason the "real" PCI driver API didn't fit at the time. If someone were to whip up a working patch, I'd happily apply it. > > Feel free to nuke it and walk the list forward. > > So something like the patch below would be fine? Yep, looks good. Acked-by: Muli Ben-Yehuda <[EMAIL PROTECTED]> Cheers, Muli - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote: > On Wednesday 13 February 2008, Greg KH wrote: > > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > > > Why does the calgary driver need this? Can we just use pci_get_device() > > > > instead? Why do you need to walk the device list backwards? Do you get > > > > false positives going forward? > > > > > > It doesn't look to be performance critical so the driver can > > > pci_get_device until the end and use the final hit anyway. > > > > That would make more sense. > > > > > IDE reverse is more problematic but nobody seems to use it. > > > > I've seen two posters say they use it. I'm wondering what it is really > > solving if they use it, and why if it's really needed, scsi never had to > > implement such a hack... > > It is no longer solving anything, just adds more pain. ;) > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/ > became popular). Some "off-board" controllers integrated on motherboards > used to appear before "on-board" IDE on PCI bus so this option was meant > to preserve the legacy ordering. ] > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order > (DEPRECATED)" config option is used it is already on its way out (though > marking it as obsoleted would make it more explicit). > > I think that removing "ide=reverse" in 2.6.26 would be OK... Great, thanks for your blessing. I'll make up a patch and send it to you for approval. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 11:32:26AM +0200, Muli Ben-Yehuda wrote:
> On Tue, Feb 12, 2008 at 04:16:38PM -0800, Greg KH wrote:
>
> > Why does the calgary driver need this? Can we just use
> > pci_get_device() instead? Why do you need to walk the device list
> > backwards? Do you get false positives going forward?
>
> It's not strictly needed, we used it for symmetry.
symetry for what? Ah, unwinding from an error condition, that makes
sense.
Is there some reason you aren't using the "real" PCI driver api here and
registering a pci driver for these devices? That would take the whole
"loop over all pci devices" logic out of the code entirely.
> Feel free to nuke it and walk the list forward.
So something like the patch below would be fine?
thanks,
greg k-h
---
arch/x86/kernel/pci-calgary_64.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -1232,8 +1232,7 @@ static int __init calgary_init(void)
error:
do {
- dev = pci_get_device_reverse(PCI_VENDOR_ID_IBM,
-PCI_ANY_ID, dev);
+ dev = pci_get_device(PCI_VENDOR_ID_IBM, PCI_ANY_ID, dev);
if (!dev)
break;
if (!is_cal_pci_dev(dev->device))
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wednesday 13 February 2008, Greg KH wrote: > On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > > Why does the calgary driver need this? Can we just use pci_get_device() > > > instead? Why do you need to walk the device list backwards? Do you get > > > false positives going forward? > > > > It doesn't look to be performance critical so the driver can > > pci_get_device until the end and use the final hit anyway. > > That would make more sense. > > > IDE reverse is more problematic but nobody seems to use it. > > I've seen two posters say they use it. I'm wondering what it is really > solving if they use it, and why if it's really needed, scsi never had to > implement such a hack... It is no longer solving anything, just adds more pain. ;) [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/ became popular). Some "off-board" controllers integrated on motherboards used to appear before "on-board" IDE on PCI bus so this option was meant to preserve the legacy ordering. ] Since it is valid only when "Probe IDE PCI devices in the PCI bus order (DEPRECATED)" config option is used it is already on its way out (though marking it as obsoleted would make it more explicit). I think that removing "ide=reverse" in 2.6.26 would be OK... Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Tue, Feb 12, 2008 at 04:16:38PM -0800, Greg KH wrote: > Why does the calgary driver need this? Can we just use > pci_get_device() instead? Why do you need to walk the device list > backwards? Do you get false positives going forward? It's not strictly needed, we used it for symmetry. Feel free to nuke it and walk the list forward. Cheers, Muli - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
On Wed, Feb 13, 2008 at 02:17:37AM +, Alan Cox wrote: > > Why does the calgary driver need this? Can we just use pci_get_device() > > instead? Why do you need to walk the device list backwards? Do you get > > false positives going forward? > > It doesn't look to be performance critical so the driver can > pci_get_device until the end and use the final hit anyway. That would make more sense. > IDE reverse is more problematic but nobody seems to use it. I've seen two posters say they use it. I'm wondering what it is really solving if they use it, and why if it's really needed, scsi never had to implement such a hack... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_get_device_reverse(), why does Calgary need this?
> Why does the calgary driver need this? Can we just use pci_get_device() > instead? Why do you need to walk the device list backwards? Do you get > false positives going forward? It doesn't look to be performance critical so the driver can pci_get_device until the end and use the final hit anyway. IDE reverse is more problematic but nobody seems to use it. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
