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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.