Re: [PATCH v3 2/2] media: video-i2c: append register data on MLX90640's frame

[Seongyong Park <euphoriccatface@gmail.com>]

2021년 6월 10일 (목) 오전 8:13, Seongyong Park <euphoriccatface@gmail.com>님이 작성:
>
> 2021년 6월 9일 (수) 오후 4:14, Matt Ranostay <matt.ranostay@konsulko.com>님이 작성:
> >
> > On Tue, Jun 8, 2021 at 8:25 AM Seongyong Park <euphoriccatface@gmail.com> wrote:
> >
> > > -       .height = 26, /* 24 lines of pixel data + 2 lines of processing data */
> > > +       .height = 27,
> > > +       /* 24 lines of pixel data + 2 lines of processing data + 1 line of registers */
> >
> > Guess you hit the 80 character line here and checkpatch.pl complained
> > .. But should all be one line since it is
> > much more clear on one line.
> >
> > >  };
> > >
> > >  static const struct regmap_config amg88xx_regmap_config = {
> > > @@ -168,8 +169,12 @@ static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> > >
> > >  static int mlx90640_xfer(struct video_i2c_data *data, char *buf)
> > >  {
> > > -       return regmap_bulk_read(data->regmap, 0x400, buf,
> > > -                               data->chip->buffer_size);
> > > +       int ret = regmap_bulk_read(data->regmap, 0x400, buf,
> > > +                                  data->chip->buffer_size - 64);
> > > +       if (ret)
> > > +               return ret;
> > > +       return regmap_bulk_read(data->regmap, 0x8000, buf + (data->chip->buffer_size - 64),
> > > +                               64);
> > >  }
> > >
> > >  static int amg88xx_setup(struct video_i2c_data *data)
> > > @@ -375,7 +380,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> > >                 .format         = &mlx90640_format,
> > >                 .frame_intervals        = mlx90640_frame_intervals,
> > >                 .num_frame_intervals    = ARRAY_SIZE(mlx90640_frame_intervals),
> > > -               .buffer_size    = 1664,
> > > +               .buffer_size    = 1728,
> >
> > Minus nitpick above looks good to me. You can keep the acked-by if
> > that is only change
> >
> > Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>
> >
> Yes, other than your suggestion, indeed that is the only change I made
> for this commit between v1 and v2 (and v3 is the same as v2.)
> and that is because of the checkpatch.pl indeed :)
>
> Thanks,
> Seongyong Park

Re-sending this mail mainly because I have made a direct reply, rather
than a reply to all.
Sorry about that.

Matt, while we're at it, if I happen to make another revision of this patchset,
would you find it looking okay to make a line break after the second parameter?
The odd arrangement was partly because I wanted to make a line break
after the same number of parameters.

The lines will look like this:
+       int ret = regmap_bulk_read(data->regmap, 0x400,
+                                  buf, data->chip->buffer_size - 64);
...
+       return regmap_bulk_read(data->regmap, 0x8000,
+                               buf + (data->chip->buffer_size - 64), 64);

Thanks,
- Seongyong Park