| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260114130522.GE25101@pendragon.ideasonboard.com>
Date: Wed, 14 Jan 2026 15:05:22 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Jai Luthra <jai.luthra@...asonboard.com>
Cc: Rishikesh Donadkar <r-donadkar@...com>, jai.luthra@...ux.dev,
mripard@...nel.org, y-abhilashchandra@...com, devarsht@...com,
s-jain1@...com, vigneshr@...com, mchehab@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, p.zabel@...gutronix.de,
conor+dt@...nel.org, sakari.ailus@...ux.intel.com,
hverkuil-cisco@...all.nl, tomi.valkeinen@...asonboard.com,
changhuang.liang@...rfivetech.com, jack.zhu@...rfivetech.com,
sjoerd@...labora.com, dan.carpenter@...aro.org,
hverkuil+cisco@...nel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v9 08/19] media: staging: starfive: Move to
enabel-disable streams in starfive drivers
On Wed, Jan 14, 2026 at 06:21:21PM +0530, Jai Luthra wrote:
> Hi Rishikesh,
>
> Thanks for the patch.
We should actually drop the driver. Starfive has confirmed they don't
plan to develop it further, so it shouldn't stay in staging.
> > Subject: [PATCH v9 08/19] media: staging: starfive: Move to enabel-disable streams in starfive drivers
>
> s/enabel/enable
>
> Quoting Rishikesh Donadkar (2025-12-30 14:02:09)
> > The enable_streams() API in v4l2 supports passing a bitmask to enable
> > each pad/stream combination individually on any media subdev. Use this
> > API instead of s_stream() API in the starfive drivers
>
> nit: I think the description can be explicit that this driver does not
> support "multiple streams" (at least right now), but just switching to the
> new API while ignoring the passed streams mask.
>
> >
> > Signed-off-by: Rishikesh Donadkar <r-donadkar@...com>
> > ---
> > .../staging/media/starfive/camss/stf-isp.c | 43 ++++++++++++-------
> > .../staging/media/starfive/camss/stf-video.c | 4 +-
> > 2 files changed, 30 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
> > index df7a903fbb1b0..4930ffb0e07a6 100644
> > --- a/drivers/staging/media/starfive/camss/stf-isp.c
> > +++ b/drivers/staging/media/starfive/camss/stf-isp.c
> > @@ -55,27 +55,43 @@ int stf_isp_init(struct stfcamss *stfcamss)
> > return 0;
> > }
> >
> > -static int isp_set_stream(struct v4l2_subdev *sd, int enable)
> > +static int isp_sd_enable_stream(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64 streams_mask)
> > {
> > struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> > struct v4l2_subdev_state *sd_state;
> > struct v4l2_mbus_framefmt *fmt;
> > struct v4l2_rect *crop;
> > + int ret;
> >
> > - sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> > + sd_state = v4l2_subdev_get_locked_active_state(sd);
> > fmt = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SINK);
> > crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SRC);
> >
> > - if (enable) {
> > - stf_isp_reset(isp_dev);
> > - stf_isp_init_cfg(isp_dev);
> > - stf_isp_settings(isp_dev, crop, fmt->code);
> > - stf_isp_stream_set(isp_dev);
> > - }
> > + stf_isp_reset(isp_dev);
> > + stf_isp_init_cfg(isp_dev);
> > + stf_isp_settings(isp_dev, crop, fmt->code);
> > + stf_isp_stream_set(isp_dev);
> > +
> > + ret = v4l2_subdev_enable_streams(isp_dev->source_subdev, 1, BIT(0));
>
> Given you have a streams_mask argument in this function now, it might be
> cleaner to use it here (and let stf-video populate it with BIT(0)).
>
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> >
> > - v4l2_subdev_call(isp_dev->source_subdev, video, s_stream, enable);
> > +static int isp_sd_disable_stream(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64 streams_mask)
> > +{
> > + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> > + int ret;
> > +
> > + ret = v4l2_subdev_disable_streams(isp_dev->source_subdev, 1, BIT(0));
>
> Same here.
>
> > + if (ret)
> > + return ret;
> >
> > - v4l2_subdev_unlock_state(sd_state);
> > return 0;
> > }
> >
> > @@ -300,20 +316,17 @@ static int isp_init_formats(struct v4l2_subdev *sd,
> > return isp_set_format(sd, sd_state, &format);
> > }
> >
> > -static const struct v4l2_subdev_video_ops isp_video_ops = {
> > - .s_stream = isp_set_stream,
> > -};
> > -
> > static const struct v4l2_subdev_pad_ops isp_pad_ops = {
> > .enum_mbus_code = isp_enum_mbus_code,
> > .get_fmt = v4l2_subdev_get_fmt,
> > .set_fmt = isp_set_format,
> > .get_selection = isp_get_selection,
> > .set_selection = isp_set_selection,
> > + .enable_streams = isp_sd_enable_stream,
> > + .disable_streams = isp_sd_disable_stream,
> > };
> >
> > static const struct v4l2_subdev_ops isp_v4l2_ops = {
> > - .video = &isp_video_ops,
> > .pad = &isp_pad_ops,
> > };
> >
> > diff --git a/drivers/staging/media/starfive/camss/stf-video.c b/drivers/staging/media/starfive/camss/stf-video.c
> > index a0420eb6a0aa0..2db29bf8bdef8 100644
> > --- a/drivers/staging/media/starfive/camss/stf-video.c
> > +++ b/drivers/staging/media/starfive/camss/stf-video.c
> > @@ -287,7 +287,7 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
> >
> > video->ops->start_streaming(video);
> >
> > - ret = v4l2_subdev_call(video->source_subdev, video, s_stream, true);
> > + ret = v4l2_subdev_enable_streams(video->source_subdev, 1, BIT(0));
>
> Now that I think of it, it was not necessary to implement enable / disable
> API for the ISP subdev driver given v4l2_subdev_*_streams falls back on
> s_stream. But it's anyway good to move drivers, so I guess it's alright.
>
> > if (ret) {
> > dev_err(video->stfcamss->dev, "stream on failed\n");
> > goto err_pm_put;
> > @@ -311,7 +311,7 @@ static void video_stop_streaming(struct vb2_queue *q)
> >
> > video->ops->stop_streaming(video);
> >
> > - v4l2_subdev_call(video->source_subdev, video, s_stream, false);
> > + v4l2_subdev_disable_streams(video->source_subdev, 1, BIT(0));
> >
> > pm_runtime_put(video->stfcamss->dev);
> >
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists