Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH 0/2] media: v4l2-cci/ov9282: support single-read I2C transactions
@ 2026-07-21 13:25 Richard Leitner
  2026-07-21 13:25 ` [PATCH 1/2] media: v4l2-cci: Add support for custom regmap configuration Richard Leitner
  2026-07-21 13:25 ` [PATCH 2/2] media: ov9282: enable single-read I2C transactions Richard Leitner
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 13:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson
  Cc: Laurent Pinchart, Hans de Goede, linux-media, linux-kernel,
	Richard Leitner

This series adds support for configuring transport-specific CCI regmap
behavior for I2C sensors, which is then used in the OV9282 driver to
force single-register read transactions.

The motivation for this change is that some OV9282 camera modules require
single-read I2C transactions to operate correctly. In particular, OV9282
modules from Vision Components return invalid data on multi-byte reads,
while switching the regmap to single-read transactions fixes the problem.

This series does not change the default behavior of existing CCI users.
Only ov9282 opts into single-read transactions.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
Richard Leitner (2):
      media: v4l2-cci: Add support for custom regmap configuration
      media: ov9282: enable single-read I2C transactions

 drivers/media/i2c/ov9282.c         |  7 ++++++-
 drivers/media/v4l2-core/v4l2-cci.c | 23 +++++++++++++++++++----
 include/media/v4l2-cci.h           | 37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 6 deletions(-)
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260717-cci-single-29bbb89775c4

Best regards,
--  
Richard Leitner <richard.leitner@linux.dev>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] media: v4l2-cci: Add support for custom regmap configuration
  2026-07-21 13:25 [PATCH 0/2] media: v4l2-cci/ov9282: support single-read I2C transactions Richard Leitner
@ 2026-07-21 13:25 ` Richard Leitner
  2026-07-21 13:25 ` [PATCH 2/2] media: ov9282: enable single-read I2C transactions Richard Leitner
  1 sibling, 0 replies; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 13:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson
  Cc: Laurent Pinchart, Hans de Goede, linux-media, linux-kernel,
	Richard Leitner

Some sensors, e.g. OV9282, require custom regmap configuration like setting
use_single_read. Therefore introduce a new custom cci configuration
struct, which exposes optional transport-specific regmap configuration for
I2C devices using the cci_*() register access helpers.

The existing devm_cci_regmap_init_i2c function is changed to use the
newly introduced devm_cci_regmap_init_i2c_cfg and its return value
documentation is fixed.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 drivers/media/v4l2-core/v4l2-cci.c | 23 +++++++++++++++++++----
 include/media/v4l2-cci.h           | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-cci.c b/drivers/media/v4l2-core/v4l2-cci.c
