[BK PATCH] And yet more PCI fixes for 2.5.70

[BK PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

Hi,

And again, here's some more PCI changes against the latest 2.5.70 bk tree.
They contain the following:
	- removed the last of the usages of pci_present(), and the
	  pci_present() function.
	- added some __user markings to make sparse happy.

Please pull from:
	bk://kernel.bkbits.net/gregkh/linux/pci-2.5

thanks,

greg k-h

p.s. I'll send these as patches in response to this email to lkml for
those who want to see them.


 drivers/ide/ide.c    |    2 +-
 drivers/pci/proc.c   |    6 +++---
 drivers/pci/search.c |   11 -----------
 drivers/sbus/sbus.c  |    2 +-
 include/linux/pci.h  |    3 ---
 5 files changed, 5 insertions(+), 19 deletions(-)
-----

Greg Kroah-Hartman:
  o PCI: sparse fixups for drivers/pci/proc.c
  o PCI: pci_present() can finally be removed, as there are no more users of it
  o PCI: replace usage of pci_present() in drivers/sbus/sbus.c
  o PCI: fix up usage of pci_present in drivers/ide/ide.c

[PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

ChangeSet 1.1395, 2003/06/10 14:16:41-07:00, greg@kroah.com

[PATCH] PCI: fix up usage of pci_present in drivers/ide/ide.c


 drivers/ide/ide.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/ide/ide.c b/drivers/ide/ide.c
--- a/drivers/ide/ide.c	Tue Jun 10 17:04:13 2003
+++ b/drivers/ide/ide.c	Tue Jun 10 17:04:13 2003
@@ -332,7 +332,7 @@
 		if (idebus_parameter) {
 			/* user supplied value */
 			system_bus_speed = idebus_parameter;
-		} else if (pci_present()) {
+		} else if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) != NULL) {
 			/* safe default value for PCI */
 			system_bus_speed = 33;
 		} else {

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

ChangeSet 1.1396, 2003/06/10 14:17:24-07:00, greg@kroah.com

[PATCH] PCI: replace usage of pci_present() in drivers/sbus/sbus.c


 drivers/sbus/sbus.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/sbus/sbus.c b/drivers/sbus/sbus.c
--- a/drivers/sbus/sbus.c	Tue Jun 10 17:04:09 2003
+++ b/drivers/sbus/sbus.c	Tue Jun 10 17:04:09 2003
@@ -334,7 +334,7 @@
 		nd = prom_searchsiblings(topnd, "sbus");
 		if(nd == 0) {
 #ifdef CONFIG_PCI
-			if (!pci_present()) {
+			if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) == NULL) {
 				prom_printf("Neither SBUS nor PCI found.\n");
 				prom_halt();
 			} else {

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

ChangeSet 1.1397, 2003/06/10 14:17:50-07:00, greg@kroah.com

[PATCH] PCI: pci_present() can finally be removed, as there are no more users of it.


 drivers/pci/search.c |   11 -----------
 include/linux/pci.h  |    3 ---
 2 files changed, 14 deletions(-)


diff -Nru a/drivers/pci/search.c b/drivers/pci/search.c
--- a/drivers/pci/search.c	Tue Jun 10 17:04:04 2003
+++ b/drivers/pci/search.c	Tue Jun 10 17:04:04 2003
@@ -190,20 +190,9 @@
 	return NULL;
 }
 
-/**
- * pci_present - determine if there are any pci devices on this system
- *
- * Returns 0 if no pci devices are present, 1 if pci devices are present.
- */
-int pci_present(void)
-{
-	return !list_empty(&pci_devices);
-}
-
 EXPORT_SYMBOL(pci_find_bus);
 EXPORT_SYMBOL(pci_find_class);
 EXPORT_SYMBOL(pci_find_device);
 EXPORT_SYMBOL(pci_find_device_reverse);
 EXPORT_SYMBOL(pci_find_slot);
 EXPORT_SYMBOL(pci_find_subsys);
-EXPORT_SYMBOL(pci_present);
diff -Nru a/include/linux/pci.h b/include/linux/pci.h
--- a/include/linux/pci.h	Tue Jun 10 17:04:04 2003
+++ b/include/linux/pci.h	Tue Jun 10 17:04:04 2003
@@ -525,7 +525,6 @@
 extern struct list_head pci_root_buses;	/* list of all known PCI buses */
 extern struct list_head pci_devices;	/* list of all devices */
 
-int pci_present(void);
 void pcibios_fixup_bus(struct pci_bus *);
 int pcibios_enable_device(struct pci_dev *, int mask);
 char *pcibios_setup (char *str);
@@ -667,8 +666,6 @@
  */
 
 #ifndef CONFIG_PCI
-static inline int pci_present(void) { return 0; }
-
 #define _PCI_NOP(o,s,t) \
 	static inline int pci_##o##_config_##s (struct pci_dev *dev, int where, t val) \
 		{ return PCIBIOS_FUNC_NOT_SUPPORTED; }

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

ChangeSet 1.1398, 2003/06/10 16:33:42-07:00, greg@kroah.com

[PATCH] PCI: sparse fixups for drivers/pci/proc.c


 drivers/pci/proc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)


