Messages in this thread | | | Date | Sat, 12 Mar 2016 16:35:00 +0100 | From | Wolfram Sang <> | Subject | Re: [PATCH v3 02/14] i2c-octeon: Cleanup i2c-octeon driver |
| |
On Mon, Mar 07, 2016 at 04:10:45PM +0100, Jan Glauber wrote: > Cleanup only without functional change.
I like most of the changes, but there are still some functional changes left.
> -static int octeon_i2c_stop(struct octeon_i2c *i2c) > +/* send STOP to the bus */ > +static void octeon_i2c_stop(struct octeon_i2c *i2c) > { > u8 data; > > @@ -266,11 +259,8 @@ static int octeon_i2c_stop(struct octeon_i2c *i2c) > > data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT); > > - if (data != STAT_IDLE) { > + if (data != STAT_IDLE) > dev_err(i2c->dev, "%s: bad status(0x%x)\n", __func__, data); > - return -EIO; > - } > - return 0;
Why this change? I don't know what SW_TWSI_EOP_TWSI_STAT tells, but this is surely not a cleanup.
> octeon_i2c_stop(i2c); > > - return (ret != 0) ? ret : num; > + return ret ? -EAGAIN : num;
This is also not a cleanup and looks wrong. -EAGAIN is for lost arbitration only.
> > -static struct of_device_id octeon_i2c_match[] = { > - { > - .compatible = "cavium,octeon-3860-twsi", > - }, > +static const struct of_device_id octeon_i2c_match[] = { > + { .compatible = "cavium,octeon-3860-twsi", },
Nit: I'd prefer no tabs within the curly braces.
Thanks,
Wolfram
[unhandled content-type:application/pgp-signature] | |