[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b6425dbe-44e6-47b4-a06b-b9a172a8cac4@amd.com>
Date: Mon, 23 Jun 2025 17:55:14 -0400
From: "Nirujogi, Pratap" <pnirujog@....com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: 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, Svetoslav.Stoilov@....com,
Yana.Zheleva@....com
Subject: Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver
Hi Laurent, Sakari,
On 6/23/2025 9:42 AM, Laurent Pinchart wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> 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.
>
Thanks for your support. We will add the page number in the register
address 15:8 or 11:8 and will update the implementation accordingly in
the next version.
Thanks,
Pratap
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists