Hi Srinidhi, some more small comments...
2010/1/8 srinidhi kasagar <srinidhi.kasagar@xxxxxxxxxxxxxx>:
> This adds support for the ST-Ericsson's I2C
> block found in Ux500 and Nomadik 8815
> platforms.
>
> Signed-off-by: srinidhi kasagar <srinidhi.kasagar@xxxxxxxxxxxxxx>
> Acked-by: Andrea gallo <andrea.gallo@xxxxxxxxxxxxxx>
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx>
It's even Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> now that
I've taken some time to do it properly.
> +static unsigned int nmk_i2c_functionality(struct i2c_adapter *adap);
> +static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
> + struct i2c_msg msgs[], int num_msgs);
> +
> +static const struct i2c_algorithm nmk_i2c_algo = {
> + .master_xfer = nmk_i2c_xfer,
> + .functionality = nmk_i2c_functionality
> +};
> +
> +static unsigned int nmk_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C
> + | I2C_FUNC_SMBUS_BYTE_DATA
> + | I2C_FUNC_SMBUS_WORD_DATA
> + | I2C_FUNC_SMBUS_I2C_BLOCK;
> +}
> +
If you move nmk_i2c_functionality() before struct i2c_algorithm nmk_i2c_algo
and then move the whole shebang just before the probe() function you don't
need to forward-declare nmk_i2c_functionality() and nmk_i2c_xfer().
(...)
> +/**
> + * nmk_i2c_xfer() - I2C transfer function used by kernel framework
> + * @i2c_adap - Adapter pointer to the controller
> + * @msgs[] - Pointer to data to be written.
> + * @num_msgs - Number of messages to be executed
> + *
> + * This is the function called by the generic kernel i2c_transfer()
> + * or i2c_smbus...() API calls. Note that this code is protected by the
> + * semaphore set in the kernel i2c_transfer() function.
> + *
> + * NOTE:
> + * READ TRANSFER : We impose a restriction of the first message to be the
> + * index message for any read transaction.
> + * - a no index is coded as '0',
> + * - 2byte big endian index is coded as '3'
> + * !!! msg[0].buf holds the actual index.
> + * This is compatible with generic messages of smbus emulator
> + * that send a one byte index.
> + * eg. a I2C transation to read 2 bytes from index 0
> + * idx = 0;
> + * msg[0].addr = client->addr;
> + * msg[0].flags = 0x0;
> + * msg[0].len = 1;
> + * msg[0].buf = &idx;
> + *
> + * msg[1].addr = client->addr;
> + * msg[1].flags = I2C_M_RD;
> + * msg[1].len = 2;
> + * msg[1].buf = rd_buff
> + * i2c_transfer(adap, msg, 2);
> + *
> + * WRITE TRANSFER : The I2C standard interface interprets all data as payload.
> + * If you want to emulate an SMBUS write transaction put the
> + * index as first byte(or first and second) in the payload.
> + * eg. a I2C transation to write 2 bytes from index 1
> + * wr_buff[0] = 0x1;
> + * wr_buff[1] = 0x23;
> + * wr_buff[2] = 0x46;
> + * msg[0].flags = 0x0;
> + * msg[0].len = 3;
> + * msg[0].buf = wr_buff;
> + * i2c_transfer(adap, msg, 1);
> + *
> + * To read or write a block of data (multiple bytes) using SMBUS emulation
> + * please use the i2c_smbus_read_i2c_block_data()
> + * or i2c_smbus_write_i2c_block_data() API
> + *
> + * i2c_master_recv() API is not supported as single read message is not
> + * accepted by our interface (it has to be preceded by an index message)
It looks like this restriction is gone, so the comment can go too?
> + */
> +static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
> + struct i2c_msg msgs[], int num_msgs)
> +{
> + int status;
> + int i;
> + u32 cause;
> + struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
> +
> + status = init_hw(dev);
> + if (status)
> + return status;
> +
> + /* setup the i2c controller */
> + setup_i2c_controller(dev);
> +
> + for (i = 0; i < num_msgs; i++) {
> + if (unlikely(msgs[i].flags & I2C_M_TEN)) {
> + dev_err(&dev->pdev->dev, "10 bit addressing"
> + "not supported\n");
> + return -EINVAL;
> + }
> + dev->cli.slave_adr = msgs[i].addr;
> + dev->cli.buffer = msgs[i].buf;
> + dev->cli.count = msgs[i].len;
> + dev->stop = (i < (num_msgs - 1)) ? 0 : 1;
> + dev->result = 0;
> +
> + if (msgs[i].flags & I2C_M_RD) {
> + /* it is a read operation */
> + dev->cli.operation = I2C_READ;
> + status = read_i2c(dev);
> + } else {
> + /* write operation */
> + dev->cli.operation = I2C_WRITE;
> + status = write_i2c(dev);
> + }
> + if (status || (dev->result)) {
> + /* get the abort cause */
> + cause = (readl(dev->virtbase + I2C_SR) >> 4) & 0x7;
> + dev_err(&dev->pdev->dev, "error during I2C"
> + "message xfer: %d\n", cause);
> + dev_err(&dev->pdev->dev, "%s\n",
> + cause >= ARRAY_SIZE(abort_causes)
> + ? "unknown reason" : abort_causes[cause]);
> + return status;
> + }
> + mdelay(1);
> + }
> + return status;
> +}
> +
This thing should return the number of messages transferred, see
include/linux/i2c.h.
Something like
if (status)
return status;
else
return i;
should work I think?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
