linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Yan, Dongcheng" <dongcheng.yan@linux.intel.com>
To: "Nirujogi, Pratap" <pnirujog@amd.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Hao Yao <hao.yao@intel.com>,
	Pratap Nirujogi <pratap.nirujogi@amd.com>,
	mchehab@kernel.org, hverkuil@xs4all.nl,
	bryan.odonoghue@linaro.org, krzk@kernel.org,
	dave.stevenson@raspberrypi.com, hdegoede@redhat.com,
	jai.luthra@ideasonboard.com, tomi.valkeinen@ideasonboard.com,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	benjamin.chan@amd.com, bin.du@amd.com, grosikop@amd.com,
	king.li@amd.com, dantony@amd.com, vengutta@amd.com,
	dongcheng.yan@intel.com, jason.z.chen@intel.com,
	jimmy.su@intel.com
Subject: Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver
Date: Wed, 2 Jul 2025 14:08:26 +0800	[thread overview]
Message-ID: <0d45fca3-5b6f-42e5-acec-bca2dda25f15@linux.intel.com> (raw)
In-Reply-To: <702594a4-ebc3-471e-8551-d94f0dee2982@amd.com>

Hi Pratap,

On 7/1/2025 6:31 AM, Nirujogi, Pratap wrote:
> Hi Sakari, Hi Laurent,
> 
> On 6/29/2025 3:40 AM, Sakari Ailus wrote:
>> Caution: This message originated from an External Source. Use proper
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> Hi Pratap,
>>
>> On 6/17/25 01:33, Nirujogi, Pratap wrote:
>> ...
>>>>>>> +static const struct cci_reg_sequence ov05c10_2888x1808_regs[] = {
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0x20),  0x00 },
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0x20),  0x0b },
>>>>>>> + { CCI_REG8(0xc1),  0x09 },
>>>>>>> + { CCI_REG8(0x21),  0x06 },
>>>>>>> + { CCI_REG8(0x14),  0x78 },
>>>>>>> + { CCI_REG8(0xe7),  0x03 },
>>>>>>> + { CCI_REG8(0xe7),  0x00 },
>>>>>>> + { CCI_REG8(0x21),  0x00 },
>>>>>>> + { CCI_REG8(0xfd),  0x01 },
>>>>>>> + { CCI_REG8(0x03),  0x00 },
>>>>>>> + { CCI_REG8(0x04),  0x06 },
>>>>>>> + { CCI_REG8(0x05),  0x07 },
>>>>>>> + { CCI_REG8(0x06),  0x44 },
>>>>>>> + { CCI_REG8(0x07),  0x08 },
>>>>>>> + { CCI_REG8(0x1b),  0x01 },
>>>>>>> + { CCI_REG8(0x24),  0xff },
>>>>>>> + { CCI_REG8(0x32),  0x03 },
>>>>>>> + { CCI_REG8(0x42),  0x5d },
>>>>>>> + { CCI_REG8(0x43),  0x08 },
>>>>>>> + { CCI_REG8(0x44),  0x81 },
>>>>>>> + { CCI_REG8(0x46),  0x5f },
>>>>>>> + { CCI_REG8(0x48),  0x18 },
>>>>>>> + { CCI_REG8(0x49),  0x04 },
>>>>>>> + { CCI_REG8(0x5c),  0x18 },
>>>>>>> + { CCI_REG8(0x5e),  0x13 },
>>>>>>> + { CCI_REG8(0x70),  0x15 },
>>>>>>> + { CCI_REG8(0x77),  0x35 },
>>>>>>> + { CCI_REG8(0x79),  0x00 },
>>>>>>> + { CCI_REG8(0x7b),  0x08 },
>>>>>>> + { CCI_REG8(0x7d),  0x08 },
>>>>>>> + { CCI_REG8(0x7e),  0x08 },
>>>>>>> + { CCI_REG8(0x7f),  0x08 },
>>>>>>> + { CCI_REG8(0x90),  0x37 },
>>>>>>> + { CCI_REG8(0x91),  0x05 },
>>>>>>> + { CCI_REG8(0x92),  0x18 },
>>>>>>> + { CCI_REG8(0x93),  0x27 },
>>>>>>> + { CCI_REG8(0x94),  0x05 },
>>>>>>> + { CCI_REG8(0x95),  0x38 },
>>>>>>> + { CCI_REG8(0x9b),  0x00 },
>>>>>>> + { CCI_REG8(0x9c),  0x06 },
>>>>>>> + { CCI_REG8(0x9d),  0x28 },
>>>>>>> + { CCI_REG8(0x9e),  0x06 },
>>>>>>> + { CCI_REG8(0xb2),  0x0f },
>>>>>>> + { CCI_REG8(0xb3),  0x29 },
>>>>>>> + { CCI_REG8(0xbf),  0x3c },
>>>>>>> + { CCI_REG8(0xc2),  0x04 },
>>>>>>> + { CCI_REG8(0xc4),  0x00 },
>>>>>>> + { CCI_REG8(0xca),  0x20 },
>>>>>>> + { CCI_REG8(0xcb),  0x20 },
>>>>>>> + { CCI_REG8(0xcc),  0x28 },
>>>>>>> + { CCI_REG8(0xcd),  0x28 },
>>>>>>> + { CCI_REG8(0xce),  0x20 },
>>>>>>> + { CCI_REG8(0xcf),  0x20 },
>>>>>>> + { CCI_REG8(0xd0),  0x2a },
>>>>>>> + { CCI_REG8(0xd1),  0x2a },
>>>>>>> + { CCI_REG8(0xfd),  0x0f },
>>>>>>> + { CCI_REG8(0x00),  0x00 },
>>>>>>> + { CCI_REG8(0x01),  0xa0 },
>>>>>>> + { CCI_REG8(0x02),  0x48 },
>>>>>>> + { CCI_REG8(0x07),  0x8f },
>>>>>>> + { CCI_REG8(0x08),  0x70 },
>>>>>>> + { CCI_REG8(0x09),  0x01 },
>>>>>>> + { CCI_REG8(0x0b),  0x40 },
>>>>>>> + { CCI_REG8(0x0d),  0x07 },
>>>>>>> + { CCI_REG8(0x11),  0x33 },
>>>>>>> + { CCI_REG8(0x12),  0x77 },
>>>>>>> + { CCI_REG8(0x13),  0x66 },
>>>>>>> + { CCI_REG8(0x14),  0x65 },
>>>>>>> + { CCI_REG8(0x15),  0x37 },
>>>>>>> + { CCI_REG8(0x16),  0xbf },
>>>>>>> + { CCI_REG8(0x17),  0xff },
>>>>>>> + { CCI_REG8(0x18),  0xff },
>>>>>>> + { CCI_REG8(0x19),  0x12 },
>>>>>>> + { CCI_REG8(0x1a),  0x10 },
>>>>>>> + { CCI_REG8(0x1c),  0x77 },
>>>>>>> + { CCI_REG8(0x1d),  0x77 },
>>>>>>> + { CCI_REG8(0x20),  0x0f },
>>>>>>> + { CCI_REG8(0x21),  0x0f },
>>>>>>> + { CCI_REG8(0x22),  0x0f },
>>>>>>> + { CCI_REG8(0x23),  0x0f },
>>>>>>> + { CCI_REG8(0x2b),  0x20 },
>>>>>>> + { CCI_REG8(0x2c),  0x20 },
>>>>>>> + { CCI_REG8(0x2d),  0x04 },
>>>>>>> + { CCI_REG8(0xfd),  0x03 },
>>>>>>> + { CCI_REG8(0x9d),  0x0f },
>>>>>>> + { CCI_REG8(0x9f),  0x40 },
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0x20),  0x1b },
>>>>>>> + { CCI_REG8(0xfd),  0x04 },
>>>>>>> + { CCI_REG8(0x19),  0x60 },
>>>>>>> + { CCI_REG8(0xfd),  0x02 },
>>>>>>> + { CCI_REG8(0x75),  0x05 },
>>>>>>> + { CCI_REG8(0x7f),  0x06 },
>>>>>>> + { CCI_REG8(0x9a),  0x03 },
>>>>>>> + { CCI_REG8(0xa2),  0x07 },
>>>>>>> + { CCI_REG8(0xa3),  0x10 },
>>>>>>> + { CCI_REG8(0xa5),  0x02 },
>>>>>>> + { CCI_REG8(0xa6),  0x0b },
>>>>>>> + { CCI_REG8(0xa7),  0x48 },
>>>>>>> + { CCI_REG8(0xfd),  0x07 },
>>>>>>> + { CCI_REG8(0x42),  0x00 },
>>>>>>> + { CCI_REG8(0x43),  0x80 },
>>>>>>> + { CCI_REG8(0x44),  0x00 },
>>>>>>> + { CCI_REG8(0x45),  0x80 },
>>>>>>> + { CCI_REG8(0x46),  0x00 },
>>>>>>> + { CCI_REG8(0x47),  0x80 },
>>>>>>> + { CCI_REG8(0x48),  0x00 },
>>>>>>> + { CCI_REG8(0x49),  0x80 },
>>>>>>> + { CCI_REG8(0x00),  0xf7 },
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0xe7),  0x03 },
>>>>>>> + { CCI_REG8(0xe7),  0x00 },
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0x93),  0x18 },
>>>>>>> + { CCI_REG8(0x94),  0xff },
>>>>>>> + { CCI_REG8(0x95),  0xbd },
>>>>>>> + { CCI_REG8(0x96),  0x1a },
>>>>>>> + { CCI_REG8(0x98),  0x04 },
>>>>>>> + { CCI_REG8(0x99),  0x08 },
>>>>>>> + { CCI_REG8(0x9b),  0x10 },
>>>>>>> + { CCI_REG8(0x9c),  0x3f },
>>>>>>> + { CCI_REG8(0xa1),  0x05 },
>>>>>>> + { CCI_REG8(0xa4),  0x2f },
>>>>>>> + { CCI_REG8(0xc0),  0x0c },
>>>>>>> + { CCI_REG8(0xc1),  0x08 },
>>>>>>> + { CCI_REG8(0xc2),  0x00 },
>>>>>>> + { CCI_REG8(0xb6),  0x20 },
>>>>>>> + { CCI_REG8(0xbb),  0x80 },
>>>>>>> + { CCI_REG8(0xfd),  0x00 },
>>>>>>> + { CCI_REG8(0xa0),  0x01 },
>>>>>>> + { CCI_REG8(0xfd),  0x01 },
>>>>
>>>> Please replace these with names macros where possible. I'm sure quite a
>>>> few of the registers configured here are documented in the datasheet.
>>>> The registers that configure the mode (analog crop, digital crop,
>>>> binning, skipping, ...) should be computed dynamically from the subdev
>>>> pad format and selection rectangles, not hardcoded.
>>>>
>>> I agree, but we get the sensor settings based on our requirements from
>>> the vendor, i will check if we can get some more info regarding the
>>> crop, binning, skipping etc...
>>
>> Some of this infomation should be available in the datasheet. Use at
>> least the register names that can be found, for those that can't there's
>> not much that could be done.
>>
> Sorry to say that I don't have the details in this case. We have
> previously reached out to the sensor vendor, but they are not willing to
> disclose any of these details. We hope for your understanding of the
> constraints we're facing and truly value your support.
> 

