linux-kernel - Re: [PATCH v7 2/7] fixup! media: i2c: rdacm21: Fix GPIO handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jan 2021 01:09:42 +0200
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Jacopo Mondi <jacopo+renesas@...ndi.org>
Cc:     kieran.bingham+renesas@...asonboard.com,
        laurent.pinchart+renesas@...asonboard.com,
        niklas.soderlund+renesas@...natech.se, geert@...ux-m68k.org,
        linux-media@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Hyun Kwon <hyunk@...inx.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        sergei.shtylyov@...il.com
Subject: Re: [PATCH v7 2/7] fixup! media: i2c: rdacm21: Fix GPIO handling

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@...ndi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.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