[KJ] Re: pci_find_device() in drivers/media/radio/

[KJ] Re: pci_find_device() in drivers/media/radio/

[Domen Puncer]

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

Sorry for late reply.

On 02/02/05 17:22 +0100, Christophe Lucas wrote:
> Hi,
> 
> Description:
> >From : http://kerneljanitors.org/TODO
> >- delete all pci_find_* functions from the kernel tree.
> >  Instead of pci_find_device use pci_get_device()..
> >-  convert from pci_module_init to pci_register_driver

Patch per concept would be nice.

> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc2-vanilla/drivers/media/radio/radio-maestro.c linux-2.6.11-rc2/drivers/media/radio/radio-maestro.c
> --- linux-2.6.11-rc2-vanilla/drivers/media/radio/radio-maestro.c	2005-01-26 08:31:14.000000000 +0100
> +++ linux-2.6.11-rc2/drivers/media/radio/radio-maestro.c	2005-02-02 15:43:24.000000000 +0100
> @@ -265,11 +265,11 @@ static int __init maestro_radio_init(voi
>  {
>  	register __u16 found=0;
>  	struct pci_dev *pcidev = NULL;
> -	while(!found && (pcidev = pci_find_device(PCI_VENDOR_ESS, 
> +	while(!found && (pcidev = pci_get_device(PCI_VENDOR_ESS, 
			you could loose trailing whitespace     ^
>  						  PCI_DEVICE_ID_ESS_ESS1968,
>  						  pcidev)))
>  		found |= radio_install(pcidev);
> -	while(!found && (pcidev = pci_find_device(PCI_VENDOR_ESS,
> +	while(!found && (pcidev = pci_get_device(PCI_VENDOR_ESS,
>  						  PCI_DEVICE_ID_ESS_ESS1978, 
>  						  pcidev)))
>  		found |= radio_install(pcidev);
> @@ -316,6 +316,7 @@ static __u16 radio_install(struct pci_de
>  	if(radio_power_on(&radio_unit)) {
>  		if(video_register_device(&maestro_radio, VFL_TYPE_RADIO, radio_nr)==-1) {
>  			printk("radio-maestro: can't register device!");
> +			pci_dev_put(pcidev);
>  			return 0;
>  		}
>  		printk(KERN_INFO "radio-maestro: version "
> @@ -326,7 +327,9 @@ static __u16 radio_install(struct pci_de
>  		       "\n");
>  		printk(KERN_INFO "radio-maestro: radio chip initialized\n");
>  		return 1;
> -	} else
> +	} else {
> +		pci_dev_put(pcidev); 
ditto.

>  		return 0;   
> +	}
>  }

Whitespace is nitpicking. :-)
About pci_dev_put... it should probably also be in _exit paths, so module
can be reinserted?

Comments?


	Domen

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

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

[KJ] Re: pci_find_device() in drivers/media/radio/

[Christophe Lucas]

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

Domen Puncer (domen@coderock.org) wrote:
> Sorry for late reply.
> 
> On 02/02/05 17:22 +0100, Christophe Lucas wrote:
> > Hi,
> > 
> > Description:
> > >From : http://kerneljanitors.org/TODO
> > >- delete all pci_find_* functions from the kernel tree.
> > >  Instead of pci_find_device use pci_get_device()..
> > >-  convert from pci_module_init to pci_register_driver
> 
> Patch per concept would be nice.

Yeah, afterwards, I have released patches by concept as you advise me
here.

> Whitespace is nitpicking. :-)

I will pay attention on this. Thanks for all your comments.
I am here to help and learn a lot from all of you.

> About pci_dev_put... it should probably also be in _exit paths, so module
> can be reinserted?

You are right, I will soon released a patch that I hope correct.

> Comments?

Anymore comments, you are totally right.

	~Christophe


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

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