Re: [PATCH v4 3/7] media: i2c: ov9282: Add ov9281 compatible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/08/2022 10:23, Sakari Ailus wrote:
> On Mon, Aug 01, 2022 at 08:08:58PM +0200, Krzysztof Kozlowski wrote:
>> On 01/08/2022 20:07, Krzysztof Kozlowski wrote:
>>> On 29/07/2022 10:18, Laurent Pinchart wrote:
>>>> Hi Sakari,
>>>>
>>>> (Adding Dave and Naush to the CC list)
>>>>
>>>> On Fri, Jul 29, 2022 at 10:07:36AM +0300, Sakari Ailus wrote:
>>>>> On Thu, Jul 28, 2022 at 03:13:11PM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 28/07/2022 15:02, Alexander Stein wrote:
>>>>>>> According to product brief they are identical from software point of view.
>>>>>>> Differences are a different chief ray angle (CRA) and the package.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
>>>>>>> Acked-by: Daniele Alessandrelli <daniele.alessandrelli@xxxxxxxxx>
>>>>>>> ---
>>>>>>>  drivers/media/i2c/ov9282.c | 1 +
>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
>>>>>>> index 8a252bf3b59f..c8d83a29f9bb 100644
>>>>>>> --- a/drivers/media/i2c/ov9282.c
>>>>>>> +++ b/drivers/media/i2c/ov9282.c
>>>>>>> @@ -1113,6 +1113,7 @@ static const struct dev_pm_ops ov9282_pm_ops = {
>>>>>>>  };
>>>>>>>  
>>>>>>>  static const struct of_device_id ov9282_of_match[] = {
>>>>>>> +	{ .compatible = "ovti,ov9281" },
>>>>>>
>>>>>> The devices seem entirely compatible, so why you add a new compatible
>>>>>> and not re-use existing?
>>>>>>
>>>>>> The difference in lens does not explain this.
>>>>>
>>>>> It is typically necessary to know what kind of related hardware can be
>>>>> found in the system, beyond just the device's register interface. Apart
>>>>> from USB cameras, less integrated cameras require low-level software
>>>>> control in which specific device properties are important. In this case it
>>>>> could be the lens shading table, among other things.
>>>>>
>>>>> 	https://www.ovt.com/sensor/ov9282/
>>>>>
>>>>> Therefore I think adding a specific compatible string for this one is
>>>>> justified.
>>>
>>> Specific compatible in binding is a requirement. No one discussed this.
>>> However not in the driver. None of the arguments above justify adding
>>> such binding, unless user-space depends on matching compatible, but not
>>> real compatible?
>>
>> Eh, now I used vague words. This should be instead:
>>
>> "However not in the driver. None of the arguments above justify adding
>> such compatible to driver, unless user-space depends on matching
>> compatible, but not real compatible?"
> 
> If I understand you right, you'd put the more specific model name as well
> as the more generic one to the compatible property and let the driver match
> against the more generic one?

Yes.

> 
> But in this case neither of these models is more generic than the other.

It's not a problem. Also the spec explains it similar way:
"They
 allow a device to express its compatibility with a family of similar
devices, potentially allowing a single
 device driver to match against several devices."

Of course the numbers would suggest that ov9281 should be the family (as
lower number usually means designed earlier), but it is a matter of
convention which here can be skipped. The point is that ov9281 and
ov9282 are compatible between each other, therefore they belong to
single family.

Best regards,
Krzysztof



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux