Hello,
Am Dienstag, 2. August 2022, 10:30:40 CEST schrieb Krzysztof Kozlowski:
> 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
So what is the conclusion of this?
If using the "family" name there is no way for userspace to see the actual
device name rather than the driver name. This might be confusing, especially
of both ov9281 and ov9282 are attached to the same platform. The only
difference would be the i2c-bus-address.
You can also go for ov928x but this is not a real improvement.
Best regards,
Alexander
