All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Leitner <richard.leitner@linux.dev>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>,
	 Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	 Hans de Goede <hansg@kernel.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] media: ov9282: enable single-read I2C transactions
Date: Tue, 21 Jul 2026 17:44:21 +0200	[thread overview]
Message-ID: <al-OpcU5M8UL_c8f@bombadil> (raw)
In-Reply-To: <20260721151614.GN50424@killaraus.ideasonboard.com>

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

  reply	other threads:[~2026-07-21 15:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-07-21 15:58         ` Laurent Pinchart
2026-07-21 16:01           ` Richard Leitner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=al-OpcU5M8UL_c8f@bombadil \
    --to=richard.leitner@linux.dev \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=hansg@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.