drivers/media/i2c/ov5640.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
From: Jason Yang <jason98166@gmail.com>
ov5640_set_stream_mipi() always programs IO_MIPI_CTRL00 with 0x45,
which selects the two data lane mode: the number of data lanes
described in the devicetree endpoint only feeds the sensor's clock
tree computations, so a module wired with one data lane starts
streaming in two lane mode and the receiver never assembles a
frame.
Select the lane mode from the endpoint instead. The one lane
encoding is [7:5] = 001 per the current sensor manual (version
2.33). The 2.03 manual documented 000/001 for one/two lanes;
OmniVision corrected the table in version 2.1, which is why the
long-standing comment here found 001 unusable for two lanes and
validated 010 instead.
The power-up path also programs a two data lane mode, but that
value is overwritten when streaming starts, so it is left alone.
Tested with a single data lane module on an i.MX8MP board
(imx-mipi-csis receiver), where the unpatched value produces no
frames at all, and on an RK3588 board.
Fixes: 19a81c1426c1 ("[media] add Omnivision OV5640 sensor driver")
Cc: stable+noautosel@kernel.org # no in-tree 1-lane users
Signed-off-by: Jason Yang <jason98166@gmail.com>
Assisted-by: Claude:claude-opus-5
---
The three [7:5] encodings were exercised individually on the
i.MX8MP board (v7.2-rc4, data-lanes = <1>) by patching the value
and capturing with v4l2-ctl:
001 (this patch): 30/30 frames, zero PHY error events in
steady state (3 x 300 frames)
000 (2.03 manual / NXP KB): same result
010 (unpatched two lane mode): no frames; the receiver logs
only start-of-transmission errors and never assembles one
The RK3588 run used the same module and devicetree (data-lanes =
<1>) through a Rockchip CSI-2 receiver, streaming to natural EOS
with a clean kernel log.
Happy to run additional tests on either platform if that would
help.
---
drivers/media/i2c/ov5640.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 8deb5f5501fa..ef731709bdc8 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1826,27 +1826,27 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
{
+ u8 val;
int ret;
/*
* Enable/disable the MIPI interface
*
- * 0x300e = on ? 0x45 : 0x40
- *
- * FIXME: the sensor manual (version 2.03) reports
- * [7:5] = 000 : 1 data lane mode
- * [7:5] = 001 : 2 data lanes mode
- * But this settings do not work, while the following ones
- * have been validated for 2 data lanes mode.
- *
+ * [7:5] = 001 : 1 data lane mode
* [7:5] = 010 : 2 data lanes mode
+ * Encodings per version 2.33 of the sensor manual.
* [4] = 0 : Power up MIPI HS Tx
* [3] = 0 : Power up MIPI LS Rx
* [2] = 1/0 : MIPI interface enable/disable
* [1:0] = 01/00: FIXME: 'debug'
*/
- ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
- on ? 0x45 : 0x40);
+ if (on)
+ val = sensor->ep.bus.mipi_csi2.num_data_lanes == 1 ?
+ 0x25 : 0x45;
+ else
+ val = 0x40;
+
+ ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, val);
if (ret)
return ret;
@@ -2535,7 +2535,7 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
* Power up MIPI HS Tx and LS Rx; 2 data lanes mode
*
* 0x300e = 0x40
- * [7:5] = 010 : 2 data lanes mode (see FIXME note in
+ * [7:5] = 010 : 2 data lanes mode (see the note in
* "ov5640_set_stream_mipi()")
* [4] = 0 : Power up MIPI HS Tx
* [3] = 0 : Power up MIPI LS Rx
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260811-ov5640-1lane-v1-b0fe37eab4d8
Best regards,
--
Jason Yang <jason98166@gmail.com>Hi Jason,
Thanks for the patch.
On Tue, Aug 11, 2026 at 12:40:13PM +0800, Jason Yang via B4 Relay wrote:
> From: Jason Yang <jason98166@gmail.com>
>
> ov5640_set_stream_mipi() always programs IO_MIPI_CTRL00 with 0x45,
> which selects the two data lane mode: the number of data lanes
> described in the devicetree endpoint only feeds the sensor's clock
> tree computations, so a module wired with one data lane starts
> streaming in two lane mode and the receiver never assembles a
> frame.
>
> Select the lane mode from the endpoint instead. The one lane
> encoding is [7:5] = 001 per the current sensor manual (version
> 2.33). The 2.03 manual documented 000/001 for one/two lanes;
> OmniVision corrected the table in version 2.1, which is why the
> long-standing comment here found 001 unusable for two lanes and
> validated 010 instead.
>
> The power-up path also programs a two data lane mode, but that
> value is overwritten when streaming starts, so it is left alone.
>
> Tested with a single data lane module on an i.MX8MP board
> (imx-mipi-csis receiver), where the unpatched value produces no
> frames at all, and on an RK3588 board.
>
> Fixes: 19a81c1426c1 ("[media] add Omnivision OV5640 sensor driver")
> Cc: stable+noautosel@kernel.org # no in-tree 1-lane users
I guess there wouldn't be harm from backporting either.
> Signed-off-by: Jason Yang <jason98166@gmail.com>
> Assisted-by: Claude:claude-opus-5
> ---
> The three [7:5] encodings were exercised individually on the
> i.MX8MP board (v7.2-rc4, data-lanes = <1>) by patching the value
> and capturing with v4l2-ctl:
>
> 001 (this patch): 30/30 frames, zero PHY error events in
> steady state (3 x 300 frames)
> 000 (2.03 manual / NXP KB): same result
> 010 (unpatched two lane mode): no frames; the receiver logs
> only start-of-transmission errors and never assembles one
>
> The RK3588 run used the same module and devicetree (data-lanes =
> <1>) through a Rockchip CSI-2 receiver, streaming to natural EOS
> with a clean kernel log.
>
> Happy to run additional tests on either platform if that would
> help.
> ---
> drivers/media/i2c/ov5640.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 8deb5f5501fa..ef731709bdc8 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1826,27 +1826,27 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
>
> static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
> {
> + u8 val;
> int ret;
>
> /*
> * Enable/disable the MIPI interface
> *
> - * 0x300e = on ? 0x45 : 0x40
> - *
> - * FIXME: the sensor manual (version 2.03) reports
> - * [7:5] = 000 : 1 data lane mode
> - * [7:5] = 001 : 2 data lanes mode
> - * But this settings do not work, while the following ones
> - * have been validated for 2 data lanes mode.
> - *
> + * [7:5] = 001 : 1 data lane mode
> * [7:5] = 010 : 2 data lanes mode
> + * Encodings per version 2.33 of the sensor manual.
> * [4] = 0 : Power up MIPI HS Tx
> * [3] = 0 : Power up MIPI LS Rx
> * [2] = 1/0 : MIPI interface enable/disable
> * [1:0] = 01/00: FIXME: 'debug'
> */
> - ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
> - on ? 0x45 : 0x40);
> + if (on)
> + val = sensor->ep.bus.mipi_csi2.num_data_lanes == 1 ?
> + 0x25 : 0x45;
How about:
ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
on ? 0x5 | ep.bus.mipi_csi2.num_data_lanes << 5 :
0x40);
> + else
> + val = 0x40;
> +
> + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, val);
> if (ret)
> return ret;
>
> @@ -2535,7 +2535,7 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
> * Power up MIPI HS Tx and LS Rx; 2 data lanes mode
> *
> * 0x300e = 0x40
> - * [7:5] = 010 : 2 data lanes mode (see FIXME note in
> + * [7:5] = 010 : 2 data lanes mode (see the note in
> * "ov5640_set_stream_mipi()")
Do we need quotes?
> * [4] = 0 : Power up MIPI HS Tx
> * [3] = 0 : Power up MIPI LS Rx
>
--
Kind regards,
Sakari Ailus
Hi Sakari,
Thanks for the review.
> I guess there wouldn't be harm from backporting either.
Agreed, switched to a plain stable Cc.
> How about:
>
> ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
> on ? 0x5 | ep.bus.mipi_csi2.num_data_lanes << 5 :
> 0x40);
Nicer, thanks - applied, with the lane count in a local variable to
stay within 80 columns.
> Do we need quotes?
Right, dropped.
Thanks,
Jason
Sakari Ailus <sakari.ailus@linux.intel.com> 於 2026年8月11日週二 下午1:54寫道：
>
> Hi Jason,
>
> Thanks for the patch.
>
> On Tue, Aug 11, 2026 at 12:40:13PM +0800, Jason Yang via B4 Relay wrote:
> > From: Jason Yang <jason98166@gmail.com>
> >
> > ov5640_set_stream_mipi() always programs IO_MIPI_CTRL00 with 0x45,
> > which selects the two data lane mode: the number of data lanes
> > described in the devicetree endpoint only feeds the sensor's clock
> > tree computations, so a module wired with one data lane starts
> > streaming in two lane mode and the receiver never assembles a
> > frame.
> >
> > Select the lane mode from the endpoint instead. The one lane
> > encoding is [7:5] = 001 per the current sensor manual (version
> > 2.33). The 2.03 manual documented 000/001 for one/two lanes;
> > OmniVision corrected the table in version 2.1, which is why the
> > long-standing comment here found 001 unusable for two lanes and
> > validated 010 instead.
> >
> > The power-up path also programs a two data lane mode, but that
> > value is overwritten when streaming starts, so it is left alone.
> >
> > Tested with a single data lane module on an i.MX8MP board
> > (imx-mipi-csis receiver), where the unpatched value produces no
> > frames at all, and on an RK3588 board.
> >
> > Fixes: 19a81c1426c1 ("[media] add Omnivision OV5640 sensor driver")
> > Cc: stable+noautosel@kernel.org # no in-tree 1-lane users
>
> I guess there wouldn't be harm from backporting either.
>
> > Signed-off-by: Jason Yang <jason98166@gmail.com>
> > Assisted-by: Claude:claude-opus-5
> > ---
> > The three [7:5] encodings were exercised individually on the
> > i.MX8MP board (v7.2-rc4, data-lanes = <1>) by patching the value
> > and capturing with v4l2-ctl:
> >
> > 001 (this patch): 30/30 frames, zero PHY error events in
> > steady state (3 x 300 frames)
> > 000 (2.03 manual / NXP KB): same result
> > 010 (unpatched two lane mode): no frames; the receiver logs
> > only start-of-transmission errors and never assembles one
> >
> > The RK3588 run used the same module and devicetree (data-lanes =
> > <1>) through a Rockchip CSI-2 receiver, streaming to natural EOS
> > with a clean kernel log.
> >
> > Happy to run additional tests on either platform if that would
> > help.
> > ---
> > drivers/media/i2c/ov5640.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 8deb5f5501fa..ef731709bdc8 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -1826,27 +1826,27 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> >
> > static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
> > {
> > + u8 val;
> > int ret;
> >
> > /*
> > * Enable/disable the MIPI interface
> > *
> > - * 0x300e = on ? 0x45 : 0x40
> > - *
> > - * FIXME: the sensor manual (version 2.03) reports
> > - * [7:5] = 000 : 1 data lane mode
> > - * [7:5] = 001 : 2 data lanes mode
> > - * But this settings do not work, while the following ones
> > - * have been validated for 2 data lanes mode.
> > - *
> > + * [7:5] = 001 : 1 data lane mode
> > * [7:5] = 010 : 2 data lanes mode
> > + * Encodings per version 2.33 of the sensor manual.
> > * [4] = 0 : Power up MIPI HS Tx
> > * [3] = 0 : Power up MIPI LS Rx
> > * [2] = 1/0 : MIPI interface enable/disable
> > * [1:0] = 01/00: FIXME: 'debug'
> > */
> > - ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
> > - on ? 0x45 : 0x40);
> > + if (on)
> > + val = sensor->ep.bus.mipi_csi2.num_data_lanes == 1 ?
> > + 0x25 : 0x45;
>
> How about:
>
> ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
> on ? 0x5 | ep.bus.mipi_csi2.num_data_lanes << 5 :
> 0x40);
>
> > + else
> > + val = 0x40;
> > +
> > + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, val);
> > if (ret)
> > return ret;
> >
> > @@ -2535,7 +2535,7 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
> > * Power up MIPI HS Tx and LS Rx; 2 data lanes mode
> > *
> > * 0x300e = 0x40
> > - * [7:5] = 010 : 2 data lanes mode (see FIXME note in
> > + * [7:5] = 010 : 2 data lanes mode (see the note in
> > * "ov5640_set_stream_mipi()")
>
> Do we need quotes?
>
> > * [4] = 0 : Power up MIPI HS Tx
> > * [3] = 0 : Power up MIPI LS Rx
> >
>
> --
> Kind regards,
>
> Sakari Ailus
Hi Jason, On Tue, Aug 11, 2026 at 03:18:01PM +0800, 楊智成 wrote: > Hi Sakari, > > Thanks for the review. > > > I guess there wouldn't be harm from backporting either. > > Agreed, switched to a plain stable Cc. > > > How about: > > > > ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, > > on ? 0x5 | ep.bus.mipi_csi2.num_data_lanes << 5 : > > 0x40); > > Nicer, thanks - applied, with the lane count in a local variable to > stay within 80 columns. There's no need for a local variable, the above remains within 80 columns. -- Sakari Ailus
Hi Sakari, > There's no need for a local variable, the above remains within 80 > columns. Dropped - v3 has the write exactly as you wrote it. Thanks, Jason Sakari Ailus <sakari.ailus@linux.intel.com> 於 2026年8月11日週二 下午3:22寫道： > > Hi Jason, > > On Tue, Aug 11, 2026 at 03:18:01PM +0800, 楊智成 wrote: > > Hi Sakari, > > > > Thanks for the review. > > > > > I guess there wouldn't be harm from backporting either. > > > > Agreed, switched to a plain stable Cc. > > > > > How about: > > > > > > ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, > > > on ? 0x5 | ep.bus.mipi_csi2.num_data_lanes << 5 : > > > 0x40); > > > > Nicer, thanks - applied, with the lane count in a local variable to > > stay within 80 columns. > > There's no need for a local variable, the above remains within 80 columns. > > -- > Sakari Ailus
On Tue, Aug 11, 2026 at 03:30:40PM +0800, 楊智成 wrote: > Hi Sakari, > > > There's no need for a local variable, the above remains within 80 > > columns. > > Dropped - v3 has the write exactly as you wrote it. Ah, I think I missed 'sensor->' there. 80 isn't a hard limit though and I wouldn't introduce local variable in this case. Thanks! -- Sakari Ailus
© 2016 - 2026 Red Hat, Inc.