[PATCH] media: mt9p031: Increase post-reset delay

[PATCH] media: mt9p031: Increase post-reset delay

[Marek Vasut]

The MT9P006 sensor driver sporadically fails to probe because the sensor
responds with a NACK condition to I2C address on the bus during an attempt
to read the sensor MT9P031_CHIP_VERSION register in mt9p031_registered().

Neither the MT9P006 nor MT9P031 datasheets are clear on reset signal timing.
Older MT9M034 [1] datasheet provides those timing figures in Appendix-A and
indicates it is necessary to wait 850000 EXTCLK cycles before starting any
I2C communication.

Add such a delay, which does make the sporadic I2C NACK go away, so it is
likely similar constraint applies to this sensor.

[1] https://www.onsemi.com/pdf/datasheet/mt9m034-d.pdf

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/mt9p031.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 45f7b5e52bc39..7c83be5b9ec9d 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -307,6 +307,7 @@ static inline int mt9p031_pll_disable(struct mt9p031 *mt9p031)
 
 static int mt9p031_power_on(struct mt9p031 *mt9p031)
 {
+	unsigned long delay;
 	int ret;
 
 	/* Ensure RESET_BAR is active */
@@ -334,7 +335,9 @@ static int mt9p031_power_on(struct mt9p031 *mt9p031)
 	/* Now RESET_BAR must be high */
 	if (mt9p031->reset) {
 		gpiod_set_value(mt9p031->reset, 0);
-		usleep_range(1000, 2000);
+		/* Wait 850000 EXTCLK cycles before de-asserting reset. */
+		delay = DIV_ROUND_UP(850000 * 1000, clk_get_rate(mt9p031->clk));
+		msleep(delay);
 	}
 
 	return 0;
-- 
2.35.1

Re: [PATCH] media: mt9p031: Increase post-reset delay

[Sakari Ailus]

Hi Marek,

On Tue, Aug 23, 2022 at 06:57:23PM +0200, Marek Vasut wrote:
> The MT9P006 sensor driver sporadically fails to probe because the sensor
> responds with a NACK condition to I2C address on the bus during an attempt
> to read the sensor MT9P031_CHIP_VERSION register in mt9p031_registered().
> 
> Neither the MT9P006 nor MT9P031 datasheets are clear on reset signal timing.
> Older MT9M034 [1] datasheet provides those timing figures in Appendix-A and
> indicates it is necessary to wait 850000 EXTCLK cycles before starting any
> I2C communication.
> 
> Add such a delay, which does make the sporadic I2C NACK go away, so it is
> likely similar constraint applies to this sensor.
> 
> [1] https://www.onsemi.com/pdf/datasheet/mt9m034-d.pdf
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/media/i2c/mt9p031.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 45f7b5e52bc39..7c83be5b9ec9d 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -307,6 +307,7 @@ static inline int mt9p031_pll_disable(struct mt9p031 *mt9p031)
>  
>  static int mt9p031_power_on(struct mt9p031 *mt9p031)
>  {
> +	unsigned long delay;
>  	int ret;
>  
>  	/* Ensure RESET_BAR is active */
> @@ -334,7 +335,9 @@ static int mt9p031_power_on(struct mt9p031 *mt9p031)
>  	/* Now RESET_BAR must be high */
>  	if (mt9p031->reset) {
>  		gpiod_set_value(mt9p031->reset, 0);
> -		usleep_range(1000, 2000);
> +		/* Wait 850000 EXTCLK cycles before de-asserting reset. */
> +		delay = DIV_ROUND_UP(850000 * 1000, clk_get_rate(mt9p031->clk));

clk_get_rate() will return 0 if there's no clock. I guess that doesn't
happen commonly but it'd be nice to guard against that.

> +		msleep(delay);
>  	}
>  
>  	return 0;

-- 
Regards,

Sakari Ailus