RE: [PATCH 3/3] media: ivsc: csi: remove privacy status in struct mei_csi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> 
> Hi Wentong,
> 
> Thanks for the patch.
> 
> On Mon, Jun 03, 2024 at 04:26:14PM +0800, Wentong Wu wrote:
> > The privacy status is maintained by privacy_ctrl, on which all of the
> > privacy status changes will go through, so there is no point in
> > maintaining one more element any more.
> >
> > Reported-by: Hao Yao <hao.yao@xxxxxxxxx>
> > Signed-off-by: Wentong Wu <wentong.wu@xxxxxxxxx>
> > Tested-by: Jason Chen <jason.z.chen@xxxxxxxxx>
> > ---
> >  drivers/media/pci/intel/ivsc/mei_csi.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c
> > b/drivers/media/pci/intel/ivsc/mei_csi.c
> > index d6ba0d9efca1..1d1b9181a50a 100644
> > --- a/drivers/media/pci/intel/ivsc/mei_csi.c
> > +++ b/drivers/media/pci/intel/ivsc/mei_csi.c
> > @@ -138,9 +138,6 @@ struct mei_csi {
> >  	u32 nr_of_lanes;
> >  	/* frequency of the CSI-2 link */
> >  	u64 link_freq;
> > -
> > -	/* privacy status */
> > -	enum ivsc_privacy_status status;
> >  };
> >
> >  static const struct v4l2_mbus_framefmt mei_csi_format_mbus_default =
> > { @@ -271,10 +268,8 @@ static void mei_csi_rx(struct mei_cl_device
> > *cldev)
> >
> >  	switch (notif.cmd_id) {
> >  	case CSI_PRIVACY_NOTIF:
> > -		if (notif.cont.cont < CSI_PRIVACY_MAX) {
> > -			csi->status = notif.cont.cont;
> > -			v4l2_ctrl_s_ctrl(csi->privacy_ctrl, csi->status);
> > -		}
> > +		if (notif.cont.cont < CSI_PRIVACY_MAX)
> > +			v4l2_ctrl_s_ctrl(csi->privacy_ctrl, notif.cont.cont);
> 
> notif.cont.cont represents is MEI's idea of the privacy state which just
> happens to be aligned with V4L2's.
> 
> While this issue is not related to this patch, it'd be nice to use e.g.
> 
> 			v4l2_ctrl_s_ctrl(csi->privacy_ctrl,
> 					 notif.cont.cont == CSI_PRIVACY_ON);
> 

Agree, thanks

> So could you add one more patch to the set for v2?

Sure, thanks

BR,
Wentong
> 
> >  		break;
> >  	case CSI_SET_OWNER:
> >  	case CSI_SET_CONF:
> 
> --
> Kind regards,
> 
> Sakari Ailus





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux