From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org, hverkuil@xs4all.nl,
tomi.valkeinen@ideasonboard.com, jacopo.mondi@ideasonboard.com,
bingbu.cao@intel.com, hongju.wang@intel.com
Subject: Re: [PATCH v3 6/8] media: v4l: subdev: Switch to stream-aware state functions
Date: Tue, 24 Oct 2023 10:21:02 +0300 [thread overview]
Message-ID: <20231024072102.GB31956@pendragon.ideasonboard.com> (raw)
In-Reply-To: <ZTdcHwhwNwCm3Q_r@kekkonen.localdomain>
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@ideasonboard.com>
>
> Thank you!
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2023-10-24 7:21 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-23 17:44 [PATCH v3 0/8] Unify sub-device state access functions Sakari Ailus
2023-10-23 17:44 ` [PATCH v3 1/8] media: v4l: subdev: Store the sub-device in the sub-device state Sakari Ailus
2023-10-23 22:04 ` Laurent Pinchart
2023-10-24 7:35 ` Tomi Valkeinen
2023-10-23 17:44 ` [PATCH v3 2/8] media: v4l: subdev: Also return pads array information on stream functions Sakari Ailus
2023-10-23 22:07 ` Laurent Pinchart
2023-10-23 22:18 ` Sakari Ailus
2023-10-24 14:45 ` Tomi Valkeinen
2023-10-24 18:07 ` Sakari Ailus
2023-10-25 9:32 ` Hans Verkuil
2023-10-25 10:20 ` Sakari Ailus
2023-10-23 17:44 ` [PATCH v3 3/8] media: v4l: subdev: Rename sub-device state information access functions Sakari Ailus
2023-10-24 7:37 ` Tomi Valkeinen
2023-10-23 17:44 ` [PATCH v3 4/8] media: v4l: subdev: v4l2_subdev_state_get_format always returns format now Sakari Ailus
2023-10-24 14:47 ` Tomi Valkeinen
2023-10-23 17:44 ` [PATCH v3 5/8] media: v4l: subdev: Make stream argument optional in state access functions Sakari Ailus
2023-10-23 22:15 ` Laurent Pinchart
2023-10-25 9:53 ` Hans Verkuil
2023-10-25 10:19 ` Sakari Ailus
2023-10-25 10:50 ` Laurent Pinchart
2023-10-25 11:03 ` Sakari Ailus
2023-10-25 11:20 ` Hans Verkuil
2023-10-25 11:30 ` Sakari Ailus
2023-10-23 17:44 ` [PATCH v3 6/8] media: v4l: subdev: Switch to stream-aware state functions Sakari Ailus
2023-10-23 22:13 ` Laurent Pinchart
2023-10-24 5:54 ` Sakari Ailus
2023-10-24 7:21 ` Laurent Pinchart [this message]
2023-10-24 9:20 ` Sakari Ailus
2023-10-23 17:44 ` [PATCH v3 7/8] media: v4l: subdev: Remove stream-unaware sub-device state access Sakari Ailus
2023-10-23 22:14 ` Laurent Pinchart
2023-10-23 17:44 ` [PATCH v3 8/8] media: v4l: subdev: Also assert acquired mutex for non-stream drivers Sakari Ailus
2023-10-23 22:14 ` Laurent Pinchart
2023-10-23 22:33 ` Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231024072102.GB31956@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=bingbu.cao@intel.com \
--cc=hongju.wang@intel.com \
--cc=hverkuil@xs4all.nl \
--cc=jacopo.mondi@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=tomi.valkeinen@ideasonboard.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox