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]
Albert Cranford wrote:
> +static void
> +cpm_iic_interrupt(void *dev_id, void *regs)
> +{
> + volatile i2c8xx_t *i2c = (i2c8xx_t *)dev_id;
no need for volatile
> +}
> +
> +static void
> +cpm_iic_init(struct i2c_algo_8xx_data *cpm_adap)
> +{
> + volatile iic_t *iip = cpm_adap->iip;
> + volatile i2c8xx_t *i2c = cpm_adap->i2c;
no need for volatile. repeat this comment several times...
> + if (cpm_adap->reloc == 0) {
> + volatile cpm8xx_t *cp = cpm_adap->cp;
> +
> + cp->cp_cpcr =
> + mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_INIT_TRX) | CPM_CR_FLG;
> + while (cp->cp_cpcr & CPM_CR_FLG)
> + cpu_relax(); /* prevent Athlons self-destruct */
> + }
don't busy-wait in a context where you can sleep
> +static void force_close(struct i2c_algo_8xx_data *cpm)
> +{
> + if (cpm->reloc == 0) {
> + volatile cpm8xx_t *cp = cpm->cp;
> +
> + if (cpm_debug) printk("force_close()\n");
> + cp->cp_cpcr =
> + mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_CLOSE_RXBD) |
> + CPM_CR_FLG;
> +
> + while (cp->cp_cpcr & CPM_CR_FLG)
> + cpu_relax(); /* prevent Athlons self-destruct */
> + }
likewise for volatile and busy-wait
> + tbdf->cbd_bufaddr = __pa(tb);
> + tbdf->cbd_datlen = count + 1;
> + tbdf->cbd_sc =
> + BD_SC_READY | BD_SC_INTRPT | BD_SC_LAST |
> + BD_SC_WRAP | BD_IIC_START;
> +
> + rbdf->cbd_datlen = 0;
> + rbdf->cbd_bufaddr = __pa(buf);
> + rbdf->cbd_sc = BD_SC_EMPTY | BD_SC_WRAP;
> +
> + dma_cache_inv((unsigned long) buf, (unsigned long) (buf+count));
> +
> + /* Chip bug, set enable here */
> + local_irq_save(flags);
> + i2c->i2c_i2cmr = 0x13; /* Enable some interupts */
> + i2c->i2c_i2cer = 0xff;
> + i2c->i2c_i2mod = 1; /* Enable */
> + i2c->i2c_i2com = 0x81; /* Start master */
__pa should very rarely be used in drivers
> + /* Wait for IIC transfer */
> + interruptible_sleep_on(&iic_wait);
> + local_irq_restore(flags);
> + if (signal_pending(current))
> + return -EIO;
local_irq_xxx should not be used...
And it is very likely you should be using a semaphore here...
> + /* Chip bug, set enable here */
> + local_irq_save(flags);
> + i2c->i2c_i2cmr = 0x13; /* Enable some interupts */
> + i2c->i2c_i2cer = 0xff;
> + i2c->i2c_i2mod = 1; /* Enable */
> + i2c->i2c_i2com = 0x81; /* Start master */
> +
> + /* Wait for IIC transfer */
> + interruptible_sleep_on(&iic_wait);
> + local_irq_restore(flags);
> + if (signal_pending(current))
> + return -EIO;
likewise
> + printk("i2c-algo-8xx.o: adapter unregistered: %s\n",adap->name);
> +
> +#ifdef MODULE
> + MOD_DEC_USE_COUNT;
> +#endif
> + return 0;
Is this MOD_DEC_USE_COUNT is for a file op, use ->owner instead.
> +#ifdef MODULE
> +MODULE_AUTHOR("Brad Parker <[email protected]>");
> +MODULE_DESCRIPTION("I2C-Bus MPC8XX algorithm");
> +#ifdef MODULE_LICENSE
> +MODULE_LICENSE("GPL");
> +#endif
> +
> +int init_module(void)
> +{
> + return i2c_algo_8xx_init();
> +}
> +
> +void cleanup_module(void)
> +{
> +}
kill ifdef MODULE.
use module_init, module_exit