Re: [PATCH] media: i2c: max9286: Remove unneeded mutex for get_fmt and set_fmt

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

 



Hi Niklas,

On Thu, Jul 08, 2021 at 11:55:50AM +0200, Niklas Söderlund wrote:
> There is no need to protect 'cfg_fmt' in get_fmt() and set_fmt() as the
> core protects these callbacks. As this is the only usage of the mutex it
> can be removed.

You know, I tried chasing where the vdev->lock used to protect the
subdev's ioctl is set for mex9286 and I wasn't able to find it.

Please validate my understanding:

- The lock used by the core to protect the set/get format subdev ioctl
  is the one in subdev_do_ioctl_lock()

  static long subdev_do_ioctl_lock(struct file *file, unsigned int cmd, void *arg)
  {
          struct video_device *vdev = video_devdata(file);
          struct mutex *lock = vdev->lock;

- the max9286 video subdevice node is registered (on R-Car) by
  __v4l2_device_register_subdev_nodes() called by the root notifier
  complete() callback

- The video_device created by __v4l2_device_register_subdev_nodes()
  doesn't initialize any lock

What am I missing ?

Thanks
   j

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
>  drivers/media/i2c/max9286.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 1aa2c58fd38c5d2b..b1d11a50d6e53ecc 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -18,7 +18,6 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-mux.h>
>  #include <linux/module.h>
> -#include <linux/mutex.h>
>  #include <linux/of_graph.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> @@ -173,9 +172,6 @@ struct max9286_priv {
>
>  	struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
>
> -	/* Protects controls and fmt structures */
> -	struct mutex mutex;
> -
>  	unsigned int nsources;
>  	unsigned int source_mask;
>  	unsigned int route_mask;
> @@ -768,9 +764,7 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
>  	if (!cfg_fmt)
>  		return -EINVAL;
>
> -	mutex_lock(&priv->mutex);
>  	*cfg_fmt = format->format;
> -	mutex_unlock(&priv->mutex);
>
>  	return 0;
>  }
> @@ -796,9 +790,7 @@ static int max9286_get_fmt(struct v4l2_subdev *sd,
>  	if (!cfg_fmt)
>  		return -EINVAL;
>
> -	mutex_lock(&priv->mutex);
>  	format->format = *cfg_fmt;
> -	mutex_unlock(&priv->mutex);
>
>  	return 0;
>  }
> @@ -1259,8 +1251,6 @@ static int max9286_probe(struct i2c_client *client)
>  	if (!priv)
>  		return -ENOMEM;
>
> -	mutex_init(&priv->mutex);
> -
>  	priv->client = client;
>  	i2c_set_clientdata(client, priv);
>
> --
> 2.32.0
>



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux