[PATCH v2 0/4] media: Introduce post_register() subdev operation

[PATCH v2 0/4] media: Introduce post_register() subdev operation

[Jacopo Mondi]

Hello, this series adds a new core operation to the v4l2 subdev ops to allow
deferring part of the initialization sequence to a later point in time.

I'll copy the commit message of the now superseded patch that was using the
deprecated init() subdev operation to achieve the same result

------------------------------------------------------------------------------
The current probe() procedure of the RDACM20 and RDACM21 GMSL cameras is
performed with the embedded MAX9271 serializer's noise immunity
threshold disabled. Once the camera has been initialized by probing the
embedded chips, the threshold is enabled and then compensated on the
deserializer's side by increasing the reverse channel signal amplitude
once all cameras have bound.

The probe routine is thus run without noise immunity activated which
in noisy environment conditions makes the probe sequence less reliable as
the chips configuration requires a relatively high amount of i2c
transactions.

Break chip initialization in two:

- At probe time only configure the serializer's reverse channel with
  noise immunity activated, to reduce the number of transactions
  performed without noise immunity protection enabled
- Move the chips initialization to the .post_register() core subdev operation
  called by the deserializer after all camera have probed and
  have increased their noise immunity threshold

The initialization routine looks like the following:

            MAX9286                  RDACM20/21

            probe()
               |
               ---------------------> |
                                      probe() {
                                         enable_threshold()
                                      }
               |<--------------------|
           v4l2 async bound {
                compensate_amplitude()
                call post_register()
               |-------------------->|
                                     init() {
                                         access camera registers()
                                    }
               |<-------------------
            }
-------------------------------------------------------------------------------

The usage of post_register() is opt-in. The receiver should explicitly require
to be in control of calling post_register() by setting the newly introduced
V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER flag.

If the receiver does not specify such flag, post_registered() is called by the
core just after the subdevice has been registered.

Thanks
   j

Jacopo Mondi (4):
  media: v4l2-subdev: Introduce post_register() core op
  media: v4l2-async: Add notifier flags
  media: v4l2-async: Call post_register() subdev op
  media: i2c: gmsl: Defer camera initialization

 drivers/media/i2c/max9286.c          | 21 ++++++--
 drivers/media/i2c/rdacm20.c          | 81 ++++++++++++++++------------
 drivers/media/i2c/rdacm21.c          | 73 ++++++++++++++-----------
 drivers/media/v4l2-core/v4l2-async.c | 11 ++++
 include/media/v4l2-async.h           | 10 ++++
 include/media/v4l2-subdev.h          |  3 ++
 6 files changed, 130 insertions(+), 69 deletions(-)

--
2.32.0

[PATCH v2 1/4] media: v4l2-subdev: Introduce post_register() core op

[Jacopo Mondi]

Introduce the post_register() subdevice core operation.

