Re: [PATCH v3 4/4] media: ox05b1s: Add support for Omnivision OS08A20 raw sensor

[Krzysztof Kozlowski]

On 03/02/2025 09:43, Mirela Rabulea wrote:
> Hi Krzysztof,
> 
> thanks again for feedback.
> 
> On 24.01.2025 10:09, Krzysztof Kozlowski wrote:
>> On Fri, Jan 24, 2025 at 02:12:43AM +0200, Mirela Rabulea wrote:
>>> @@ -758,6 +914,9 @@ static int ox05b1s_read_chip_id(struct ox05b1s *sensor)
>>>        }
>>>
>>>        switch (chip_id) {
>>> +     case 0x530841:
>>> +             camera_name = "os08a20";
>>> +             break;
>> Ah, so here I see missing second device support.
>>
>> It is still confusing to see that you use here some sort of
>> autodetection, but in the same time not.
> 
> The two sensors that I included in this driver have some similarities, 
> but also differences, for which I used the platform data. I made 
> separate patches for the two sensors, such that it is visible how much 
> is common/different.  The chip_id reading is for validating that the 
> sensor that is actually attached matches the device tree. It happens to 
> me sometimes, that I switch the sensors, but forget to switch the dtb, 
> and it helps to see which sensor is actually attached. Also, it helps a 
> lot when the evaluation board is in some remote lab and I am unsure what 
> sensor is attached to it.
> 
> I saw most sensor drivers have some kind of identification of the sensor 
> module by the means of reading the chip_id register. Some examples with 
> multiple compatibles supported and chip_id validation: imx296, ov9650, 
> ov772x.
> 
> Please let me know what you suggest further.


If devices are from the same family and support reliable autodetection,
they should be made compatible. At least that's generic approach.


Best regards,
Krzysztof