[PATCH 0/3] Media device file handle support, prepare for requests

[PATCH 0/3] Media device file handle support, prepare for requests

[Sakari Ailus]

Hi everyone,

These patches add support for media device file handle and make it more
practical to have callback functions in the media device. Also pad
init_cfg() operation is called at sub-device open.

While not directly related to the request API, these will be needed, or
are at least beneficial in implementing it.

I believe we could merge them before the request API as I don't think they
contain controversial aspects (there will be enough of them in the request
API patches themselves) so we could better focus on the problem at hand.

The patches have been written by Laurent, I've rebased them and fixed
minor issues as well.

-- 
Kind regards,
Sakari

[PATCH 1/3] media: Move media_device link_notify operation to an ops structure

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

This will allow adding new operations without increasing the
media_device structure size for drivers that don't implement any media
device operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Fix compilation error for the omap3isp driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-entity.c                  | 11 ++++++-----
 drivers/media/platform/exynos4-is/media-dev.c |  6 +++++-
 drivers/media/platform/omap3isp/isp.c         |  6 +++++-
 drivers/staging/media/omap4iss/iss.c          |  6 +++++-
 include/media/media-device.h                  | 16 ++++++++++++----
 5 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index c53c1d5..301fd4f 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -806,17 +806,18 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
 
 	mdev = source->graph_obj.mdev;
 
-	if (mdev->link_notify) {
-		ret = mdev->link_notify(link, flags,
-					MEDIA_DEV_NOTIFY_PRE_LINK_CH);
+	if (mdev->ops && mdev->ops->link_notify) {
+		ret = mdev->ops->link_notify(link, flags,
+					     MEDIA_DEV_NOTIFY_PRE_LINK_CH);
 		if (ret < 0)
 			return ret;
 	}
 
 	ret = __media_entity_setup_link_notify(link, flags);
 
-	if (mdev->link_notify)
-		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
+	if (mdev->ops && mdev->ops->link_notify)
+		mdev->ops->link_notify(link, flags,
+				       MEDIA_DEV_NOTIFY_POST_LINK_CH);
 
 	return ret;
 }
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 04348b5..1ed1a9e 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1190,6 +1190,10 @@ static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
 	return ret ? -EPIPE : 0;
 }
 
+static const struct media_device_ops fimc_md_ops = {
+	.link_notify = fimc_md_link_notify,
+};
+
 static ssize_t fimc_md_sysfs_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -1416,7 +1420,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 
 	strlcpy(fmd->media_dev.model, "SAMSUNG S5P FIMC",
 		sizeof(fmd->media_dev.model));
-	fmd->media_dev.link_notify = fimc_md_link_notify;
+	fmd->media_dev.ops = &fimc_md_ops;
 	fmd->media_dev.dev = dev;
 
 	v4l2_dev = &fmd->v4l2_dev;
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 5d54e2c..0321d84 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -657,6 +657,10 @@ static irqreturn_t isp_isr(int irq, void *_isp)
 	return IRQ_HANDLED;
 }
 
+static const struct media_device_ops isp_media_ops = {
+	.link_notify = v4l2_pipeline_link_notify,
+};
+
 /* -----------------------------------------------------------------------------
  * Pipeline stream management
  */
@@ -1680,7 +1684,7 @@ static int isp_register_entities(struct isp_device *isp)
 	strlcpy(isp->media_dev.model, "TI OMAP3 ISP",
 		sizeof(isp->media_dev.model));
 	isp->media_dev.hw_revision = isp->revision;
-	isp->media_dev.link_notify = v4l2_pipeline_link_notify;
+	isp->media_dev.ops = &isp_media_ops;
 	media_device_init(&isp->media_dev);
 
 	isp->v4l2_dev.mdev = &isp->media_dev;
diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index c5a5138..355a704 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -362,6 +362,10 @@ static irqreturn_t iss_isr(int irq, void *_iss)
 	return IRQ_HANDLED;
 }
 
