All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Wentong Wu <wentong.wu@intel.com>
Cc: linux-media@vger.kernel.org, hao.yao@intel.com,
	Jason Chen <jason.z.chen@intel.com>
Subject: Re: [PATCH 3/3] media: ivsc: csi: remove privacy status in struct mei_csi
Date: Fri, 7 Jun 2024 12:45:06 +0000	[thread overview]
Message-ID: <ZmMA0pahCizgRdE2@kekkonen.localdomain> (raw)
In-Reply-To: <20240603082614.1567712-4-wentong.wu@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);

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

>  		break;
>  	case CSI_SET_OWNER:
>  	case CSI_SET_CONF:

-- 
Kind regards,

Sakari Ailus

  reply	other threads:[~2024-06-07 12:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03  8:26 [PATCH 0/3] Fix privacy issue for MEI CSI Wentong Wu
2024-06-03  8:26 ` [PATCH 1/3] media: ivsc: csi: don't count privacy on as error Wentong Wu
2024-06-07 12:45   ` Sakari Ailus
2024-06-07 12:55     ` Wu, Wentong
2024-06-03  8:26 ` [PATCH 2/3] media: ivsc: csi: add separate lock for v4l2 control handler Wentong Wu
2024-06-07 12:41   ` Sakari Ailus
2024-06-07 12:53     ` Wu, Wentong
2024-06-03  8:26 ` [PATCH 3/3] media: ivsc: csi: remove privacy status in struct mei_csi Wentong Wu
2024-06-07 12:45   ` Sakari Ailus [this message]
2024-06-07 12:54     ` Wu, Wentong

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=ZmMA0pahCizgRdE2@kekkonen.localdomain \
    --to=sakari.ailus@linux.intel.com \
    --cc=hao.yao@intel.com \
    --cc=jason.z.chen@intel.com \
    --cc=linux-media@vger.kernel.org \
    --cc=wentong.wu@intel.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.