| Message ID | 20240603082614.1567712-4-wentong.wu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix privacy issue for MEI CSI | expand |
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@intel.com> > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > Tested-by: Jason Chen <jason.z.chen@intel.com> > --- > 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); So could you add one more patch to the set for v2? > break; > case CSI_SET_OWNER: > case CSI_SET_CONF:
> From: Sakari Ailus <sakari.ailus@linux.intel.com> > > 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@intel.com> > > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > > Tested-by: Jason Chen <jason.z.chen@intel.com> > > --- > > 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
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); break; case CSI_SET_OWNER: case CSI_SET_CONF: