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] [day] [month] [year] [list]

Message-ID: <20250623134200.GB29597@pendragon.ideasonboard.com>
Date: Mon, 23 Jun 2025 16:42:00 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: "Nirujogi, Pratap" <pnirujog@....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 01:22:00PM +0000, Sakari Ailus wrote:
> On Mon, Jun 23, 2025 at 03:09:29PM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 16, 2025 at 07:12:28PM -0400, Nirujogi, Pratap wrote:
> > > On 6/13/2025 6:02 PM, Sakari Ailus wrote:
> > > > On Fri, Jun 13, 2025 at 12:55:46PM +0800, Hao Yao wrote:
> > > >> Hi Pratap,
> > > >>
> > > >> Thanks for your patch.
> > > >>
> > > >> This patch is written for your camera sensor module, which seems very
> > > >> different from those already applied on Dell laptops (some of "Dell Pro"
> > > >> series). Looking into the driver, I think this version will break the
> > > >> devices using ov05c10 sensor.
> > > > 
> > > > There never was such a driver in upstream so nothing breaks. However, in
> > > > order to support these, could you check what would it take to support them
> > > > using this driver and post patches, please?
> > > > 
> > > >> I think this patch is better to be validated on existing devices, but please
> > > >> do some fixes before we can do validation. Please check my comments inline.
> > > >>
> > > >> On 2025/6/10 03:42, Pratap Nirujogi wrote:
> > > >>> Add driver for OmniVision 5.2M OV05C10 sensor. This driver
> > > >>> supports only the full size normal 2888x1808@...ps 2-lane
> > > >>> sensor profile.
> > > >>>
> > > >>> Co-developed-by: Venkata Narendra Kumar Gutta <vengutta@....com>
> > > >>> Signed-off-by: Venkata Narendra Kumar Gutta <vengutta@....com>
> > > >>> Co-developed-by: Bin Du <bin.du@....com>
> > > >>> Signed-off-by: Bin Du <bin.du@....com>
> > > >>> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@....com>
> > 
> > [snip]
> > 
> > > >> Hi Sakari,
> > > >>
> > > >> 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.
> > > 
> > > Hi Sakari, thank you for sharing your insights and guiding us. Could you 
> > > please suggest if we should take up this work implementing the helpers 
> > > in CCI and submit the patch or is it okay to leave it as-is for now and 
> > > take care of updating in future once the implementation is ready.
> > 
> > I think it can live in the driver for now. Given that the device uses
> > only 8 bits of register address, I would store the page number in bits
> > 15:8 instead of bits 31:24, as the CCI helpers do not make bits 27:24
> > available for driver-specific purpose.
> 
> I'd use the CCI private bits, the driver uses page numbers up to 4 so 4
> bits are plenty for that. If we add pages to CCI later, this may be
> refactored then.

That works too.

-- 
Regards,

Laurent Pinchart