lkml.org 
[lkml]   [2018]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v7 5/7] drivers/i2c: Add transfer implementation for FSI algorithm
    On Wed, May 30, 2018 at 1:24 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
    > From: "Edward A. James" <eajames@us.ibm.com>
    >
    > Execute I2C transfers from the FSI-attached I2C master. Use polling
    > instead of interrupts as we have no hardware IRQ over FSI.

    > + if (msg->flags & I2C_M_RD)
    > + cmd |= I2C_CMD_READ;

    I think we have a helper for this, though not sure.

    > +static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
    > + u8 fifo_count)
    > +{
    > + int write;

    > + int rc = 0;

    Redundant assignment.

    > + struct fsi_i2c_master *i2c = port->master;
    > + int bytes_to_write = i2c->fifo_size - fifo_count;
    > + int bytes_remaining = msg->len - port->xfrd;

    > + if (bytes_to_write > bytes_remaining)
    > + bytes_to_write = bytes_remaining;

    _write = min(_write, _remaining);

    > + while (bytes_to_write > 0) {

    > + write = bytes_to_write;
    > + /* fsi limited to max 4 byte aligned ops */
    > + if (bytes_to_write > 4)
    > + write = 4;
    > + else if (write == 3)
    > + write = 2;

    write = min_t(int, 4, rounddown_pow_of_two(bytes_to_write));

    Also check it carefully, it might be optimized even more, though I
    didn't think much.

    > + }

    > + return rc;

    How it can be non-zero?

    > +}

    > +
    > +static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
    > + u8 fifo_count)
    > +{
    > + int read;

    > + int rc = 0;

    Redundant assignment.

    > + struct fsi_i2c_master *i2c = port->master;
    > + int xfr_remaining = msg->len - port->xfrd;
    > + u32 dummy;
    > +
    > + while (fifo_count) {

    > + read = fifo_count;
    > + /* fsi limited to max 4 byte aligned ops */
    > + if (fifo_count > 4)
    > + read = 4;
    > + else if (read == 3)
    > + read = 2;

    See above for write case and do in similar way.

    > +
    > + if (xfr_remaining) {

    > + if (xfr_remaining < read)
    > + read = xfr_remaining;

    read = min(read, _remaining);

    > +
    > + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
    > + &msg->buf[port->xfrd], read);
    > + if (rc)
    > + return rc;
    > +
    > + port->xfrd += read;
    > + xfr_remaining -= read;
    > + } else {
    > + /* no more buffer but data in fifo, need to clear it */
    > + rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
    > + read);
    > + if (rc)
    > + return rc;
    > + }
    > +
    > + fifo_count -= read;
    > + }
    > +

    > + return rc;

    How non-zero possible here?

    > +}

    > + if (port->xfrd < msg->len)
    > + rc = -ENODATA;
    > + else
    > + rc = msg->len;
    > +
    > + return rc;

    if (...)
    return -ENODATA;
    return msg->len;
    ?

    > + u32 status = 0;

    Redundant assignment.

    > + return -ETIME;

    ETIMEDOUT ?


    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2018-05-30 02:10    [W:4.674 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site