diff -Nru a/drivers/pci/proc.c b/drivers/pci/proc.c
--- a/drivers/pci/proc.c	Tue Jun 10 17:03:59 2003
+++ b/drivers/pci/proc.c	Tue Jun 10 17:03:59 2003
@@ -44,7 +44,7 @@
 }
 
 static ssize_t
-proc_bus_pci_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
+proc_bus_pci_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
 {
 	const struct inode *ino = file->f_dentry->d_inode;
 	const struct proc_dir_entry *dp = PDE(ino);
@@ -126,7 +126,7 @@
 }
 
 static ssize_t
-proc_bus_pci_write(struct file *file, const char *buf, size_t nbytes, loff_t *ppos)
+proc_bus_pci_write(struct file *file, const char __user *buf, size_t nbytes, loff_t *ppos)
 {
 	const struct inode *ino = file->f_dentry->d_inode;
 	const struct proc_dir_entry *dp = PDE(ino);
@@ -323,7 +323,7 @@
 {
 	struct list_head *p = v;
 	(*pos)++;
-	return p->next != &pci_devices ? p->next : NULL;
+	return p->next != &pci_devices ? (void *)p->next : NULL;
 }
 static void pci_seq_stop(struct seq_file *m, void *v)
 {

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Alan Cox]

On Mer, 2003-06-11 at 01:11, Greg KH wrote:
>  			/* user supplied value */
>  			system_bus_speed = idebus_parameter;
> -		} else if (pci_present()) {
> +		} else if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) != NULL) {

That is just gross. pci_present() is far more readable even if you make
it an inline in pci.h that is pci_find_device(PCI_ANY_ID, PCI_ANY_ID,
NULL)

Alan

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Christoph Hellwig]

On Wed, Jun 11, 2003 at 01:37:37PM +0100, Alan Cox wrote:
> On Mer, 2003-06-11 at 01:11, Greg KH wrote:
> >  			/* user supplied value */
> >  			system_bus_speed = idebus_parameter;
> > -		} else if (pci_present()) {
> > +		} else if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) != NULL) {
> 
> That is just gross. pci_present() is far more readable even if you make
> it an inline in pci.h that is pci_find_device(PCI_ANY_ID, PCI_ANY_ID,
> NULL)

The whole surround code seems rather bogus.  Most of the drivers actually
using this are PCI ones so we _know_ pci is present when this gets
called.  And for the few other it should probably be a per-driver
thing instead, i.e. if the device hangs of a specific pci bus use
the ide busspeed, else not.

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Dave Jones]

On Wed, Jun 11, 2003 at 01:37:37PM +0100, Alan Cox wrote:
 > On Mer, 2003-06-11 at 01:11, Greg KH wrote:
 > >  			/* user supplied value */
 > >  			system_bus_speed = idebus_parameter;
 > > -		} else if (pci_present()) {
 > > +		} else if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) != NULL) {
 > 
 > That is just gross. pci_present() is far more readable even if you make
 > it an inline in pci.h that is pci_find_device(PCI_ANY_ID, PCI_ANY_ID,
 > NULL)

That was my argument I was trying to get across with the previous
lot of ugly PCI changes.  I lost that one, and lost the will to fight
this one.

		Dave

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Greg KH]

