Skip to contentSkip to conversation

i6300esb.c uses pci_find_device

PATCH New Jiri Slaby 3 participants 14 messages Aug 8, 2005 – Aug 16, 2005

Subsystems (4)
Documentation PCI Char and misc drivers Parallel port
lore
Share & tools
lore

Jiri Slaby

author
#0 lore raw

PATCH 1/1 i6300esb.c uses pci_find_device

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1

Signed-off-by Jiri Slaby <[email protected]>
drivers/char/watchdog/i6300esb.c +1 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
         *      Find the PCI device
         */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                if (pci_match_id(esb_pci_tbl, dev)) {
                        esb_pci = dev;
                        break;
                }
-        }
 
        if (esb_pci) {
        	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:

		pci_release_region(esb_pci, 0);
err_disable:
		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);
	}
out:
	return 0;
@@ -481,6 +481,7 @@ err_unmap:
	pci_release_region(esb_pci, 0);
/* err_disable: */
	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
/* out: */
        return ret;
}
@@ -497,6 +498,7 @@ static void __exit watchdog_cleanup (voi
	iounmap(BASEADDR);
	pci_release_region(esb_pci, 0);
	pci_disable_device(esb_pci);

+ pci_dev_put(esb_pci);
}

module_init(watchdog_init);

author
#1 lore raw

PATCH 1/1 removes pci_find_device from i6300esb.c

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
drivers/char/watchdog/i6300esb.c +4 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
          *      Find the PCI device
          */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                 if (pci_match_id(esb_pci_tbl, dev)) {
                         esb_pci = dev;
                         break;
                 }
-        }
 
         if (esb_pci) {
         	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);
 err_disable:
 		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);
 	}
 out:
 	return 0;

@@ -481,6 +481,7 @@ err_unmap:
 	pci_release_region(esb_pci, 0);
 /* err_disable: */
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 /* out: */
         return ret;
 }

@@ -497,6 +498,7 @@ static void __exit watchdog_cleanup (voi
 	iounmap(BASEADDR);
 	pci_release_region(esb_pci, 0);
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 }
 
 module_init(watchdog_init);
#2 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

Quoted 2 lines · Jiri Slaby

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Can you please route all of these changes through me? It's some tricky code to get right, and lots of people have failed in the past, I don't want this to be applied without lots of review.

So, care to resend all of your pci changes, including the documentation ones, to me?

thanks,

greg k-h

author
#3 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

Quoted 2 lines · Greg KH

So, care to resend all of your pci changes, including the documentation ones, to me?

Sure:

This removes very old functions from pci docs, which aren't no longer in the kernel.

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
Documentation/pci.txt +0 -14
@@ -266,20 +266,6 @@ port an old driver to the new PCI interf
 in the kernel as they aren't compatible with hotplug or PCI domains or
 having sane locking.
 
-pcibios_present() and		Since ages, you don't need to test presence
-pci_present()			of PCI subsystem when trying to talk to it.
-				If it's not there, the list of PCI devices
-				is empty and all functions for searching for
-				devices just return NULL.
-pcibios_(read|write)_*		Superseded by their pci_(read|write)_*
-				counterparts.
-pcibios_find_*			Superseded by their pci_get_* counterparts.
-pci_for_each_dev()		Superseded by pci_get_device()
-pci_for_each_dev_reverse()	Superseded by pci_find_device_reverse()
-pci_for_each_bus()		Superseded by pci_find_next_bus()
 pci_find_device()		Superseded by pci_get_device()
 pci_find_subsys()		Superseded by pci_get_subsys()
 pci_find_slot()			Superseded by pci_get_slot()
-pcibios_find_class()		Superseded by pci_get_class()
-pci_find_class()		Superseded by pci_get_class()
-pci_(read|write)_*_nodev()	Superseded by pci_bus_(read|write)_*()

------------------
This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
drivers/char/watchdog/i6300esb.c +4 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
          *      Find the PCI device
          */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                 if (pci_match_id(esb_pci_tbl, dev)) {
                         esb_pci = dev;
                         break;
                 }
-        }
 
         if (esb_pci) {
         	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);
 err_disable:
 		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);
 	}
 out:
 	return 0;

@@ -481,6 +481,7 @@ err_unmap:
 	pci_release_region(esb_pci, 0);
 /* err_disable: */
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 /* out: */
         return ret;
 }

@@ -497,6 +498,7 @@ static void __exit watchdog_cleanup (voi
 	iounmap(BASEADDR);
 	pci_release_region(esb_pci, 0);
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 }
 
 module_init(watchdog_init);

------------------
This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev).

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
drivers/parport/parport_pc.c +1 -1
@@ -3007,7 +3007,7 @@ static int __init parport_pc_init_superi
 	struct pci_dev *pdev = NULL;
 	int ret = 0;
 
