diff mbox series

[v3,5/7] media: i2c: max9286: Configure reverse channel amplitude

Message ID 20201016120625.64337-6-jacopo+renesas@jmondi.org (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series media: i2c: Add support for RDACM21 camera module | expand

Commit Message

Jacopo Mondi Oct. 16, 2020, 12:06 p.m. UTC
Adjust reverse channel amplitude according to the presence of
the 'high-threshold" DTS property.

If no high threshold compensation is required, start with a low
amplitude (100mV) and increase it after the remote serializers
have probed and have enabled noise immunity on their reverse
channels.

If high threshold compensation is required, configure the reverse
channel with a 170mV amplitude before the remote serializers have
probed.

This change is required for both rdacm20 and rdacm21 camera modules
to be correctly probed when used in combination with the max9286
deserializer.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Kieran Bingham Oct. 16, 2020, 11:04 a.m. UTC | #1
Hi Jacopo,

On 16/10/2020 13:06, Jacopo Mondi wrote:
> Adjust reverse channel amplitude according to the presence of
> the 'high-threshold" DTS property.
> 
> If no high threshold compensation is required, start with a low
> amplitude (100mV) and increase it after the remote serializers
> have probed and have enabled noise immunity on their reverse
> channels.
> 
> If high threshold compensation is required, configure the reverse
> channel with a 170mV amplitude before the remote serializers have
> probed.
> 
> This change is required for both rdacm20 and rdacm21 camera modules
> to be correctly probed when used in combination with the max9286
> deserializer.

My only fear here would be that perhaps on other cameras we need a more
fine-grained control of the amplitudes?

But I'll leave that discussion to the binding itself,

For this patch ...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/max9286.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 31e27d0f34f1..4c72e1e6b27b 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -163,6 +163,8 @@ struct max9286_priv {
>  	unsigned int mux_channel;
>  	bool mux_open;
>  
> +	bool high_threshold;
> +
>  	struct v4l2_ctrl_handler ctrls;
>  	struct v4l2_ctrl *pixelrate;
>  
> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
>  	 * All enabled sources have probed and enabled their reverse control
>  	 * channels:
>  	 *
> +	 * - Increase the reverse channel amplitude to compensate for the
> +	 *   remote ends high threshold, if not done already
>  	 * - Verify all configuration links are properly detected
>  	 * - Disable auto-ack as communication on the control channel are now
>  	 *   stable.
>  	 */
> +	if (!priv->high_threshold)
> +		max9286_reverse_channel_setup(priv, 170);
>  	max9286_check_config_link(priv, priv->source_mask);
>  
>  	/*
> @@ -967,7 +973,12 @@ static int max9286_setup(struct max9286_priv *priv)
>  	 * only. This should be disabled after the mux is initialised.
>  	 */
>  	max9286_configure_i2c(priv, true);
> -	max9286_reverse_channel_setup(priv, 170);
> +
> +	/*
> +	 * Compensate the remote end high threshold with a larger channel
> +	 * amplitude if necessary.
> +	 */
> +	max9286_reverse_channel_setup(priv, priv->high_threshold ? 170 : 100);
>  
>  	/*
>  	 * Enable GMSL links, mask unused ones and autodetect link
> @@ -1235,6 +1246,12 @@ static int max9286_parse_dt(struct max9286_priv *priv)
>  	}
>  	of_node_put(node);
>  
> +	/*
> +	 * Parse 'high_threshold' property to configure the reverse channel
> +	 * amplitude.
> +	 */
> +	priv->high_threshold = device_property_present(dev, "high_threshold");
> +
>  	priv->route_mask = priv->source_mask;
>  
>  	return 0;
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 31e27d0f34f1..4c72e1e6b27b 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -163,6 +163,8 @@  struct max9286_priv {
 	unsigned int mux_channel;
 	bool mux_open;
 
+	bool high_threshold;
+
 	struct v4l2_ctrl_handler ctrls;
 	struct v4l2_ctrl *pixelrate;
 
@@ -557,10 +559,14 @@  static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
 	 * All enabled sources have probed and enabled their reverse control
 	 * channels:
 	 *
+	 * - Increase the reverse channel amplitude to compensate for the
+	 *   remote ends high threshold, if not done already
 	 * - Verify all configuration links are properly detected
 	 * - Disable auto-ack as communication on the control channel are now
 	 *   stable.
 	 */
+	if (!priv->high_threshold)
+		max9286_reverse_channel_setup(priv, 170);
 	max9286_check_config_link(priv, priv->source_mask);
 
 	/*
@@ -967,7 +973,12 @@  static int max9286_setup(struct max9286_priv *priv)
 	 * only. This should be disabled after the mux is initialised.
 	 */
 	max9286_configure_i2c(priv, true);
-	max9286_reverse_channel_setup(priv, 170);
+
+	/*
+	 * Compensate the remote end high threshold with a larger channel
+	 * amplitude if necessary.
+	 */
+	max9286_reverse_channel_setup(priv, priv->high_threshold ? 170 : 100);
 
 	/*
 	 * Enable GMSL links, mask unused ones and autodetect link
@@ -1235,6 +1246,12 @@  static int max9286_parse_dt(struct max9286_priv *priv)
 	}
 	of_node_put(node);
 
+	/*
+	 * Parse 'high_threshold' property to configure the reverse channel
+	 * amplitude.
+	 */
+	priv->high_threshold = device_property_present(dev, "high_threshold");
+
 	priv->route_mask = priv->source_mask;
 
 	return 0;