Re: [PATCH v2 4/6] media: uapi: Add mlx7502x header file

[Volodymyr Kharuk <vkh@melexis.com>]

On Fri, Jul 15, 2022 at 12:36:18PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> CC'ing Benjamin Mugnier who I recall expressed an interest in ToF
> sensors (if I recall incorrectly, my apologies).
> 
> On Fri, Jul 15, 2022 at 11:57:20AM +0300, Volodymyr Kharuk wrote:
> > On Thu, Jul 14, 2022 at 01:31:35PM +0300, Laurent Pinchart wrote:
> > > On Thu, Jul 14, 2022 at 11:34:46AM +0300, Volodymyr Kharuk wrote:
> > > > Define user controls for mlx7502x driver and update MAINTAINERS
> > > > 
> > > > Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
> > > > ---
> > > >  MAINTAINERS                   |  7 +++++++
> > > >  include/uapi/linux/mlx7502x.h | 31 +++++++++++++++++++++++++++++++
> > > >  2 files changed, 38 insertions(+)
> > > >  create mode 100644 include/uapi/linux/mlx7502x.h
> > > > 
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index ef3ec334fae9..1a68d888ee14 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -12673,6 +12673,13 @@ S:	Supported
> > > >  W:	http://www.melexis.com
> > > >  F:	drivers/iio/temperature/mlx90632.c
> > > >  
> > > > +MELEXIS MLX7502X DRIVER
> > > > +M:	Volodymyr Kharuk <vkh@melexis.com>
> > > > +L:	linux-media@vger.kernel.org
> > > > +S:	Supported
> > > > +W:	http://www.melexis.com
> > > > +F:	include/uapi/linux/mlx7502x.h
> > > > +
> > > >  MELFAS MIP4 TOUCHSCREEN DRIVER
> > > >  M:	Sangwon Jee <jeesw@melfas.com>
> > > >  S:	Supported
> > > > diff --git a/include/uapi/linux/mlx7502x.h b/include/uapi/linux/mlx7502x.h
> > > > new file mode 100644
> > > > index 000000000000..44386f3d6f5a
> > > > --- /dev/null
> > > > +++ b/include/uapi/linux/mlx7502x.h
> > > > @@ -0,0 +1,31 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +/*
> > > > + * Melexis 7502x ToF cameras driver.
> > > > + *
> > > > + * Copyright (C) 2021 Melexis N.V.
> > > > + *
> > > > + */
> > > > +
> > > > +#ifndef __UAPI_MLX7502X_H_
> > > > +#define __UAPI_MLX7502X_H_
> > > > +
> > > > +#include <linux/v4l2-controls.h>
> > > > +
> > > 
> > > These controls should be documented, in
> > > Documentation/userspace-api/media/drivers/.
> > 
> > Ok, will do in v3
> > 
> > > > +/* number of phases per frame: 1..8 */
> > > > +#define V4L2_CID_MLX7502X_PHASE_NUMBER  (V4L2_CID_USER_MLX7502X_BASE + 0)
> > > > +/* shift of each phase in frame, this is an array of 8 elements, each 16bits */
> > > > +#define V4L2_CID_MLX7502X_PHASE_SEQ	(V4L2_CID_USER_MLX7502X_BASE + 1)
> > > > +/* frequency modulation in MHz */
> > > > +#define V4L2_CID_MLX7502X_FMOD		(V4L2_CID_USER_MLX7502X_BASE + 2)
> > > > +/* time integration of each phase in us */
> > > > +#define V4L2_CID_MLX7502X_TINT		(V4L2_CID_USER_MLX7502X_BASE + 3)
> > > 
> > > Are these control very device-specific, or are they concept that apply
> > > in general to ToF sensors ? Same for V4L2_CID_MLX7502X_OUTPUT_MODE.
> > 
> > These controls(except V4L2_CID_MLX7502X_OUTPUT_MODE) are general for ToF
> > sensors. Do you think we should standardize them?
> 
> I would really really like to see control standardization for ToF
> sensors, yes :-)
Sounds great :)
> 
> Do you know of any public litterature that explains the operating
> principles of ToF sensors ? I don't expect most of the V4L2 developers
> to be familiar with the concept, so something that could bring us up to
> speed on ToF would be useful for the discussion.

Here what I have:
1. ToF Basics from Melexis
https://media.melexis.com/-/media/files/documents/application-notes/time-of-flight-basics-application-note-melexis.pdf
2. ToF Basics from TI
https://www.ti.com/lit/wp/sloa190b/sloa190b.pdf?ts=1657842732275&ref_url=https%253A%252F%252Fwww.google.com%252F
2. ToF systems from TI
https://www.ti.com/lit/ug/sbau219d/sbau219d.pdf
4. This more related to ToF algorithms
https://hal.inria.fr/hal-00725654/document

I hope it helps.
> 
> > Note that the control V4L2_CID_MLX7502X_TINT is similar to
> > V4L2_CID_EXPOSURE, but the way it is done in ToF is different. They don't
> > have a shutter. So I gave a separate control name. Is it ok?
> 
> Yes, I think that's fine.
> 
> > > > +/* mode could sw(sending i2c packet), hw(pin triggering), and continuous(self triggering) */
> > > > +#define V4L2_CID_MLX7502X_TRIGGER_MODE	(V4L2_CID_USER_MLX7502X_BASE + 4)
> > > > +/* in case sw or hw trigger mode is used */
> > > > +#define V4L2_CID_MLX7502X_TRIGGER	(V4L2_CID_USER_MLX7502X_BASE + 5)
> > > 
> > > Trigger control is likely something we need to standardize at the V4L2
> > > level.
> > 
> > Ok, then I'll remove these controls for now and I will back with this as
> > a separate patch.
> > 
> > > > +/* this is related to the taps in ToF cameras, usually A minus B is the best option */
> > > > +#define V4L2_CID_MLX7502X_OUTPUT_MODE	(V4L2_CID_USER_MLX7502X_BASE + 6)
> > > > +/* ToF camers has its own temperature sensor, which can be read out only during streaming */
> > > > +#define V4L2_CID_MLX7502X_TEMPERATURE	(V4L2_CID_USER_MLX7502X_BASE + 7)
> > > 
> > > This should probably use the proposed temperature control from
> > > https://lore.kernel.org/linux-media/20220415111845.27130-3-benjamin.mugnier@foss.st.com/
> > 
> > Ok, then I'll remove these controls for now.
> > 
> > > > +
> > > > +#endif /* __UAPI_MLX7502X_H_ */
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
--
Volodymyr Kharuk