2002-09-24 04:40:55

by Albert Cranford

[permalink] [raw]
Subject: [patch] linux-2.5.38 new i2c parallel port adapter

Hello Linus,
New i2c-pport parallel port adapter for linux-2.5.38
Thanks
Albert
--
[email protected]


Attachments:
3-pport-patch (6.19 kB)

2002-09-24 05:07:15

by David Miller

[permalink] [raw]
Subject: Re: [patch] linux-2.5.38 new i2c parallel port adapter

From: Jeff Garzik <[email protected]>
Date: Tue, 24 Sep 2002 01:07:22 -0400

Albert Cranford wrote:
> +#define DEFAULT_BASE 0x378

surely there is a parport define for this?

Probably this driver should be hooked into the generic
parport infrastructure, then every architecture no matter
where the parport regs actually live could make use of it.

2002-09-24 05:02:42

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch] linux-2.5.38 new i2c parallel port adapter

Albert Cranford wrote:
> +#define DEFAULT_BASE 0x378

surely there is a parport define for this?


> +static int base=0;
> +static unsigned char PortData = 0;

don't explicitly init to zero... you were told this before :)


> +static int bit_pport_init(void)
> +{
> + if (!request_region((base+2),1, "i2c (PPORT adapter)")) {
> + 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;

leak, no release_region


> + } else {
> +
> + /*SCL high and SDA low*/
> + PortData = PortData SET_SCL CLR_SDA;
> + outb(PortData,base+2);
> + schedule_timeout(400);

ugly... use a constant based on HZ



> + if ( !(inb(base+2) | 0x4) ) {
> + //outb(0x04,base+2);
> + DEBINIT(printk("i2c-port.o: SDA was high.\n"));
> + return -ENODEV;

likewise


> +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
> +}

remove the ifdefs... you were told this too :)



> +int __init i2c_bitpport_init(void)
> +{
> + printk("i2c-pport.o: i2c Primitive parallel port adapter module version %s (%s)\n", I2C_VERSION, I2C_DATE);
> +
> + if (base==0) {
> + /* probe some values */
> + base=DEFAULT_BASE;
> + bit_pport_data.data=(void*)DEFAULT_BASE;
> + if (bit_pport_init()==0) {
> + if(i2c_bit_add_bus(&bit_pport_ops) < 0)
> + return -ENODEV;

bug, no release_region


> + } else {
> + return -ENODEV;
> + }
> + } else {
> + bit_pport_data.data=(void*)base;
> + if (bit_pport_init()==0) {
> + if(i2c_bit_add_bus(&bit_pport_ops) < 0)
> + return -ENODEV;

likewise