[parisc-linux] Re: [chris@qwirx.com: eisa_eeprom.c misc_register patch]

[Daniel Engstrom]

On 2002.11.17 17:13 Matthew Wilcox wrote:
> 
> Daniel, I presume you're interested in maintaining this code?
Well, I have spent too much time on PA hacking for a while.

But the patch seems ok to me. As for the return-code: it is not 
checked by is only caller drivers/gsc/eisa.c:eisa_probe() so it does
not matter, currently. 

One other problem with this driver is the following line:

#define         EISA_EEPROM_MINOR 241

I tried to get a minor number when I wrote the code, but my emails
was not answered. Do we currently have a methos of aquiring misc 
minor numbers that work?

/Daniel
 
> ----- Forwarded message from Chris Wilson <chris@qwirx.com> -----
> 
> Envelope-to: willy@ftp.uk.linux.org
> Delivery-date: Sun, 17 Nov 2002 15:53:19 +0000
> Date: Sun, 17 Nov 2002 15:53:16 +0000 (GMT)
> From: Chris Wilson <chris@qwirx.com>
> X-X-Sender: chris@top.qwarx.com
> To: Matthew Wilcox <willy@debian.org>,
>    Linux Kernel Janitors <kernel-janitor-discuss@lists.sourceforge.net>
> cc: Trivial Patch Monkey <trivial@rustcorp.com.au>
> Subject: eisa_eeprom.c misc_register patch
> 
> Dear Sirs,
> 
> As part of my work on the Linux Kernel Janitors project, cleaning up on 
> functions which call misc_register and don't check for an error return, I 
> would like to submit my patch to drivers/parisc/eisa_eeprom.c. 
> 
> This patch simply causes the function to printk() an error and return the
> error code from misc_register() if it fails. I hope this is the right
> thing to do. It also changes the code layout (but not function) slightly,
> and flags a condition that I'm not sure about: If no address is supplied
> to its initialisation function, should it really return with no error,
> despite the fact that it hasn't done anything useful?
> 
> Cheers, Chris.
> -- 
> _ ___ __     _
>  / __/ / ,__(_)_  | Chris Wilson <0000 at qwirx.com> - Cambs UK |
> / (_/ ,\/ _/ /_ \ | Security/C/C++/Java/Perl/SQL/HTML Developer |
> \ _/_/_/_//_/___/ | We are GNU-free your mind-and your software |
> 
> 
> diff -ru8 linux-2.5.47/drivers/parisc/eisa_eeprom.c linux-2.5.47-chris/drivers/parisc/eisa_eeprom.c
> --- linux-2.5.47/drivers/parisc/eisa_eeprom.c	Mon Nov 11 03:28:29 2002
> +++ linux-2.5.47-chris/drivers/parisc/eisa_eeprom.c	Sun Nov 17 15:42:30 2002
> @@ -91,17 +91,27 @@
>  {
>  	EISA_EEPROM_MINOR,
>  	"eisa eeprom",
>  	&eisa_eeprom_fops
>  };
>  
>  int __init eisa_eeprom_init(unsigned long addr)
>  {
> -	if (addr) {
> -		eeprom_addr = addr;
> -		misc_register(&eisa_eeprom_dev);
> -		printk(KERN_INFO "EISA EEPROM at 0x%lx\n", eeprom_addr);
> +	int retval;
> +
> +	/* XXX why return success when we haven't done anything? */
> +	if (!addr)
> +		return 0;
> +
> +	eeprom_addr = addr;
> +
> +	retval = misc_register(&eisa_eeprom_dev);
> +	if (retval < 0) {
> +		printk(KERN_ERR "EISA EEPROM: cannot register misc device.\n");
> +		return retval;
>  	}
> +
> +	printk(KERN_INFO "EISA EEPROM at 0x%lx\n", eeprom_addr);
>  	return 0;
>  }
>  
>  MODULE_LICENSE("GPL");
> 
> 
> ----- End forwarded message -----
> 
> -- 
> Revolutions do not require corporate support.
> 
--