index e9ecf47859465..584ca080bc6e5 100644
--- a/drivers/media/v4l2-core/v4l2-cci.c
+++ b/drivers/media/v4l2-core/v4l2-cci.c
@@ -183,18 +183,33 @@ int cci_multi_reg_write(struct regmap *map, const struct cci_reg_sequence *regs,
 EXPORT_SYMBOL_GPL(cci_multi_reg_write);
 
 #if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
-struct regmap *devm_cci_regmap_init_i2c(struct i2c_client *client,
-					int reg_addr_bits)
+struct regmap *devm_cci_regmap_init_i2c_cfg(struct i2c_client *client,
+					    const struct cci_regmap_config *cci_cfg)
 {
-	struct regmap_config config = {
-		.reg_bits = reg_addr_bits,
+	const struct regmap_config config = {
+		.reg_bits = cci_cfg->reg_addr_bits,
 		.val_bits = 8,
 		.reg_format_endian = REGMAP_ENDIAN_BIG,
 		.disable_locking = true,
+		.use_single_read = cci_cfg->use_single_read,
+		.use_single_write = cci_cfg->use_single_write,
 	};
 
 	return devm_regmap_init_i2c(client, &config);
 }
+EXPORT_SYMBOL_GPL(devm_cci_regmap_init_i2c_cfg);
+
+struct regmap *devm_cci_regmap_init_i2c(struct i2c_client *client,
+					int reg_addr_bits)
+{
+	const struct cci_regmap_config cci_cfg = {
+		.reg_addr_bits = reg_addr_bits,
+		.use_single_read = false,
+		.use_single_write = false,
+	};
+
+	return devm_cci_regmap_init_i2c_cfg(client, &cci_cfg);
+}
 EXPORT_SYMBOL_GPL(devm_cci_regmap_init_i2c);
 #endif
 
diff --git a/include/media/v4l2-cci.h b/include/media/v4l2-cci.h
index 4e96e90ee6369..8296a8c122b4d 100644
--- a/include/media/v4l2-cci.h
+++ b/include/media/v4l2-cci.h
@@ -27,6 +27,27 @@ struct cci_reg_sequence {
 	u64 val;
 };
 
+/**
+ * struct cci_regmap_config - Optional configuration for CCI I2C regmap creation
+ *
+ * @reg_addr_bits: Number of bits in the register address.
+ * @use_single_read: Force single-register read transactions.
+ * @use_single_write: Force single-register write transactions.
+ *
+ * This structure describes optional transport-specific regmap configuration for
+ * I2C devices using the cci_*() register access helpers.
+ *
+ * The CCI helper initializes the underlying regmap with the CCI defaults:
+ * 16-bit or 8-bit register addresses as selected by @reg_addr_bits, 8-bit
+ * register values, big-endian register address formatting, and disabled
+ * regmap locking.
+ */
+struct cci_regmap_config {
+	int reg_addr_bits;
+	bool use_single_read;
+	bool use_single_write;
+};
+
 /*
  * Macros to define register address with the register width encoded
  * into the higher bits.
@@ -123,6 +144,20 @@ int cci_multi_reg_write(struct regmap *map, const struct cci_reg_sequence *regs,
 			unsigned int num_regs, int *err);
 
 #if IS_ENABLED(CONFIG_V4L2_CCI_I2C)
+/**
+ * devm_cci_regmap_init_i2c_cfg() - Create regmap with a custom config to use
+ *                                  with cci_*() register access functions
+ *
+ * @client: i2c_client to create the regmap for
+ * @config: Configuration for CCI register map.
+ *
+ * Note the memory for the created regmap is devm() managed, tied to the client.
+ *
+ * Return: a pointer to the regmap on success, or an ERR_PTR() encoded error on failure.
+ */
+struct regmap *devm_cci_regmap_init_i2c_cfg(struct i2c_client *client,
+					    const struct cci_regmap_config *config);
+
 /**
  * devm_cci_regmap_init_i2c() - Create regmap to use with cci_*() register
  *                              access functions
@@ -132,7 +167,7 @@ int cci_multi_reg_write(struct regmap *map, const struct cci_reg_sequence *regs,
  *
  * Note the memory for the created regmap is devm() managed, tied to the client.
  *
- * Return: %0 on success or a negative error code on failure.
+ * Return: a pointer to the regmap on success, or an ERR_PTR() encoded error on failure.
  */
 struct regmap *devm_cci_regmap_init_i2c(struct i2c_client *client,
 					int reg_addr_bits);

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:25 [PATCH 0/2] media: v4l2-cci/ov9282: support single-read I2C transactions Richard Leitner
  2026-07-21 13:25 ` [PATCH 1/2] media: v4l2-cci: Add support for custom regmap configuration Richard Leitner
@ 2026-07-21 13:25 ` Richard Leitner
  2026-07-21 13:38   ` Alexander Stein
  2026-07-21 13:39   ` Dave Stevenson
  1 sibling, 2 replies; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 13:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson
  Cc: Laurent Pinchart, Hans de Goede, linux-media, linux-kernel,
	Richard Leitner

Some OV9282 camera modules, such as those from Vision Components, fail
multi-byte register reads with the default CCI I2C regmap access pattern
and only work when single-register read transactions are used. This
results in the driver being unable to probe due to a chip id mismatch.

As the driver has no reliable way to identify affected modules enable
use_single_read for OV9282 unconditionally when creating the CCI regmap.

Tested on an i.M8MP system with OV9282 modules from Vision Components.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 drivers/media/i2c/ov9282.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 5b6f897a74fcd..8267a0a9c6f2d 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
 	struct ov9282 *ov9282;
 	int ret;
 
+	const struct cci_regmap_config config = {
+		.reg_addr_bits = 16,
+		.use_single_read = true,
+	};
+
 	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
 	if (!ov9282)
 		return -ENOMEM;
@@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
+	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
 	if (IS_ERR(ov9282->regmap))
 		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
 				     "Failed to init CCI\n");

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:25 ` [PATCH 2/2] media: ov9282: enable single-read I2C transactions Richard Leitner
@ 2026-07-21 13:38   ` Alexander Stein
  2026-07-21 15:12     ` Laurent Pinchart
  2026-07-21 15:15     ` Richard Leitner
  2026-07-21 13:39   ` Dave Stevenson
  1 sibling, 2 replies; 14+ messages in thread
From: Alexander Stein @ 2026-07-21 13:38 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson,
	Richard Leitner
  Cc: Laurent Pinchart, Hans de Goede, linux-media, linux-kernel,
	Richard Leitner

Hi Richard,

thanks for addressing this long-standing issue.

Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> Some OV9282 camera modules, such as those from Vision Components, fail
> multi-byte register reads with the default CCI I2C regmap access pattern
> and only work when single-register read transactions are used. This
> results in the driver being unable to probe due to a chip id mismatch.
> 
> As the driver has no reliable way to identify affected modules enable
> use_single_read for OV9282 unconditionally when creating the CCI regmap.

What about a property in the DT instead? In DT it should be clear if a
Vision Components camera is used.

Best regards,
Alexander

> 
> Tested on an i.M8MP system with OV9282 modules from Vision Components.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
>  drivers/media/i2c/ov9282.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 5b6f897a74fcd..8267a0a9c6f2d 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
>  	struct ov9282 *ov9282;
>  	int ret;
>  
> +	const struct cci_regmap_config config = {
> +		.reg_addr_bits = 16,
> +		.use_single_read = true,
> +	};
> +
>  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
>  	if (!ov9282)
>  		return -ENOMEM;
> @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
>  	if (IS_ERR(ov9282->regmap))
>  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
>  				     "Failed to init CCI\n");
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:25 ` [PATCH 2/2] media: ov9282: enable single-read I2C transactions Richard Leitner
  2026-07-21 13:38   ` Alexander Stein
@ 2026-07-21 13:39   ` Dave Stevenson
  2026-07-21 15:16     ` Laurent Pinchart
  1 sibling, 1 reply; 14+ messages in thread
From: Dave Stevenson @ 2026-07-21 13:39 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Laurent Pinchart,
	Hans de Goede, linux-media, linux-kernel

Hi Richard

On Tue, 21 Jul 2026 at 14:25, Richard Leitner <richard.leitner@linux.dev> wrote:
>
> Some OV9282 camera modules, such as those from Vision Components, fail
> multi-byte register reads with the default CCI I2C regmap access pattern
> and only work when single-register read transactions are used. This
> results in the driver being unable to probe due to a chip id mismatch.

Those damn stupid Vision Components modules with the MCU that prevents
you reading registers.

I seem to recall Laurent had a plan for how to handle those without
having to patch all the drivers, however I can't find that thread
quickly.

> As the driver has no reliable way to identify affected modules enable
> use_single_read for OV9282 unconditionally when creating the CCI regmap.
>
> Tested on an i.M8MP system with OV9282 modules from Vision Components.
>
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>

As regmap does provide a simple way to request this behaviour, and the
only read is the ID register from ov9282_detect, I'll give it an

Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

but it's not an ideal situation. Others can debate the alternative
approaches for handling VC modules.

  Dave

> ---
>  drivers/media/i2c/ov9282.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 5b6f897a74fcd..8267a0a9c6f2d 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
>         struct ov9282 *ov9282;
>         int ret;
>
> +       const struct cci_regmap_config config = {
> +               .reg_addr_bits = 16,
> +               .use_single_read = true,
> +       };
> +
>         ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
>         if (!ov9282)
>                 return -ENOMEM;
> @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
>                 return ret;
>         }
>
> -       ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> +       ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
>         if (IS_ERR(ov9282->regmap))
>                 return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
>                                      "Failed to init CCI\n");
>
> --
> 2.53.0
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:38   ` Alexander Stein
@ 2026-07-21 15:12     ` Laurent Pinchart
  2026-07-21 15:15     ` Richard Leitner
  1 sibling, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2026-07-21 15:12 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson,
	Richard Leitner, Hans de Goede, linux-media, linux-kernel

On Tue, Jul 21, 2026 at 03:38:26PM +0200, Alexander Stein wrote:
> Hi Richard,
> 
> thanks for addressing this long-standing issue.
> 
> Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> > Some OV9282 camera modules, such as those from Vision Components, fail
> > multi-byte register reads with the default CCI I2C regmap access pattern
> > and only work when single-register read transactions are used. This
> > results in the driver being unable to probe due to a chip id mismatch.
> > 
> > As the driver has no reliable way to identify affected modules enable
> > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> 
> What about a property in the DT instead? In DT it should be clear if a
> Vision Components camera is used.

Yes, this should not be done by default, or we'll have to do the same
for all sensors used in any module from Vision Components.

> > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> >  drivers/media/i2c/ov9282.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> >  	struct ov9282 *ov9282;
> >  	int ret;
> >  
> > +	const struct cci_regmap_config config = {
> > +		.reg_addr_bits = 16,
> > +		.use_single_read = true,
> > +	};
> > +
> >  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> >  	if (!ov9282)
> >  		return -ENOMEM;
> > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> >  		return ret;
> >  	}
> >  
> > -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> >  	if (IS_ERR(ov9282->regmap))
> >  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> >  				     "Failed to init CCI\n");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:38   ` Alexander Stein
  2026-07-21 15:12     ` Laurent Pinchart