+static const struct media_device_ops iss_media_ops = {
+	.link_notify = v4l2_pipeline_link_notify,
+};
+
 /* -----------------------------------------------------------------------------
  * Pipeline stream management
  */
@@ -988,7 +992,7 @@ static int iss_register_entities(struct iss_device *iss)
 	strlcpy(iss->media_dev.model, "TI OMAP4 ISS",
 		sizeof(iss->media_dev.model));
 	iss->media_dev.hw_revision = iss->revision;
-	iss->media_dev.link_notify = v4l2_pipeline_link_notify;
+	iss->media_dev.ops = &iss_media_ops;
 	ret = media_device_register(&iss->media_dev);
 	if (ret < 0) {
 		dev_err(iss->dev, "Media device registration failed (%d)\n",
diff --git a/include/media/media-device.h b/include/media/media-device.h
index a9b33c4..19c8ed4 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -280,6 +280,16 @@ struct media_entity_notify {
 };
 
 /**
+ * struct media_device_ops - Media device operations
+ * @link_notify: Link state change notification callback. This callback is
+ *		 called with the graph_mutex held.
+ */
+struct media_device_ops {
+	int (*link_notify)(struct media_link *link, u32 flags,
+			   unsigned int notification);
+};
+
+/**
  * struct media_device - Media device
  * @dev:	Parent device
  * @devnode:	Media device node
@@ -311,8 +321,7 @@ struct media_entity_notify {
  * @enable_source: Enable Source Handler function pointer
  * @disable_source: Disable Source Handler function pointer
  *
- * @link_notify: Link state change notification callback. This callback is
- *		 called with the graph_mutex held.
+ * @ops:	Operation handler callbacks
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -379,8 +388,7 @@ struct media_device {
 			     struct media_pipeline *pipe);
 	void (*disable_source)(struct media_entity *entity);
 
-	int (*link_notify)(struct media_link *link, u32 flags,
-			   unsigned int notification);
+	const struct media_device_ops *ops;
 };
 
 /* We don't need to include pci.h or usb.h here */
-- 
1.9.1

[PATCH 2/3] media: Add per-file-handle data support

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The media devnode core associates devnodes with files by storing the
devnode pointer in the file structure private_data field. In order to
allow tracking of per-file-handle data introduce a new media devnode
file handle structure that stores the devnode pointer, and store a
pointer to that structure in the file private_data field.

Users of the media devnode code (the only existing user being
media_device) are responsible for managing their own subclass of the
media_devnode_fh structure.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/media-device.c  | 21 +++++++++++++++++++++
 drivers/media/media-devnode.c | 21 ++++++++++-----------
 include/media/media-devnode.h | 18 +++++++++++++++++-
 3 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 898a3cf..89602a7 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -39,6 +39,15 @@
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 
+struct media_device_fh {
+	struct media_devnode_fh fh;
+};
+
+static inline struct media_device_fh *media_device_fh(struct file *filp)
+{
+	return container_of(filp->private_data, struct media_device_fh, fh);
+}
+
 /* -----------------------------------------------------------------------------
  * Userspace API
  */
@@ -50,11 +59,23 @@ static inline void __user *media_get_uptr(__u64 arg)
 
 static int media_device_open(struct file *filp)
 {
+	struct media_device_fh *fh;
+
+	fh = kzalloc(sizeof(*fh), GFP_KERNEL);
+	if (!fh)
+		return -ENOMEM;
+
+	filp->private_data = &fh->fh;
+
 	return 0;
 }
 
 static int media_device_close(struct file *filp)
 {
+	struct media_device_fh *fh = media_device_fh(filp);
+
+	kfree(fh);
+
 	return 0;
 }
 
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 64a4b1e..d4d2917 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -154,6 +154,7 @@ static long media_compat_ioctl(struct file *filp, unsigned int cmd,
 /* Override for the open function */
 static int media_open(struct inode *inode, struct file *filp)
 {
+	struct media_devnode_fh *fh;
 	struct media_devnode *mdev;
 	int ret;
 
@@ -175,17 +176,16 @@ static int media_open(struct inode *inode, struct file *filp)
 	get_device(&mdev->dev);
 	mutex_unlock(&media_devnode_lock);
 
-	filp->private_data = mdev;
-
-	if (mdev->fops->open) {
-		ret = mdev->fops->open(filp);
-		if (ret) {
-			put_device(&mdev->dev);
-			filp->private_data = NULL;
-			return ret;
-		}
+	ret = mdev->fops->open(filp);
+	if (ret) {
+		put_device(&mdev->dev);
+		filp->private_data = NULL;
+		return ret;
 	}
 
+	fh = filp->private_data;
+	fh->devnode = mdev;
+
 	return 0;
 }
 
@@ -194,8 +194,7 @@ static int media_release(struct inode *inode, struct file *filp)
 {
 	struct media_devnode *mdev = media_devnode_data(filp);
 
-	if (mdev->fops->release)
-		mdev->fops->release(filp);
+	mdev->fops->release(filp);
 
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index fe42f08..09aafc3 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -66,6 +66,20 @@ struct media_file_operations {
 };
 
 /**
+ * struct media_devnode_fh - Media device node file handle
+ * @devnode:	pointer to the media device node
+ *
+ * This structure serves as a base for per-file-handle data storage. Media
+ * device node users embed media_devnode_fh in their custom file handle data
+ * structures and store the media_devnode_fh in the file private_data in order
+ * to let the media device node core locate the media_devnode corresponding to a
+ * file handle.
+ */
+struct media_devnode_fh {
+	struct media_devnode *devnode;
+};
+
+/**
  * struct media_devnode - Media device node
  * @fops:	pointer to struct &media_file_operations with media device ops
  * @dev:	struct device pointer for the media controller device
@@ -138,7 +152,9 @@ void media_devnode_unregister(struct media_devnode *mdev);
  */
 static inline struct media_devnode *media_devnode_data(struct file *filp)
 {
-	return filp->private_data;
+	struct media_devnode_fh *fh = filp->private_data;
+
+	return fh->devnode;
 }
 
 /**
-- 
1.9.1

[PATCH 3/3] v4l: subdev: Call pad init_cfg operation when opening subdevs

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The subdev core code currently rely on the subdev open handler to
initialize the file handle's pad configuration, even though subdevs now
have a pad operation dedicated for that purpose.

As a first step towards migration to init_cfg, call the operation
operation in the subdev core open implementation. Subdevs that are
haven't been moved to init_cfg yet will just continue implementing pad
config initialization in their open handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 224ea60..9cbd011 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -85,6 +85,8 @@ static int subdev_open(struct file *file)
 	}
 #endif
 
+	v4l2_subdev_call(sd, pad, init_cfg, subdev_fh->pad);
+
 	if (sd->internal_ops && sd->internal_ops->open) {
 		ret = sd->internal_ops->open(sd, subdev_fh);
 		if (ret < 0)
-- 
1.9.1

[PATCH 1/3] media: Move media_device link_notify operation to an ops structure

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

This will allow adding new operations without increasing the
media_device structure size for drivers that don't implement any media
device operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Fix compilation error for the omap3isp driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-entity.c                  | 11 ++++++-----
 drivers/media/platform/exynos4-is/media-dev.c |  6 +++++-
 drivers/media/platform/omap3isp/isp.c         |  6 +++++-
 drivers/staging/media/omap4iss/iss.c          |  6 +++++-
 include/media/media-device.h                  | 16 ++++++++++++----
 5 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index c53c1d5..301fd4f 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -806,17 +806,18 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
 
 	mdev = source->graph_obj.mdev;
 
-	if (mdev->link_notify) {
-		ret = mdev->link_notify(link, flags,
-					MEDIA_DEV_NOTIFY_PRE_LINK_CH);
+	if (mdev->ops && mdev->ops->link_notify) {
+		ret = mdev->ops->link_notify(link, flags,
+					     MEDIA_DEV_NOTIFY_PRE_LINK_CH);
 		if (ret < 0)
 			return ret;
 	}
 
 	ret = __media_entity_setup_link_notify(link, flags);
 
-	if (mdev->link_notify)
-		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
+	if (mdev->ops && mdev->ops->link_notify)
+		mdev->ops->link_notify(link, flags,
+				       MEDIA_DEV_NOTIFY_POST_LINK_CH);
 
 	return ret;
 }
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 04348b5..1ed1a9e 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1190,6 +1190,10 @@ static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
 	return ret ? -EPIPE : 0;
 }
 
+static const struct media_device_ops fimc_md_ops = {
+	.link_notify = fimc_md_link_notify,
+};
+
 static ssize_t fimc_md_sysfs_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -1416,7 +1420,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 
 	strlcpy(fmd->media_dev.model, "SAMSUNG S5P FIMC",
 		sizeof(fmd->media_dev.model));
-	fmd->media_dev.link_notify = fimc_md_link_notify;
+	fmd->media_dev.ops = &fimc_md_ops;
 	fmd->media_dev.dev = dev;
 
 	v4l2_dev = &fmd->v4l2_dev;
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 5d54e2c..0321d84 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -657,6 +657,10 @@ static irqreturn_t isp_isr(int irq, void *_isp)
 	return IRQ_HANDLED;
 }
 
+static const struct media_device_ops isp_media_ops = {
+	.link_notify = v4l2_pipeline_link_notify,
+};
+
 /* -----------------------------------------------------------------------------
  * Pipeline stream management
  */
@@ -1680,7 +1684,7 @@ static int isp_register_entities(struct isp_device *isp)
 	strlcpy(isp->media_dev.model, "TI OMAP3 ISP",
 		sizeof(isp->media_dev.model));
 	isp->media_dev.hw_revision = isp->revision;
-	isp->media_dev.link_notify = v4l2_pipeline_link_notify;
+	isp->media_dev.ops = &isp_media_ops;
 	media_device_init(&isp->media_dev);
 
 	isp->v4l2_dev.mdev = &isp->media_dev;
diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index c5a5138..355a704 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -362,6 +362,10 @@ static irqreturn_t iss_isr(int irq, void *_iss)
 	return IRQ_HANDLED;
 }
 
+static const struct media_device_ops iss_media_ops = {
+	.link_notify = v4l2_pipeline_link_notify,
+};
+
 /* -----------------------------------------------------------------------------
  * Pipeline stream management
  */
@@ -988,7 +992,7 @@ static int iss_register_entities(struct iss_device *iss)
 	strlcpy(iss->media_dev.model, "TI OMAP4 ISS",
 		sizeof(iss->media_dev.model));
 	iss->media_dev.hw_revision = iss->revision;
-	iss->media_dev.link_notify = v4l2_pipeline_link_notify;
+	iss->media_dev.ops = &iss_media_ops;
 	ret = media_device_register(&iss->media_dev);
 	if (ret < 0) {
 		dev_err(iss->dev, "Media device registration failed (%d)\n",
diff --git a/include/media/media-device.h b/include/media/media-device.h
index a9b33c4..19c8ed4 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -280,6 +280,16 @@ struct media_entity_notify {
 };
 
 /**
+ * struct media_device_ops - Media device operations
+ * @link_notify: Link state change notification callback. This callback is
+ *		 called with the graph_mutex held.
+ */
+struct media_device_ops {
+	int (*link_notify)(struct media_link *link, u32 flags,
+			   unsigned int notification);
+};
+
+/**
  * struct media_device - Media device
  * @dev:	Parent device
  * @devnode:	Media device node
@@ -311,8 +321,7 @@ struct media_entity_notify {
  * @enable_source: Enable Source Handler function pointer
  * @disable_source: Disable Source Handler function pointer
  *
- * @link_notify: Link state change notification callback. This callback is
- *		 called with the graph_mutex held.
+ * @ops:	Operation handler callbacks
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -379,8 +388,7 @@ struct media_device {
 			     struct media_pipeline *pipe);
 	void (*disable_source)(struct media_entity *entity);
 
-	int (*link_notify)(struct media_link *link, u32 flags,
-			   unsigned int notification);
+	const struct media_device_ops *ops;
 };
 
 /* We don't need to include pci.h or usb.h here */
-- 
1.9.1

Re: [PATCH 1/3] media: Move media_device link_notify operation to an ops structure

[Hans Verkuil]

On 05/04/2016 02:26 PM, Sakari Ailus wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> This will allow adding new operations without increasing the
> media_device structure size for drivers that don't implement any media
> device operation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Fix compilation error for the omap3isp driver.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [PATCH 2/3] media: Add per-file-handle data support

[Hans Verkuil]

On 05/04/2016 01:25 PM, Sakari Ailus wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The media devnode core associates devnodes with files by storing the
> devnode pointer in the file structure private_data field. In order to
> allow tracking of per-file-handle data introduce a new media devnode
> file handle structure that stores the devnode pointer, and store a
> pointer to that structure in the file private_data field.
> 
> Users of the media devnode code (the only existing user being
> media_device) are responsible for managing their own subclass of the
> media_devnode_fh structure.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [PATCH 3/3] v4l: subdev: Call pad init_cfg operation when opening subdevs

[Hans Verkuil]

On 05/04/2016 01:25 PM, Sakari Ailus wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The subdev core code currently rely on the subdev open handler to
> initialize the file handle's pad configuration, even though subdevs now
> have a pad operation dedicated for that purpose.
> 
> As a first step towards migration to init_cfg, call the operation
> operation in the subdev core open implementation. Subdevs that are
> haven't been moved to init_cfg yet will just continue implementing pad
> config initialization in their open handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [PATCH 3/3] v4l: subdev: Call pad init_cfg operation when opening subdevs

[Laurent Pinchart]

Hi Sakari,

On Wednesday 04 May 2016 14:25:33 Sakari Ailus wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The subdev core code currently rely on the subdev open handler to
> initialize the file handle's pad configuration, even though subdevs now
> have a pad operation dedicated for that purpose.
> 
> As a first step towards migration to init_cfg, call the operation
> operation in the subdev core open implementation. Subdevs that are
> haven't been moved to init_cfg yet will just continue implementing pad
> config initialization in their open handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> b/drivers/media/v4l2-core/v4l2-subdev.c index 224ea60..9cbd011 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -85,6 +85,8 @@ static int subdev_open(struct file *file)
>  	}
>  #endif
> 
> +	v4l2_subdev_call(sd, pad, init_cfg, subdev_fh->pad);
> +

Given that v4l2_subdev_alloc_pad_config(), called by subdev_fh_init(), already 
calls the init_cfg operation, is this still needed ?

>  	if (sd->internal_ops && sd->internal_ops->open) {
>  		ret = sd->internal_ops->open(sd, subdev_fh);
>  		if (ret < 0)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] v4l: subdev: Call pad init_cfg operation when opening subdevs

[Sakari Ailus]

Hi Laurent,

On Mon, May 09, 2016 at 07:18:11PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 04 May 2016 14:25:33 Sakari Ailus wrote:
> > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > The subdev core code currently rely on the subdev open handler to
> > initialize the file handle's pad configuration, even though subdevs now
> > have a pad operation dedicated for that purpose.
> > 
> > As a first step towards migration to init_cfg, call the operation
> > operation in the subdev core open implementation. Subdevs that are
> > haven't been moved to init_cfg yet will just continue implementing pad
> > config initialization in their open handler.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> > b/drivers/media/v4l2-core/v4l2-subdev.c index 224ea60..9cbd011 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -85,6 +85,8 @@ static int subdev_open(struct file *file)
> >  	}
> >  #endif
> > 
> > +	v4l2_subdev_call(sd, pad, init_cfg, subdev_fh->pad);
> > +
> 
> Given that v4l2_subdev_alloc_pad_config(), called by subdev_fh_init(), already 
> calls the init_cfg operation, is this still needed ?

It's your patch. ;-)

Yeah, after looking at the code, I agree to drop it.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk