And yet more PCI fixes for 2.5.70

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

ChangeSet 1.1395, 2003/06/10 14:16:41-07:00, [email protected]

[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
@@ -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 {

ChangeSet 1.1396, 2003/06/10 14:17:24-07:00, [email protected]

[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
@@ -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 {

ChangeSet 1.1397, 2003/06/10 14:17:50-07:00, [email protected]

[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
@@ -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
@@ -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; }

ChangeSet 1.1398, 2003/06/10 16:33:42-07:00, [email protected]

[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
@@ -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)
 {

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.

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

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

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

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

Is this a PCI box is a sane thing to ask

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)