@ 2026-07-21 15:15     ` Richard Leitner
  2026-07-21 15:32       ` Laurent Pinchart
  1 sibling, 1 reply; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 15:15 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson,
	Laurent Pinchart, Hans de Goede, linux-media, linux-kernel

Hi Alexander,

thanks for your feedback!

On Tue, Jul 21, 2026 at 03:38:26PM +0200, Alexander Stein wrote:
> Hi Richard,
> 
> thanks for addressing this long-standing issue.
> 
> Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> > Some OV9282 camera modules, such as those from Vision Components, fail
> > multi-byte register reads with the default CCI I2C regmap access pattern
> > and only work when single-register read transactions are used. This
> > results in the driver being unable to probe due to a chip id mismatch.
> > 
> > As the driver has no reliable way to identify affected modules enable
> > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> 
> What about a property in the DT instead? In DT it should be clear if a
> Vision Components camera is used.

I've talked to Laurent about this topic (I guess a year ago or so...)
and he pointed me to his downstream work regarding the vc-mipi driver
(which I am currently also using):

https://gitlab.com/ideasonboard/nxp/linux/-/commits/v7.0/sensors/vcmipi

There is a new driver (and therefore DT compatible) for the controller
(without the sensor) on the Vision Components board implemented. This
separates basically the "regulator" part of the VC microprocessor from
the "forward I2C traffic to the sensor" part. AFAICT...

