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

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

 



On Thu, Jul 14, 2022 at 12:35:52PM +0200, Krzysztof Kozlowski wrote:
> On 14/07/2022 12:06, Laurent Pinchart wrote:
> > Hi Volodymyr,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote:
> >> Add device tree binding of the mlx7502x and update MAINTAINERS
> >>
> >> Signed-off-by: Volodymyr Kharuk <vkh@xxxxxxxxxxx>
> >> ---
> >>  .../bindings/media/i2c/melexis,mlx7502x.yaml  | 146 ++++++++++++++++++
> >>  MAINTAINERS                                   |   1 +
> >>  2 files changed, 147 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
> >> new file mode 100644
> >> index 000000000000..4ac91f7a26b6
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
> >> @@ -0,0 +1,146 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Melexis ToF 7502x MIPI CSI-2 Sensor
> >> +
> >> +maintainers:
> >> +  - Volodymyr Kharuk <vkh@xxxxxxxxxxx>
> >> +
> >> +description: |-
> >> +  Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes,
> >> +  and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12.
> >> +  Sensor 75026 is QVGA, while 75027 is VGA sensor.
> >> +  If you use compatible = "melexis,mlx7502x", then autodetect will be called.
> > 
> > I'd move this last line as a description of the compatible property, but
> > I'm also not sure this should be mentioned in the DT bindings, as it's a
> > driver implementation detail. I'm actually not sure we should support it
> > with three different compatible values as proposed, as without this
> > documentation users will have a hard time figuring out what compatible
> > value to pick.
> > 
> > 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.

> Anyway what does this autodetection mean?

As far as I understand, it means that the driver will use a hardware
identification register to figure out if the sensor is a 75026 or 75027.
The upside is that one doesn't need to change the device tree when
swapping between those two sensors. The downside is that the sensor
needs to be powered up at probe time. Depending on the platform, one of
those two behaviours is preferred. Auto-detection is nice, but in
laptops or tablets (not a use case for this particular device, but the
problem applies to camera sensors in general), it would mean that the
privacy LED of the camera could be briefly lit at boot time due to the
sensor being powered on, which can worry users.

-- 
Regards,

Laurent Pinchart



[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