Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 14 Jan 2021 01:09:42 +0200
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc:     kieran.bingham+renesas@ideasonboard.com,
        laurent.pinchart+renesas@ideasonboard.com,
        niklas.soderlund+renesas@ragnatech.se, geert@linux-m68k.org,
        linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
        linux-kernel@vger.kernel.org, Hyun Kwon <hyunk@xilinx.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        sergei.shtylyov@gmail.com
Subject: Re: [PATCH v7 2/7] fixup! media: i2c: rdacm21: Fix GPIO handling
Message-ID: <X/99thQ5z/lJGrjJ@pendragon.ideasonboard.com>
References: <20210113185506.119808-1-jacopo+renesas@jmondi.org>
 <20210113185506.119808-3-jacopo+renesas@jmondi.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20210113185506.119808-3-jacopo+renesas@jmondi.org>
Precedence: bulk

Hi Jacopo,

Thank you for the patch.

On Wed, Jan 13, 2021 at 07:55:00PM +0100, Jacopo Mondi wrote:
> The MAX9271 GPIO line connected to the OV490 RESETB line is
> GPIO1, not GPO. As the GPIO1 line is not enabled by default, first
> enable it then control the OV490 reset during the MAX9271 configuration
> procedure.
> 
> Before this change the embedded OV490 ISP was not actually reset.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/rdacm21.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c
> index f3841369768c..0428e3209463 100644
> --- a/drivers/media/i2c/rdacm21.c
> +++ b/drivers/media/i2c/rdacm21.c
> @@ -425,27 +425,23 @@ static int rdacm21_initialize(struct rdacm21_device *dev)
>  	if (ret)
>  		return ret;
>  
> -	ret = max9271_configure_gmsl_link(&dev->serializer);
> +	/* Enable GPIO1 and hold OV490 in reset during max9271 configuration. */
> +	ret = max9271_enable_gpios(&dev->serializer, MAX9271_GPIO1OUT);
>  	if (ret)
>  		return ret;
>  
> -	/* Set GPO high to hold OV490 in reset. */
> -	ret = max9271_set_gpios(&dev->serializer, MAX9271_GPO);
> +	ret = max9271_clear_gpios(&dev->serializer, MAX9271_GPIO1OUT);
>  	if (ret)
>  		return ret;
>  
> -	ret = max9271_set_address(&dev->serializer, dev->addrs[0]);
> +	ret = max9271_configure_gmsl_link(&dev->serializer);
>  	if (ret)
>  		return ret;
> -	dev->serializer.client->addr = dev->addrs[0];
>  
> -	/*
> -	 * Release OV490 from reset and program address translation
> -	 * before performing OV490 configuration.
> -	 */
> -	ret = max9271_clear_gpios(&dev->serializer, MAX9271_GPO);
> +	ret = max9271_set_address(&dev->serializer, dev->addrs[0]);
>  	if (ret)
>  		return ret;
> +	dev->serializer.client->addr = dev->addrs[0];
>  
>  	ret = max9271_set_translation(&dev->serializer, dev->addrs[1],
>  				      OV490_I2C_ADDRESS);
> @@ -453,6 +449,12 @@ static int rdacm21_initialize(struct rdacm21_device *dev)
>  		return ret;
>  	dev->isp->addr = dev->addrs[1];
>  
> +	/* Release OV490 from reset and initialize it. */
> +	ret = max9271_set_gpios(&dev->serializer, MAX9271_GPIO1OUT);
> +	if (ret)
> +		return ret;
> +	usleep_range(3000, 5000);
> +
>  	ret = ov490_initialize(dev);
>  	if (ret)
>  		return ret;

-- 
Regards,

Laurent Pinchart