Talking about that separation I vaguely remember he argued against a
new device-tree property or compatible for the vcmipi variants of the
ov9282. Therefore I went for the "easy" approach of just enabling single
register reads.

Nonetheless if I understood this wrong or "things" changed, please feel
free to correct me.

Just to be clear: I'm also perfectly fine with introducing a new
"vcmipi,ov9282" compatible or something like a "quirk-vision-components"
property if that's preferred.

Thanks & regards;rl

> 
> Best regards,
> Alexander
> 
> > 
> > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> >  drivers/media/i2c/ov9282.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> >  	struct ov9282 *ov9282;
> >  	int ret;
> >  
> > +	const struct cci_regmap_config config = {
> > +		.reg_addr_bits = 16,
> > +		.use_single_read = true,
> > +	};
> > +
> >  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> >  	if (!ov9282)
> >  		return -ENOMEM;
> > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> >  		return ret;
> >  	}
> >  
> > -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> >  	if (IS_ERR(ov9282->regmap))
> >  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> >  				     "Failed to init CCI\n");
> > 
> > 
> 
> 
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 13:39   ` Dave Stevenson
@ 2026-07-21 15:16     ` Laurent Pinchart
  2026-07-21 15:44       ` Richard Leitner
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2026-07-21 15:16 UTC (permalink / raw)
  To: Dave Stevenson
  Cc: Richard Leitner, Mauro Carvalho Chehab, Sakari Ailus,
	Hans de Goede, linux-media, linux-kernel

On Tue, Jul 21, 2026 at 02:39:46PM +0100, Dave Stevenson wrote:
> On Tue, 21 Jul 2026 at 14:25, Richard Leitner wrote:
> >
> > Some OV9282 camera modules, such as those from Vision Components, fail
> > multi-byte register reads with the default CCI I2C regmap access pattern
> > and only work when single-register read transactions are used. This
> > results in the driver being unable to probe due to a chip id mismatch.
> 
> Those damn stupid Vision Components modules with the MCU that prevents
> you reading registers.

There's good news, the situation is improving with recent modules. We
still have to handle the other ones of course.

> I seem to recall Laurent had a plan for how to handle those without
> having to patch all the drivers, however I can't find that thread
> quickly.

I'm thinking of two options:

- We can introduce a new standard DT property for I2C device that will
  be parsed by regmap or the I2C core directly, and switch to single
  reads.

- We could model the MCU as some sort of I2C mux, sitting in DT between
  the I2C controller and the image sensor, and translate multi-byte
  reads to single-byte reads in that driver.

> > As the driver has no reliable way to identify affected modules enable
> > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> >
> > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> >
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> 
> As regmap does provide a simple way to request this behaviour, and the
> only read is the ID register from ov9282_detect, I'll give it an
> 
> Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> but it's not an ideal situation. Others can debate the alternative
> approaches for handling VC modules.
> 
> > ---
> >  drivers/media/i2c/ov9282.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> >         struct ov9282 *ov9282;
> >         int ret;
> >
> > +       const struct cci_regmap_config config = {
> > +               .reg_addr_bits = 16,
> > +               .use_single_read = true,
> > +       };
> > +
> >         ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> >         if (!ov9282)
> >                 return -ENOMEM;
> > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> >                 return ret;
> >         }
> >
> > -       ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > +       ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> >         if (IS_ERR(ov9282->regmap))
> >                 return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> >                                      "Failed to init CCI\n");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:15     ` Richard Leitner
@ 2026-07-21 15:32       ` Laurent Pinchart
  2026-07-21 15:57         ` Richard Leitner
  2026-07-22  8:00         ` Alexander Stein
  0 siblings, 2 replies; 14+ messages in thread
From: Laurent Pinchart @ 2026-07-21 15:32 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Alexander Stein, Mauro Carvalho Chehab, Sakari Ailus,
	Dave Stevenson, Hans de Goede, linux-media, linux-kernel

Hi Richard,

On Tue, Jul 21, 2026 at 05:15:06PM +0200, Richard Leitner wrote:
> On Tue, Jul 21, 2026 at 03:38:26PM +0200, Alexander Stein wrote:
> > Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > and only work when single-register read transactions are used. This
> > > results in the driver being unable to probe due to a chip id mismatch.
> > > 
> > > As the driver has no reliable way to identify affected modules enable
> > > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> > 
> > What about a property in the DT instead? In DT it should be clear if a
> > Vision Components camera is used.
> 
> I've talked to Laurent about this topic (I guess a year ago or so...)
> and he pointed me to his downstream work regarding the vc-mipi driver
> (which I am currently also using):
> 
> https://gitlab.com/ideasonboard/nxp/linux/-/commits/v7.0/sensors/vcmipi
> 
> There is a new driver (and therefore DT compatible) for the controller
> (without the sensor) on the Vision Components board implemented. This
> separates basically the "regulator" part of the VC microprocessor from
> the "forward I2C traffic to the sensor" part. AFAICT...
> 
> Talking about that separation I vaguely remember he argued against a
> new device-tree property or compatible for the vcmipi variants of the
> ov9282. Therefore I went for the "easy" approach of just enabling single
> register reads.
> 
> Nonetheless if I understood this wrong or "things" changed, please feel
> free to correct me.
> 
> Just to be clear: I'm also perfectly fine with introducing a new
> "vcmipi,ov9282" compatible or something like a "quirk-vision-components"
> property if that's preferred.

My main concern is that I don't want to see this being handled manually
in drivers. Whatever solution we decide to implement (I proposed two in
a separate e-mail in this thread) should not require modifying image
sensor drivers to implement logic specific to the Vision Components
module. I'm fine relying on drivers using specific helpers (for instance
requiring drivers to be ported to regmap or v4l2-cci in order to support
the quirk).

> > > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > > 
> > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > > ---
> > >  drivers/media/i2c/ov9282.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > > --- a/drivers/media/i2c/ov9282.c
> > > +++ b/drivers/media/i2c/ov9282.c
> > > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> > >  	struct ov9282 *ov9282;
> > >  	int ret;
> > >  
> > > +	const struct cci_regmap_config config = {
> > > +		.reg_addr_bits = 16,
> > > +		.use_single_read = true,
> > > +	};
> > > +
> > >  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> > >  	if (!ov9282)
> > >  		return -ENOMEM;
> > > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> > >  		return ret;
> > >  	}
> > >  
> > > -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> > >  	if (IS_ERR(ov9282->regmap))
> > >  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> > >  				     "Failed to init CCI\n");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:16     ` Laurent Pinchart
@ 2026-07-21 15:44       ` Richard Leitner
  2026-07-21 15:58         ` Laurent Pinchart
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 15:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Dave Stevenson, Mauro Carvalho Chehab, Sakari Ailus,
	Hans de Goede, linux-media, linux-kernel

