2005-09-10 20:33:05

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Generated in 2.6.13-mm2 kernel version.

Signed-off-by: Jiri Slaby <[email protected]>

Repost, posted on:
02 Sep 2005

ide.h | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
--- a/include/asm-i386/ide.h
+++ b/include/asm-i386/ide.h
@@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un

static __inline__ unsigned long ide_default_io_base(int index)
{
- if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
+ struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
+ unsigned int a = !pdev;
+
+ pci_dev_put(pdev);
+
+ if (a) {
switch(index) {
case 2: return 0x1e8;
case 3: return 0x168;


2005-09-10 20:55:26

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Jiri Slaby wrote:
> diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
> --- a/include/asm-i386/ide.h
> +++ b/include/asm-i386/ide.h
> @@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un
>
> static __inline__ unsigned long ide_default_io_base(int index)
> {
> - if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
> + struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
> + unsigned int a = !pdev;
> +
> + pci_dev_put(pdev);


Looks like we need to resurrect pci_present() from the ancient past.

Jeff


2005-09-10 21:20:05

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sat, Sep 10, 2005 at 04:55:10PM -0400, Jeff Garzik wrote:
> Jiri Slaby wrote:
> >diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
> >--- a/include/asm-i386/ide.h
> >+++ b/include/asm-i386/ide.h
> >@@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un
> >
> > static __inline__ unsigned long ide_default_io_base(int index)
> > {
> >- if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
> >+ struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
> >+ unsigned int a = !pdev;
> >+
> >+ pci_dev_put(pdev);
>
>
> Looks like we need to resurrect pci_present() from the ancient past.

Heh, ick, no :)

Jiri, any other way to do this instead?

thanks,

greg k-h

2005-09-10 21:28:54

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Greg KH napsal(a):
> On Sat, Sep 10, 2005 at 04:55:10PM -0400, Jeff Garzik wrote:
>
>>Jiri Slaby wrote:
>>
>>>diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
>>>--- a/include/asm-i386/ide.h
>>>+++ b/include/asm-i386/ide.h
>>>@@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un
>>>
>>>static __inline__ unsigned long ide_default_io_base(int index)
>>>{
>>>- if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
>>>+ struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
>>>+ unsigned int a = !pdev;
>>>+
>>>+ pci_dev_put(pdev);
>>
>>
>>Looks like we need to resurrect pci_present() from the ancient past.
>
>
> Heh, ick, no :)
>
> Jiri, any other way to do this instead?
I have no idea, how to do it more elegant. So hint me a little bit...

thanks,
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10

2005-09-10 21:40:00

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Greg KH wrote:
> On Sat, Sep 10, 2005 at 04:55:10PM -0400, Jeff Garzik wrote:
>
>>Jiri Slaby wrote:
>>
>>>diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
>>>--- a/include/asm-i386/ide.h
>>>+++ b/include/asm-i386/ide.h
>>>@@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un
>>>
>>>static __inline__ unsigned long ide_default_io_base(int index)
>>>{
>>>- if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
>>>+ struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
>>>+ unsigned int a = !pdev;
>>>+
>>>+ pci_dev_put(pdev);
>>
>>
>>Looks like we need to resurrect pci_present() from the ancient past.
>
>
> Heh, ick, no :)
>
> Jiri, any other way to do this instead?

Look at what the IDE code is trying to do. All it cares about is
whether -any PCI device at all- is present, a boolean value.

Jeff


2005-09-10 22:33:42

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sat, Sep 10, 2005 at 05:39:52PM -0400, Jeff Garzik wrote:
> Look at what the IDE code is trying to do. All it cares about is
> whether -any PCI device at all- is present, a boolean value.

Why not change it to query whether any IDE device is present, perhaps
using pci_get_class()?

2005-09-10 23:35:26

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Matthew Wilcox wrote:
> On Sat, Sep 10, 2005 at 05:39:52PM -0400, Jeff Garzik wrote:
>
>>Look at what the IDE code is trying to do. All it cares about is
>>whether -any PCI device at all- is present, a boolean value.
>
>
> Why not change it to query whether any IDE device is present, perhaps
> using pci_get_class()?

Because that's not what the code is attempting to discover.

Jeff



2005-09-10 23:41:24

by Alan

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sad, 2005-09-10 at 14:19 -0700, Greg KH wrote:
> > Looks like we need to resurrect pci_present() from the ancient past.
>
> Heh, ick, no :)
>
> Jiri, any other way to do this instead?

IDE really does want to know if you have a PCI bus of any kind attached.
Perhaps pci_present should really come back - its better than hiding the
gory details in drivers

2005-09-11 00:28:08

by Grant Grundler

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sat, Sep 10, 2005 at 07:35:10PM -0400, Jeff Garzik wrote:
> >Why not change it to query whether any IDE device is present, perhaps
> >using pci_get_class()?
>
> Because that's not what the code is attempting to discover.

If ide_scan_pcibus() finds any pci device, it calls ide_scan_pcidev().
ide_scan_pcidev() only seems to handle PCI devices.
Are you saying there are PCI IDE devices out there that
don't advertise PCI_CLASS_STORAGE_IDE?

thanks,
grant

2005-09-11 00:44:16

by Alan

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sad, 2005-09-10 at 18:34 -0600, Grant Grundler wrote:
> If ide_scan_pcibus() finds any pci device, it calls ide_scan_pcidev().
> ide_scan_pcidev() only seems to handle PCI devices.
> Are you saying there are PCI IDE devices out there that
> don't advertise PCI_CLASS_STORAGE_IDE?

Lots of them. We also want to know if PCI is present so we can know
whether to do the IDE tertiary scan which isn't safe on a PCI bus box.

2005-09-11 00:48:07

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

Grant Grundler wrote:
> On Sat, Sep 10, 2005 at 07:35:10PM -0400, Jeff Garzik wrote:
>
>>>Why not change it to query whether any IDE device is present, perhaps
>>>using pci_get_class()?
>>
>>Because that's not what the code is attempting to discover.
>
>
> If ide_scan_pcibus() finds any pci device, it calls ide_scan_pcidev().
> ide_scan_pcidev() only seems to handle PCI devices.
> Are you saying there are PCI IDE devices out there that
> don't advertise PCI_CLASS_STORAGE_IDE?

The code is not searching for PCI devices. The code is searching for...
precisely what it is searching for: presence of PCI in the system.

Jeff



2005-09-11 01:18:23

by Grant Grundler

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sun, Sep 11, 2005 at 02:06:56AM +0100, Alan Cox wrote:
> On Sad, 2005-09-10 at 18:34 -0600, Grant Grundler wrote:
> > If ide_scan_pcibus() finds any pci device, it calls ide_scan_pcidev().
> > ide_scan_pcidev() only seems to handle PCI devices.
> > Are you saying there are PCI IDE devices out there that
> > don't advertise PCI_CLASS_STORAGE_IDE?
>
> Lots of them. We also want to know if PCI is present so we can know
> whether to do the IDE tertiary scan which isn't safe on a PCI bus box.

ah ok. I'm not seeing where that happens.

Anyway, pci_present() could be as simple as "(pci_root_buses!=NULL)".

ide_system_bus_speed() in drivers/ide/ide.c might want this too.


BTW, I'm not convinced the current code does *exactly* what you want.
Hypothetically, we can have a PCI bus but no PCI devices.
Maybe no such system exists but I wouldn't bet on it.


thanks,
grant

2005-09-11 01:30:10

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sun, Sep 11, 2005 at 02:06:56AM +0100, Alan Cox wrote:
> On Sad, 2005-09-10 at 18:34 -0600, Grant Grundler wrote:
> > If ide_scan_pcibus() finds any pci device, it calls ide_scan_pcidev().
> > ide_scan_pcidev() only seems to handle PCI devices.
> > Are you saying there are PCI IDE devices out there that
> > don't advertise PCI_CLASS_STORAGE_IDE?
>
> Lots of them. We also want to know if PCI is present so we can know
> whether to do the IDE tertiary scan which isn't safe on a PCI bus box.

surely this is worthy of a comment in the code. there's at least 3
people on the cc who're confused bby what it's for.

2005-09-12 09:53:06

by Alan

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Sad, 2005-09-10 at 19:30 -0600, Matthew Wilcox wrote:
> > Lots of them. We also want to know if PCI is present so we can know
> > whether to do the IDE tertiary scan which isn't safe on a PCI bus box.
>
> surely this is worthy of a comment in the code. there's at least 3
> people on the cc who're confused bby what it's for.

Thats because someone removed the obvious pci_present() function some
time ago.

Alan

2005-09-12 11:21:43

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Mon, Sep 12, 2005 at 11:17:21AM +0100, Alan Cox wrote:
> On Sad, 2005-09-10 at 19:30 -0600, Matthew Wilcox wrote:
> > > Lots of them. We also want to know if PCI is present so we can know
> > > whether to do the IDE tertiary scan which isn't safe on a PCI bus box.
> >
> > surely this is worthy of a comment in the code. there's at least 3
> > people on the cc who're confused bby what it's for.
>
> Thats because someone removed the obvious pci_present() function some
> time ago.

Huh? Even if we had pci_present(), it still wouldn't be obvious that a
tertiary scan is unsafe on a PCI-based box.

2005-09-12 12:10:30

by Alan

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On Llu, 2005-09-12 at 05:21 -0600, Matthew Wilcox wrote:
> > > surely this is worthy of a comment in the code. there's at least 3
> > > people on the cc who're confused bby what it's for.
> >
> > Thats because someone removed the obvious pci_present() function some
> > time ago.
>
> Huh? Even if we had pci_present(), it still wouldn't be obvious that a
> tertiary scan is unsafe on a PCI-based box.

Ah sorry - I misunderstood what you meant was not obvious. I sent in the
original patch for this so I'll send Bartlomiej a comment update to go
with it.

2005-10-06 22:24:54

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] include: pci_find_device remove (include/asm-i386/ide.h)

On 9/10/05, Jeff Garzik <[email protected]> wrote:
> Jiri Slaby wrote:
> > diff --git a/include/asm-i386/ide.h b/include/asm-i386/ide.h
> > --- a/include/asm-i386/ide.h
> > +++ b/include/asm-i386/ide.h
> > @@ -41,7 +41,12 @@ static __inline__ int ide_default_irq(un
> >
> > static __inline__ unsigned long ide_default_io_base(int index)
> > {
> > - if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
> > + struct pci_dev *pdev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, NULL);
> > + unsigned int a = !pdev;
> > +
> > + pci_dev_put(pdev);
>
>
> Looks like we need to resurrect pci_present() from the ancient past.
So, what was the result of this debate? I can't see any solution in
that thread, not even in 2.6.14-rc2-mm2.

thanks,
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
B67499670407CE62ACC8 22A032CC55C339D47A7E