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