Hi Laurent,

thank you very much for your quick feedback!

On Tue, Jul 21, 2026 at 06:16:14PM +0300, Laurent Pinchart wrote:
> On Tue, Jul 21, 2026 at 02:39:46PM +0100, Dave Stevenson wrote:
> > On Tue, 21 Jul 2026 at 14:25, Richard Leitner wrote:
> > >
> > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > and only work when single-register read transactions are used. This
> > > results in the driver being unable to probe due to a chip id mismatch.
> > 
> > Those damn stupid Vision Components modules with the MCU that prevents
> > you reading registers.
> 
> There's good news, the situation is improving with recent modules. We
> still have to handle the other ones of course.
> 
> > I seem to recall Laurent had a plan for how to handle those without
> > having to patch all the drivers, however I can't find that thread
> > quickly.
> 
> I'm thinking of two options:
> 
> - We can introduce a new standard DT property for I2C device that will
>   be parsed by regmap or the I2C core directly, and switch to single
>   reads.
> 
> - We could model the MCU as some sort of I2C mux, sitting in DT between
>   the I2C controller and the image sensor, and translate multi-byte
>   reads to single-byte reads in that driver.

I'm basically fine with both. Personally I would prefer the first
option, as this would be a generic approach which other devices could also
use. Furthermore it's IMHO the simpler solution. But that's just my 2 cents.

regards;rl