If you have a spec of OV05C10 (I assume you do, as the developer of this
driver), it is not a issue.
Take P0:0x14 as an example, it's named as DPLL_NC_SEL in spec and set to
0x78 in your reglist ov05c10_2888x1808_regs. If define all named
registers rather than the confusing magic hardcode, the driver code will
be more readable and easy to review.
I think this is what Sakari thought.

Thanks,
Dongcheng

> Thanks,
> Pratap
> 
>> -- 
>> Regards,
>>
>> Sakari Ailus
> 
> 


  reply	other threads:[~2025-07-02  6:08 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-09 19:42 [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver Pratap Nirujogi
2025-06-13  4:55 ` Hao Yao
2025-06-13 11:02   ` Kieran Bingham
2025-06-13 12:05     ` Bryan O'Donoghue
2025-06-16 23:15       ` Nirujogi, Pratap
2025-06-16 23:46     ` Nirujogi, Pratap
2025-06-28 19:23       ` Sakari Ailus
2025-06-13 22:02   ` Sakari Ailus
2025-06-14 22:52     ` Laurent Pinchart
2025-06-16 22:33       ` Nirujogi, Pratap
2025-06-29  7:40         ` Sakari Ailus
2025-06-30 22:31           ` Nirujogi, Pratap
2025-07-02  6:08             ` Yan, Dongcheng [this message]
2025-07-02  6:12               ` Sakari Ailus
2025-07-02 16:47                 ` Nirujogi, Pratap
2025-07-03  6:24                   ` Yan, Dongcheng
2025-07-05 11:45                     ` Nirujogi, Pratap
2025-07-06  9:11                   ` Sakari Ailus
2025-07-08 15:31                     ` Nirujogi, Pratap
2025-06-23 11:27       ` Sakari Ailus
2025-06-23 12:06         ` Laurent Pinchart
2025-06-23 21:53           ` Nirujogi, Pratap
2025-06-23 22:11             ` Laurent Pinchart
2025-06-16 23:12     ` Nirujogi, Pratap
2025-06-23 12:09       ` Laurent Pinchart
2025-06-23 13:22         ` Sakari Ailus
2025-06-23 13:42           ` Laurent Pinchart
2025-06-23 21:55             ` Nirujogi, Pratap
2025-06-23 22:06               ` Laurent Pinchart
2025-06-23 23:21                 ` Nirujogi, Pratap
2025-06-25 22:06               ` Nirujogi, Pratap
2025-06-26 11:11                 ` Kieran Bingham
2025-06-26 12:23                   ` Laurent Pinchart
2025-06-26 18:22                     ` Nirujogi, Pratap
2025-06-26 18:56                       ` Laurent Pinchart
2025-06-26 19:21                         ` Nirujogi, Pratap
2025-06-26 18:14                   ` Nirujogi, Pratap
2025-06-17  0:19   ` Nirujogi, Pratap
2025-06-15  0:09 ` Laurent Pinchart
2025-06-16 22:49   ` Nirujogi, Pratap
2025-06-23 21:51     ` Nirujogi, Pratap
2025-06-23 22:05       ` Laurent Pinchart
2025-06-23 23:28         ` Nirujogi, Pratap
2025-06-24  0:19           ` Laurent Pinchart
2025-06-24 18:49             ` Nirujogi, Pratap
2025-06-24 19:00               ` Laurent Pinchart
2025-06-24 19:49                 ` Nirujogi, Pratap
2025-06-24  8:35         ` Mehdi Djait
2025-06-24 10:19           ` Sakari Ailus
2025-06-24 10:20             ` Sakari Ailus
2025-06-24 10:27               ` Laurent Pinchart
2025-06-24 11:27                 ` Mehdi Djait
2025-06-24 11:33                   ` Laurent Pinchart
2025-06-24 11:46                   ` Sakari Ailus
2025-06-24 16:34                     ` Mehdi Djait
2025-06-24 20:24                       ` Nirujogi, Pratap
2025-06-25  6:11                       ` Sakari Ailus
2025-06-24 18:26                   ` Nirujogi, Pratap
2025-06-24 20:01                     ` Nirujogi, Pratap
  -- strict thread matches above, loose matches on Subject: below --
2025-05-15 22:41 Pratap Nirujogi

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=0d45fca3-5b6f-42e5-acec-bca2dda25f15@linux.intel.com \
    --to=dongcheng.yan@linux.intel.com \
    --cc=benjamin.chan@amd.com \
    --cc=bin.du@amd.com \
    --cc=bryan.odonoghue@linaro.org \
    --cc=dantony@amd.com \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=dongcheng.yan@intel.com \
    --cc=grosikop@amd.com \
    --cc=hao.yao@intel.com \
    --cc=hdegoede@redhat.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jai.luthra@ideasonboard.com \
    --cc=jason.z.chen@intel.com \
    --cc=jimmy.su@intel.com \
    --cc=king.li@amd.com \
    --cc=krzk@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=pnirujog@amd.com \
    --cc=pratap.nirujogi@amd.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=vengutta@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).