Re: [PATCH v5] media: i2c: Add ar0234 camera sensor driver

["Yan, Dongcheng" <dongcheng.yan@intel.com>]

Hi Sakari,

On 4/17/2025 2:53 PM, Sakari Ailus wrote:
> Hi Dongcheng,
> 
> On Fri, Apr 11, 2025 at 12:12:49AM +0800, Dongcheng Yan wrote:
>> The driver is implemented with V4L2 framework,
>> and supports following features:
>>
>>     - manual exposure and analog/digital gain control
>>     - vblank/hblank control
>>     - vflip/hflip control
>>     - runtime PM support
>>     - 1280x960@30fps and 1920x1200@60fps support
>>     - dynamical pll_params set
>>
>> Signed-off-by: Dongcheng Yan <dongcheng.yan@intel.com>
> 
> I reviewed Laurent's ar0144 driver and it turns out that the ar0144 and
> ar0234 sensors are nearly identical in their register interface. Thus they
> could be controlled using the same driver, with relatively small changes.
> Laurent's driver programmatically sets the register values in general and
> does not rely on register lists, which is why I prefer that driver.
> 
> Would you be able to add ar0234 support to the ar0144 driver, please?
> Sensor configuration independent MSRs are totally fine, I presume there are
> some the ar0234 benefits from.
> 
> My apologies for telling you this now, after 5 versions of this driver, but
> I only learned of this very recently.
> 
> I expect Laurent to send v3 of his driver by early next week.
> 

This is the first time I have come across such a suggestion to merge one
sensor driver into another sensor driver.
Why should ar0234 be merged into the ar0144? If it is because of "nearly
identical in their register interface", then most sensor drivers can be
merged into one driver according to vendor, such as AR/OV/IMX. So I
don't quite understand the meaning of doing this.

And AR0234 is the first patch, why should it be merged into the later patch?
Considering that ar0144 is fully implemented using ccs-pll, do you want
me to use the parameter constraints on pll in AR0144? The lack of
information in the AR0234 spec makes it impossible to implement dynamic
calculations in ccs-pll. This can also be seen from Dave's comments. The
pll parameters we use are very different. The parameter value
constraints found only from the existing spec cannot meet a
multi-platform compatible ccs-pll implementation.

Thanks,
Dongcheng