Re: [PATCH v2 18/18] media: i2c: max9286: Rework comments in .bound()

2021-03-15 Thread Laurent Pinchart
Hi Jacopo,

Thank you for the patch.

On Mon, Mar 15, 2021 at 02:15:12PM +0100, Jacopo Mondi wrote:
> Re-phrase a comment in .bound() callback to make it clear we register

s/Re-phrase/Rephrase/

> a subdev notifier and remove a redundant comment about disabling i2c
> auto-ack.
> 
> No functional changes intended.
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/i2c/max9286.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index b6347639901e..16b2cb9b44a2 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -556,9 +556,9 @@ static int max9286_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   subdev->name, src_pad, index);
>  
>   /*
> -  * We can only register v4l2_async_notifiers, which do not provide a
> -  * means to register a complete callback. bound_sources allows us to
> -  * identify when all remote serializers have completed their probe.
> +  * As we register a subdev notifiers we won't get a .complete() callback
> +  * here, so we have to use bound_sources to identify when all remote
> +  * serializers have probed.
>*/
>   if (priv->bound_sources != priv->source_mask)
>   return 0;
> @@ -581,16 +581,12 @@ static int max9286_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   /*
>* All enabled sources have probed and enabled their reverse control
>* channels:
> +  * - The reverse channel amplitude stays high
>* - Verify all configuration links are properly detected
> -  * - Disable auto-ack as communication on the control channel are now
> -  *   stable.
> +  * - Disable auto-ack as communications on the control channel are now
> +  *   stable
>*/
>   max9286_check_config_link(priv, priv->source_mask);
> -
> - /*
> -  * Re-configure I2C with local acknowledge disabled after cameras have
> -  * probed.
> -  */
>   max9286_configure_i2c(priv, false);
>  
>   return max9286_set_pixelrate(priv);

-- 
Regards,

Laurent Pinchart


Re: [PATCH v2 18/18] media: i2c: max9286: Rework comments in .bound()

2021-03-15 Thread Kieran Bingham
On 15/03/2021 13:15, Jacopo Mondi wrote:
> Re-phrase a comment in .bound() callback to make it clear we register
> a subdev notifier and remove a redundant comment about disabling i2c
> auto-ack.
> 
> No functional changes intended.
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/i2c/max9286.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index b6347639901e..16b2cb9b44a2 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -556,9 +556,9 @@ static int max9286_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   subdev->name, src_pad, index);
>  
>   /*
> -  * We can only register v4l2_async_notifiers, which do not provide a
> -  * means to register a complete callback. bound_sources allows us to
> -  * identify when all remote serializers have completed their probe.
> +  * As we register a subdev notifiers we won't get a .complete() callback
> +  * here, so we have to use bound_sources to identify when all remote
> +  * serializers have probed.
>*/
>   if (priv->bound_sources != priv->source_mask)
>   return 0;
> @@ -581,16 +581,12 @@ static int max9286_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   /*
>* All enabled sources have probed and enabled their reverse control
>* channels:
> +  * - The reverse channel amplitude stays high
>* - Verify all configuration links are properly detected
> -  * - Disable auto-ack as communication on the control channel are now
> -  *   stable.
> +  * - Disable auto-ack as communications on the control channel are now
> +  *   stable
>*/
>   max9286_check_config_link(priv, priv->source_mask);
> -
> - /*
> -  * Re-configure I2C with local acknowledge disabled after cameras have
> -  * probed.
> -  */
>   max9286_configure_i2c(priv, false);
>  
>   return max9286_set_pixelrate(priv);
> 



[PATCH v2 18/18] media: i2c: max9286: Rework comments in .bound()

2021-03-15 Thread Jacopo Mondi
Re-phrase a comment in .bound() callback to make it clear we register
a subdev notifier and remove a redundant comment about disabling i2c
auto-ack.

No functional changes intended.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/max9286.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index b6347639901e..16b2cb9b44a2 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -556,9 +556,9 @@ static int max9286_notify_bound(struct v4l2_async_notifier 
*notifier,
subdev->name, src_pad, index);
 
/*
-* We can only register v4l2_async_notifiers, which do not provide a
-* means to register a complete callback. bound_sources allows us to
-* identify when all remote serializers have completed their probe.
+* As we register a subdev notifiers we won't get a .complete() callback
+* here, so we have to use bound_sources to identify when all remote
+* serializers have probed.
 */
if (priv->bound_sources != priv->source_mask)
return 0;
@@ -581,16 +581,12 @@ static int max9286_notify_bound(struct 
v4l2_async_notifier *notifier,
/*
 * All enabled sources have probed and enabled their reverse control
 * channels:
+* - The reverse channel amplitude stays high
 * - Verify all configuration links are properly detected
-* - Disable auto-ack as communication on the control channel are now
-*   stable.
+* - Disable auto-ack as communications on the control channel are now
+*   stable
 */
max9286_check_config_link(priv, priv->source_mask);
-
-   /*
-* Re-configure I2C with local acknowledge disabled after cameras have
-* probed.
-*/
max9286_configure_i2c(priv, false);
 
return max9286_set_pixelrate(priv);
-- 
2.30.0