> 
> > > As the driver has no reliable way to identify affected modules enable
> > > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> > >
> > > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > >
> > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > 
> > As regmap does provide a simple way to request this behaviour, and the
> > only read is the ID register from ov9282_detect, I'll give it an
> > 
> > Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > 
> > but it's not an ideal situation. Others can debate the alternative
> > approaches for handling VC modules.
> > 
> > > ---
> > >  drivers/media/i2c/ov9282.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > > --- a/drivers/media/i2c/ov9282.c
> > > +++ b/drivers/media/i2c/ov9282.c
> > > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> > >         struct ov9282 *ov9282;
> > >         int ret;
> > >
> > > +       const struct cci_regmap_config config = {
> > > +               .reg_addr_bits = 16,
> > > +               .use_single_read = true,
> > > +       };
> > > +
> > >         ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> > >         if (!ov9282)
> > >                 return -ENOMEM;
> > > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> > >                 return ret;
> > >         }
> > >
> > > -       ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > +       ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> > >         if (IS_ERR(ov9282->regmap))
> > >                 return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> > >                                      "Failed to init CCI\n");
> 
> -- 
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:32       ` Laurent Pinchart
@ 2026-07-21 15:57         ` Richard Leitner
  2026-07-22  8:00         ` Alexander Stein
  1 sibling, 0 replies; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 15:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Alexander Stein, Mauro Carvalho Chehab, Sakari Ailus,
	Dave Stevenson, Hans de Goede, linux-media, linux-kernel

Hi Laurent,

On Tue, Jul 21, 2026 at 06:32:12PM +0300, Laurent Pinchart wrote:
> Hi Richard,
> 
> On Tue, Jul 21, 2026 at 05:15:06PM +0200, Richard Leitner wrote:
> > On Tue, Jul 21, 2026 at 03:38:26PM +0200, Alexander Stein wrote:
> > > Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> > > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > > and only work when single-register read transactions are used. This
> > > > results in the driver being unable to probe due to a chip id mismatch.
> > > > 
> > > > As the driver has no reliable way to identify affected modules enable
> > > > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> > > 
> > > What about a property in the DT instead? In DT it should be clear if a
> > > Vision Components camera is used.
> > 
> > I've talked to Laurent about this topic (I guess a year ago or so...)
> > and he pointed me to his downstream work regarding the vc-mipi driver
> > (which I am currently also using):
> > 
> > https://gitlab.com/ideasonboard/nxp/linux/-/commits/v7.0/sensors/vcmipi
> > 
> > There is a new driver (and therefore DT compatible) for the controller
> > (without the sensor) on the Vision Components board implemented. This
> > separates basically the "regulator" part of the VC microprocessor from
> > the "forward I2C traffic to the sensor" part. AFAICT...
> > 
> > Talking about that separation I vaguely remember he argued against a
> > new device-tree property or compatible for the vcmipi variants of the
> > ov9282. Therefore I went for the "easy" approach of just enabling single
> > register reads.
> > 
> > Nonetheless if I understood this wrong or "things" changed, please feel
> > free to correct me.
> > 
> > Just to be clear: I'm also perfectly fine with introducing a new
> > "vcmipi,ov9282" compatible or something like a "quirk-vision-components"
> > property if that's preferred.
> 
> My main concern is that I don't want to see this being handled manually
> in drivers. Whatever solution we decide to implement (I proposed two in
> a separate e-mail in this thread) should not require modifying image
> sensor drivers to implement logic specific to the Vision Components
> module. I'm fine relying on drivers using specific helpers (for instance
> requiring drivers to be ported to regmap or v4l2-cci in order to support
> the quirk).

Sure. That makes perfect sense. I've answered that e-mail already.

Just wanted to share my current downstream solution to start the discussion ;-)

Thanks for those quick replies and feedback!

regards;rl

> 
> > > > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > > > 
> > > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > > > ---
> > > >  drivers/media/i2c/ov9282.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > > > --- a/drivers/media/i2c/ov9282.c
> > > > +++ b/drivers/media/i2c/ov9282.c
> > > > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> > > >  	struct ov9282 *ov9282;
> > > >  	int ret;
> > > >  
> > > > +	const struct cci_regmap_config config = {
> > > > +		.reg_addr_bits = 16,
> > > > +		.use_single_read = true,
> > > > +	};
> > > > +
> > > >  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> > > >  	if (!ov9282)
> > > >  		return -ENOMEM;
> > > > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> > > >  		return ret;
> > > >  	}
> > > >  
> > > > -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > > +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> > > >  	if (IS_ERR(ov9282->regmap))
> > > >  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> > > >  				     "Failed to init CCI\n");
> 
> -- 
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:44       ` Richard Leitner
@ 2026-07-21 15:58         ` Laurent Pinchart
  2026-07-21 16:01           ` Richard Leitner
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2026-07-21 15:58 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Dave Stevenson, Mauro Carvalho Chehab, Sakari Ailus,
	Hans de Goede, linux-media, linux-kernel

On Tue, Jul 21, 2026 at 05:44:21PM +0200, Richard Leitner wrote:
> Hi Laurent,
> 
> thank you very much for your quick feedback!
> 
> On Tue, Jul 21, 2026 at 06:16:14PM +0300, Laurent Pinchart wrote:
> > On Tue, Jul 21, 2026 at 02:39:46PM +0100, Dave Stevenson wrote:
> > > On Tue, 21 Jul 2026 at 14:25, Richard Leitner wrote:
> > > >
> > > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > > and only work when single-register read transactions are used. This
> > > > results in the driver being unable to probe due to a chip id mismatch.
> > > 
> > > Those damn stupid Vision Components modules with the MCU that prevents
> > > you reading registers.
> > 
> > There's good news, the situation is improving with recent modules. We
> > still have to handle the other ones of course.
> > 
> > > I seem to recall Laurent had a plan for how to handle those without
> > > having to patch all the drivers, however I can't find that thread
> > > quickly.
> > 
> > I'm thinking of two options:
> > 
> > - We can introduce a new standard DT property for I2C device that will
> >   be parsed by regmap or the I2C core directly, and switch to single
> >   reads.
> > 
> > - We could model the MCU as some sort of I2C mux, sitting in DT between
> >   the I2C controller and the image sensor, and translate multi-byte
> >   reads to single-byte reads in that driver.
> 
> I'm basically fine with both. Personally I would prefer the first
> option, as this would be a generic approach which other devices could also
> use. Furthermore it's IMHO the simpler solution. But that's just my 2 cents.

