2002-09-15 22:31:30

by Albert Cranford

[permalink] [raw]
Subject: [patch 2/9]Four new i2c drivers and __init/__exit cleanup to i2c

Hello Linus,
New I2C drivers that have been adjusted after Russell King comments of August.
o i2c-algo-8xx.c
o i2c-pport.c
o i2c-adap-ibm_ocp.c
o i2c-pcf-epp.c
o Add new drivers to Config.in and Makefile.
o Add new drivers to i2c-core for initialization.
o Remove EXPORT_NO_SYMBOLS statement from i2c-dev, i2c-elektor and i2c-frodo.
o Cleanup init_module and cleanup_module adding __init and __exit to most drivers.
o Adjust i2c-elektor with cli/sti replacement.
--
[email protected]


Attachments:
47-i2c-3b-patch (6.35 kB)

2002-09-15 22:56:22

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch 2/9]Four new i2c drivers and __init/__exit cleanup to i2c

Albert Cranford wrote:

> +#ifdef MODULE_LICENSE
> +MODULE_LICENSE("GPL");
> +#endif

kill the ifdef


> +static int bit_pport_init(void)
> +{
> + //release_region( (base+2) ,1);
> +
> + if (check_region((base+2),1) < 0 ) {

wrong. race. use request_region, and check its return value.
check_region should never be used.


> + return -ENODEV;
> + } else {
> +
> + /* test for PPORT adap. */
> +
> +
> + PortData=inb(base+2);
> + PortData= (PortData SET_SDA) SET_SCL;
> + outb(PortData,base+2);
> +
> + if (!(inb(base+2) | 0x06)) { /* SDA and SCL will be high */
> + DEBINIT(printk("i2c-pport.o: SDA and SCL was low.\n"));
> + return -ENODEV;
> + } else {
> +
> + /*SCL high and SDA low*/
> + PortData = PortData SET_SCL CLR_SDA;
> + outb(PortData,base+2);
> + udelay(400);

long udelay in process context, where you should sleep instead



> +static void bit_pport_inc_use(struct i2c_adapter *adap)
> +{
> +#ifdef MODULE
> + MOD_INC_USE_COUNT;
> +#endif
> +}
> +
> +static void bit_pport_dec_use(struct i2c_adapter *adap)
> +{
> +#ifdef MODULE
> + MOD_DEC_USE_COUNT;
> +#endif

kill the ifdef. use ->owner instead if possible.



> +#ifdef MODULE
> +MODULE_AUTHOR("Daniel Smolik <[email protected]>");
> +MODULE_DESCRIPTION("I2C-Bus adapter routines for Primitive parallel port adapter")
> +;
> +
> +MODULE_PARM(base, "i");
> +
> +int init_module(void)
> +{
> + return i2c_bitpport_init();
> +}
> +
> +void cleanup_module(void)
> +{
> + i2c_bit_del_bus(&bit_pport_ops);
> + bit_pport_exit();
> +}
> +
> +#endif

kill the ifdef, use module_init, module_exit