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: <aFk6K4mYtq24MnNj@valkosipuli.retiisi.eu>
Date: Mon, 23 Jun 2025 11:27:39 +0000
From: Sakari Ailus <sakari.ailus@....fi>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
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

Hi Laurent,

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.

> > > 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? The current CCI register addresses are limited to 16 bits. To
support that, we'd need to use u64 most likely. For 16-bit register
addresses and 8-bit values which probably are the most common, that starts
to appear a bit wasteful.

-- 
Regards,

Sakari Ailus