-	while ((pdev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pdev)) != NULL) {
+	for_each_pci_dev(pdev) {
 		id = pci_match_id(parport_pc_pci_tbl, pdev);
 		if (id == NULL || id->driver_data >= last_sio)
 			continue;

------------------ [this was refused by you:]
This marks these functions as deprecated not to use in latest drivers (it doesn't use reference counts and the device returned by it can disappear in any time).

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
include/linux/pci.h +4 -2
@@ -328,9 +328,11 @@ void pci_setup_cardbus(struct pci_bus *b
 
 /* Generic PCI functions exported to card drivers */
 
-struct pci_dev *pci_find_device (unsigned int vendor, unsigned int device, const struct pci_dev *from);
+struct pci_dev *pci_find_device (unsigned int vendor, unsigned int device,
+	const struct pci_dev *from) __deprecated;
 struct pci_dev *pci_find_device_reverse (unsigned int vendor, unsigned int device, const struct pci_dev *from);
-struct pci_dev *pci_find_slot (unsigned int bus, unsigned int devfn);
+struct pci_dev *pci_find_slot (unsigned int bus, unsigned int devfn)
+	__deprecated;
 int pci_find_capability (struct pci_dev *dev, int cap);
 int pci_find_ext_capability (struct pci_dev *dev, int cap);
 struct pci_bus * pci_find_next_bus(const struct pci_bus *from);
#4 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

Quoted 4 lines · Jiri Slaby
Quoted 2 lines · Greg KH

So, care to resend all of your pci changes, including the documentation ones, to me?

Sure:

Um, one patch per email please.

Doesn't anyone read Documentation/SubmittingPatches anymore...

thanks,

greg k-h

author
#5 lore raw

cover pci_find_device patches

Quoted 3 lines

Um, one patch per email please.

Doesn't anyone read Documentation/SubmittingPatches anymore...

I did. But I only thought... Let it be. Sorry.

Here are they. The two, which you haven't accepted (one, which marks deprecation isn't included because of refusing it and docpci has been added to your tree).

regards,
jiri

author
#6 lore raw

PATCH 1/2 removes pci_find_device from i6300esb.c

In reply to Greg KH

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
drivers/char/watchdog/i6300esb.c +4 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
          *      Find the PCI device
          */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                 if (pci_match_id(esb_pci_tbl, dev)) {
                         esb_pci = dev;
                         break;
                 }
-        }
 
         if (esb_pci) {
         	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);
 err_disable:
 		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);
 	}
 out:
 	return 0;

@@ -481,6 +481,7 @@ err_unmap:
 	pci_release_region(esb_pci, 0);
 /* err_disable: */
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 /* out: */
         return ret;
 }

@@ -497,6 +498,7 @@ static void __exit watchdog_cleanup (voi
 	iounmap(BASEADDR);
 	pci_release_region(esb_pci, 0);
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 }
 
 module_init(watchdog_init);
author
#7 lore raw

PATCH 1/1 removes pci_find_device from i6300esb.c

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1 kernel version.

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

This is repost, the patch was posted yet:
8 Aug 2005

drivers/char/watchdog/i6300esb.c +4 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
          *      Find the PCI device
          */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                 if (pci_match_id(esb_pci_tbl, dev)) {
                         esb_pci = dev;
                         break;
                 }
-        }
 
         if (esb_pci) {
         	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);
 err_disable:
 		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);
 	}
 out:
 	return 0;

@@ -481,6 +481,7 @@ err_unmap:
 	pci_release_region(esb_pci, 0);
 /* err_disable: */
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 /* out: */
         return ret;
 }

@@ -497,6 +498,7 @@ static void __exit watchdog_cleanup (voi
 	iounmap(BASEADDR);
 	pci_release_region(esb_pci, 0);
 	pci_disable_device(esb_pci);
+	pci_dev_put(esb_pci);
 }
 
 module_init(watchdog_init);
#8 lore raw

Re: [PATCH] removes pci_find_device from i6300esb.c

Quoted 9 lines · Jiri Slaby

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev) in i6300esb watchdog card with appropriate adding pci_dev_put.

Generated in 2.6.13-rc5-mm1 kernel version.

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

This is repost, the patch was posted yet:
8 Aug 2005

I can't take this as the driver is only in the -mm tree, not mainline. Andrew will have to pick it up (if it's even correct, haven't verified it or not...)

thanks,

greg k-h

author
#9 lore raw

PATCH 2/2 removes pci_find_device from parport_pc.c

In reply to Greg KH

[Andrew has added this into his tree yet.]

This patch changes pci_find_device to pci_get_device (encapsulated in for_each_pci_dev).

Generated in 2.6.13-rc5-mm1 kernel version.

