Re: [PATCH] media: i2c: ov5645: Fix CSI-2 CRC errors on RK3576 after init sequence

[Xiaolei Wang <xiaoleiwangxiaolei@gmail.com>]

Hi Laurent,

Thank you so much for your review.

在 2026/9/8 18:36, Laurent Pinchart 写道:
> On Tue, Sep 08, 2026 at 01:14:32PM +0300, Sakari Ailus wrote:
>> On Wed, Aug 26, 2026 at 08:54:32AM +0000, Xiaolei Wang wrote:
>>> On the RK3576 platform, after ov5645_global_init_setting completes and
>>> before stream on, the CSI host reports a high rate of CRC errors:
>>>
>>>    MIPI_CSI2 ERR1:0x1000000 (crc,vc: 0)
>>>
>>> These errors occur exclusively in the window between init completion and
>>> stream on. Once preview starts and the sensor outputs valid HS data, no
>>> further CRC errors are observed.
>>>
>>> Commit 092e8eb90a7d ("media: i2c: ov5645: Fix power sequence") reworked
>>> the power sequence to properly enter LP-11 during power-up. As part of
>>> this change, the SYSTEM_CTRL0_STOP (0x42) write in s_power() was replaced
>>> with usleep_range(500, 1000), leaving the sensor in wake state (0x3008 =
>>> 0x02) from init through to s_stream. Reverting this commit resolves the
>>> CRC errors, confirming it as the triggering change.
>>>
>>> Restore the power-down state after init by appending { 0x3008, 0x42 } to
>>> the end of ov5645_global_init_setting[]. This powers down the sensor core
>>> after MIPI configuration is complete, while keeping the MIPI PHY active in
>>> LP-11. The sensor is then woken up only at s_stream time.
>>>
>>> This preserves all LP-11 compliance improvements from
>>> commit 092e8eb90a7d ("media: i2c: ov5645: Fix power sequence") while
>>> eliminating the CRC errors on RK3576.
>>>
>>> Fixes: 092e8eb90a7d ("media: i2c: ov5645: Fix power sequence")
>>> Signed-off-by: Xiaolei Wang <xiaoleiwangxiaolei@gmail.com>
>>> ---
>>>   drivers/media/i2c/ov5645.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
>>> index c772ef6e51d2..be85e0155b81 100644
>>> --- a/drivers/media/i2c/ov5645.c
>>> +++ b/drivers/media/i2c/ov5645.c
>>> @@ -351,7 +351,8 @@ static const struct reg_value ov5645_global_init_setting[] = {
>>>   	{ 0x3008, 0x02 },
>>>   	{ OV5645_IO_MIPI_CTRL00, 0x40 },
>>>   	{ OV5645_MIPI_CTRL00, 0x24 },
>>> -	{ OV5645_PAD_OUTPUT00, 0x70 }
>>> +	{ OV5645_PAD_OUTPUT00, 0x70 },
>>> +	{ 0x3008, 0x42 }
> While at it, please use the register macro.
Will do in v2.
>
>> Is the earlier write of 0x3008 still appropriate?
> There are four earlier writes of register 0x3008. The whole
> initialization sequence likely needs a cleanup.

The ov5645_global_init_setting register may come directly from

the manufacturer.

  I gave this a try. Three of the four writes can simply be dropped,

   leaving only the { 0x3008, 0x42 } near the beginning of the table:

         { 0x5a24, 0x00 },
   -     { 0x3008, 0x02 },
         { 0x3503, 0x00 },
         ...
         { 0x0601, 0x02 },
   -     { 0x3008, 0x42 },
   -     { 0x3008, 0x02 },
         { OV5645_IO_MIPI_CTRL00, 0x40 },

I tested this change on the Rockchip 3576 and IMX8MP platforms and found 
no problems.


>   It's not clear why
> software suspend needs to be disabled briefly during initialization.

Yes, I noticed that the driver was configured with software suspend upon

power-up at the initial commit.

thanks

xiaolei

>
>>>   };
>>>   
>>>   static const struct reg_value ov5645_setting_sxga[] = {