[KJ] [PATCH] drivers/char/mxser.c : replace pci_find_device by

[KJ] [PATCH] drivers/char/mxser.c : replace pci_find_device by

[trem]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

From: trem <tremyfr@yahoo.fr>
 
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 <tremyfr@yahoo.fr>
---



[-- Attachment #2: mxser_replace_pci_find_device_by_pci_get_device.patch --]
[-- Type: text/x-patch, Size: 612 bytes --]

diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index 0fb2fb9..8d235a1 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -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);

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/char/mxser.c : replace pci_find_device by

[Nish Aravamudan]

On 5/27/06, trem <tremyfr@yahoo.fr> wrote:
> From: trem <tremyfr@yahoo.fr>
>
> 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
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/char/mxser.c : replace pci_find_device by

[trem]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

Nish Aravamudan wrote:
> On 5/27/06, trem <tremyfr@yahoo.fr> wrote:
>> From: trem <tremyfr@yahoo.fr>
>>
>> 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
>
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


[-- Attachment #2: mxser_replace_pci_find_device_by_pci_get_device.patch --]
[-- Type: text/x-patch, Size: 585 bytes --]

diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index 0fb2fb9..28298dc 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -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);

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/char/mxser.c : replace pci_find_device by

[Greg KH]

On Sat, May 27, 2006 at 10:33:30PM +0200, trem wrote:
> diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
> index 0fb2fb9..28298dc 100644
> --- a/drivers/char/mxser.c
> +++ b/drivers/char/mxser.c
> @@ -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

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
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors