linux-kernel - Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250623120613.GB19319@pendragon.ideasonboard.com>
Date: Mon, 23 Jun 2025 15:06:13 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Sakari Ailus <sakari.ailus@....fi>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Hao Yao <hao.yao@...el.com>,
	Pratap Nirujogi <pratap.nirujogi@....com>, mchehab@...nel.org,
	hverkuil@...all.nl, bryan.odonoghue@...aro.org, krzk@...nel.org,
	dave.stevenson@...pberrypi.com, hdegoede@...hat.com,
	jai.luthra@...asonboard.com, tomi.valkeinen@...asonboard.com,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	benjamin.chan@....com, bin.du@....com, grosikop@....com,
	king.li@....com, dantony@....com, vengutta@....com,
	dongcheng.yan@...el.com, jason.z.chen@...el.com, jimmy.su@...el.com
Subject: Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver

On Mon, Jun 23, 2025 at 11:27:39AM +0000, Sakari Ailus wrote:
> On Sun, Jun 15, 2025 at 01:52:57AM +0300, Laurent Pinchart wrote:
> > > > > +#define OV05C10_REF_CLK			(24 * HZ_PER_MHZ)
> > > > 
> > > > Seems your module use 24 MHz clock input. The Dell's modules always use
> > > > 19.2MHz, which means your the PLL settings will not work on Dell's.
> > > 
> > > This is ok as further work. Please send a patch. :-)
> > 
> > The patch should calculate PLL configuration dynamically, perhaps using
> > the ccs-pll calculator if applicable.
> 
> As much as I do like your suggestion, I don't think it's really feasible to
> often do this for Omnivision sensors (most others largely do just work
> without much hassle wrt. PLL, as long as a PLL calculator exists). This
> sensor's PLL tree is different from CCS and badly documented, as expected.

How much do we know about the PLL structure ?

> > > > Seems there are already several camera sensors using page-based registers.
> > > > Is it a good idea to add page support in CCI interface?
> > > 
> > > Sounds like a good idea as such but I'm not sure how common this really is,
> > > I think I've seen a few Omnivision sensors doing this. If implemented, I
> > > think it would be nice if the page could be encoded in the register address
> > > which V4L2 CCI would store and switch page if needed only. This would
> > > require serialising accesses, too. There's some room in CCI register raw
> > > value space so this could be done without even changing that, say, with
> > > 8-bit page and 8-bit register address.
> > 
> > Ack. I've worked on a driver for the AP1302 external ISP, which also
> > uses pages registers. The full address space spans 32 bits though, but
> > fortunately the driver doesn't need to access anything above 0x00ffffff.
> 
> 0xffffff?

Yes.

> The current CCI register addresses are limited to 16 bits. To
> support that, we'd need to use u64 most likely.

I handled it in the ap1302 driver, by using bits 31:24 of address to
store a 8 bits page value. It's a hack as the CCI helper currently only
allocates 4 bits of the address to driver-specific purpose.

> For 16-bit register
> addresses and 8-bit values which probably are the most common, that starts
> to appear a bit wasteful.

It is wasteful, I don't want to turn the register address to a 64-bit
value.

-- 
Regards,

Laurent Pinchart