The next step is to then write and send an RFC, to get feedback on the
idea from the DT and I2C maintainers (and possibly regmap, if we handle
the quirk at that level). Is that something you could work on ?

> > > > As the driver has no reliable way to identify affected modules enable
> > > > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> > > >
> > > > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > > >
> > > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > > 
> > > As regmap does provide a simple way to request this behaviour, and the
> > > only read is the ID register from ov9282_detect, I'll give it an
> > > 
> > > Acked-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > 
> > > but it's not an ideal situation. Others can debate the alternative
> > > approaches for handling VC modules.
> > > 
> > > > ---
> > > >  drivers/media/i2c/ov9282.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > > > --- a/drivers/media/i2c/ov9282.c
> > > > +++ b/drivers/media/i2c/ov9282.c
> > > > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> > > >         struct ov9282 *ov9282;
> > > >         int ret;
> > > >
> > > > +       const struct cci_regmap_config config = {
> > > > +               .reg_addr_bits = 16,
> > > > +               .use_single_read = true,
> > > > +       };
> > > > +
> > > >         ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> > > >         if (!ov9282)
> > > >                 return -ENOMEM;
> > > > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> > > >                 return ret;
> > > >         }
> > > >
> > > > -       ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > > +       ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> > > >         if (IS_ERR(ov9282->regmap))
> > > >                 return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> > > >                                      "Failed to init CCI\n");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:58         ` Laurent Pinchart
@ 2026-07-21 16:01           ` Richard Leitner
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Leitner @ 2026-07-21 16:01 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Dave Stevenson, Mauro Carvalho Chehab, Sakari Ailus,
	Hans de Goede, linux-media, linux-kernel

On Tue, Jul 21, 2026 at 06:58:22PM +0300, Laurent Pinchart wrote:
> On Tue, Jul 21, 2026 at 05:44:21PM +0200, Richard Leitner wrote:
> > Hi Laurent,
> > 
> > thank you very much for your quick feedback!
> > 
> > On Tue, Jul 21, 2026 at 06:16:14PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jul 21, 2026 at 02:39:46PM +0100, Dave Stevenson wrote:
> > > > On Tue, 21 Jul 2026 at 14:25, Richard Leitner wrote:
> > > > >
> > > > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > > > and only work when single-register read transactions are used. This
> > > > > results in the driver being unable to probe due to a chip id mismatch.
> > > > 
> > > > Those damn stupid Vision Components modules with the MCU that prevents
> > > > you reading registers.
> > > 
> > > There's good news, the situation is improving with recent modules. We
> > > still have to handle the other ones of course.
> > > 
> > > > I seem to recall Laurent had a plan for how to handle those without
> > > > having to patch all the drivers, however I can't find that thread
> > > > quickly.
> > > 
> > > I'm thinking of two options:
> > > 
> > > - We can introduce a new standard DT property for I2C device that will
> > >   be parsed by regmap or the I2C core directly, and switch to single
> > >   reads.
> > > 
> > > - We could model the MCU as some sort of I2C mux, sitting in DT between
> > >   the I2C controller and the image sensor, and translate multi-byte
> > >   reads to single-byte reads in that driver.
> > 
> > I'm basically fine with both. Personally I would prefer the first
> > option, as this would be a generic approach which other devices could also
> > use. Furthermore it's IMHO the simpler solution. But that's just my 2 cents.
> 
> The next step is to then write and send an RFC, to get feedback on the
> idea from the DT and I2C maintainers (and possibly regmap, if we handle
> the quirk at that level). Is that something you could work on ?

Sure. I'll take a more detailled look at it tomorrow.

regards;rl

...

> -- 
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
  2026-07-21 15:32       ` Laurent Pinchart
  2026-07-21 15:57         ` Richard Leitner
