| Message ID | 20220823222216.411656-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] media: mt9p031: Increase post-reset delay | expand |
On 8/24/22 00:22, 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> > --- > V2: - In case clk_get_rate() returns 0, use slowest supported clock > to avoid division by zero Any news on this patch ?
Hi Marek, Thank you for the patch. On Wed, Aug 24, 2022 at 12:22:16AM +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> Given the lack of information regarding the MT9P006 reset timings, this seems to be the best we can do. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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> > --- > V2: - In case clk_get_rate() returns 0, use slowest supported clock > to avoid division by zero > --- > drivers/media/i2c/mt9p031.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c > index 45f7b5e52bc39..5f5caafe56887 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 rate, delay; > int ret; > > /* Ensure RESET_BAR is active */ > @@ -334,7 +335,12 @@ 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. */ > + rate = clk_get_rate(mt9p031->clk); > + if (!rate) > + rate = 6000000; /* Slowest supported clock, 6 MHz */ > + delay = DIV_ROUND_UP(850000 * 1000, rate); > + msleep(delay); > } > > return 0;
Hi Marek, On Sun, Nov 20, 2022 at 02:55:22PM +0100, Marek Vasut wrote: > On 8/24/22 00:22, 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> > > --- > > V2: - In case clk_get_rate() returns 0, use slowest supported clock > > to avoid division by zero > > Any news on this patch ? Thanks for the ping. It's in my tree now.
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 45f7b5e52bc39..5f5caafe56887 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 rate, delay; int ret; /* Ensure RESET_BAR is active */ @@ -334,7 +335,12 @@ 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. */ + rate = clk_get_rate(mt9p031->clk); + if (!rate) + rate = 6000000; /* Slowest supported clock, 6 MHz */ + delay = DIV_ROUND_UP(850000 * 1000, rate); + msleep(delay); } return 0;
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> --- V2: - In case clk_get_rate() returns 0, use slowest supported clock to avoid division by zero --- drivers/media/i2c/mt9p031.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)