On Thu, Aug 15, 2019 at 11:24:22AM +0300, Sakari Ailus wrote:

Hi Helen,

On Wed, Aug 14, 2019 at 09:58:05PM -0300, Helen Koike wrote:

...

quoted

quoted

quoted

+static int rkisp1_isp_sd_set_fmt(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_pad_config *cfg,
+				 struct v4l2_subdev_format *fmt)
+{
+	struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
+	struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev;
+	struct v4l2_mbus_framefmt *mf = &fmt->format;
+

Note that for sub-device nodes, the driver is itself responsible for
serialising the access to its data structures.

But looking at subdev_do_ioctl_lock(), it seems that it serializes the
ioctl calls for subdevs, no? Or I'm misunderstanding something (which is
most probably) ?

Good question. I had missed this change --- subdev_do_ioctl_lock() is
relatively new. But setting that lock is still not possible as the struct
is allocated in the framework and the device is registered before the
driver gets hold of it. It's a good idea to provide the same serialisation
for subdevs as well.

I'll get back to this later.

The main reason is actually that these ops are also called through the
sub-device kAPI, not only through the uAPI, and the locks are only taken
through the calls via uAPI.

So adding the locks to uAPI calls alone would not address the issue.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel