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: <44b71f16-edc5-46c3-96d1-53aceba537be@linux.intel.com>
Date: Sat, 28 Jun 2025 22:23:41 +0300
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: "Nirujogi, Pratap" <pnirujog@....com>,
 Kieran Bingham <kieran.bingham@...asonboard.com>, Hao Yao
 <hao.yao@...el.com>, Pratap Nirujogi <pratap.nirujogi@....com>
Cc: mchehab@...nel.org, laurent.pinchart@...asonboard.com,
 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 Pratap, Kieran,

On 6/17/25 02:46, Nirujogi, Pratap wrote:
> Hi Kieran,
> 
> Thank you for reviewing and sharing your feedback.
> 
> On 6/13/2025 7:02 AM, Kieran Bingham wrote:
>> Caution: This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> Quoting Hao Yao (2025-06-13 05:55:46)
>> I think it will highlight that werever possible - the code below should
>> be factored out to support the different configuration requirements.
>> Cleaning up the large tables of register addresses and making those
>> configurable functions for example configuring the link rate
>> independently would be really beneficial!
>>
>> That's precisely why we continually push for reducing the large
>> "undocumented register" tables in sensor drivers...
>>
> Yes, I agree, it is best to have documented settings and make 
> calculation for sensor modes instead of array of undocumented settings. 
> However the usual practice is that we send requirements to sensor vendor 
> and they provide us the settings array to be applied. May be we can try 
> to move PLL settings to different array but it is not always just the 
> PLL, there are a lot of undocumented settings which are in sync with PLL 
> and sometimes changing the PLL may result in misbehaviour of the sensor...
> 
> We will try to move PLL settings to separate array but cannot guarantee 
> changing these settings alone will make the sensor functional.

I did discuss this with Laurent as well and based on that I'd suggest 
the following split:

- power-on register list,
- external clock / link frequency specific list (often these are tied)
   and
- sensor mode

Which of these lists a particular register is written in isn't crucial 
at the moment, this can be always changed later on.

I'd leave the possible PLL calculator for later. Having more 
configurations is also useful for validating its function. A different 
PLL configuration from the original doesn't necessarily mean it would be 
wrong.

...

>>>> +     page = OV05C10_GET_PAGE_NUM(reg);
>>>> +     addr = OV05C10_GET_REG_ADDR(reg);
>>>> +     ov05c10_switch_page(ov05c10, page, &ret);
>>>> +     cci_write(ov05c10->regmap, addr, val, &ret);
>>>> +     if (err)
>>>> +             *err = ret;
>>>> +
>>>> +     return ret;
>>>> +}
>>
>> One of the main goals of CCI helpers was to avoid all of the custom
>> device accessors being duplicated in each driver, so I think extending
>> the CCI helpers to support page based accesses in some common way would
>> be beneficial.
>>
> Yes, I agree. We can take on this enhancement either now or in the 
> future, depending on the direction. We'll wait for Sakari's feedback to 
> determine the best way to proceed.

As Laurent suggested, keep the page-based helper macros/functions in 
this driver, we can generalise them later on as needed.

-- 
Kind regards,

Sakari Ailus


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