Skip to contentSkip to conversation

drivers/char/mxser.c : replace pci_find_device by

PATCH In review trem 4 participants 4 messages May 27, 2006 – May 29, 2006

Subsystems (3)
Arm i.MX and Layerscape Kernel janitors Char and misc drivers
lore
Share & tools
lore

trem

author
#0 lore raw

PATCH 1/1 drivers/char/mxser.c : replace pci_find_device by

From: trem <[email protected]>

This patch simply change pci_find_device by pci_get_device, because pci_find_device is deprecated. pci_dev_put has been added after the loop to decrement the counter if necessary.

Signed-off-by trem <[email protected]>

---

drivers/char/mxser.c +4 -1
index 0fb2fb9..8d235a1 100644
@@ -817,7 +817,7 @@ static int mxser_init(void)
 	index = 0;
 	b = 0;
 	while (b < n) {
-		pdev = pci_find_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
+		pdev = pci_get_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
 			if (pdev == NULL) {
 			b++;
 			continue;

@@ -854,6 +854,9 @@ static int mxser_init(void)
 			m++;
 		}
 	}
+	if (pdev != NULL) {
+		pci_dev_put(pdev);
+	}
 #endif
 
 	retval = tty_register_driver(mxvar_sdriver);

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw
Quoted 5 lines · trem

From: trem <[email protected]>

This patch simply change pci_find_device by pci_get_device, because pci_find_device is deprecated. pci_dev_put has been added after the loop to decrement the counter if necessary.

Can't the pci_dev_put() be unconditional? pci_dev_put() does the checking for NULL itself, it would seem. There are two cases that are (logicall, not practically, possible):

1) We don't even enter the while loop, which can only occur if someone alters mxser_pcibrds[]. In this case, we'll call pci_dev_put(pdev), pdev = NULL, and return from the call immediately.

2) We need to clean-up the last iteration of the loop. In this case pci_dev_put(pdev) will do the right thing.

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to trem
Quoted 15 lines · Nish Aravamudan
Quoted 5 lines · trem

From: trem <[email protected]>

This patch simply change pci_find_device by pci_get_device, because pci_find_device is deprecated. pci_dev_put has been added after the loop to decrement the counter if necessary.

Can't the pci_dev_put() be unconditional? pci_dev_put() does the checking for NULL itself, it would seem. There are two cases that are (logicall, not practically, possible):

1) We don't even enter the while loop, which can only occur if someone alters mxser_pcibrds[]. In this case, we'll call pci_dev_put(pdev), pdev == NULL, and return from the call immediately.

Quoted 6 lines

2) We need to clean-up the last iteration of the loop. In this case pci_dev_put(pdev) will do the right thing.

Thanks,
Nish

Hi

you're right pci_dev_put check if the parameter is NULL, so checking it before
is useless. It just avoid to call the function.

I send a new patch

trem

drivers/char/mxser.c +2 -1
index 0fb2fb9..28298dc 100644
@@ -817,7 +817,7 @@ static int mxser_init(void)
 	index = 0;
 	b = 0;
 	while (b < n) {
-		pdev = pci_find_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
+		pdev = pci_get_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
 			if (pdev == NULL) {
 			b++;
 			continue;

@@ -854,6 +854,7 @@ static int mxser_init(void)
 			m++;
 		}
 	}
+	pci_dev_put(pdev);
 #endif
 
 	retval = tty_register_driver(mxvar_sdriver);

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to trem
Quoted 14 lines · trem
drivers/char/mxser.c +1 -1
index 0fb2fb9..28298dc 100644
@@ -817,7 +817,7 @@ static int mxser_init(void)
 	index = 0;
 	b = 0;
 	while (b < n) {
-		pdev = pci_find_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
+		pdev = pci_get_device(mxser_pcibrds[b].vendor, mxser_pcibrds[b].device, pdev);
 			if (pdev = NULL) {
 			b++;
 			continue;

drivers/char/mxser.c static int mxser_init(void) · line 858

Quoted 5 lines
drivers/char/mxser.c +1 -0
@@ -854,6 +854,7 @@ static int mxser_init(void)
 			m++;
 		}
 	}
+	pci_dev_put(pdev);
 #endif

No, this is not correct as you are saving off that pdev somewhere else and then referencing it later. Which could be a stale pointer.

Please just convert the driver to the proper pci_register_driver() api instead.

thanks,

greg k-h
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors