On Wed, Feb 12, 2003 at 10:25:49PM +0900, Osamu Tomita wrote:
> +ifneq ($(CONFIG_PC9800),y)
> obj-$(CONFIG_BLK_DEV_FD) += floppy.o
> +else
> +obj-$(CONFIG_BLK_DEV_FD) += floppy98.o
> +endif
Please use a different config option for your floppy driver, i.e.
CONFIG_BLK_DEV_FD98
> +#if !defined(CONFIG_PC9800) && !defined(CONFIG_PC98)
> +#error This driver works only for NEC PC-9800 series
> +#endif
this shoiuld be handled by the config system..
> +
> +#if LINUX_VERSION_CODE < 0x20200
> +# define LP_STATS
> +#endif
> +
> +#if LINUX_VERSION_CODE >= 0x2030b
> +# define CONFIG_RESOURCE98
> +#endif
it would be nice if you could get rid of those obsolete version ifdefs for a
new port..
> + /* Following `TAG: INITIALIZER' notations are GNU CC extension. */
> + flags: LP_EXIST | LP_ABORTOPEN,
> + chars: LP_INIT_CHAR,
> + time: LP_INIT_TIME,
> + wait: LP_INIT_WAIT,
> +};
please use C99-style initializers here.
> +static inline void nanodelay(unsigned long nanosecs) /* Evil ? */
yes.
> +static long long lp_old98_llseek(struct file * file,
> + long long offset, int whence)
> +{
> + return -ESPIPE; /* cannot seek like pipe */
> +}
use no_llseek instead.
> + unsigned long eflags;
> +
> + save_flags(eflags);
> + cli(); /* interrupts while check is fairly bad */
use proper spinlocking.
> +
> + if (!lp_old98_char(DC1)) {
> + restore_flags(eflags);
> + return -EBUSY;
you don't free an buffer allocated above here
> +static int lp_old98_release(struct inode * inode, struct file * file)
> +{
> + kfree(lp.lp_buffer);
> + lp.lp_buffer = NULL;
> + lp.flags &= ~LP_BUSY;
> +#ifdef CONFIG_PC9800_OLDLP_CONSOLE
> + lp_old98_console.flags = saved_console_flags;
> +#endif
> + MOD_DEC_USE_COUNT;
please use fops.owner based refcounting instead.
> +/*
> + * Many use of `put_user' macro enlarge code size...
> + */
> +static /* not inline */ int lp_old98_put_user(int val, int *addr)
> +{
> + return put_user(val, addr);
> +}
umm..
> +#ifdef MODULE
> +#define lp_old98_init init_module
> +#endif
> +
> +int __init lp_old98_init(void)
use module_init/module_exit instead.
> + if (check_region(LP_PORT_DATA, 1) || check_region(LP_PORT_STATUS, 1)
> + || check_region(LP_PORT_STROBE, 1)
> +#ifdef PC98_HW_H98
> + || ((pc98_hw_flags & PC98_HW_H98)
> + && check_region(LP_PORT_H98MODE, 1))
> +#endif
> + || check_region(LP_PORT_EXTMODE, 1)) {
> + printk(KERN_ERR
> + "lp_old98: I/O ports already occupied, giving up.\n");
> + return -EBUSY;
> + }
check_region is obsolete, use the return value from request_region instead.
> +static long long rtc_llseek(struct file *file, loff_t offset, int origin)
> +{
> + return -ESPIPE;
> +}
again, use no_llseek
> +EXPORT_NO_SYMBOLS;
EXPORT_NO_SYMBOLS is obsolete, don't use it in 2.5.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Feb 15 2003 - 22:00:42 EST