Signed-off-by Jiri Slaby <[email protected]>
drivers/parport/parport_pc.c +1 -1
@@ -3007,7 +3007,7 @@ static int __init parport_pc_init_superi
 	struct pci_dev *pdev = NULL;
 	int ret = 0;
 
-	while ((pdev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pdev)) != NULL) {
+	for_each_pci_dev(pdev) {
 		id = pci_match_id(parport_pc_pci_tbl, pdev);
 		if (id == NULL || id->driver_data >= last_sio)
 			continue;
#10 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

In reply to Jiri Slaby
Quoted 15 lines · Jiri Slaby

--- a/drivers/char/watchdog/i6300esb.c
+++ b/drivers/char/watchdog/i6300esb.c
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
* Find the PCI device
*/

- while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+ for_each_pci_dev(dev)

                  if (pci_match_id(esb_pci_tbl, dev)) {
                          esb_pci = dev;
                          break;
                  }

- }

Quoted 7 lines
          if (esb_pci) {
          	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
pci_release_region(esb_pci, 0);
err_disable:
pci_disable_device(esb_pci);
+ pci_dev_put(esb_pci);

That doesn't look right. Each iteration of for_each_pci_dev() needs a pci_dev_put(), not just the final one.

author
#11 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

Andrew Morton napsal(a):

Quoted 17 lines
Quoted 14 lines · Jiri Slaby
drivers/char/watchdog/i6300esb.c +1 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
          *      Find the PCI device
          */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+        for_each_pci_dev(dev)
                 if (pci_match_id(esb_pci_tbl, dev)) {
                         esb_pci = dev;
                         break;
                 }
-        }
Quoted 14 lines

drivers/char/watchdog/i6300esb.c err_release: · line 434

Quoted 9 lines
drivers/char/watchdog/i6300esb.c +1 -0
         if (esb_pci) {
         	if (pci_enable_device(esb_pci)) {

@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);
 err_disable:
 		pci_disable_device(esb_pci);
+		pci_dev_put(esb_pci);

That doesn't look right. Each iteration of for_each_pci_dev() needs a pci_dev_put(), not just the final one.

But pci_get_device do it for us on pci_get_subsys, line 249, doesn't it?

Signature · 3 lines
Jiri Slaby         www.fi.muni.cz/~xslaby
~\-/~      [email protected]      ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
#12 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

Quoted 32 lines · Jiri Slaby

Andrew Morton napsal(a):

Quoted 30 lines
Quoted 23 lines · Jiri Slaby
drivers/char/watchdog/i6300esb.c +1 -2
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
         *      Find the PCI device
         */
 
-        while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != 
 NULL) {
+        for_each_pci_dev(dev)
                if (pci_match_id(esb_pci_tbl, dev)) {
                        esb_pci = dev;
                        break;
                }
-        }
 
        if (esb_pci) {

        	if (pci_enable_device(esb_pci)) {
drivers/char/watchdog/i6300esb.c +0 -0
@@ -430,6 +429,7 @@ err_release:
 		pci_release_region(esb_pci, 0);

err_disable:

		pci_disable_device(esb_pci);

drivers/char/watchdog/i6300esb.c

Quoted 1 line
drivers/char/watchdog/i6300esb.c +1 -0
+		pci_dev_put(esb_pci);

That doesn't look right. Each iteration of for_each_pci_dev() needs a pci_dev_put(), not just the final one.

Not true, see the documentation for pci_get_device(), it's only required if you break out of the loop.

thanks,

greg k-h

author
#13 lore raw

Re: [PATCH -mm] removes pci_find_device from i6300esb.c

In reply to Andrew Morton

[lkml sorry for reposting, but my nat was on cbl blacklist again, grrr.]

Quoted 16 lines · Andrew Morton
Quoted 15 lines · Jiri Slaby

--- a/drivers/char/watchdog/i6300esb.c
+++ b/drivers/char/watchdog/i6300esb.c
@@ -368,12 +368,11 @@ static unsigned char __init esb_getdevic
* Find the PCI device
*/

- while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+ for_each_pci_dev(dev)

                  if (pci_match_id(esb_pci_tbl, dev)) {
                          esb_pci = dev;
                          break;
                  }

- }

Quoted 11 lines
Quoted 7 lines
          if (esb_pci) {
              if (pci_enable_device(esb_pci)) {
 @@ -430,6 +429,7 @@ err_release:
              pci_release_region(esb_pci, 0);
  err_disable:
              pci_disable_device(esb_pci);

+ pci_dev_put(esb_pci);

That doesn't look right. Each iteration of for_each_pci_dev() needs a pci_dev_put(), not just the final one.

But pci_get_device do it for us in pci_get_subsys, line 249 in pci/search.c, doesn't it?