The operation is meant to be called after the subdevice has been
registered.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 include/media/v4l2-subdev.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 95f8bfd63273..4c226c4b69a6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -187,6 +187,8 @@ struct v4l2_subdev_io_pin_config {
  *		     for it to be warned when the value of a control changes.
  *
  * @unsubscribe_event: remove event subscription from the control framework.
+ *
+ * @post_register: called after the subdevice has been registered.
  */
 struct v4l2_subdev_core_ops {
 	int (*log_status)(struct v4l2_subdev *sd);
@@ -213,6 +215,7 @@ struct v4l2_subdev_core_ops {
 			       struct v4l2_event_subscription *sub);
 	int (*unsubscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
 				 struct v4l2_event_subscription *sub);
+	int (*post_register)(struct v4l2_subdev *sd);
 };
 
 /**
-- 
2.32.0

[PATCH v2 2/4] media: v4l2-async: Add notifier flags

[Jacopo Mondi]

Add a 'flags' field to the async notifier structure and define the
V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER flag.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-async.c |  1 +
 include/media/v4l2-async.h           | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index cd9e78c63791..0836e01e59ca 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -472,6 +472,7 @@ static int v4l2_async_notifier_asd_valid(struct v4l2_async_notifier *notifier,
 void v4l2_async_notifier_init(struct v4l2_async_notifier *notifier)
 {
 	INIT_LIST_HEAD(&notifier->asd_list);
+	notifier->flags = 0;
 }
 EXPORT_SYMBOL(v4l2_async_notifier_init);
 
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 5b275a845c20..96471fb44646 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -92,6 +92,13 @@ struct v4l2_async_notifier_operations {
 		       struct v4l2_async_subdev *asd);
 };
 
+/*
+ * Set this flag to instruct the core framework not to call the post_register()
+ * core operation. The driver that registered the notifier will take care to
+ * do so eventually.
+ */
+#define V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER			BIT(0)
+
 /**
  * struct v4l2_async_notifier - v4l2_device notifier data
  *
@@ -103,6 +110,8 @@ struct v4l2_async_notifier_operations {
  * @waiting:	list of struct v4l2_async_subdev, waiting for their drivers
  * @done:	list of struct v4l2_subdev, already probed
  * @list:	member in a global list of notifiers
+ * @flags:	notifier's flags. Can be:
+ *	%V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER
  */
 struct v4l2_async_notifier {
 	const struct v4l2_async_notifier_operations *ops;
@@ -113,6 +122,7 @@ struct v4l2_async_notifier {
 	struct list_head waiting;
 	struct list_head done;
 	struct list_head list;
+	u32 flags;
 };
 
 /**
-- 
2.32.0

[PATCH v2 3/4] media: v4l2-async: Call post_register() subdev op

[Jacopo Mondi]

Call the "post_register()" subdev operation on a just registered
subdevice, immediately after having called the 'bound' notifier
operation on the parent.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-async.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 0836e01e59ca..fcc32ec15832 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -293,6 +293,16 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
 		return ret;
 	}
 
+	if (!(notifier->flags & V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER) &&
+	    sd->ops->core && sd->ops->core->post_register) {
+		ret = sd->ops->core->post_register(sd);
+		if (ret) {
+			v4l2_async_notifier_call_unbind(notifier, sd, sd->asd);
+			v4l2_device_unregister_subdev(sd);
+			return ret;
+		}
+	}
+
 	/* Remove from the waiting list */
 	list_del(&asd->list);
 	sd->asd = asd;
-- 
2.32.0

[PATCH v2 4/4] media: i2c: gmsl: Defer camera initialization

[Jacopo Mondi]

Use the post_register() callback to defer the bulk of the camera
modules initialization. The deserializer driver waits for all the
registered camera to probe, increases its channel amplitude and the
calls the subdevice post_register() operation that completes the
initialization procedure.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 21 ++++++++--
 drivers/media/i2c/rdacm20.c | 81 +++++++++++++++++++++----------------
 drivers/media/i2c/rdacm21.c | 73 ++++++++++++++++++---------------
 3 files changed, 106 insertions(+), 69 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 8cd3340ea9bb..1b92d18a1f94 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -567,17 +567,31 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
 	if (priv->bound_sources != priv->source_mask)
 		return 0;
 
+	/*
+	 * Once all cameras have probed, increase the channel amplitude
+	 * to compensate for the remote noise immunity threshold and call
+	 * the camera post_register operation to complete initialization with
+	 * noise immunity enabled.
+	 */
+	max9286_reverse_channel_setup(priv, MAX9286_REV_AMP_HIGH);
+	for_each_source(priv, source) {
+		ret = v4l2_subdev_call(source->sd, core, post_register);
+		if (ret) {
+			dev_err(&priv->client->dev,
+				"Failed to initialize camera device %u\n",
+				index);
+			return ret;
+		}
+	}
+
 	/*
 	 * All enabled sources have probed and enabled their reverse control
 	 * channels:
 	 *
-	 * - Increase the reverse channel amplitude to compensate for the
-	 *   remote ends high threshold
 	 * - Verify all configuration links are properly detected
 	 * - Disable auto-ack as communication on the control channel are now
 	 *   stable.
 	 */
-	max9286_reverse_channel_setup(priv, MAX9286_REV_AMP_HIGH);
 	max9286_check_config_link(priv, priv->source_mask);
 	max9286_configure_i2c(priv, false);
 
@@ -630,6 +644,7 @@ static int max9286_v4l2_notifier_register(struct max9286_priv *priv)
 	}
 
 	priv->notifier.ops = &max9286_notify_ops;
+	priv->notifier.flags |= V4L2_ASYNC_NOTIFIER_DEFER_POST_REGISTER;
 
 	ret = v4l2_async_subdev_notifier_register(&priv->sd, &priv->notifier);
 	if (ret) {
diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
index 025a610de893..ba956733aa89 100644
--- a/drivers/media/i2c/rdacm20.c
+++ b/drivers/media/i2c/rdacm20.c
@@ -435,44 +435,12 @@ static int rdacm20_get_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops rdacm20_video_ops = {
-	.s_stream	= rdacm20_s_stream,
-};
-
-static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {
-	.enum_mbus_code = rdacm20_enum_mbus_code,
-	.get_fmt	= rdacm20_get_fmt,
-	.set_fmt	= rdacm20_get_fmt,
-};
-
-static const struct v4l2_subdev_ops rdacm20_subdev_ops = {
-	.video		= &rdacm20_video_ops,
-	.pad		= &rdacm20_subdev_pad_ops,
-};
-
-static int rdacm20_initialize(struct rdacm20_device *dev)
+static int rdacm20_post_register(struct v4l2_subdev *sd)
 {
+	struct rdacm20_device *dev = sd_to_rdacm20(sd);
 	unsigned int retry = 3;
 	int ret;
 
-	max9271_wake_up(&dev->serializer);
-
-	/* Serial link disabled during config as it needs a valid pixel clock. */
-	ret = max9271_set_serial_link(&dev->serializer, false);
-	if (ret)
-		return ret;
-
-	/*
-	 *  Ensure that we have a good link configuration before attempting to
-	 *  identify the device.
-	 */
-	ret = max9271_configure_i2c(&dev->serializer,
-				    MAX9271_I2CSLVSH_469NS_234NS |
-				    MAX9271_I2CSLVTO_1024US |
-				    MAX9271_I2CMSTBT_105KBPS);
-	if (ret)
-		return ret;
-
 	/*
 	 * Hold OV10635 in reset during max9271 configuration. The reset signal
 	 * has to be asserted for at least 200 microseconds.
@@ -549,6 +517,51 @@ static int rdacm20_initialize(struct rdacm20_device *dev)
 
 	dev_info(dev->dev, "Identified RDACM20 camera module\n");
 
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops rdacm20_video_ops = {
+	.s_stream	= rdacm20_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {
+	.enum_mbus_code = rdacm20_enum_mbus_code,
+	.get_fmt	= rdacm20_get_fmt,
+	.set_fmt	= rdacm20_get_fmt,
+};
+
+static const struct v4l2_subdev_core_ops rdacm20_core_ops = {
+	.post_register	= rdacm20_post_register,
+};
+
+static const struct v4l2_subdev_ops rdacm20_subdev_ops = {
+	.core		= &rdacm20_core_ops,
+	.video		= &rdacm20_video_ops,
+	.pad		= &rdacm20_subdev_pad_ops,
+};
+
+static int rdacm20_initialize(struct rdacm20_device *dev)
+{
+	int ret;
+
+	max9271_wake_up(&dev->serializer);
+
+	/* Serial link disabled during config as it needs a valid pixel clock. */
+	ret = max9271_set_serial_link(&dev->serializer, false);
+	if (ret)
+		return ret;
+
+	/*
+	 *  Ensure that we have a good link configuration before attempting to
+	 *  identify the device.
+	 */
+	ret = max9271_configure_i2c(&dev->serializer,
+				    MAX9271_I2CSLVSH_469NS_234NS |
+				    MAX9271_I2CSLVTO_1024US |
+				    MAX9271_I2CMSTBT_105KBPS);
+	if (ret)
+		return ret;
+
 	/*
 	 * Set reverse channel high threshold to increase noise immunity.
 	 *
diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c
index 12ec5467ed1e..35217782f693 100644
--- a/drivers/media/i2c/rdacm21.c
+++ b/drivers/media/i2c/rdacm21.c
@@ -315,21 +315,6 @@ static int rdacm21_get_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops rdacm21_video_ops = {
-	.s_stream	= rdacm21_s_stream,
-};
-
-static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {
-	.enum_mbus_code = rdacm21_enum_mbus_code,
-	.get_fmt	= rdacm21_get_fmt,
-	.set_fmt	= rdacm21_get_fmt,
-};
-
-static const struct v4l2_subdev_ops rdacm21_subdev_ops = {
-	.video		= &rdacm21_video_ops,
-	.pad		= &rdacm21_subdev_pad_ops,
-};
-
 static void ov10640_power_up(struct rdacm21_device *dev)
 {
 	/* Enable GPIO0#0 (reset) and GPIO1#0 (pwdn) as output lines. */
@@ -470,25 +455,11 @@ static int ov490_initialize(struct rdacm21_device *dev)
 	return 0;
 }
 
-static int rdacm21_initialize(struct rdacm21_device *dev)
+static int rdacm21_post_register(struct v4l2_subdev *sd)
 {
+	struct rdacm21_device *dev = sd_to_rdacm21(sd);
 	int ret;
 
-	max9271_wake_up(&dev->serializer);
-
-	/* Enable reverse channel and disable the serial link. */
-	ret = max9271_set_serial_link(&dev->serializer, false);
-	if (ret)
-		return ret;
-
-	/* Configure I2C bus at 105Kbps speed and configure GMSL. */
-	ret = max9271_configure_i2c(&dev->serializer,
-				    MAX9271_I2CSLVSH_469NS_234NS |
-				    MAX9271_I2CSLVTO_1024US |
-				    MAX9271_I2CMSTBT_105KBPS);
-	if (ret)
-		return ret;
-
 	ret = max9271_verify_id(&dev->serializer);
 	if (ret)
 		return ret;
@@ -527,7 +498,45 @@ static int rdacm21_initialize(struct rdacm21_device *dev)
 		return ret;
 	usleep_range(3000, 5000);
 
-	ret = ov490_initialize(dev);
+	return ov490_initialize(dev);
+}
+
+static const struct v4l2_subdev_video_ops rdacm21_video_ops = {
+	.s_stream	= rdacm21_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {
+	.enum_mbus_code = rdacm21_enum_mbus_code,
+	.get_fmt	= rdacm21_get_fmt,
+	.set_fmt	= rdacm21_get_fmt,
+};
+
+static const struct v4l2_subdev_core_ops rdacm21_core_ops = {
+	.post_register	= rdacm21_post_register,
+};
+
+static const struct v4l2_subdev_ops rdacm21_subdev_ops = {
+	.core		= &rdacm21_core_ops,
+	.video		= &rdacm21_video_ops,
+	.pad		= &rdacm21_subdev_pad_ops,
+};
+
+static int rdacm21_initialize(struct rdacm21_device *dev)
+{
+	int ret;
+
+	max9271_wake_up(&dev->serializer);
+
+	/* Enable reverse channel and disable the serial link. */
+	ret = max9271_set_serial_link(&dev->serializer, false);
+	if (ret)
+		return ret;
+
+	/* Configure I2C bus at 105Kbps speed and configure GMSL. */
+	ret = max9271_configure_i2c(&dev->serializer,
+				    MAX9271_I2CSLVSH_469NS_234NS |
+				    MAX9271_I2CSLVTO_1024US |
+				    MAX9271_I2CMSTBT_105KBPS);
 	if (ret)
 		return ret;
 
-- 
2.32.0