| Message ID | 20170816081305.GA19601@amd (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 16, 2017 at 10:13:05AM +0200, Pavel Machek wrote: > On Wed 2017-08-16 10:33:45, Sakari Ailus wrote: > > The et8ek8 driver combines I²C register writes to a single array that it > > passes to i2c_transfer(). The maximum number of writes is 48 at once, > > decrease it to 8 and make more transfers if needed, thus avoiding a > > warning on stack usage. > > Dunno. Slowing down code to save stack does not sound attractive. > > What about this one? Way simpler, too... (Unless there's some rule > about i2c, DMA and static buffers. Is it?) > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > (untested) > Pavel > > diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c > index f39f517..64da731 100644 > --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c > +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c > @@ -227,7 +227,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client, > int cnt) > { > struct i2c_msg msg[ET8EK8_MAX_MSG]; > - unsigned char data[ET8EK8_MAX_MSG][6]; > + static unsigned char data[ET8EK8_MAX_MSG][6]; Works, but we'll need to serialise calls to the function then. I'm not really sure if passing multiple messages to i2c_transfer() really even helps here. I think it could be removed altogether as well. > int wcnt = 0; > u16 reg, data_length; > u32 val; > > > > > --- > > Pavel: this is just compile tested. Could you test it on N900, please? > > > > drivers/media/i2c/et8ek8/et8ek8_driver.c | 26 +++++++++++++++++--------- > > 1 file changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c > > index f39f517..c14f0fd 100644 > > --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c > > +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c > > @@ -43,7 +43,7 @@ > > > > #define ET8EK8_NAME "et8ek8" > > #define ET8EK8_PRIV_MEM_SIZE 128 > > -#define ET8EK8_MAX_MSG 48 > > +#define ET8EK8_MAX_MSG 8 > > > > struct et8ek8_sensor { > > struct v4l2_subdev subdev; > > @@ -220,7 +220,8 @@ static void et8ek8_i2c_create_msg(struct i2c_client *client, u16 len, u16 reg, > > > > /* > > * A buffered write method that puts the wanted register write > > - * commands in a message list and passes the list to the i2c framework > > + * commands in smaller number of message lists and passes the lists to > > + * the i2c framework > > */ > > static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client, > > const struct et8ek8_reg *wnext, > > @@ -231,11 +232,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client, > > int wcnt = 0; > > u16 reg, data_length; > > u32 val; > > - > > - if (WARN_ONCE(cnt > ET8EK8_MAX_MSG, > > - ET8EK8_NAME ": %s: too many messages.\n", __func__)) { > > - return -EINVAL; > > - } > > + int rval; > > > > /* Create new write messages for all writes */ > > while (wcnt < cnt) { > > @@ -249,10 +246,21 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client, > > > > /* Update write count */ > > wcnt++; > > + > > + if (wcnt < ET8EK8_MAX_MSG) > > + continue; > > + > > + rval = i2c_transfer(client->adapter, msg, wcnt); > > + if (rval < 0) > > + return rval; > > + > > + cnt -= wcnt; > > + wcnt = 0; > > } > > > > - /* Now we send everything ... */ > > - return i2c_transfer(client->adapter, msg, wcnt); > > + rval = i2c_transfer(client->adapter, msg, wcnt); > > + > > + return rval < 0 ? rval : 0; > > } > > > > /* > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c index f39f517..64da731 100644 --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c @@ -227,7 +227,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client, int cnt) { struct i2c_msg msg[ET8EK8_MAX_MSG]; - unsigned char data[ET8EK8_MAX_MSG][6]; + static unsigned char data[ET8EK8_MAX_MSG][6]; int wcnt = 0; u16 reg, data_length; u32 val;