@ 2026-07-22  8:00         ` Alexander Stein
  1 sibling, 0 replies; 14+ messages in thread
From: Alexander Stein @ 2026-07-22  8:00 UTC (permalink / raw)
  To: Richard Leitner, Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Dave Stevenson,
	Hans de Goede, linux-media, linux-kernel

Hi everyone,

Am Dienstag, 21. Juli 2026, 17:32:12 CEST schrieb Laurent Pinchart:
> Hi Richard,
> 
> On Tue, Jul 21, 2026 at 05:15:06PM +0200, Richard Leitner wrote:
> > On Tue, Jul 21, 2026 at 03:38:26PM +0200, Alexander Stein wrote:
> > > Am Dienstag, 21. Juli 2026, 15:25:29 CEST schrieb Richard Leitner:
> > > > Some OV9282 camera modules, such as those from Vision Components, fail
> > > > multi-byte register reads with the default CCI I2C regmap access pattern
> > > > and only work when single-register read transactions are used. This
> > > > results in the driver being unable to probe due to a chip id mismatch.
> > > > 
> > > > As the driver has no reliable way to identify affected modules enable
> > > > use_single_read for OV9282 unconditionally when creating the CCI regmap.
> > > 
> > > What about a property in the DT instead? In DT it should be clear if a
> > > Vision Components camera is used.
> > 
> > I've talked to Laurent about this topic (I guess a year ago or so...)
> > and he pointed me to his downstream work regarding the vc-mipi driver
> > (which I am currently also using):
> > 
> > https://gitlab.com/ideasonboard/nxp/linux/-/commits/v7.0/sensors/vcmipi

Yep, I've also been using that one. And yes, this I2C access peculiarity has
been an issue for a long time already.

> > There is a new driver (and therefore DT compatible) for the controller
> > (without the sensor) on the Vision Components board implemented. This
> > separates basically the "regulator" part of the VC microprocessor from
> > the "forward I2C traffic to the sensor" part. AFAICT...
> > 
> > Talking about that separation I vaguely remember he argued against a
> > new device-tree property or compatible for the vcmipi variants of the
> > ov9282. Therefore I went for the "easy" approach of just enabling single
> > register reads.
> > 
> > Nonetheless if I understood this wrong or "things" changed, please feel
> > free to correct me.
> > 
> > Just to be clear: I'm also perfectly fine with introducing a new
> > "vcmipi,ov9282" compatible or something like a "quirk-vision-components"
> > property if that's preferred.
> 
> My main concern is that I don't want to see this being handled manually
> in drivers. Whatever solution we decide to implement (I proposed two in
> a separate e-mail in this thread) should not require modifying image
> sensor drivers to implement logic specific to the Vision Components
> module. I'm fine relying on drivers using specific helpers (for instance
> requiring drivers to be ported to regmap or v4l2-cci in order to support
> the quirk).

I had the idea of a generic property like 'i2c-disable-autoincrement' usable
on all i2c devices. This property might be parsed in regmap-i2c (or even
i2c-core) so it is enabled and available for all i2c devices without
modifying any driver.

Best regards,
Alexander

> > > > Tested on an i.M8MP system with OV9282 modules from Vision Components.
> > > > 
> > > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > > > ---
> > > >  drivers/media/i2c/ov9282.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > > index 5b6f897a74fcd..8267a0a9c6f2d 100644
> > > > --- a/drivers/media/i2c/ov9282.c
> > > > +++ b/drivers/media/i2c/ov9282.c
> > > > @@ -1283,6 +1283,11 @@ static int ov9282_probe(struct i2c_client *client)
> > > >  	struct ov9282 *ov9282;
> > > >  	int ret;
> > > >  
> > > > +	const struct cci_regmap_config config = {
> > > > +		.reg_addr_bits = 16,
> > > > +		.use_single_read = true,
> > > > +	};
> > > > +
> > > >  	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
> > > >  	if (!ov9282)
> > > >  		return -ENOMEM;
> > > > @@ -1301,7 +1306,7 @@ static int ov9282_probe(struct i2c_client *client)
> > > >  		return ret;
> > > >  	}
> > > >  
> > > > -	ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > > +	ov9282->regmap = devm_cci_regmap_init_i2c_cfg(client, &config);
> > > >  	if (IS_ERR(ov9282->regmap))
> > > >  		return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> > > >  				     "Failed to init CCI\n");
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-07-22  8:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 13:25 [PATCH 0/2] media: v4l2-cci/ov9282: support single-read I2C transactions Richard Leitner
2026-07-21 13:25 ` [PATCH 1/2] media: v4l2-cci: Add support for custom regmap configuration Richard Leitner
2026-07-21 13:25 ` [PATCH 2/2] media: ov9282: enable single-read I2C transactions Richard Leitner
2026-07-21 13:38   ` Alexander Stein
2026-07-21 15:12     ` Laurent Pinchart
2026-07-21 15:15     ` Richard Leitner
2026-07-21 15:32       ` Laurent Pinchart
2026-07-21 15:57         ` Richard Leitner
2026-07-22  8:00         ` Alexander Stein
2026-07-21 13:39   ` Dave Stevenson
2026-07-21 15:16     ` Laurent Pinchart
2026-07-21 15:44       ` Richard Leitner
2026-07-21 15:58         ` Laurent Pinchart
2026-07-21 16:01           ` Richard Leitner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox