Re: [PATCH v2 5/6] media: dt-bindings: media: i2c: Add mlx7502x camera sensor binding

[Volodymyr Kharuk <vkh@melexis.com>]

Hi Krzysztof,

On Thu, Jul 14, 2022 at 01:56:13PM +0200, Krzysztof Kozlowski wrote:
> On 14/07/2022 13:29, Laurent Pinchart wrote:
> > On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote:
> >> On 14/07/2022 13:12, Laurent Pinchart wrote:
> >>>>>>> One option would be to support the following three compatible values:
> >>>>>>>
> >>>>>>> 	compatible = "melexis,mlx75026", "melexis,mlx7502x";
> >>>>>>> 	compatible = "melexis,mlx75027", "melexis,mlx7502x";
> >>>>>>> 	compatible = "melexis,mlx7502x";
> >>>>>>>
> >>>>>>> The last one only would trigger autodetection. I'm still not sure how to
> >>>>>>> document that properly in bindings though.
> >>>>>>
> >>>>>> I missed that part of binding.
> >>>>>>
> >>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go.
> >>>>>
> >>>>> Really ? We've had fallback generic compatible strings since the
> >>>>> beginning.
> >>>>
> >>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were
> >>>> actually never explicitly allowed, they just slipped in to many
> >>>> bindings... We have several discussions on this on mailing list, so no
> >>>> real point to repeat the arguments.
> >>>>
> >>>> There is a difference between generic fallback. If the device follows
> >>>> clear specification and version, e.g. "foo-bar-v4", you can use it for
> >>>> generic compatible. This is more common in SoC components. Requirement -
> >>>> there is a clear mapping between versions and SoCs.
> >>>
> >>> I'm not sure to see a clear difference between the two concepts.
> >>
> >> The clear difference is that you have a versioned and re-usable hardware
> >> block plus clear mapping which version goes to which SoC. Version
> >> numbers usually start with 1, not with 75025. 75025 is a model name.
> > 
> > How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for
> > instance, where the version number isn't known and the SoC name is used
> > instead ? Is that acceptable ?
> 
> This is the second case I mentioned - family of devices where the family
> fallback is not allowed to be alone. You cannot use just "renesas,scif"
> in DTS.
> 
> > 
> > How should we deal with devices that have different models, where the
> > model is irrelevant to the kernel driver, but relevant to userspace ?
> > Imagine, for instance, a light sensor with 10 models than only differ by
> > the filter they use to tune the sensitivity to different light spectrums
> > ? They are all "compatible" from a software point of view, would the
> > driver need to list all 10 compatible strings ?
> 
> I don't understand that example, I mean, what's the problem here? If
> they are all compatible, you can use only one comaptible, e.g.
> melexis,mlx75026.
> 
> If you ever need to differentiate it for user-space, you add specific
> compatible for the model and you have:
> 
> melexis,mlx75027, melexis,mlx75026
> 
> If user-space needs dedicated compatibles - add them, no one here argues
> to not to use specific compatibles.
Thanks for explanation. Now I understand the device tree better and
the whole idea behind it. I'll remove wildcard and autodetect.
Instead I will use of_match_table only.
> 
> 
> >>> For cameras, we often deal with complex pipelines with multiple external
> >>> devices and multiple IP cores, with drivers that need to communicate
> >>> with each other to initialize the complete camera system. For instance,
> >>> each camera-related component in the system registers itself in a media
> >>> graph that can be queried from userspace and exposes information about
> >>> all devices, including their model. There's no power up of any device
> >>> when this query is being performed from userspace. It could possibly be
> >>> changed (and maybe it should, for reasons unrelated to this discussion),
> >>> but we're looking at pretty much a complete redesign of V4L2 and MC
> >>> then.
> >>
> >> Is then autodetection a real use case since you have to power up the
> >> sensor each time system boots and this violates privacy? Several I2C
> >> sensors do not care about this and they always do it on power up, so
> >> aren't we solving here something unimportant?
> > 
> > In a laptop or tablet with a camera sensor, you likely don't want
> > autodetection. In an industrial device, you don't care, and having the
> > ability to auto-detect the exact sensor model when booting saves cost in
> > the production chain as a single image can work across different models.
> 
> We talk about the case here, not generic. Do you want to have
> autodetection possible here or not?
> 
> Best regards,
> Krzysztof

-- 
--
Volodymyr Kharuk
Embedded Software Engineer
Melexis-Ukraine
Mykhaila Kotel'nykova St, 4, Kyiv
Mobile phone: +38 050 346 5527
www.melexis.com
---
The contents of this e-mail are CONFIDENTIAL AND PROPRIETARY. Please read
our disclaimer at http://www.melexis.com/mailpolicy