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 <pa...@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
--
Sakari Ailus
sakari.ai...@linux.intel.com
