Messages in this thread | | | Date | Sat, 11 Jan 2003 08:22:23 +0000 | From | Christoph Hellwig <> | Subject | Re: [PATCH] i2c-isa and w83871d sensors support |
| |
On Sun, Jan 05, 2003 at 06:44:55PM +0100, GertJan Spoelman wrote: > This patch adds the i2c-isa pseudo ISA adapter and the w8387* series > sensors support to 2.5.54 (tested on bk1 and bk3). > > I've just took the code from the lm_sensor package and changed it > to be more or less consistent with the lm75 and amd* patches Christoph > has done. > Christoph, maybe you could check this because I don't really understand > all of the code and maybe it needs some more changes.
Looks fine in principle. A few nitpicks:
> +static struct i2c_algorithm isa_algorithm = { > + .name = "ISA bus algorithm", > + .id = I2C_ALGO_ISA, > + .smbus_xfer = NULL, > + .functionality = &isa_func, > +};
There's no need to initialize a struct member to zero/NULL.
> +static int __init isa_init(void) > +{ > + int res; > + if (isa_initialized) { > + pr_debug(DRV_NAME > + ": i2c-isa.o: Oops, isa_init called a second time!\n"); > + return -EBUSY; > + } > + isa_initialized = 0; > + if ((res = i2c_add_adapter(&isa_adapter))) { > + printk(KERN_ERR DRV_NAME > + ": Adapter registration failed, module not inserted\n."); > + isa_cleanup(); > + return res; > + } > + isa_initialized++; > + printk("i2c-isa.o: ISA bus access for i2c modules initialized.\n"); > + return 0;
Please remove the <foo>_initialized handling. It just obsfucates the code.
> +static void __exit isa_exit(void) > +{ > + isa_cleanup(); > +}
Just move the code from isa_cleanup into isa_exit directly.
- 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/
| |