On Wed, Jun 11, 2003 at 01:37:37PM +0100, Alan Cox wrote:
> On Mer, 2003-06-11 at 01:11, Greg KH wrote:
> >  			/* user supplied value */
> >  			system_bus_speed = idebus_parameter;
> > -		} else if (pci_present()) {
> > +		} else if (pci_find_device(PCI_ANY_ID, PCI_ANY_ID, NULL) != NULL) {
> 
> That is just gross. pci_present() is far more readable even if you make
> it an inline in pci.h that is pci_find_device(PCI_ANY_ID, PCI_ANY_ID,
> NULL)

Bartlomiej was actually the one who suggested this patch, I didn't do it
on my own :)

Anyway, there are only 2 places in the whole kernel that want a
pci_present() check, this place, and drivers/sbus/sbus.c.  sbus.c can
probably be changed to not need it at all, but I did it this way to be
safe.

So that leaves only this file.  Jeff Garzik and I talked about removing
pci_present() as it's not needed, and I think for this one case we can
live without it.  Do you want me to make the pci_present() macro earlier
in this file, so it's readable again?  I don't want to put it back into
pci.h.

thanks,

greg k-h

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Alan Cox]

On Mer, 2003-06-11 at 17:38, Greg KH wrote:
> So that leaves only this file.  Jeff Garzik and I talked about removing
> pci_present() as it's not needed, and I think for this one case we can
> live without it.  Do you want me to make the pci_present() macro earlier
> in this file, so it's readable again?  I don't want to put it back into
> pci.h.

I still think it belongs in pci.h. Its an API and the API makes sense. The
implementation may change in time so it belongs in pci.h in one spot

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Jeff Garzik]

On Wed, Jun 11, 2003 at 06:19:49PM +0100, Alan Cox wrote:
> On Mer, 2003-06-11 at 17:38, Greg KH wrote:
> > So that leaves only this file.  Jeff Garzik and I talked about removing
> > pci_present() as it's not needed, and I think for this one case we can
> > live without it.  Do you want me to make the pci_present() macro earlier
> > in this file, so it's readable again?  I don't want to put it back into
> > pci.h.
> 
> I still think it belongs in pci.h. Its an API and the API makes sense. The

Its an API that doesn't make sense.

99% of the uses can simply be eliminated (in 2.4, too).
They are entirely redundant.

The remaining two cases are really arch-specific checks that were
being done wrong anyway.  Note the history:  the definition morphed
in 2.4 from being "PCI BIOS seems to be present, so we'll assume a
PCI bus is present" to "PCI devices are present."  Neither definition
is correct for the question the remaining two cases want answered:
"Is a PCI bus present?"  Further, the IDE code calculating system
bus speed it should really be calling a PCI callback, not asking "Do
I have a PCI bus?" and making a guess...  a guess which seems wrong
in several cases, including my Dual Athlon box w/ 100% 66 Mhz PCI bus.

So, I conclude that pci_present() is wrong for all cases except one --
and that case is sparc64-specific and can be handled with arch-specific
code, I bet.

	Jeff

Re: [PATCH] And yet more PCI fixes for 2.5.70

[Alan Cox]

On Mer, 2003-06-11 at 18:46, Jeff Garzik wrote:
> Its an API that doesn't make sense.

Is this a PCI box is a sane thing to ask

> "Is a PCI bus present?"  Further, the IDE code calculating system
> bus speed it should really be calling a PCI callback, not asking "Do
> I have a PCI bus?" and making a guess...  a guess which seems wrong
> in several cases, including my Dual Athlon box w/ 100% 66 Mhz PCI bus.

Wrong. You misunderstand why this is done. We want to know for various
wackomtic old IDE devices and the key to knowing if its going to be
33Mhz is "does it have a PCI bus".

BTW if you want to be pedantic your "fix" is as broken as the original
since PCI root bridges can live on hotpluggable cards on a different
system bus 8)