[PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers

Wenmeng Liu posted 1 patch 4 weeks, 1 day ago
drivers/media/i2c/imx412.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Wenmeng Liu 4 weeks, 1 day ago
During sensor bring-up, the IMX412 performs CCI ID read (T6 ~0.6 ms) and
parameter loading from NVM (T7 ≤ 8 ms) after INCK/XCLR rise. Writing the
mode register list while T7 is in progress can cause  failed
register programming.

Move the usleep_range(7400, 8000) to the beginning of
imx412_start_streaming(), so the driver waits for the NVM read window (T7)
to complete before pushing the mode registers and sending the streaming
command (T8). This change preserves the original delay length but fixes
the ordering to match the datasheet timing:

- T6: CCI ID read wait (~0.6 ms)
- T7: NVM parameter read (≤ 8 ms) — now fully elapsed before any
      register writes
- T8: start of first streaming after issuing MODE_SELECT

Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
Changes in v2:
- Move the 7.4–8 ms delay before mode-register programming to satisfy T7 (NVM read).
- Link to v1: https://lore.kernel.org/all/20251222-imx412-v1-1-51c7e724b376@oss.qualcomm.com/
---
 drivers/media/i2c/imx412.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index b3826f803547..ed249a95ff35 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -798,6 +798,9 @@ static int imx412_start_streaming(struct imx412 *imx412)
 	const struct imx412_reg_list *reg_list;
 	int ret;
 
+	/* Wait T7 (≤8ms) so NVM read finishes; avoid I2C NACK when writing mode regs */
+	usleep_range(7400, 8000);
+
 	/* Write sensor mode registers */
 	reg_list = &imx412->cur_mode->reg_list;
 	ret = imx412_write_regs(imx412, reg_list->regs,
@@ -814,9 +817,6 @@ static int imx412_start_streaming(struct imx412 *imx412)
 		return ret;
 	}
 
-	/* Delay is required before streaming*/
-	usleep_range(7400, 8000);
-
 	/* Start streaming */
 	ret = imx412_write_reg(imx412, IMX412_REG_MODE_SELECT,
 			       1, IMX412_MODE_STREAMING);
-- 
2.34.1

Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Bryan O'Donoghue 4 weeks, 1 day ago
On 09/01/2026 04:49, Wenmeng Liu wrote:
> During sensor bring-up, the IMX412 performs CCI ID read (T6 ~0.6 ms) and
> parameter loading from NVM (T7 ≤ 8 ms) after INCK/XCLR rise. Writing the
> mode register list while T7 is in progress can cause  failed
> register programming.
> 
> Move the usleep_range(7400, 8000) to the beginning of
> imx412_start_streaming(), so the driver waits for the NVM read window (T7)
> to complete before pushing the mode registers and sending the streaming
> command (T8). This change preserves the original delay length but fixes
> the ordering to match the datasheet timing:
> 
> - T6: CCI ID read wait (~0.6 ms)
> - T7: NVM parameter read (≤ 8 ms) — now fully elapsed before any
>        register writes
> - T8: start of first streaming after issuing MODE_SELECT
> 
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> Changes in v2:
> - Move the 7.4–8 ms delay before mode-register programming to satisfy T7 (NVM read).
> - Link to v1: https://lore.kernel.org/all/20251222-imx412-v1-1-51c7e724b376@oss.qualcomm.com/
> ---
>   drivers/media/i2c/imx412.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index b3826f803547..ed249a95ff35 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -798,6 +798,9 @@ static int imx412_start_streaming(struct imx412 *imx412)
>   	const struct imx412_reg_list *reg_list;
>   	int ret;
> 
> +	/* Wait T7 (≤8ms) so NVM read finishes; avoid I2C NACK when writing mode regs */
> +	usleep_range(7400, 8000);
> +
>   	/* Write sensor mode registers */
>   	reg_list = &imx412->cur_mode->reg_list;
>   	ret = imx412_write_regs(imx412, reg_list->regs,
> @@ -814,9 +817,6 @@ static int imx412_start_streaming(struct imx412 *imx412)
>   		return ret;
>   	}
> 
> -	/* Delay is required before streaming*/
> -	usleep_range(7400, 8000);
> -
>   	/* Start streaming */
>   	ret = imx412_write_reg(imx412, IMX412_REG_MODE_SELECT,
>   			       1, IMX412_MODE_STREAMING);
> --
> 2.34.1
> 
> 

This delay should go at the end of the operation that requires the delay 
not at the start of the streaming operation.

The delay after the stream write, should be related to the stream write 
command, not the antecedent - the command that came before start_streaming.

Basically I think you need to put your delay into the CCI_ID read NVM 
parameter load routine so that it guarantees its own completion.

Because for argument's sake if start_streaming() were not to be the 
thing to happen after CCI_ID/NVM loading, the logic would no longer work.

And you need a Fixes: tag for this patch too.

---
bod
Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Wenmeng Liu 3 weeks, 5 days ago

On 1/9/2026 8:32 PM, Bryan O'Donoghue wrote:
> This delay should go at the end of the operation that requires the delay 
> not at the start of the streaming operation.
> 
> The delay after the stream write, should be related to the stream write 
> command, not the antecedent - the command that came before start_streaming.
> 
> Basically I think you need to put your delay into the CCI_ID read NVM 
> parameter load routine so that it guarantees its own completion.
> 
> Because for argument's sake if start_streaming() were not to be the 
> thing to happen after CCI_ID/NVM loading, the logic would no longer work.
> 
> And you need a Fixes: tag for this patch too.

Reading the sensor ID only occurs during the sensor probe process. After 
the probe is completed, the IMX577 will power down. When stream on 
occurs, the driver will power on again and then start streaming, but the 
sensor ID is not read during the stream on process.I have tested this 
change on imx577 modules of different models.

So this change can only happen during power on or stream on.

Hi Bryan, Sakari,
May I ask if you have any suggestions regarding this?


Thanks,
Wenmeng
Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Sakari Ailus 3 weeks, 5 days ago
Hi Wenmeng, Bryan,

On Mon, Jan 12, 2026 at 11:07:39AM +0800, Wenmeng Liu wrote:
> 
> 
> On 1/9/2026 8:32 PM, Bryan O'Donoghue wrote:
> > This delay should go at the end of the operation that requires the delay
> > not at the start of the streaming operation.

I would have thought that, too, but I understand there's an issue with an
Arducam module. It's also not exactly clear to me if all other registers
are writable at the sensor identification time or is the required delay
only concerning starting streaming (I'd hope so).

Also see
<URL:https://git.retiisi.eu/?p=~sailus/linux.git;a=shortlog;h=refs/heads/pm-resume-delay>.
I haven't posted these yet, but I think it'd be useful to avoid extra
delays here and elsewhere.

> > 
> > The delay after the stream write, should be related to the stream write
> > command, not the antecedent - the command that came before
> > start_streaming.
> > 
> > Basically I think you need to put your delay into the CCI_ID read NVM
> > parameter load routine so that it guarantees its own completion.
> > 
> > Because for argument's sake if start_streaming() were not to be the
> > thing to happen after CCI_ID/NVM loading, the logic would no longer
> > work.
> > 
> > And you need a Fixes: tag for this patch too.
> 
> Reading the sensor ID only occurs during the sensor probe process. After the
> probe is completed, the IMX577 will power down. When stream on occurs, the
> driver will power on again and then start streaming, but the sensor ID is
> not read during the stream on process.I have tested this change on imx577
> modules of different models.
> 
> So this change can only happen during power on or stream on.
> 
> Hi Bryan, Sakari,
> May I ask if you have any suggestions regarding this?

Could you add a comment this delay is there for the Arducam module (and
which one), that doesn't work without it?

-- 
Kind regards,

Sakari Ailus
Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Wenmeng Liu 3 weeks, 5 days ago

On 1/12/2026 5:06 PM, Sakari Ailus wrote:
> Hi Wenmeng, Bryan,
> 
> On Mon, Jan 12, 2026 at 11:07:39AM +0800, Wenmeng Liu wrote:
>>
>>
>> On 1/9/2026 8:32 PM, Bryan O'Donoghue wrote:
>>> This delay should go at the end of the operation that requires the delay
>>> not at the start of the streaming operation.
> 
> I would have thought that, too, but I understand there's an issue with an
> Arducam module. It's also not exactly clear to me if all other registers
> are writable at the sensor identification time or is the required delay
> only concerning starting streaming (I'd hope so).
> 

Hi Sakari,

I tried adding a read ID at the end of power_on func and found that it 
could only read the ID during probe; subsequent attempts during stream 
on would fail to read every power on read.

[   11.298460] imx412 2-001a: read reg chip id: 577
[   11.310703] imx412 2-001a: read reg chip id: 577
[   35.392396] imx412 2-001a: read reg failed ret = -5
[   39.583990] imx412 2-001a: read reg failed ret = -5


> Also see
> <URL:https://git.retiisi.eu/?p=~sailus/linux.git;a=shortlog;h=refs/heads/pm-resume-delay>.
> I haven't posted these yet, but I think it'd be useful to avoid extra
> delays here and elsewhere.

compile failed in my side...
+void pm_runtime_resume_fsleep(struct device *dev, u64 sleep_us)
+{
+       u64 ns_diff = ktime_get_mono_fast_ns() - 
READ_ONCE(dev->power.last_busy);
+
+       if (ns_diff < NSEC_PER_USEC * sleep_us)
+               fsleep(sleep_us - (ns_diff >> 10)); /* avoid div_u64 */
+}
use this func in imx412.c for all usleep_range, have no effect.

> 
>>>
>>> The delay after the stream write, should be related to the stream write
>>> command, not the antecedent - the command that came before
>>> start_streaming.
>>>
>>> Basically I think you need to put your delay into the CCI_ID read NVM
>>> parameter load routine so that it guarantees its own completion.
>>>
>>> Because for argument's sake if start_streaming() were not to be the
>>> thing to happen after CCI_ID/NVM loading, the logic would no longer
>>> work.
>>>
>>> And you need a Fixes: tag for this patch too.
>>
>> Reading the sensor ID only occurs during the sensor probe process. After the
>> probe is completed, the IMX577 will power down. When stream on occurs, the
>> driver will power on again and then start streaming, but the sensor ID is
>> not read during the stream on process.I have tested this change on imx577
>> modules of different models.
>>
>> So this change can only happen during power on or stream on.
>>
>> Hi Bryan, Sakari,
>> May I ask if you have any suggestions regarding this?
> 
> Could you add a comment this delay is there for the Arducam module (and
> which one), that doesn't work without it?
>
https://www.arducam.com/arducam-imx577-mini-camera-module-for-qualcomm-rb3g2.html
https://www.arducam.com/arducam-imx477-camera-module-for-depthai-oak-b0369.html

I have tested on these two IMX577 sensor module form Arducam,all have 
this issue.Did not encounter this problem when using one non-arducam 
IMX577.

Thanks,
Wenmeng
Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Bryan O'Donoghue 3 weeks, 5 days ago
On 12/01/2026 10:09, Wenmeng Liu wrote:
>>>> This delay should go at the end of the operation that requires the delay
>>>> not at the start of the streaming operation.
>> I would have thought that, too, but I understand there's an issue with an
>> Arducam module. It's also not exactly clear to me if all other registers
>> are writable at the sensor identification time or is the required delay
>> only concerning starting streaming (I'd hope so).
>>
> Hi Sakari,
> 
> I tried adding a read ID at the end of power_on func and found that it
> could only read the ID during probe; subsequent attempts during stream
> on would fail to read every power on read.
> 
> [   11.298460] imx412 2-001a: read reg chip id: 577
> [   11.310703] imx412 2-001a: read reg chip id: 577
> [   35.392396] imx412 2-001a: read reg failed ret = -5
> [   39.583990] imx412 2-001a: read reg failed ret = -5

This "smell wrong" points to the power_on() sequence not being correct.

You should be able to read the identity register at the end of 
power_on() every single time, if not, power_on - isn't working.

In fact, looking at the power_on() sequence, I'd say we should have put 
reset 1, switched on power, and clock and then taken the part out of reset.

You should be able to put exactly the same delay into power_on() and 
have the same result as having no delay in power_on() instead having it 
in start_streaming().

Are you sure something else isn't happening - a reset line, pm_runtime .. ?

I think either power_off() is happening without you knowing it or more 
likely the reset line logic in the power_on() sequence isn't correct, 
which is why detecting the chip then fails.

---
bod
Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Wenmeng Liu 3 weeks, 5 days ago

On 1/12/2026 6:21 PM, Bryan O'Donoghue wrote:
> On 12/01/2026 10:09, Wenmeng Liu wrote:
>>>>> This delay should go at the end of the operation that requires the 
>>>>> delay
>>>>> not at the start of the streaming operation.
>>> I would have thought that, too, but I understand there's an issue 
>>> with an
>>> Arducam module. It's also not exactly clear to me if all other registers
>>> are writable at the sensor identification time or is the required delay
>>> only concerning starting streaming (I'd hope so).
>>>
>> Hi Sakari,
>>
>> I tried adding a read ID at the end of power_on func and found that it
>> could only read the ID during probe; subsequent attempts during stream
>> on would fail to read every power on read.
>>
>> [   11.298460] imx412 2-001a: read reg chip id: 577
>> [   11.310703] imx412 2-001a: read reg chip id: 577
>> [   35.392396] imx412 2-001a: read reg failed ret = -5
>> [   39.583990] imx412 2-001a: read reg failed ret = -5
> 
> This "smell wrong" points to the power_on() sequence not being correct.
> 
> You should be able to read the identity register at the end of 
> power_on() every single time, if not, power_on - isn't working.
> 
> 
> You should be able to put exactly the same delay into power_on() and 
> have the same result as having no delay in power_on() instead having it 
> in start_streaming().
> 
> Are you sure something else isn't happening - a reset line, pm_runtime .. ?
> 
> I think either power_off() is happening without you knowing it or more 
> likely the reset line logic in the power_on() sequence isn't correct, 
> which is why detecting the chip then fails.

-- if disable gpiod_set_value_cansleep(imx412->reset_gpio, 1); form 
imx412_power_off, the issue will not happen.

Through this analysis, the issue was ultimately identified as the 
Arducam IMX577 sensor module requiring a relatively long wait time after 
the reset pin is pulled high.

The reason why the ID can be read successfully the first time is that 
the GPIO’s default state is not output low. Therefore, to fix this 
problem, it is necessary to increase the delay time in the power_on 
sequence.

 > In fact, looking at the power_on() sequence, I'd say we should have put
 > reset 1, switched on power, and clock and then taken the part out of 
reset.

I tried this, but it didn't work at all.

Thanks,
Wenmeng








Re: [PATCH v2] media: i2c: imx412: wait for NVM read (T7) before programming mode registers
Posted by Bryan O'Donoghue 3 weeks, 5 days ago
On 12/01/2026 11:50, Wenmeng Liu wrote:
> -- if disable gpiod_set_value_cansleep(imx412->reset_gpio, 1); form
> imx412_power_off, the issue will not happen.

Yeah this is basically what I said in the last email.

The state transition diagram for starting the part will assume reset is 
asserted.

So in power_on() - you need to assert reset prior to powering on and 
clocking the part - which is the eqivalent logic to what you posted above.

The reason you should do this in power_on() is it doesn't assume 
power_off has run first.

power_on should establish the initial conditions required to power on 
the device, in this case, we've established you need to have reset 
asserted first.

---
bod