On Tue, Oct 24, 2023 at 05:54:39AM +0000, Sakari Ailus wrote:
> Hi Laurent,
>
> On Tue, Oct 24, 2023 at 01:13:39AM +0300, Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > Thank you for the patch.
> >
> > On Mon, Oct 23, 2023 at 08:44:06PM +0300, Sakari Ailus wrote:
> > > Switch all drivers accessing sub-device state to use the stream-aware
> > > functions. We will soon remove the old ones.
> > >
> > > This patch has been generated using the following Coccinelle script:
> > >
> > > ---------8<------------
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_pad_format(E1, E2, E3)
> > > + v4l2_subdev_state_get_format(E2, E3)
> > >
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_pad_crop(E1, E2, E3)
> > > + v4l2_subdev_state_get_crop(E2, E3)
> > >
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_pad_compose(E1, E2, E3)
> > > + v4l2_subdev_state_get_compose(E2, E3)
> > >
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_try_format(E1, E2, E3)
> > > + v4l2_subdev_state_get_format(E2, E3)
> > >
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_try_crop(E1, E2, E3)
> > > + v4l2_subdev_state_get_crop(E2, E3)
> > >
> > > @@
> > > expression E1, E2, E3;
> > >
> > > @@
> > >
> > > - v4l2_subdev_get_try_compose(E1, E2, E3)
> > > + v4l2_subdev_state_get_compose(E2, E3)
> > > ---------8<------------
> > >
> > > Additionally drivers/media/i2c/s5k5baf.c and
> > > drivers/media/platform/samsung/s3c-camif/camif-capture.c have been
> > > manually changed as Coccinelle didn't.
> >
> > Strange, I wonder why.
>
> I wondered that, too, and I guess in some cases Coccinelle doesn't
> recognise these as function calls as they're in variable declaration but
> some are just odd.
>
> >
> > > Further local variables have been
> > > removed as they became unused as a result of the other changes. The diff
> > > from Coccinelle-generated changes are:
> > >
> > > diff --git b/drivers/media/i2c/imx319.c a/drivers/media/i2c/imx319.c
> > > index e549692ff478..420984382173 100644
> > > --- b/drivers/media/i2c/imx319.c
> > > +++ a/drivers/media/i2c/imx319.c
> >
> > I can imagine git am getting confused :-)
>
> Hopefully no-one uses it with this patch.
>
> >
> > > @@ -2001,7 +2001,6 @@ static int imx319_do_get_pad_format(struct imx319 *imx319,
> > > struct v4l2_subdev_format *fmt)
> > > {
> > > struct v4l2_mbus_framefmt *framefmt;
> > > - struct v4l2_subdev *sd = &imx319->sd;
> > >
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> > > diff --git b/drivers/media/i2c/imx355.c a/drivers/media/i2c/imx355.c
> > > index 96bdde685d65..e1b1d2fc79dd 100644
> > > --- b/drivers/media/i2c/imx355.c
> > > +++ a/drivers/media/i2c/imx355.c
> > > @@ -1299,7 +1299,6 @@ static int imx355_do_get_pad_format(struct imx355 *imx355,
> > > struct v4l2_subdev_format *fmt)
> > > {
> > > struct v4l2_mbus_framefmt *framefmt;
> > > - struct v4l2_subdev *sd = &imx355->sd;
> > >
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> > > diff --git b/drivers/media/i2c/ov08x40.c a/drivers/media/i2c/ov08x40.c
> > > index ca799bbcfdb7..abbb0b774d43 100644
> > > --- b/drivers/media/i2c/ov08x40.c
> > > +++ a/drivers/media/i2c/ov08x40.c
> > > @@ -2774,7 +2774,6 @@ static int ov08x40_do_get_pad_format(struct ov08x40 *ov08x,
> > > struct v4l2_subdev_format *fmt)
> > > {
> > > struct v4l2_mbus_framefmt *framefmt;
> > > - struct v4l2_subdev *sd = &ov08x->sd;
> > >
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> > > diff --git b/drivers/media/i2c/ov13858.c a/drivers/media/i2c/ov13858.c
> > > index 7816d9787c61..09387e335d80 100644
> > > --- b/drivers/media/i2c/ov13858.c
> > > +++ a/drivers/media/i2c/ov13858.c
> > > @@ -1316,7 +1316,6 @@ static int ov13858_do_get_pad_format(struct ov13858 *ov13858,
> > > struct v4l2_subdev_format *fmt)
> > > {
> > > struct v4l2_mbus_framefmt *framefmt;
> > > - struct v4l2_subdev *sd = &ov13858->sd;
> > >
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> > > diff --git b/drivers/media/i2c/ov13b10.c a/drivers/media/i2c/ov13b10.c
> > > index 268cd4b03f9c..c06411d5ee2b 100644
> > > --- b/drivers/media/i2c/ov13b10.c
> > > +++ a/drivers/media/i2c/ov13b10.c
> > > @@ -1001,7 +1001,6 @@ static int ov13b10_do_get_pad_format(struct ov13b10 *ov13b,
> > > struct v4l2_subdev_format *fmt)
> > > {
> > > struct v4l2_mbus_framefmt *framefmt;
> > > - struct v4l2_subdev *sd = &ov13b->sd;
> > >
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> > > diff --git b/drivers/media/i2c/s5c73m3/s5c73m3-core.c a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> > > index 47605e36bc60..8f9b5713daf7 100644
> > > --- b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> > > +++ a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> > > @@ -819,7 +819,6 @@ static void s5c73m3_oif_try_format(struct s5c73m3 *state,
> > > struct v4l2_subdev_format *fmt,
> > > const struct s5c73m3_frame_size **fs)
> > > {
> > > - struct v4l2_subdev *sd = &state->sensor_sd;
> > > u32 code;
> > >
> > > switch (fmt->pad) {
> > > diff --git b/drivers/media/i2c/s5k5baf.c a/drivers/media/i2c/s5k5baf.c
> > > index a36b310b32e1..3f3005cca9d0 100644
> > > --- b/drivers/media/i2c/s5k5baf.c
> > > +++ a/drivers/media/i2c/s5k5baf.c
> > > @@ -1473,12 +1473,10 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd,
> > > if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
> > > rects = (struct v4l2_rect * []) {
> > > &s5k5baf_cis_rect,
> > > - v4l2_subdev_get_try_crop(sd, sd_state,
> > > - PAD_CIS),
> > > - v4l2_subdev_get_try_compose(sd, sd_state,
> > > - PAD_CIS),
> > > - v4l2_subdev_get_try_crop(sd, sd_state,
> > > - PAD_OUT)
> > > + v4l2_subdev_state_get_crop(sd_state, PAD_CIS),
> > > + v4l2_subdev_state_get_compose(sd_state,
> > > + PAD_CIS),
> >
> > A single line would be more readable I think.
>
> But over 80.
By one character. Given the relaxed limit, this is one of the cases
where going to 81 columns inmproves readability.
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Thank you!
--
Regards,
Laurent Pinchart
