[PATCH v2 0/6] media: i2c: mlx7502x ToF camera support

[PATCH v2 0/6] media: i2c: mlx7502x ToF camera support

[Volodymyr Kharuk]

Hello,

This series adds support for the Melexis 75026 and 75027 Time of Flight
camera sensors, with DT bindings in patch 5/6 and a driver in patch 6/6.
Also it adds 1X12 greyscale format to xilinx MIPI receiver
in patch 1/6 and to xilinx video in patch 2/6.

v2:
- added external clock to the sensor
- added all regulators required by the sensor
- added posibility to choose sensor type in device tree
- added prefixes to all custom types in device tree and driver as well
- style fixes

Volodymyr Kharuk (6):
  media: xilinx: csi2rxss: Add 1X12 greyscale format
  media: xilinx: video: Add 1X12 greyscale format
  media: v4l: ctrls: Add user control base for mlx7502x
  media: uapi: Add mlx7502x header file
  media: dt-bindings: media: i2c: Add mlx7502x camera sensor binding
  media: i2c: Add driver for mlx7502x ToF sensor

 .../bindings/media/i2c/melexis,mlx7502x.yaml  |  146 ++
 MAINTAINERS                                   |    9 +
 drivers/media/i2c/Kconfig                     |   13 +
 drivers/media/i2c/Makefile                    |    1 +
 drivers/media/i2c/mlx7502x.c                  | 2020 +++++++++++++++++
 .../media/platform/xilinx/xilinx-csi2rxss.c   |    1 +
 drivers/media/platform/xilinx/xilinx-vip.c    |    2 +
 include/uapi/linux/mlx7502x.h                 |   31 +
 include/uapi/linux/v4l2-controls.h            |    6 +
 9 files changed, 2229 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
 create mode 100644 drivers/media/i2c/mlx7502x.c
 create mode 100644 include/uapi/linux/mlx7502x.h


base-commit: d8e8aa866ed8636fd6c1017c3d9453eab2922496
-- 
BR,
Volodymyr Kharuk

[PATCH v2 1/6] media: xilinx: csi2rxss: Add 1X12 greyscale format

[Volodymyr Kharuk]

Extend the csi2rxss with Y12_1X12 greyscale format

Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
---
 drivers/media/platform/xilinx/xilinx-csi2rxss.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
index 051c60cba1e0..67574244eb2b 100644
--- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c
+++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
@@ -188,6 +188,7 @@ static const u32 xcsi2dt_mbus_lut[][2] = {
 	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
 	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
 	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
+	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_Y12_1X12 },
 	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
 	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
 	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },
-- 
BR,
Volodymyr Kharuk

Re: [PATCH v2 1/6] media: xilinx: csi2rxss: Add 1X12 greyscale format

[Laurent Pinchart]

Hi Volodymyr,

Thank you for the patch.

On Thu, Jul 14, 2022 at 11:34:43AM +0300, Volodymyr Kharuk wrote:
> Extend the csi2rxss with Y12_1X12 greyscale format
> 
> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/xilinx/xilinx-csi2rxss.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> index 051c60cba1e0..67574244eb2b 100644
> --- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> @@ -188,6 +188,7 @@ static const u32 xcsi2dt_mbus_lut[][2] = {
>  	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
>  	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
>  	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
> +	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_Y12_1X12 },
>  	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
>  	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
>  	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/6] media: xilinx: video: Add 1X12 greyscale format

[Volodymyr Kharuk]

Extend the xilinx video driver with Y12_1X12 greyscale format

Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
---
 drivers/media/platform/xilinx/xilinx-vip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
index a0073122798f..5b214bf7f93a 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -40,6 +40,8 @@ static const struct xvip_video_format xvip_video_formats[] = {
 	  1, V4L2_PIX_FMT_SGBRG8 },
 	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
 	  1, V4L2_PIX_FMT_SBGGR8 },
+	{ XVIP_VF_MONO_SENSOR, 12, "mono", MEDIA_BUS_FMT_Y12_1X12,
+	  2, V4L2_PIX_FMT_Y12 },
 };
 
 /**
-- 
BR,
Volodymyr Kharuk

Re: [PATCH v2 2/6] media: xilinx: video: Add 1X12 greyscale format

[Laurent Pinchart]

Hi Volodymyr,

Thank you for the patch.

On Thu, Jul 14, 2022 at 11:34:44AM +0300, Volodymyr Kharuk wrote:
> Extend the xilinx video driver with Y12_1X12 greyscale format
> 
> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/xilinx/xilinx-vip.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
> index a0073122798f..5b214bf7f93a 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.c
> +++ b/drivers/media/platform/xilinx/xilinx-vip.c
> @@ -40,6 +40,8 @@ static const struct xvip_video_format xvip_video_formats[] = {
>  	  1, V4L2_PIX_FMT_SGBRG8 },
>  	{ XVIP_VF_MONO_SENSOR, 8, "bggr", MEDIA_BUS_FMT_SBGGR8_1X8,
>  	  1, V4L2_PIX_FMT_SBGGR8 },
> +	{ XVIP_VF_MONO_SENSOR, 12, "mono", MEDIA_BUS_FMT_Y12_1X12,
> +	  2, V4L2_PIX_FMT_Y12 },
>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/6] media: v4l: ctrls: Add user control base for mlx7502x

[Volodymyr Kharuk]

Add a control base for mlx7502x and reserve 16 controls.
ToF sensor requires a bunch of new user controls.

Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
---
 include/uapi/linux/v4l2-controls.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index dfff69ed88f7..8f68d5b53430 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -225,6 +225,12 @@ enum v4l2_colorfx {
  */
 #define V4L2_CID_USER_ISL7998X_BASE		(V4L2_CID_USER_BASE + 0x1180)
 
+/*
+ * The base for Melexis ToF 7502x driver controls.
+ * We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_USER_MLX7502X_BASE		(V4L2_CID_USER_BASE + 0x1190)
+
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
  * and the 'MPEG' part of the define is historical */
-- 
BR,
Volodymyr Kharuk

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

[Volodymyr Kharuk]

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>
+
+/* 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)
+/* 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)
+/* 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)
+
+#endif /* __UAPI_MLX7502X_H_ */
-- 
BR,
Volodymyr Kharuk

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

[Laurent Pinchart]

Hi Volodymyr,

Thank you for the patch.

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/.

> +/* 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.

> +/* 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.

> +/* 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/

> +
> +#endif /* __UAPI_MLX7502X_H_ */

-- 
Regards,

Laurent Pinchart

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

[Volodymyr Kharuk]

Hi Laurent,

Thank you for your review.

On Thu, Jul 14, 2022 at 01:31:35PM +0300, Laurent Pinchart wrote:
> Hi Volodymyr,
> 
> Thank you for the patch.
> 
> 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?

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?
> 
> > +/* 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

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

[Laurent Pinchart]

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 :-)

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.

> 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

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

[Volodymyr Kharuk]

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

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

[Benjamin Mugnier]

Hi Volodymyr,


On 15/07/2022 17:03, Volodymyr Kharuk wrote:
> 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).

I am indeed very interested. Thank you :)

>>
>> 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 :)

Thanks a lot for your efforts in standardizing these controls. This is pretty close to what I expected :)

Sensors may require multiple fmod from the user, and may not be able to deduce them from a single one.
Subframes may be acquired for each fmod (composed themselves of acquisitions for each phase), and then generate a frame from these.
Here is a quick drawing example with 2 fmod and 2 phases. Hope this makes sense.

|-------------------------------------------------------------------------> time
|FMOD1 PHASE1|FMOD1 PHASE2|FMOD2 PHASE1|FMOD2 PHASE2|FMOD1 PHASE1|...
|         SUBFRAME1       |         SUBFRAME2       |
|                       FRAME1                      |

This allows greater ranges.
I suggest changing V4L2_CID_MLX7502X_FMOD to an array, if it suits you.
I'm curious how are you doing this? Are you using only one fmod or do you compute some others from the first one? Either in the driver or the sensor.

>>
>> 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.
>>

Having only one integration time control is problematic for HDR sensors as they require both a short and long integration time setting.
I have the same issue for the vgxy61 camera with V4L2_CID_EXPOSURE and ended up defining 2 custom controls for both short and long exposure, but I understand this is not ideal. Maybe Laurent have an idea on this?

>>>>> +/* 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.
>>>

We discussed the standardization of the temperature control with linux-hwmon subsystem team [1].
As this happened to be a trickier problem than I thought, I decided to remove the temperature control I initially proposed. You can find the v3 of the vgxy61 without the temperature control [2].

So no temperature control for now. I plan on giving it another go after the vgxy61 is accepted, but for now the simpler the better.
Of course feel free to do it, I'll gratefully rebase on your work ;)

[1] https://lore.kernel.org/linux-media/d4c868d5ef05f338bdc2237d9b9304077d268c8b.camel@ndufresne.ca/
[2] https://lore.kernel.org/all/20220512074037.3829926-1-benjamin.mugnier@foss.st.com/

>>>>> +
>>>>> +#endif /* __UAPI_MLX7502X_H_ */
>>
>> -- 
>> Regards,
>>
>> Laurent Pinchart
> 

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

[Dave Stevenson]

On Tue, 19 Jul 2022 at 16:21, Benjamin Mugnier
<benjamin.mugnier@foss.st.com> wrote:
>
> Hi Volodymyr,
>
>
> On 15/07/2022 17:03, Volodymyr Kharuk wrote:
> > 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).
>
> I am indeed very interested. Thank you :)
>
> >>
> >> 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 :)
>
> Thanks a lot for your efforts in standardizing these controls. This is pretty close to what I expected :)
>
> Sensors may require multiple fmod from the user, and may not be able to deduce them from a single one.
> Subframes may be acquired for each fmod (composed themselves of acquisitions for each phase), and then generate a frame from these.
> Here is a quick drawing example with 2 fmod and 2 phases. Hope this makes sense.
>
> |-------------------------------------------------------------------------> time
> |FMOD1 PHASE1|FMOD1 PHASE2|FMOD2 PHASE1|FMOD2 PHASE2|FMOD1 PHASE1|...
> |         SUBFRAME1       |         SUBFRAME2       |
> |                       FRAME1                      |
>
> This allows greater ranges.
> I suggest changing V4L2_CID_MLX7502X_FMOD to an array, if it suits you.
> I'm curious how are you doing this? Are you using only one fmod or do you compute some others from the first one? Either in the driver or the sensor.
>
> >>
> >> 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.
> >>
>
> Having only one integration time control is problematic for HDR sensors as they require both a short and long integration time setting.
> I have the same issue for the vgxy61 camera with V4L2_CID_EXPOSURE and ended up defining 2 custom controls for both short and long exposure, but I understand this is not ideal. Maybe Laurent have an idea on this?
>
> >>>>> +/* 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.
> >>>
>
> We discussed the standardization of the temperature control with linux-hwmon subsystem team [1].
> As this happened to be a trickier problem than I thought, I decided to remove the temperature control I initially proposed. You can find the v3 of the vgxy61 without the temperature control [2].
>
> So no temperature control for now. I plan on giving it another go after the vgxy61 is accepted, but for now the simpler the better.
> Of course feel free to do it, I'll gratefully rebase on your work ;)

I've thrown it into the ring as one of a number of image sensor
related subjects to discuss at the Media Summit alongside ELCE in
September[1]. Whether it makes the agenda or we get a decision there
or not are different questions.

An option that may be available is reading it from embedded metadata
(if we had a standardised way of exporting that, which is dependent on
the multiplexed streams patchset).
Certainly several of the Sony image sensors that we're using drop the
temperature registers in the embedded data, so we can lift it out in
libcamera without needing kernel support beyond enabling it.

  Dave

[1] https://lore.kernel.org/linux-media/CAPY8ntCwoLys1uwpoy3AW=wbBZod5cxj==z0XjUrBxK=0cwr8g@mail.gmail.com/

> [1] https://lore.kernel.org/linux-media/d4c868d5ef05f338bdc2237d9b9304077d268c8b.camel@ndufresne.ca/
> [2] https://lore.kernel.org/all/20220512074037.3829926-1-benjamin.mugnier@foss.st.com/
>
> >>>>> +
> >>>>> +#endif /* __UAPI_MLX7502X_H_ */
> >>
> >> --
> >> Regards,
> >>
> >> Laurent Pinchart
> >

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

[Volodymyr Kharuk]

Hi Benjamin,

On Tue, Jul 19, 2022 at 05:20:40PM +0200, Benjamin Mugnier wrote:
> Hi Volodymyr,
> 
> 
> On 15/07/2022 17:03, Volodymyr Kharuk wrote:
> > 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).
> 
> I am indeed very interested. Thank you :)
> 
> >>
> >> 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 :)
> 
> Thanks a lot for your efforts in standardizing these controls. This is pretty close to what I expected :)
> 
> Sensors may require multiple fmod from the user, and may not be able to deduce them from a single one.
> Subframes may be acquired for each fmod (composed themselves of acquisitions for each phase), and then generate a frame from these.
> Here is a quick drawing example with 2 fmod and 2 phases. Hope this makes sense.
> 
> |-------------------------------------------------------------------------> time
> |FMOD1 PHASE1|FMOD1 PHASE2|FMOD2 PHASE1|FMOD2 PHASE2|FMOD1 PHASE1|...
> |         SUBFRAME1       |         SUBFRAME2       |
> |                       FRAME1                      |
Just to be clear: one camera trigger generates one frame.
The number of subframes can be configured via register.
One phase generates one image. Do we have the same understanding?
> 
> This allows greater ranges.
> I suggest changing V4L2_CID_MLX7502X_FMOD to an array, if it suits you.
> I'm curious how are you doing this? Are you using only one fmod or do you compute some others from the first one? Either in the driver or the sensor.
The mlx7502x supports only one fmod per frame. Instead it is possible to use
a few phase shifts(V4L2_CID_MLX7502X_PHASE_SEQ) per one frame. Using
cross-correlation it is possible to calculate the distance from received images.
> 
> >>
> >> 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.
> >>
> 
> Having only one integration time control is problematic for HDR sensors as they require both a short and long integration time setting.
> I have the same issue for the vgxy61 camera with V4L2_CID_EXPOSURE and ended up defining 2 custom controls for both short and long exposure, but I understand this is not ideal. Maybe Laurent have an idea on this?
It is possible to have integration time per phase. For now I just copy tint into all phase registers.
> 
> >>>>> +/* 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.
> >>>
> 
> We discussed the standardization of the temperature control with linux-hwmon subsystem team [1].
> As this happened to be a trickier problem than I thought, I decided to remove the temperature control I initially proposed. You can find the v3 of the vgxy61 without the temperature control [2].
> 
> So no temperature control for now. I plan on giving it another go after the vgxy61 is accepted, but for now the simpler the better.
> Of course feel free to do it, I'll gratefully rebase on your work ;)
> 
> [1] https://lore.kernel.org/linux-media/d4c868d5ef05f338bdc2237d9b9304077d268c8b.camel@ndufresne.ca/
> [2] https://lore.kernel.org/all/20220512074037.3829926-1-benjamin.mugnier@foss.st.com/
> 
> >>>>> +
> >>>>> +#endif /* __UAPI_MLX7502X_H_ */
> >>
> >> -- 
> >> Regards,
> >>
> >> Laurent Pinchart
> > 

-- 
--
BR,
Volodymyr Kharuk

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

[Benjamin Mugnier]

Hi Volodymyr,

On 20/07/2022 16:44, Volodymyr Kharuk wrote:
> Hi Benjamin,
> 
> On Tue, Jul 19, 2022 at 05:20:40PM +0200, Benjamin Mugnier wrote:
>> Hi Volodymyr,
>>
>>
>> On 15/07/2022 17:03, Volodymyr Kharuk wrote:
>>> 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).
>>
>> I am indeed very interested. Thank you :)
>>
>>>>
>>>> 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 :)
>>
>> Thanks a lot for your efforts in standardizing these controls. This is pretty close to what I expected :)
>>
>> Sensors may require multiple fmod from the user, and may not be able to deduce them from a single one.
>> Subframes may be acquired for each fmod (composed themselves of acquisitions for each phase), and then generate a frame from these.
>> Here is a quick drawing example with 2 fmod and 2 phases. Hope this makes sense.
>>
>> |-------------------------------------------------------------------------> time
>> |FMOD1 PHASE1|FMOD1 PHASE2|FMOD2 PHASE1|FMOD2 PHASE2|FMOD1 PHASE1|...
>> |         SUBFRAME1       |         SUBFRAME2       |
>> |                       FRAME1                      |
> Just to be clear: one camera trigger generates one frame.
> The number of subframes can be configured via register.

Yes exactly. It's mostly vocabulary differences afaik, but we are on the same 

> One phase generates one image. Do we have the same understanding?

Yes, one frame on the MIPI is one phase. The reconstitution is done later on.
But some sensors tend to put their "start of frame" and "end of frame" markers for the concatenation of subframes (the frame on my drawing), and not for each phase. I'm not really familiar with this yet.
This is used example if you want to plug your sensor in a software stack that can't handle more than a limited number of fps. You trick it into getting one big frame and then slice after. This is afaik the same mechanism as used for high fps cameras. I don't really know how to handle this kind of usage yet.

>>
>> This allows greater ranges.
>> I suggest changing V4L2_CID_MLX7502X_FMOD to an array, if it suits you.
>> I'm curious how are you doing this? Are you using only one fmod or do you compute some others from the first one? Either in the driver or the sensor.
> The mlx7502x supports only one fmod per frame. Instead it is possible to use
> a few phase shifts(V4L2_CID_MLX7502X_PHASE_SEQ) per one frame. Using
> cross-correlation it is possible to calculate the distance from received images.

Alright. In my opinion if we standardize this control we could make it an array from the start to prepare for other sensors using multiple fmod.

>>
>>>>
>>>> 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.
>>>>
>>
>> Having only one integration time control is problematic for HDR sensors as they require both a short and long integration time setting.
>> I have the same issue for the vgxy61 camera with V4L2_CID_EXPOSURE and ended up defining 2 custom controls for both short and long exposure, but I understand this is not ideal. Maybe Laurent have an idea on this?
> It is possible to have integration time per phase. For now I just copy tint into all phase registers.

Yes I see little to no use cases for variable integration time per phase. So your control is fine.

For HDR it's kind of special, as every frame is acquired is in fact 2 frames: one in HDR and one in SDR, and they require 2 different integration time.
As I said the problem already exists for camera sensors, so this is fine as is for now :)

>>
>>>>>>> +/* 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.
>>>>>
>>
>> We discussed the standardization of the temperature control with linux-hwmon subsystem team [1].
>> As this happened to be a trickier problem than I thought, I decided to remove the temperature control I initially proposed. You can find the v3 of the vgxy61 without the temperature control [2].
>>
>> So no temperature control for now. I plan on giving it another go after the vgxy61 is accepted, but for now the simpler the better.
>> Of course feel free to do it, I'll gratefully rebase on your work ;)
>>
>> [1] https://lore.kernel.org/linux-media/d4c868d5ef05f338bdc2237d9b9304077d268c8b.camel@ndufresne.ca/
>> [2] https://lore.kernel.org/all/20220512074037.3829926-1-benjamin.mugnier@foss.st.com/
>>
>>>>>>> +
>>>>>>> +#endif /* __UAPI_MLX7502X_H_ */
>>>>
>>>> -- 
>>>> Regards,
>>>>
>>>> Laurent Pinchart
>>>
> 

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

[Volodymyr Kharuk]

Add device tree binding of the mlx7502x and update MAINTAINERS

Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
---
 .../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@melexis.com>
+
+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.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - melexis,mlx7502x
+          - melexis,mlx75026
+          - melexis,mlx75027
+
+  reg:
+    maxItems: 1
+
+  assigned-clocks: true
+  assigned-clock-parents: true
+  assigned-clock-rates: true
+
+  clocks:
+    description: Clock frequency 8MHz
+    maxItems: 1
+
+  vdda-supply:
+    description:
+      Definition of the regulator used as analog power supply(2.7V).
+
+  vddif-supply:
+    description:
+      Definition of the regulator used as interface power supply(1.8V).
+
+  vddd-supply:
+    description:
+      Definition of the regulator used as digital power supply(1.2V).
+
+  vdmix-supply:
+    description:
+      Definition of the regulator used as mixed driver power supply(1.2V).
+
+  reset-gpios:
+    maxItems: 1
+    description: Reset Sensor GPIO Control (active low)
+
+  melexis,trig-gpios:
+    maxItems: 1
+    description:
+      Hardware Trigger GPIO Control (active low)
+      When the hardware trigger mode is enabled, this GPIO is used to generate
+      a low level impulse for about 100us. The impulse triggers a new
+      measurement cycle.
+
+  melexis,leden-gpios:
+    maxItems: 1
+    description:
+      Led driver enable GPIO Control (active high)
+      This GPIO notifies the illumination driver when it is safe to use led
+      signals from the sensor.
+
+  port:
+    description: MIPI CSI-2 transmitter port
+    $ref: /schemas/graph.yaml#/$defs/port-base
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            oneOf:
+              - items:
+                  - const: 1
+                  - const: 2
+              - items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+
+          clock-noncontinuous: true
+          link-frequencies: true
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mlx7502x: camera@57 {
+            compatible = "melexis,mlx7502x";
+            reg = <0x57>;
+            clocks = <&mlx7502x_clk>;
+
+            assigned-clocks = <&mlx7502x_clk>;
+            assigned-clock-parents = <&mlx7502x_clk_parent>;
+            assigned-clock-rates = <8000000>;
+
+            vddd-supply = <&mlx_7502x_reg>;
+
+            reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>;
+            melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>;
+            melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>;
+
+            port {
+                mlx7502x_out_mipi_csi2: endpoint {
+                    remote-endpoint = <&mipi_csi2_from_mlx7502x>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 < 960000000
+                                                   904000000
+                                                   800000000
+                                                   704000000
+                                                   600000000
+                                                   300000000 >;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a68d888ee14..b00a726bb3db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12678,6 +12678,7 @@ M:	Volodymyr Kharuk <vkh@melexis.com>
 L:	linux-media@vger.kernel.org
 S:	Supported
 W:	http://www.melexis.com
+F:	Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
 F:	include/uapi/linux/mlx7502x.h
 
 MELFAS MIP4 TOUCHSCREEN DRIVER
-- 
BR,
Volodymyr Kharuk

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

[Krzysztof Kozlowski]

On 14/07/2022 10:34, Volodymyr Kharuk wrote:
> Add device tree binding of the mlx7502x and update MAINTAINERS
> 
> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>

Wrong subject prefix. Remove trailing "binding".

> ---
>  .../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@melexis.com>
> +
> +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.
> +
> +properties:
> +  compatible:
> +    items:

No items. You have just one item.

> +      - enum:
> +          - melexis,mlx7502x
> +          - melexis,mlx75026
> +          - melexis,mlx75027
> +
> +  reg:
> +    maxItems: 1
> +
> +  assigned-clocks: true
> +  assigned-clock-parents: true
> +  assigned-clock-rates: true

These are not needed.

> +
> +  clocks:
> +    description: Clock frequency 8MHz
> +    maxItems: 1
> +
> +  vdda-supply:
> +    description:
> +      Definition of the regulator used as analog power supply(2.7V).

s/Definition of the regulator used as //
(redundant)

> +
> +  vddif-supply:
> +    description:
> +      Definition of the regulator used as interface power supply(1.8V).

s/Definition of the regulator used as //

> +
> +  vddd-supply:
> +    description:
> +      Definition of the regulator used as digital power supply(1.2V).

s/Definition of the regulator used as //

> +
> +  vdmix-supply:
> +    description:
> +      Definition of the regulator used as mixed driver power supply(1.2V).

s/Definition of the regulator used as //

> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Reset Sensor GPIO Control (active low)
> +
> +  melexis,trig-gpios:
> +    maxItems: 1
> +    description:
> +      Hardware Trigger GPIO Control (active low)
> +      When the hardware trigger mode is enabled, this GPIO is used to generate
> +      a low level impulse for about 100us. The impulse triggers a new
> +      measurement cycle.
> +
> +  melexis,leden-gpios:
> +    maxItems: 1
> +    description:
> +      Led driver enable GPIO Control (active high)
> +      This GPIO notifies the illumination driver when it is safe to use led
> +      signals from the sensor.
> +
> +  port:
> +    description: MIPI CSI-2 transmitter port
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            oneOf:
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +                  - const: 3
> +                  - const: 4
> +
> +          clock-noncontinuous: true
> +          link-frequencies: true

You do not need these two, they come from video-interfaces.yaml

> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - port
> +



Best regards,
Krzysztof

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

[Laurent Pinchart]

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@melexis.com>
> ---
>  .../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@melexis.com>
> +
> +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.

> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - melexis,mlx7502x
> +          - melexis,mlx75026
> +          - melexis,mlx75027
> +
> +  reg:
> +    maxItems: 1
> +
> +  assigned-clocks: true
> +  assigned-clock-parents: true
> +  assigned-clock-rates: true
> +
> +  clocks:
> +    description: Clock frequency 8MHz
> +    maxItems: 1
> +
> +  vdda-supply:
> +    description:
> +      Definition of the regulator used as analog power supply(2.7V).
> +
> +  vddif-supply:
> +    description:
> +      Definition of the regulator used as interface power supply(1.8V).
> +
> +  vddd-supply:
> +    description:
> +      Definition of the regulator used as digital power supply(1.2V).
> +
> +  vdmix-supply:
> +    description:
> +      Definition of the regulator used as mixed driver power supply(1.2V).
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Reset Sensor GPIO Control (active low)
> +
> +  melexis,trig-gpios:
> +    maxItems: 1
> +    description:
> +      Hardware Trigger GPIO Control (active low)
> +      When the hardware trigger mode is enabled, this GPIO is used to generate
> +      a low level impulse for about 100us. The impulse triggers a new
> +      measurement cycle.
> +
> +  melexis,leden-gpios:
> +    maxItems: 1
> +    description:
> +      Led driver enable GPIO Control (active high)
> +      This GPIO notifies the illumination driver when it is safe to use led
> +      signals from the sensor.

As far as I understand this isn't an input of the sensor, but a signal
that is driven by the driver and controls the separate illuminator. It
shouldn't be specified in this binding, as it's not a property of the
sensor. You should instead have a DT node for the illumination driver.

> +  port:
> +    description: MIPI CSI-2 transmitter port
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            oneOf:
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +                  - const: 3
> +                  - const: 4
> +
> +          clock-noncontinuous: true
> +          link-frequencies: true
> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - port

Let's make the supplies mandatory too, as the chip can't operate without
them.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mlx7502x: camera@57 {
> +            compatible = "melexis,mlx7502x";
> +            reg = <0x57>;
> +            clocks = <&mlx7502x_clk>;
> +
> +            assigned-clocks = <&mlx7502x_clk>;
> +            assigned-clock-parents = <&mlx7502x_clk_parent>;
> +            assigned-clock-rates = <8000000>;
> +
> +            vddd-supply = <&mlx_7502x_reg>;
> +
> +            reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>;
> +            melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>;
> +            melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>;
> +
> +            port {
> +                mlx7502x_out_mipi_csi2: endpoint {
> +                    remote-endpoint = <&mipi_csi2_from_mlx7502x>;
> +                    data-lanes = <1 2 3 4>;
> +                    link-frequencies = /bits/ 64 < 960000000
> +                                                   904000000
> +                                                   800000000
> +                                                   704000000
> +                                                   600000000
> +                                                   300000000 >;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1a68d888ee14..b00a726bb3db 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12678,6 +12678,7 @@ M:	Volodymyr Kharuk <vkh@melexis.com>
>  L:	linux-media@vger.kernel.org
>  S:	Supported
>  W:	http://www.melexis.com
> +F:	Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
>  F:	include/uapi/linux/mlx7502x.h
>  
>  MELFAS MIP4 TOUCHSCREEN DRIVER

-- 
Regards,

Laurent Pinchart

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

[Krzysztof Kozlowski]

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@melexis.com>
>> ---
>>  .../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@melexis.com>
>> +
>> +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.

Anyway what does this autodetection mean?

Best regards,
Krzysztof

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

[Laurent Pinchart]

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@melexis.com>
> >> ---
> >>  .../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@melexis.com>
> >> +
> >> +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

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

[Krzysztof Kozlowski]

On 14/07/2022 12:45, Laurent Pinchart wrote:
> 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@melexis.com>
>>>> ---
>>>>  .../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@melexis.com>
>>>> +
>>>> +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.

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.

> 
>> 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.

Then there is no need to define 75027 compatible. DT is for cases where
autodetection does not work...

> 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.

OK, that's reasonable argument for dedicated compatible but I don't
understand why you cannot perform autodetection the moment device is
actually powered up (first time). I understand it is nice and easy to
make everything in the probe and most devices perform it that way. But
if you don't want to do it in the probe - DT is not a workaround for this...

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 14/07/2022 13:00, Krzysztof Kozlowski wrote:
> On 14/07/2022 12:45, Laurent Pinchart wrote:
>> 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@melexis.com>
>>>>> ---
>>>>>  .../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@melexis.com>
>>>>> +
>>>>> +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.
> 
> 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.

Although I forgot one more acceptable case - family of devices followed
by a specific compatible. However that "family" cannot be on its own.


Best regards,
Krzysztof

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

[Laurent Pinchart]

On Thu, Jul 14, 2022 at 01:00:24PM +0200, Krzysztof Kozlowski wrote:
> On 14/07/2022 12:45, Laurent Pinchart wrote:
> > 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@melexis.com>
> >>>> ---
> >>>>  .../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@melexis.com>
> >>>> +
> >>>> +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.
> 
> 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.

> >> 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.
> 
> Then there is no need to define 75027 compatible. DT is for cases where
> autodetection does not work...

It's autodetection of the exact device model, those are I2C devices so
we still need DT, and we still need to know that it's one of the
MLX75026 or MLX75027.

> > 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.
> 
> OK, that's reasonable argument for dedicated compatible but I don't
> understand why you cannot perform autodetection the moment device is
> actually powered up (first time). I understand it is nice and easy to
> make everything in the probe and most devices perform it that way. But
> if you don't want to do it in the probe - DT is not a workaround for this...

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.

-- 
Regards,

Laurent Pinchart

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

[Krzysztof Kozlowski]

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.

>>>> 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.
>>
>> Then there is no need to define 75027 compatible. DT is for cases where
>> autodetection does not work...
> 
> It's autodetection of the exact device model, those are I2C devices so
> we still need DT, and we still need to know that it's one of the
> MLX75026 or MLX75027.
> 
>>> 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.
>>
>> OK, that's reasonable argument for dedicated compatible but I don't
>> understand why you cannot perform autodetection the moment device is
>> actually powered up (first time). I understand it is nice and easy to
>> make everything in the probe and most devices perform it that way. But
>> if you don't want to do it in the probe - DT is not a workaround for this...
> 
> 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?

Best regards,
Krzysztof

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

[Laurent Pinchart]

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 ?

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 ?

> >>>> 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.
> >>
> >> Then there is no need to define 75027 compatible. DT is for cases where
> >> autodetection does not work...
> > 
> > It's autodetection of the exact device model, those are I2C devices so
> > we still need DT, and we still need to know that it's one of the
> > MLX75026 or MLX75027.
> > 
> >>> 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.
> >>
> >> OK, that's reasonable argument for dedicated compatible but I don't
> >> understand why you cannot perform autodetection the moment device is
> >> actually powered up (first time). I understand it is nice and easy to
> >> make everything in the probe and most devices perform it that way. But
> >> if you don't want to do it in the probe - DT is not a workaround for this...
> > 
> > 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.

-- 
Regards,

Laurent Pinchart

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

[Krzysztof Kozlowski]

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.


>>> 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

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

[Volodymyr Kharuk]

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

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

[Laurent Pinchart]

Hi Krzysztof,

Very late reply, this had fallen through the cracks.

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.

OK. Does this mean you are fine with

	compatible = "melexis,mlx75026", "melexis,mlx7502x";
	compatible = "melexis,mlx75027", "melexis,mlx7502x";

where "melexis,mlx7502x" is considered to be the family fallback, but
not

	compatible = "melexis,mlx7502x";

alone ?

> > 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.

OK.

> >>> 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?

I'd like to support auto-detection, but not make it mandatory. Assuming
a family of chips supported by one driver with hardware that makes
auto-detection possible, I have use cases where I specifically don't
want auto-detection as it would have undesirable side effects at probe
time, and other use cases where I want auto-detection as it lowers the
costs in the production chain. I thus need to be able to specify, in DT,
whether to use auto-detection or not, and when not using auto-detection,
specify the exact chip model.

-- 
Regards,

Laurent Pinchart

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

[Krzysztof Kozlowski]

On 06/02/2023 11:45, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> Very late reply, this had fallen through the cracks.
> 
> 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.
> 
> OK. Does this mean you are fine with
> 
> 	compatible = "melexis,mlx75026", "melexis,mlx7502x";
> 	compatible = "melexis,mlx75027", "melexis,mlx7502x";
> 
> where "melexis,mlx7502x" is considered to be the family fallback, but
> not
> 
> 	compatible = "melexis,mlx7502x";
> 
> alone ?

Correct.

(...)

>>>
>>> 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?
> 
> I'd like to support auto-detection, but not make it mandatory. Assuming
> a family of chips supported by one driver with hardware that makes
> auto-detection possible, I have use cases where I specifically don't
> want auto-detection as it would have undesirable side effects at probe
> time, and other use cases where I want auto-detection as it lowers the
> costs in the production chain. I thus need to be able to specify, in DT,
> whether to use auto-detection or not, and when not using auto-detection,
> specify the exact chip model.

OK, I understand. This however stretches the Devicetree approach - you
are putting OS policy of device probing into the DT binding. What's
more, it serves only Linux' purpose. If other OS/software is fine with
auto-detection on first use (thus no privacy concerns), then all this
discussion is not relevant. The binding is independent of OS, thus we
should not align it to our specific OS behavior.

Maybe Linux needs some generic runtime knob to turn on/off autodetection
for all devices. It does not look like a job for DT.

Best regards,
Krzysztof

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

[Laurent Pinchart]

Hi Krzysztof,

On Mon, Feb 06, 2023 at 07:20:21PM +0100, Krzysztof Kozlowski wrote:
> On 06/02/2023 11:45, Laurent Pinchart wrote:
> > 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.
> > 
> > OK. Does this mean you are fine with
> > 
> > 	compatible = "melexis,mlx75026", "melexis,mlx7502x";
> > 	compatible = "melexis,mlx75027", "melexis,mlx7502x";
> > 
> > where "melexis,mlx7502x" is considered to be the family fallback, but
> > not
> > 
> > 	compatible = "melexis,mlx7502x";
> > 
> > alone ?
> 
> Correct.
> 
> (...)
> 
> >>>
> >>> 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?
> > 
> > I'd like to support auto-detection, but not make it mandatory. Assuming
> > a family of chips supported by one driver with hardware that makes
> > auto-detection possible, I have use cases where I specifically don't
> > want auto-detection as it would have undesirable side effects at probe
> > time, and other use cases where I want auto-detection as it lowers the
> > costs in the production chain. I thus need to be able to specify, in DT,
> > whether to use auto-detection or not, and when not using auto-detection,
> > specify the exact chip model.
> 
> OK, I understand. This however stretches the Devicetree approach - you
> are putting OS policy of device probing into the DT binding. What's
> more, it serves only Linux' purpose. If other OS/software is fine with
> auto-detection on first use (thus no privacy concerns), then all this
> discussion is not relevant. The binding is independent of OS, thus we
> should not align it to our specific OS behavior.
> 
> Maybe Linux needs some generic runtime knob to turn on/off autodetection
> for all devices. It does not look like a job for DT.

If we want to be able to run without auto-detection, regardless of the
operating system, we need to specify the exact model in DT, otherwise a
driver wouldn't be able to identify the device.

If we want to use auto-detection, that's for the purpose of simplifying
system integration, with a single DT that covers multiple device
variants.

Those two use cases thus require DT binding that allow for both options,
specifying an exact model, or being more generic. I agree that deciding
to auto-detect based on what compatible strings are specified may be
specific to a particular Linux driver, but the fact that we need the two
options to support both use cases isn't OS-specific in my opinion.

-- 
Regards,

Laurent Pinchart

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

[Volodymyr Kharuk]

Hi Laurent,

Thank you for your review,

On Thu, Jul 14, 2022 at 01:06:35PM +0300, 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@melexis.com>
> > ---
> >  .../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@melexis.com>
> > +
> > +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.
> 
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - melexis,mlx7502x
> > +          - melexis,mlx75026
> > +          - melexis,mlx75027
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  assigned-clocks: true
> > +  assigned-clock-parents: true
> > +  assigned-clock-rates: true
> > +
> > +  clocks:
> > +    description: Clock frequency 8MHz
> > +    maxItems: 1
> > +
> > +  vdda-supply:
> > +    description:
> > +      Definition of the regulator used as analog power supply(2.7V).
> > +
> > +  vddif-supply:
> > +    description:
> > +      Definition of the regulator used as interface power supply(1.8V).
> > +
> > +  vddd-supply:
> > +    description:
> > +      Definition of the regulator used as digital power supply(1.2V).
> > +
> > +  vdmix-supply:
> > +    description:
> > +      Definition of the regulator used as mixed driver power supply(1.2V).
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: Reset Sensor GPIO Control (active low)
> > +
> > +  melexis,trig-gpios:
> > +    maxItems: 1
> > +    description:
> > +      Hardware Trigger GPIO Control (active low)
> > +      When the hardware trigger mode is enabled, this GPIO is used to generate
> > +      a low level impulse for about 100us. The impulse triggers a new
> > +      measurement cycle.
> > +
> > +  melexis,leden-gpios:
> > +    maxItems: 1
> > +    description:
> > +      Led driver enable GPIO Control (active high)
> > +      This GPIO notifies the illumination driver when it is safe to use led
> > +      signals from the sensor.
> 
> As far as I understand this isn't an input of the sensor, but a signal
> that is driven by the driver and controls the separate illuminator. It
> shouldn't be specified in this binding, as it's not a property of the
> sensor. You should instead have a DT node for the illumination driver.
Yes, you are right. There are illuminators, which are not ready to accept
the signal from the sensor, if pin levels are not right. So I need to
notify illuminator somehow.  Can the illumination driver be as a V4L2
subdevice? Then I can notify the illuminator via s_stream. In another
case it can damage the illuminator. What do you think?
> 
> > +  port:
> > +    description: MIPI CSI-2 transmitter port
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          data-lanes:
> > +            oneOf:
> > +              - items:
> > +                  - const: 1
> > +                  - const: 2
> > +              - items:
> > +                  - const: 1
> > +                  - const: 2
> > +                  - const: 3
> > +                  - const: 4
> > +
> > +          clock-noncontinuous: true
> > +          link-frequencies: true
> > +
> > +        required:
> > +          - data-lanes
> > +          - link-frequencies
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - port
> 
> Let's make the supplies mandatory too, as the chip can't operate without
> them.
Ok
> 
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        mlx7502x: camera@57 {
> > +            compatible = "melexis,mlx7502x";
> > +            reg = <0x57>;
> > +            clocks = <&mlx7502x_clk>;
> > +
> > +            assigned-clocks = <&mlx7502x_clk>;
> > +            assigned-clock-parents = <&mlx7502x_clk_parent>;
> > +            assigned-clock-rates = <8000000>;
> > +
> > +            vddd-supply = <&mlx_7502x_reg>;
> > +
> > +            reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>;
> > +            melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>;
> > +            melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>;
> > +
> > +            port {
> > +                mlx7502x_out_mipi_csi2: endpoint {
> > +                    remote-endpoint = <&mipi_csi2_from_mlx7502x>;
> > +                    data-lanes = <1 2 3 4>;
> > +                    link-frequencies = /bits/ 64 < 960000000
> > +                                                   904000000
> > +                                                   800000000
> > +                                                   704000000
> > +                                                   600000000
> > +                                                   300000000 >;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1a68d888ee14..b00a726bb3db 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12678,6 +12678,7 @@ M:	Volodymyr Kharuk <vkh@melexis.com>
> >  L:	linux-media@vger.kernel.org
> >  S:	Supported
> >  W:	http://www.melexis.com
> > +F:	Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
> >  F:	include/uapi/linux/mlx7502x.h
> >  
> >  MELFAS MIP4 TOUCHSCREEN DRIVER
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
--
Volodymyr Kharuk

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

[Laurent Pinchart]

Hi Volodymyr,

I've just realized your previous e-mail felt through the cracks. Sorry
about that. Please see below for comments.

On Fri, Jul 15, 2022 at 06:32:43PM +0300, Volodymyr Kharuk wrote:
> On Thu, Jul 14, 2022 at 01:06:35PM +0300, Laurent Pinchart wrote:
> > 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@melexis.com>
> > > ---
> > >  .../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@melexis.com>
> > > +
> > > +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.
> > 
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - melexis,mlx7502x
> > > +          - melexis,mlx75026
> > > +          - melexis,mlx75027
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  assigned-clocks: true
> > > +  assigned-clock-parents: true
> > > +  assigned-clock-rates: true
> > > +
> > > +  clocks:
> > > +    description: Clock frequency 8MHz
> > > +    maxItems: 1
> > > +
> > > +  vdda-supply:
> > > +    description:
> > > +      Definition of the regulator used as analog power supply(2.7V).
> > > +
> > > +  vddif-supply:
> > > +    description:
> > > +      Definition of the regulator used as interface power supply(1.8V).
> > > +
> > > +  vddd-supply:
> > > +    description:
> > > +      Definition of the regulator used as digital power supply(1.2V).
> > > +
> > > +  vdmix-supply:
> > > +    description:
> > > +      Definition of the regulator used as mixed driver power supply(1.2V).
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +    description: Reset Sensor GPIO Control (active low)
> > > +
> > > +  melexis,trig-gpios:
> > > +    maxItems: 1
> > > +    description:
> > > +      Hardware Trigger GPIO Control (active low)
> > > +      When the hardware trigger mode is enabled, this GPIO is used to generate
> > > +      a low level impulse for about 100us. The impulse triggers a new
> > > +      measurement cycle.
> > > +
> > > +  melexis,leden-gpios:
> > > +    maxItems: 1
> > > +    description:
> > > +      Led driver enable GPIO Control (active high)
> > > +      This GPIO notifies the illumination driver when it is safe to use led
> > > +      signals from the sensor.
> > 
> > As far as I understand this isn't an input of the sensor, but a signal
> > that is driven by the driver and controls the separate illuminator. It
> > shouldn't be specified in this binding, as it's not a property of the
> > sensor. You should instead have a DT node for the illumination driver.
>
> Yes, you are right. There are illuminators, which are not ready to accept
> the signal from the sensor, if pin levels are not right. So I need to
> notify illuminator somehow.  Can the illumination driver be as a V4L2
> subdevice? Then I can notify the illuminator via s_stream. In another
> case it can damage the illuminator. What do you think?

It can be a separate subdev, yes. There are flash-related controls in
V4L2, maybe some of them would cover your use cases ? We have a few
flash drivers in upstream that you can look at for inspiration, for
instance drivers/media/i2c/lm3560.c or drivers/media/i2c/adp1653.c.

> > > +  port:
> > > +    description: MIPI CSI-2 transmitter port
> > > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > > +
> > > +    properties:
> > > +      endpoint:
> > > +        $ref: /schemas/media/video-interfaces.yaml#
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          data-lanes:
> > > +            oneOf:
> > > +              - items:
> > > +                  - const: 1
> > > +                  - const: 2
> > > +              - items:
> > > +                  - const: 1
> > > +                  - const: 2
> > > +                  - const: 3
> > > +                  - const: 4
> > > +
> > > +          clock-noncontinuous: true
> > > +          link-frequencies: true
> > > +
> > > +        required:
> > > +          - data-lanes
> > > +          - link-frequencies
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - port
> > 
> > Let's make the supplies mandatory too, as the chip can't operate without
> > them.
>
> Ok
>
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        mlx7502x: camera@57 {
> > > +            compatible = "melexis,mlx7502x";
> > > +            reg = <0x57>;
> > > +            clocks = <&mlx7502x_clk>;
> > > +
> > > +            assigned-clocks = <&mlx7502x_clk>;
> > > +            assigned-clock-parents = <&mlx7502x_clk_parent>;
> > > +            assigned-clock-rates = <8000000>;
> > > +
> > > +            vddd-supply = <&mlx_7502x_reg>;
> > > +
> > > +            reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>;
> > > +            melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>;
> > > +            melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>;
> > > +
> > > +            port {
> > > +                mlx7502x_out_mipi_csi2: endpoint {
> > > +                    remote-endpoint = <&mipi_csi2_from_mlx7502x>;
> > > +                    data-lanes = <1 2 3 4>;
> > > +                    link-frequencies = /bits/ 64 < 960000000
> > > +                                                   904000000
> > > +                                                   800000000
> > > +                                                   704000000
> > > +                                                   600000000
> > > +                                                   300000000 >;
> > > +                };
> > > +            };
> > > +        };
> > > +    };
> > > +
> > > +...
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1a68d888ee14..b00a726bb3db 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -12678,6 +12678,7 @@ M:	Volodymyr Kharuk <vkh@melexis.com>
> > >  L:	linux-media@vger.kernel.org
> > >  S:	Supported
> > >  W:	http://www.melexis.com
> > > +F:	Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
> > >  F:	include/uapi/linux/mlx7502x.h
> > >  
> > >  MELFAS MIP4 TOUCHSCREEN DRIVER

-- 
Regards,

Laurent Pinchart

[PATCH v2 6/6] media: i2c: Add driver for mlx7502x ToF sensor

[Volodymyr Kharuk]

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.

The driver supports:
- Digital binning and cropping
- Horizontal and vertical flip
- phase shifts(up to 8)
- frequency modulation
- time integration
- internal temperature
- different trigger sources

Signed-off-by: Volodymyr Kharuk <vkh@melexis.com>
Signed-off-by: Andrii Kyselov <ays@melexis.com>
---
 MAINTAINERS                  |    1 +
 drivers/media/i2c/Kconfig    |   13 +
 drivers/media/i2c/Makefile   |    1 +
 drivers/media/i2c/mlx7502x.c | 2020 ++++++++++++++++++++++++++++++++++
 4 files changed, 2035 insertions(+)
 create mode 100644 drivers/media/i2c/mlx7502x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b00a726bb3db..6dc74dab45aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12679,6 +12679,7 @@ L:	linux-media@vger.kernel.org
 S:	Supported
 W:	http://www.melexis.com
 F:	Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml
+F:	drivers/media/i2c/mlx7502x.c
 F:	include/uapi/linux/mlx7502x.h
 
 MELFAS MIP4 TOUCHSCREEN DRIVER
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 2b20aa6c37b1..126f6bd458b7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -216,6 +216,19 @@ config VIDEO_IMX412
 config VIDEO_MAX9271_LIB
 	tristate
 
+config VIDEO_MLX7502X
+	tristate "Melexis ToF 75026 and 75027 sensors support"
+	depends on I2C && VIDEO_DEV
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select V4L2_FWNODE
+	help
+	  This is a V4L2 sensor driver for the Melexis 75026 and 75027
+	  ToF sensors.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mlx7502x.
+
 config VIDEO_MT9M001
 	tristate "mt9m001 support"
 	depends on I2C && VIDEO_DEV
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 3e1696963e7f..57c30859e537 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_M5MOLS) += m5mols/
 obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o
 obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
 obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
+obj-$(CONFIG_VIDEO_MLX7502X) += mlx7502x.o
 obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
 obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
 obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
diff --git a/drivers/media/i2c/mlx7502x.c b/drivers/media/i2c/mlx7502x.c
new file mode 100644
index 000000000000..93fb6b555a4e
--- /dev/null
+++ b/drivers/media/i2c/mlx7502x.c
@@ -0,0 +1,2020 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A V4L2 driver for Melexis 7502x ToF cameras.
+ *
+ * Copyright (C) 2022 Melexis N.V.
+ *
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/bsearch.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/mlx7502x.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/pm_runtime.h>
+#include <linux/units.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-image-sizes.h>
+
+#define MLX7502X_SENSOR_ID_REG        0x0002
+#define MLX7502X_PARAM_HOLD_REG       0x0102
+#define MLX7502X_HMAX_REG             0x0800
+#define MLX7502X_COLUMN_START_REG     0x0804
+#define MLX7502X_COLUMN_LEN_REG       0x0806
+#define MLX7502X_ROW_START_REG        0x0808
+#define MLX7502X_ROW_END_REG          0x080a
+#define MLX7502X_VFLIP_REG            0x080c
+#define MLX7502X_HFLIP_REG            0x080d
+#define MLX7502X_OUTPUT_MODE_REG      0x0828
+#define MLX7502X_STREAM_EN_REG        0x1001
+#define MLX7502X_DATA_LANE_CONFIG_REG 0x1010
+#define MLX7502X_FMOD_REG             0x1048
+#define MLX7502X_PLL_RES_REG          0x104b
+#define MLX7502X_TEMPERATURE_REG      0x1403
+#define MLX7502X_BINNING_REG          0x14a5
+#define MLX7502X_CONTINUOUS_REG       0x1c40
+#define MLX7502X_SW_TRIGGER_REG       0x2100
+#define MLX7502X_FRAME_TIME_REG       0x2108
+#define MLX7502X_TINT0_REG            0x2120
+#define MLX7502X_PX_PHASE_SHIFT_REG   0x21b4
+#define MLX7502X_DIVSELPRE_REG        0x21be
+#define MLX7502X_DIVSEL_REG           0x21bf
+#define MLX7502X_PHASE_COUNT_REG      0x21e8
+#define MLX7502X_PLLSETUP_REG         0x4010
+#define MLX7502X_PRETIME_REG          0x4015
+#define MLX7502X_RANDNM0_REG          0x5265
+
+#define MLX7502X_NIBLE_LOW_MASK		GENMASK(3, 0)
+#define MLX7502X_NIBLE_HIGH_MASK	GENMASK(7, 4)
+
+#define MLX7502X_PHASE_MAX_NUM 8
+
+#define MLX7502X_CLK_RATE_MHZ 120
+#define MLX7502X_XCLK_RATE (8 * HZ_PER_MHZ)
+
+#define MLX7502X_RESET_DELAY_MS 100
+#define MLX7502X_TRIGGER_DELAY_US 100
+#define MLX7502X_STREAMING_DELAY_US 1500
+
+#define MLX7502X_SW_TRIGGER_DEFAULT 0x0000
+#define MLX7502X_SW_TRIGGER_TRIG 0x0001
+#define MLX7502X_SW_TRIGGER_CONT 0x0008
+
+#define MLX7502X_LINK_FREQ_REG_N 11
+#define MLX7502X_LINK_FREQ_N 6
+#define MLX7502X_LANE_N 2
+#define MLX7502X_OUTPUT_MODE_N 2
+
+#define MLX7502X_PLLSETUP_US 503
+#define MLX7502X_PLLSETUP_TICKS 8
+#define MLX7502X_PRETIME_US 50
+#define MLX7502X_FRAME_ADD_TICKS 13
+
+#define MLX7502X_DEFAULT_FRAME_RATE 25
+
+#define MLX7502X_ROW_START(top) ((top) / 2)
+#define MLX7502X_ROW_END(top, height) ((((top) + (height)) / 2) + 1)
+
+#define MLX7502X_LEFT_STEP 1
+#define MLX7502X_LEFT_MIN 0
+
+#define MLX7502X_TOP_STEP 2
+#define MLX7502X_TOP_MIN 0
+
+#define MLX7502X_PLL_RES_THR 113
+
+#define MLX7502X_75026_ID 0x10
+
+static const char * const mlx7502x_supply_names[] = {
+	"vdda",		/* Analog (2.7V) supply */
+	"vddif",	/* I/O (1.8V) supply */
+	"vddd",		/* Digital core (1.2V) supply */
+	"vdmix",	/* MIX driver (1.2V) supply */
+};
+
+#define MLX7502X_NUM_SUPPLIES ARRAY_SIZE(mlx7502x_supply_names)
+
+/* the source to generate next frame */
+enum mlx7502x_trigger_mode {
+	MLX7502X_SOFTWARE = 0, /* internal sw trigger */
+	MLX7502X_HARDWARE, /* external gpio trigger */
+	MLX7502X_CONTINUOUS, /* self triggering */
+};
+
+/* output data of the sensor */
+enum mlx7502x_output_mode {
+	MLX7502X_AMB = 0, /* rawA minus rawB mode */
+	MLX7502X_APB, /* rawA plus rawB mode */
+	MLX7502X_RAW_A, /* only rawA mode */
+	MLX7502X_RAW_B, /* only rawB mode */
+	MLX7502X_RAW_ANB, /* both rawA and rawB, output frame size doubles */
+};
+
+struct mlx7502x_regval_list {
+	u16 addr;
+	u8 data;
+};
+
+struct mlx7502x_link_freq_list {
+	s64 link_freq[MLX7502X_LINK_FREQ_N];
+	u16 addr[MLX7502X_LINK_FREQ_REG_N];
+	u8 data[MLX7502X_LANE_N][MLX7502X_LINK_FREQ_N][MLX7502X_LINK_FREQ_REG_N];
+	u16 hmax[MLX7502X_LANE_N][MLX7502X_LINK_FREQ_N][MLX7502X_OUTPUT_MODE_N];
+};
+
+struct mlx7502x_binning_mode {
+	u8 reg_value;
+	u8 ratio;
+	u8 width_step;
+	u8 height_step;
+	u8 width_min;
+	u8 height_min;
+};
+
+/* configuration of divider for specific fmod */
+struct mlx7502x_fmod_list {
+	u8 fmod;
+	u8 divselpre;
+	u8 divsel;
+};
+
+struct mlx7502x_sensor_desc {
+	const struct mlx7502x_regval_list *init_cfg;
+	u32 init_cfg_size;
+	const struct mlx7502x_link_freq_list *link_freq_cfg;
+	u32 width;
+	u32 height;
+};
+
+struct mlx7502x {
+	struct v4l2_subdev sd;
+	struct media_pad pad;
+	struct v4l2_fwnode_endpoint ep;
+	struct device *dev;
+
+	/* controls */
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *tint;
+	struct v4l2_ctrl *trigger_mode;
+	struct v4l2_ctrl *phase_number;
+	struct v4l2_ctrl *output_mode;
+
+	struct clk *xclk;
+	struct regulator_bulk_data supplies[MLX7502X_NUM_SUPPLIES];
+
+	/* pins */
+	struct gpio_desc *reset;
+	struct gpio_desc *leden;
+	struct gpio_desc *trig;
+
+	const struct mlx7502x_sensor_desc *cur_desc;
+	const struct mlx7502x_binning_mode *binning_mode;
+	int (*trigger)(struct mlx7502x *sensor);
+	int streaming;
+	u16 hmax; /* internal sensor frame size in ticks */
+	struct v4l2_fract frame_interval;
+	struct v4l2_rect crop;
+	struct v4l2_rect compose;
+	struct mutex lock; /* mutex lock for serialized operations */
+};
+
+static const struct mlx7502x_regval_list mlx7502x_common_init_cfg[] = {
+	{ 0x1006, 0x08 }, { 0x1007, 0x00 }, { 0x1040, 0x00 }, { 0x1041, 0x96 },
+	{ 0x1042, 0x01 }, { 0x1043, 0x00 }, { 0x1044, 0x00 }, { 0x1046, 0x01 },
+	{ 0x104a, 0x01 }, { 0x1000, 0x00 }, { 0x10d3, 0x10 }, { 0x1448, 0x06 },
+	{ 0x1449, 0x40 }, { 0x144a, 0x06 }, { 0x144b, 0x40 }, { 0x144c, 0x06 },
+	{ 0x144d, 0x40 }, { 0x144e, 0x06 }, { 0x144f, 0x40 }, { 0x1450, 0x06 },
+	{ 0x1451, 0x40 }, { 0x1452, 0x06 }, { 0x1453, 0x40 }, { 0x1454, 0x06 },
+	{ 0x1455, 0x40 }, { 0x1456, 0x06 }, { 0x1457, 0x40 }, { 0x2203, 0x1e },
+	{ 0x2c08, 0x01 }, { 0x3c2b, 0x1b }, { 0x400e, 0x01 }, { 0x400f, 0x81 },
+	{ 0x40d1, 0x00 }, { 0x40d2, 0x00 }, { 0x40d3, 0x00 }, { 0x40db, 0x3f },
+	{ 0x40de, 0x40 }, { 0x40df, 0x01 }, { 0x4134, 0x04 }, { 0x4135, 0x04 },
+	{ 0x4136, 0x04 }, { 0x4137, 0x04 }, { 0x4138, 0x04 }, { 0x4139, 0x04 },
+	{ 0x413a, 0x04 }, { 0x413b, 0x04 }, { 0x413c, 0x04 }, { 0x4146, 0x01 },
+	{ 0x4147, 0x01 }, { 0x4148, 0x01 }, { 0x4149, 0x01 }, { 0x414a, 0x01 },
+	{ 0x414b, 0x01 }, { 0x414c, 0x01 }, { 0x414d, 0x01 }, { 0x4158, 0x01 },
+	{ 0x4159, 0x01 }, { 0x415a, 0x01 }, { 0x415b, 0x01 }, { 0x415c, 0x01 },
+	{ 0x415d, 0x01 }, { 0x415e, 0x01 }, { 0x415f, 0x01 }, { 0x4590, 0x00 },
+	{ 0x4591, 0x2e }, { 0x4684, 0x00 }, { 0x4685, 0xa0 }, { 0x4687, 0xa1 },
+	{ 0x471e, 0x07 }, { 0x471f, 0xc9 }, { 0x473a, 0x07 }, { 0x473b, 0xc9 },
+	{ 0x4770, 0x00 }, { 0x4771, 0x00 }, { 0x4772, 0x1f }, { 0x4773, 0xff },
+	{ 0x4778, 0x06 }, { 0x4779, 0xa4 }, { 0x477a, 0x07 }, { 0x477b, 0xae },
+	{ 0x4788, 0x06 }, { 0x4789, 0xa4 }, { 0x478c, 0x1f }, { 0x478d, 0xff },
+	{ 0x478e, 0x00 }, { 0x478f, 0x00 }, { 0x4792, 0x00 }, { 0x4793, 0x00 },
+	{ 0x4796, 0x00 }, { 0x4797, 0x00 }, { 0x479a, 0x00 }, { 0x479b, 0x00 },
+	{ 0x479c, 0x1f }, { 0x479d, 0xff }, { 0x479e, 0x00 }, { 0x479f, 0x00 },
+	{ 0x47a2, 0x00 }, { 0x47a3, 0x00 }, { 0x47a6, 0x00 }, { 0x47a7, 0x00 },
+	{ 0x47aa, 0x00 }, { 0x47ab, 0x00 }, { 0x47ac, 0x1f }, { 0x47ad, 0xff },
+	{ 0x47ae, 0x00 }, { 0x47af, 0x00 }, { 0x47b2, 0x00 }, { 0x47b3, 0x00 },
+	{ 0x47b6, 0x00 }, { 0x47b7, 0x00 }, { 0x47ba, 0x00 }, { 0x47bb, 0x00 },
+	{ 0x47bc, 0x1f }, { 0x47bd, 0xff }, { 0x47be, 0x00 }, { 0x47bf, 0x00 },
+	{ 0x47c2, 0x00 }, { 0x47c3, 0x00 }, { 0x47c6, 0x00 }, { 0x47c7, 0x00 },
+	{ 0x47ca, 0x00 }, { 0x47cb, 0x00 }, { 0x4834, 0x00 }, { 0x4835, 0xa0 },
+	{ 0x4837, 0xa1 }, { 0x4878, 0x00 }, { 0x4879, 0xa0 }, { 0x487b, 0xa1 },
+	{ 0x48bc, 0x00 }, { 0x48bd, 0xa0 }, { 0x48bf, 0xa1 }, { 0x49ff, 0x78 },
+	{ 0x4baf, 0x1a }, { 0x4bc7, 0x1a }, { 0x4d2a, 0x07 }, { 0x4d80, 0x06 },
+	{ 0x4d81, 0xa4 }, { 0x4d82, 0x07 }, { 0x4e39, 0x07 }, { 0x4e7b, 0x64 },
+	{ 0x4e8e, 0x0e }, { 0x4e9c, 0x01 }, { 0x4ea0, 0x01 }, { 0x4ea1, 0x03 },
+	{ 0x4ea5, 0x00 }, { 0x4ea7, 0x00 }, { 0x4f05, 0x04 }, { 0x4f0d, 0x04 },
+	{ 0x4f15, 0x04 }, { 0x4f19, 0x01 }, { 0x4f20, 0x01 }, { 0x4f66, 0x0f },
+	{ 0x500f, 0x01 }, { 0x5225, 0x2f }, { 0x5227, 0x1e }, { 0x5231, 0x19 },
+	{ 0x5245, 0x07 }, { 0x5252, 0x07 }, { 0x5253, 0x08 }, { 0x5254, 0x07 },
+	{ 0x5255, 0xb4 }, { 0x5272, 0x04 }, { 0x5273, 0x2e }, { 0x5282, 0x04 },
+	{ 0x5283, 0x2e }, { 0x5286, 0x00 }, { 0x5287, 0x5d }, { 0x1433, 0x00 },
+	{ 0x3c18, 0x00 }, { 0x100e, 0x00 }, { 0x100f, 0x00 }, { 0x10c2, 0x00 },
+	{ 0x10d0, 0x0a }, { 0x10d4, 0x00 }, { 0x10d5, 0xc5 },
+};
+
+static const struct mlx7502x_regval_list mlx7502x_75027_init_cfg[] = {
+	{ 0x477d, 0xd6 }, { 0x4954, 0x00 }, { 0x4955, 0xa0 }, { 0x4957, 0xa1 },
+	{ 0x4984, 0x00 }, { 0x4985, 0xa0 }, { 0x4987, 0xa1 }, { 0x49b9, 0x78 },
+	{ 0x49c3, 0x3c }, { 0x49c9, 0x76 }, { 0x49d3, 0x3f }, { 0x49dc, 0x00 },
+	{ 0x49dd, 0xa0 }, { 0x49df, 0xa1 }, { 0x49ef, 0x78 }, { 0x49f9, 0x3c },
+	{ 0x4a05, 0x3c }, { 0x4a0b, 0x76 }, { 0x4a11, 0x3f }, { 0x4a1a, 0x00 },
+	{ 0x4a1b, 0xa0 }, { 0x4a1d, 0xa1 }, { 0x4a1f, 0x78 }, { 0x4a29, 0x3c },
+	{ 0x4a4a, 0x00 }, { 0x4a4b, 0xa0 }, { 0x4a4d, 0xa1 }, { 0x4a7a, 0x00 },
+	{ 0x4a7b, 0xa0 }, { 0x4a7d, 0xa1 }, { 0x4aee, 0x00 }, { 0x4aef, 0xa0 },
+	{ 0x4af1, 0xa1 }, { 0x4b2e, 0x00 }, { 0x4b2f, 0xa0 }, { 0x4b31, 0xa1 },
+	{ 0x4b5a, 0x00 }, { 0x4b5b, 0xa0 }, { 0x4b5d, 0xa1 }, { 0x4b86, 0x00 },
+	{ 0x4b87, 0xa0 }, { 0x4b89, 0xa1 }, { 0x4b9f, 0x1a }, { 0x4bb7, 0x1a },
+	{ 0x4bcf, 0x1a }, { 0x4bee, 0x00 }, { 0x4bef, 0xa0 }, { 0x4bf1, 0xa1 },
+	{ 0x4bf7, 0x1a }, { 0x4c01, 0x1a }, { 0x4c58, 0x00 }, { 0x4c59, 0xa0 },
+	{ 0x4c5b, 0xa1 }, { 0x4c6e, 0x00 }, { 0x4c6f, 0xa0 }, { 0x4c71, 0xa1 },
+	{ 0x4c7a, 0x01 }, { 0x4c7b, 0x35 }, { 0x4cf2, 0x07 }, { 0x4cf3, 0xc9 },
+	{ 0x4cf8, 0x06 }, { 0x4cf9, 0x9b }, { 0x4cfa, 0x07 }, { 0x4cfb, 0xae },
+	{ 0x4cfe, 0x07 }, { 0x4cff, 0xc9 }, { 0x4d04, 0x06 }, { 0x4d05, 0x98 },
+	{ 0x4d06, 0x07 }, { 0x4d07, 0xb1 }, { 0x4d18, 0x06 }, { 0x4d19, 0xa4 },
+	{ 0x4d1a, 0x07 }, { 0x4d1b, 0x49 }, { 0x4d1e, 0x07 }, { 0x4d1f, 0xc9 },
+	{ 0x4d2b, 0xc9 }, { 0x4d4a, 0x07 }, { 0x4d4b, 0xc9 }, { 0x4d50, 0x06 },
+	{ 0x4d51, 0x9b }, { 0x4d52, 0x07 }, { 0x4d53, 0xae }, { 0x4d56, 0x07 },
+	{ 0x4d57, 0xc9 }, { 0x4d5c, 0x06 }, { 0x4d5d, 0x98 }, { 0x4d5e, 0x07 },
+	{ 0x4d5f, 0xb1 }, { 0x4d70, 0x06 }, { 0x4d71, 0xa4 }, { 0x4d72, 0x07 },
+	{ 0x4d73, 0x49 }, { 0x4d78, 0x06 }, { 0x4d79, 0xa4 }, { 0x4d7a, 0x07 },
+	{ 0x4d7b, 0xae }, { 0x4d7c, 0x1f }, { 0x4d7d, 0xff }, { 0x4d7e, 0x1f },
+	{ 0x4d7f, 0xff }, { 0x4d83, 0xae }, { 0x4d84, 0x1f }, { 0x4d85, 0xff },
+	{ 0x4d86, 0x1f }, { 0x4d87, 0xff },
+};
+
+static const struct mlx7502x_link_freq_list mlx75027_link_freq_cfg = {
+	.link_freq = { 300000000, 600000000, 704000000, 800000000, 904000000, 960000000 },
+	.addr = {
+		0x100c, 0x100d, 0x1016, 0x1017, 0x1045, 0x1047,
+		0x1060, 0x1071, 0x10c3, 0x10c4, 0x10c5
+	},
+	.data = {
+		{ /* lane 2 */
+			{ 0x02, 0x58, 0x09, 0x99, 0x4b, 0x02,
+				0x01, 0x0c, 0x1c, 0x01, 0x3a }, /* 300MBps */
+			{ 0x04, 0xb0, 0x04, 0xcc, 0x4b, 0x02,
+				0x00, 0x06, 0x0f, 0x00, 0x9d }, /* 600MBps */
+			{ 0x05, 0x80, 0x04, 0x17, 0x58, 0x02,
+				0x00, 0x06, 0x0d, 0x00, 0x86 }, /* 704MBps */
+			{ 0x06, 0x40, 0x03, 0x99, 0x64, 0x02,
+				0x00, 0x06, 0x0b, 0x00, 0x75 }, /* 800MBps */
+			{ 0x07, 0x10, 0x03, 0x2f, 0x71, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x68 }, /* 904MBps */
+			{ 0x07, 0x80, 0x03, 0x00, 0x78, 0x02,
+				0x00, 0x06, 0x0a, 0x00, 0x62 }, /* 960MBps */
+		}, { /* lane 4 */
+			{ 0x04, 0xb0, 0x09, 0x99, 0x4b, 0x02,
+				0x01, 0x0c, 0x1c, 0x01, 0x3a }, /* 300MBps */
+			{ 0x09, 0x60, 0x04, 0xcc, 0x4b, 0x02,
+				0x00, 0x06, 0x0f, 0x00, 0x9d }, /* 600MBps */
+			{ 0x0b, 0x00, 0x04, 0x17, 0x58, 0x02,
+				0x00, 0x06, 0x0d, 0x00, 0x86 }, /* 704MBps */
+			{ 0x0c, 0x80, 0x03, 0x99, 0x64, 0x02,
+				0x00, 0x06, 0x0b, 0x00, 0x75 }, /* 800MBps */
+			{ 0x0e, 0x20, 0x03, 0x2f, 0x71, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x68 }, /* 904MBps */
+			{ 0x0f, 0x00, 0x03, 0x00, 0x78, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x62 }, /* 960MBps */
+		},
+	},
+	.hmax = {
+		{ /* lane 2 */
+			{ 0x0e78, 0x1a80 }, /* 300MBps */
+			{ 0x0750, 0x0d54 }, /* 600MBps */
+			{ 0x0640, 0x0b60 }, /* 704MBps */
+			{ 0x0584, 0x0a06 }, /* 800MBps */
+			{ 0x04e8, 0x08e6 }, /* 904MBps */
+			{ 0x049e, 0x0860 }, /* 960MBps */
+		},
+		{ /* lane 4 */
+			{ 0x0860, 0x0e60 }, /* 300MBps */
+			{ 0x0444, 0x0744 }, /* 600MBps */
+			{ 0x03a8, 0x0636 }, /* 704MBps */
+			{ 0x033a, 0x057a }, /* 800MBps */
+			{ 0x02e2, 0x0514 }, /* 904MBps */
+			{ 0x02b6, 0x0514 }, /* 960MBps */
+		},
+	}
+};
+
+/* VGA sensor */
+static const struct mlx7502x_sensor_desc mlx75027 = {
+	.init_cfg = mlx7502x_75027_init_cfg,
+	.init_cfg_size = ARRAY_SIZE(mlx7502x_75027_init_cfg),
+	.link_freq_cfg = &mlx75027_link_freq_cfg,
+	.width = VGA_WIDTH,
+	.height = VGA_HEIGHT,
+};
+
+static const struct mlx7502x_regval_list mlx7502x_75026_init_cfg[] = {
+	{ 0x477c, 0x0a }, { 0x477d, 0xd4 }, { 0x4964, 0x00 }, { 0x4965, 0xa0 },
+	{ 0x4967, 0xa1 }, { 0x4994, 0x00 }, { 0x4995, 0xa0 }, { 0x4997, 0xa1 },
+	{ 0x49c9, 0x78 }, { 0x49d3, 0x3c }, { 0x49d9, 0x76 }, { 0x49e3, 0x3f },
+	{ 0x49ec, 0x00 }, { 0x49ed, 0xa0 }, { 0x49ef, 0xa1 }, { 0x4a09, 0x3c },
+	{ 0x4a0f, 0x78 }, { 0x4a15, 0x3c }, { 0x4a1b, 0x76 }, { 0x4a21, 0x3f },
+	{ 0x4a2a, 0x00 }, { 0x4a2b, 0xa0 }, { 0x4a2d, 0xa1 }, { 0x4a2f, 0x78 },
+	{ 0x4a39, 0x3c }, { 0x4a5a, 0x00 }, { 0x4a5b, 0xa0 }, { 0x4a5d, 0xa1 },
+	{ 0x4a8a, 0x00 }, { 0x4a8b, 0xa0 }, { 0x4a8d, 0xa1 }, { 0x4afe, 0x00 },
+	{ 0x4aff, 0xa0 }, { 0x4b01, 0xa1 }, { 0x4b3e, 0x00 }, { 0x4b3f, 0xa0 },
+	{ 0x4b41, 0xa1 }, { 0x4b6a, 0x00 }, { 0x4b6b, 0xa0 }, { 0x4b6d, 0xa1 },
+	{ 0x4b96, 0x00 }, { 0x4b97, 0xa0 }, { 0x4b99, 0xa1 }, { 0x4bbf, 0x1a },
+	{ 0x4bd7, 0x1a }, { 0x4bdf, 0x1a }, { 0x4bfe, 0x00 }, { 0x4bff, 0xa0 },
+	{ 0x4c01, 0xa1 }, { 0x4c07, 0x1a }, { 0x4c11, 0x1a }, { 0x4c68, 0x00 },
+	{ 0x4c69, 0xa0 }, { 0x4c6b, 0xa1 }, { 0x4c7e, 0x00 }, { 0x4c7f, 0xa0 },
+	{ 0x4c81, 0xa1 }, { 0x4c8a, 0x01 }, { 0x4c8b, 0x35 }, { 0x4d02, 0x07 },
+	{ 0x4d03, 0xc9 }, { 0x4d08, 0x06 }, { 0x4d09, 0x9b }, { 0x4d0a, 0x07 },
+	{ 0x4d0b, 0xae }, { 0x4d0e, 0x07 }, { 0x4d0f, 0xc9 }, { 0x4d14, 0x06 },
+	{ 0x4d15, 0x98 }, { 0x4d16, 0x07 }, { 0x4d17, 0xb1 }, { 0x4d28, 0x06 },
+	{ 0x4d29, 0xa4 }, { 0x4d2b, 0xa9 }, { 0x4d2e, 0x07 }, { 0x4d2f, 0xc9 },
+	{ 0x4d3a, 0x07 }, { 0x4d3b, 0xc9 }, { 0x4d5a, 0x07 }, { 0x4d5b, 0xc9 },
+	{ 0x4d60, 0x06 }, { 0x4d61, 0x9b }, { 0x4d62, 0x07 }, { 0x4d63, 0xae },
+	{ 0x4d66, 0x07 }, { 0x4d67, 0xc9 }, { 0x4d6c, 0x06 }, { 0x4d6d, 0x98 },
+	{ 0x4d6e, 0x07 }, { 0x4d6f, 0xb1 }, { 0x4d83, 0xa9 }, { 0x4d88, 0x06 },
+	{ 0x4d89, 0xa4 }, { 0x4d8a, 0x07 }, { 0x4d8b, 0xae }, { 0x4d8c, 0x1f },
+	{ 0x4d8d, 0xff }, { 0x4d8e, 0x1f }, { 0x4d8f, 0xff }, { 0x4d90, 0x06 },
+	{ 0x4d91, 0xa4 }, { 0x4d92, 0x07 }, { 0x4d93, 0xae }, { 0x4d94, 0x1f },
+	{ 0x4d95, 0xff }, { 0x4d96, 0x1f }, { 0x4d97, 0xff },
+};
+
+static const struct mlx7502x_link_freq_list mlx75026_link_freq_cfg = {
+	.link_freq = { 300000000, 600000000, 704000000, 800000000, 904000000, 960000000 },
+	.addr = {
+		0x100c, 0x100d, 0x1016, 0x1017, 0x1045, 0x1047,
+		0x1060, 0x1071, 0x10c3, 0x10c4, 0x10c5
+	},
+	.data = {
+		{ /* lane 2 */
+			{ 0x02, 0x58, 0x09, 0x99, 0x4b, 0x02,
+				0x01, 0x0c, 0x1c, 0x01, 0x3a }, /* 300MBps */
+			{ 0x04, 0xb0, 0x04, 0xcc, 0x4b, 0x02,
+				0x00, 0x06, 0x0f, 0x00, 0x9d }, /* 600MBps */
+			{ 0x05, 0x80, 0x04, 0x17, 0x58, 0x02,
+				0x00, 0x06, 0x0d, 0x00, 0x86 }, /* 704MBps */
+			{ 0x06, 0x40, 0x03, 0x99, 0x64, 0x02,
+				0x00, 0x06, 0x0b, 0x00, 0x75 }, /* 800MBps */
+			{ 0x07, 0x10, 0x03, 0x2f, 0x71, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x68 }, /* 904MBps */
+			{ 0x07, 0x80, 0x03, 0x00, 0x78, 0x02,
+				0x00, 0x06, 0x0a, 0x00, 0x62 }, /* 960MBps */
+		}, { /* lane 4 */
+			{ 0x04, 0xb0, 0x09, 0x99, 0x4b, 0x02,
+				0x01, 0x0c, 0x1c, 0x01, 0x3a }, /* 300MBps */
+			{ 0x09, 0x60, 0x04, 0xcc, 0x4b, 0x02,
+				0x00, 0x06, 0x0f, 0x00, 0x9d }, /* 600MBps */
+			{ 0x0b, 0x00, 0x04, 0x17, 0x58, 0x02,
+				0x00, 0x06, 0x0d, 0x00, 0x86 }, /* 704MBps */
+			{ 0x0c, 0x80, 0x03, 0x99, 0x64, 0x02,
+				0x00, 0x06, 0x0b, 0x00, 0x75 }, /* 800MBps */
+			{ 0x0e, 0x20, 0x03, 0x2f, 0x71, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x68 }, /* 904MBps */
+			{ 0x0f, 0x00, 0x03, 0x00, 0x78, 0x00,
+				0x00, 0x06, 0x0a, 0x00, 0x62 }, /* 960MBps */
+		},
+	},
+	.hmax = {
+		{ /* lane 2 */
+			{ 0x0878, 0x0e80 }, /* 300MBps */
+			{ 0x0450, 0x0754 }, /* 600MBps */
+			{ 0x03b2, 0x0644 }, /* 704MBps */
+			{ 0x0344, 0x0586 }, /* 800MBps */
+			{ 0x02ea, 0x0514 }, /* 904MBps */
+			{ 0x02be, 0x0514 }, /* 960MBps */
+		}, { /* lane 4 */
+			{ 0x0560, 0x0860 }, /* 300MBps */
+			{ 0x02c4, 0x0444 }, /* 600MBps */
+			{ 0x02b6, 0x03a8 }, /* 704MBps */
+			{ 0x02b6, 0x033a }, /* 800MBps */
+			{ 0x02b6, 0x02e2 }, /* 904MBps */
+			{ 0x02b6, 0x02b6 }, /* 960MBps */
+		},
+	}
+};
+
+/* QVGA sensor */
+static const struct mlx7502x_sensor_desc mlx75026 = {
+	.init_cfg = mlx7502x_75026_init_cfg,
+	.init_cfg_size = ARRAY_SIZE(mlx7502x_75026_init_cfg),
+	.link_freq_cfg = &mlx75026_link_freq_cfg,
+	.width = QVGA_WIDTH,
+	.height = QVGA_HEIGHT,
+};
+
+static const struct mlx7502x_binning_mode binning_mode[] = {
+	{ .reg_value = 0, .ratio = 1, .width_step = 4, .height_step = 2,
+	  .width_min = 8, .height_min = 2 },
+	{ .reg_value = 1, .ratio = 2, .width_step = 8, .height_step = 2,
+	  .width_min = 16, .height_min = 2 },
+	{ .reg_value = 2, .ratio = 4, .width_step = 16, .height_step = 4,
+	  .width_min = 32, .height_min = 4 },
+	{ .reg_value = 3, .ratio = 8, .width_step = 32, .height_step = 8,
+	  .width_min = 64, .height_min = 8 },
+};
+
+static const struct mlx7502x_fmod_list mlx7502x_fmod_cfg[] = {
+	{ .fmod = 100, .divselpre = 0, .divsel = 0, },
+	{ .fmod = 75,  .divselpre = 0, .divsel = 0, },
+	{ .fmod = 74,  .divselpre = 1, .divsel = 0, },
+	{ .fmod = 51,  .divselpre = 1, .divsel = 0, },
+	{ .fmod = 50,  .divselpre = 0, .divsel = 1, },
+	{ .fmod = 38,  .divselpre = 0, .divsel = 1, },
+	{ .fmod = 37,  .divselpre = 1, .divsel = 1, },
+	{ .fmod = 21,  .divselpre = 1, .divsel = 1, },
+	{ .fmod = 20,  .divselpre = 0, .divsel = 2, },
+	{ .fmod = 19,  .divselpre = 0, .divsel = 2, },
+	{ .fmod = 18,  .divselpre = 1, .divsel = 2, },
+	{ .fmod = 10,  .divselpre = 1, .divsel = 2, },
+	{ .fmod = 9,   .divselpre = 2, .divsel = 2, },
+	{ .fmod = 5,   .divselpre = 2, .divsel = 2, },
+	{ .fmod = 4,   .divselpre = 3, .divsel = 2, },
+};
+
+static const struct mlx7502x_regval_list mlx7502x_hw_trigger_cfg[] = {
+	{ 0x2020, 0x00 }, { 0x2100, 0x00 }, { 0x2f05, 0x07 },
+	{ 0x2f06, 0x00 }, { 0x2f07, 0x00 }, { 0x3071, 0x03 },
+};
+
+static const struct mlx7502x_regval_list mlx7502x_sw_trigger_cfg[] = {
+	{ 0x2020, 0x01 }, { 0x2f05, 0x01 }, { 0x2f06, 0x09 },
+	{ 0x2f07, 0x7a }, { 0x3071, 0x00 },
+};
+
+static const char * const mlx7502x_ctrl_trigger_mode[] = {
+	[MLX7502X_SOFTWARE]	= "Software",
+	[MLX7502X_HARDWARE]	= "Hardware",
+	[MLX7502X_CONTINUOUS]	= "Continuous",
+};
+
+static const char * const mlx7502x_ctrl_output[] = {
+	[MLX7502X_AMB] = "A minus B",
+	[MLX7502X_APB] = "A plus B",
+	[MLX7502X_RAW_A] = "Raw A",
+	[MLX7502X_RAW_B] = "Raw B",
+	[MLX7502X_RAW_ANB] = "Raw A and Raw B",
+};
+
+static const u16 mlx7502x_ctrl_phase_sequence[] = {
+	0, 180, 90, 270, 0, 0, 0, 0
+};
+
+static const struct mlx7502x_regval_list mlx7502x_detect_cfg[] = {
+	{ 0x1006, 0x08 }, { 0x1045, 0x78 }, { 0x1049, 0x50 }, { 0x1071, 0x06 }
+};
+
+static inline struct mlx7502x *to_mlx7502x(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct mlx7502x, sd);
+}
+
+static int mlx7502x_read(struct v4l2_subdev *sd, u16 reg, u8 *val, int val_size)
+{
+	int ret;
+	unsigned char data_w[2];
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	struct i2c_msg msg[] = {
+		{
+			.addr = client->addr,
+			.flags = 0,
+			.len = 2,
+			.buf = data_w,
+		}, {
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = val_size,
+			.buf = val,
+		}
+	};
+
+	/* write reg address into first msg */
+	put_unaligned_be16(reg, data_w);
+
+	/* Using transfer allows skip STOP between messages
+	 * so we have repeated Start here
+	 */
+	ret = i2c_transfer(client->adapter, msg, 2);
+
+	return ret != 2 ? -EIO : 0;
+}
+
+static int mlx7502x_write(struct v4l2_subdev *sd, u8 *data, u32 data_size)
+{
+	int ret;
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	dev_dbg(&client->dev, "%*ph\n", data_size, data);
+
+	ret = i2c_master_send(client, data, data_size);
+	if (ret < 0) {
+		dev_dbg(&client->dev, "%s: i2c write error, reg: 0x%x%x\n",
+			__func__, data[0], data[1]);
+
+		return ret;
+	}
+
+	return ret != data_size ? -EIO : 0;
+}
+
+static int mlx7502x_write8(struct v4l2_subdev *sd, u16 reg, u8 val)
+{
+	unsigned char data[3];
+
+	put_unaligned_be16(reg, data);
+	data[2] = val;
+
+	return mlx7502x_write(sd, data, 3);
+}
+
+static int mlx7502x_write16(struct v4l2_subdev *sd, u16 reg, u16 val)
+{
+	unsigned char data[4];
+
+	put_unaligned_be16(reg, data);
+	put_unaligned_be16(val, data + 2);
+
+	return mlx7502x_write(sd, data, 4);
+}
+
+static int mlx7502x_write24(struct v4l2_subdev *sd, u16 reg, u32 val)
+{
+	unsigned char data[5];
+
+	put_unaligned_be16(reg, data);
+	put_unaligned_be24(val, data + 2);
+
+	return mlx7502x_write(sd, data, 5);
+}
+
+static int mlx7502x_write32(struct v4l2_subdev *sd, u16 reg, u32 val)
+{
+	unsigned char data[6];
+
+	put_unaligned_be16(reg, data);
+	put_unaligned_be32(val, data + 2);
+
+	return mlx7502x_write(sd, data, 6);
+}
+
+static int mlx7502x_write_regval(struct v4l2_subdev *sd,
+				 const struct mlx7502x_regval_list *regs, int array_size)
+{
+	int i, ret;
+
+	for (i = 0; i < array_size; i++) {
+		ret = mlx7502x_write8(sd, regs[i].addr, regs[i].data);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int mlx7502x_write_array(struct v4l2_subdev *sd, const u16 *reg, const u8 *data, int size)
+{
+	int i, ret;
+
+	for (i = 0; i < size; i++) {
+		ret = mlx7502x_write8(sd, reg[i], data[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * *********************************************************************************
+ *  PM
+ * *********************************************************************************
+ */
+static int mlx7502x_init(struct v4l2_subdev *sd)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	int ret;
+
+	ret = mlx7502x_write_regval(sd, mlx7502x_common_init_cfg,
+				    ARRAY_SIZE(mlx7502x_common_init_cfg));
+	if (ret < 0) {
+		dev_err(sensor->dev, "failed to write init_cfg\n");
+		return ret;
+	}
+
+	ret = mlx7502x_write_regval(sd, sensor->cur_desc->init_cfg,
+				    sensor->cur_desc->init_cfg_size);
+	if (ret < 0) {
+		dev_err(sensor->dev, "failed to write sensor specific init_cfg\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mlx7502x_power_on(struct mlx7502x *sensor)
+{
+	int ret;
+
+	gpiod_set_value_cansleep(sensor->reset, 0);
+
+	ret = regulator_bulk_enable(MLX7502X_NUM_SUPPLIES, sensor->supplies);
+	if (ret) {
+		dev_err(sensor->dev, "Failed to enable supply: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(sensor->xclk);
+	if (ret) {
+		dev_err(sensor->dev, "Failed to enable external clock: %d\n", ret);
+		goto fail_clk;
+	}
+
+	gpiod_set_value_cansleep(sensor->reset, 1);
+	msleep(MLX7502X_RESET_DELAY_MS);
+
+	dev_dbg(sensor->dev, "power on\n");
+
+	return 0;
+
+fail_clk:
+	regulator_bulk_disable(MLX7502X_NUM_SUPPLIES, sensor->supplies);
+	return ret;
+}
+
+static int mlx7502x_power_off(struct mlx7502x *sensor)
+{
+	gpiod_set_value_cansleep(sensor->reset, 0);
+
+	clk_disable_unprepare(sensor->xclk);
+
+	regulator_bulk_disable(MLX7502X_NUM_SUPPLIES, sensor->supplies);
+
+	dev_dbg(sensor->dev, "power off\n");
+
+	return 0;
+}
+
+static int __maybe_unused mlx7502x_runtime_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	int ret;
+
+	ret = mlx7502x_power_on(sensor);
+	if (ret)
+		return ret;
+
+	return mlx7502x_init(sd);
+}
+
+static int __maybe_unused mlx7502x_runtime_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+
+	mlx7502x_power_off(sensor);
+
+	return 0;
+}
+
+/*
+ * *********************************************************************************
+ *  Subdev operations
+ * *********************************************************************************
+ */
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int mlx7502x_get_register(struct v4l2_subdev *sd,
+				 struct v4l2_dbg_register *reg)
+{
+	u8 val[4];
+	int ret;
+
+	/* v4l2-dbg set it to 0 */
+	if (reg->size == 0)
+		reg->size = 1;
+
+	ret = mlx7502x_read(sd, reg->reg & 0xffff, val, reg->size);
+	if (ret < 0)
+		return ret;
+
+	if (reg->size == 1)
+		reg->val = val[0];
+	else if (reg->size == 2)
+		reg->val = get_unaligned_be16(val);
+	else if (reg->size == 3)
+		reg->val = get_unaligned_be24(val);
+	else if (reg->size == 4)
+		reg->val = get_unaligned_be32(val);
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int mlx7502x_set_register(struct v4l2_subdev *sd,
+				 const struct v4l2_dbg_register *reg)
+{
+	if (reg->size <= 1)
+		return mlx7502x_write8(sd, reg->reg & 0xffff, reg->val);
+	else if (reg->size == 2)
+		return mlx7502x_write16(sd, reg->reg & 0xffff, reg->val);
+	else if (reg->size == 3)
+		return mlx7502x_write24(sd, reg->reg & 0xffff, reg->val);
+	else if (reg->size == 4)
+		return mlx7502x_write32(sd, reg->reg & 0xffff, reg->val);
+
+	return -EINVAL;
+}
+#endif
+
+/* from us into sensor ticks */
+static unsigned int mlx7502x_from_us(struct mlx7502x *sensor, u32 us)
+{
+	return DIV_ROUND_CLOSEST(us * MLX7502X_CLK_RATE_MHZ, sensor->hmax);
+}
+
+static unsigned int mlx7502x_to_us(struct mlx7502x *sensor, u32 reg)
+{
+	return DIV_ROUND_CLOSEST(reg * sensor->hmax, MLX7502X_CLK_RATE_MHZ);
+}
+
+static int mlx7502x_set_link_freq(struct mlx7502x *sensor)
+{
+	struct v4l2_subdev *sd = &sensor->sd;
+	const struct mlx7502x_link_freq_list *lfc = sensor->cur_desc->link_freq_cfg;
+	struct v4l2_mbus_config_mipi_csi2 *bus = &sensor->ep.bus.mipi_csi2;
+	u8 lane_n = bus->num_data_lanes;
+	u32 pretime;
+	int ret;
+
+	/* lane */
+	ret = mlx7502x_write8(sd, MLX7502X_DATA_LANE_CONFIG_REG, lane_n - 1);
+	if (ret < 0)
+		return ret;
+
+	/* link freq */
+	ret = mlx7502x_write_array(sd, lfc->addr,
+				   lfc->data[lane_n >> 2][sensor->link_freq->val],
+				   MLX7502X_LINK_FREQ_REG_N);
+	if (ret < 0)
+		return ret;
+
+	/* clock continuous mode if MIPI receiver requires it */
+	ret = mlx7502x_write8(sd, MLX7502X_CONTINUOUS_REG,
+			      bus->flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? 1u : 0u);
+	if (ret < 0)
+		return ret;
+
+	ret = mlx7502x_write16(sd, MLX7502X_HMAX_REG, sensor->hmax);
+	if (ret < 0)
+		return ret;
+
+	/* timings which depends on hmax and mipi config */
+	ret = mlx7502x_write8(sd, MLX7502X_PLLSETUP_REG,
+			      mlx7502x_from_us(sensor, MLX7502X_PLLSETUP_US)
+			      + MLX7502X_PLLSETUP_TICKS);
+	if (ret < 0)
+		return ret;
+
+	pretime = mlx7502x_from_us(sensor, MLX7502X_PRETIME_US);
+	ret = mlx7502x_write16(sd, MLX7502X_PRETIME_REG, pretime);
+	if (ret < 0)
+		return ret;
+
+	return mlx7502x_write24(sd, MLX7502X_RANDNM0_REG,
+				pretime * sensor->hmax - 1070 - 2098);
+}
+
+static int mlx7502x_update_output_format(struct mlx7502x *sensor)
+{
+	struct v4l2_rect *crop = &sensor->crop;
+	struct v4l2_subdev *sd = &sensor->sd;
+	int ret;
+
+	ret = mlx7502x_write16(sd, MLX7502X_COLUMN_START_REG, crop->left + 1u);
+	if (ret < 0)
+		return ret;
+	ret = mlx7502x_write16(sd, MLX7502X_COLUMN_LEN_REG, crop->width);
+	if (ret < 0)
+		return ret;
+	ret = mlx7502x_write16(sd, MLX7502X_ROW_START_REG, MLX7502X_ROW_START(crop->top));
+	if (ret < 0)
+		return ret;
+	ret = mlx7502x_write16(sd, MLX7502X_ROW_END_REG,
+			       MLX7502X_ROW_END(crop->top, crop->height));
+	if (ret < 0)
+		return ret;
+	ret = mlx7502x_write8(sd, MLX7502X_BINNING_REG,
+			      sensor->binning_mode->reg_value);
+	if (ret < 0)
+		return ret;
+	ret = mlx7502x_write8(sd, MLX7502X_OUTPUT_MODE_REG, sensor->output_mode->val);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int mlx7502x_sw_trigger(struct mlx7502x *sensor)
+{
+	return mlx7502x_write8(&sensor->sd, MLX7502X_SW_TRIGGER_REG, MLX7502X_SW_TRIGGER_TRIG);
+}
+
+static int mlx7502x_sw_trigger_start(struct mlx7502x *sensor)
+{
+	/* this func is called once after streaming was enabled
+	 * we need to skip first trigger as the frame is already generated,
+	 * when streaming was enabled
+	 */
+	sensor->trigger = mlx7502x_sw_trigger;
+	return 0;
+}
+
+static int mlx7502x_hw_trigger(struct mlx7502x *sensor)
+{
+	gpiod_set_value_cansleep(sensor->trig, 0);
+	usleep_range(MLX7502X_TRIGGER_DELAY_US, MLX7502X_TRIGGER_DELAY_US + 10);
+	gpiod_set_value_cansleep(sensor->trig, 1);
+
+	return 0;
+}
+
+static int mlx7502x_cont_trigger(struct mlx7502x *sensor)
+{
+	return -1; /* in continuous mode, we can't use trigger */
+}
+
+static int mlx7502x_check_frame_interval(struct mlx7502x *sensor, struct v4l2_fract fi)
+{
+	u32 req_fi_us, min_fi_us, readout_ticks;
+	int fi_register = 0;
+
+	if (fi.denominator != 0) {
+		readout_ticks = MLX7502X_ROW_END(sensor->crop.top, sensor->crop.height)
+				- MLX7502X_ROW_START(sensor->crop.top) + 1;
+		min_fi_us = MLX7502X_PLLSETUP_US
+			    + mlx7502x_to_us(sensor, MLX7502X_PLLSETUP_TICKS)
+			    + (mlx7502x_to_us(sensor, readout_ticks + MLX7502X_FRAME_ADD_TICKS)
+			       + MLX7502X_PRETIME_US + sensor->tint->val)
+			      * sensor->phase_number->val;
+
+		req_fi_us = MICRO * fi.numerator / fi.denominator;
+
+		if (req_fi_us < min_fi_us) {
+			dev_err(sensor->dev, "Too small frame interval: min = %dus, requested = %dus\n",
+				min_fi_us, req_fi_us);
+			return -EINVAL;
+		}
+
+		fi_register = mlx7502x_from_us(sensor, req_fi_us);
+	}
+
+	return fi_register;
+}
+
+static int mlx7502x_set_trigger_mode(struct mlx7502x *sensor)
+{
+	int ret;
+	struct v4l2_subdev *sd = &sensor->sd;
+	int frame_interval = 0;
+	enum mlx7502x_trigger_mode mode = sensor->trigger_mode->val;
+
+	switch (mode) {
+	case MLX7502X_SOFTWARE:
+		ret = mlx7502x_write_regval(sd, mlx7502x_sw_trigger_cfg,
+					    ARRAY_SIZE(mlx7502x_sw_trigger_cfg));
+		if (ret < 0)
+			break;
+
+		ret = mlx7502x_write8(sd, MLX7502X_SW_TRIGGER_REG, MLX7502X_SW_TRIGGER_DEFAULT);
+		sensor->trigger = mlx7502x_sw_trigger_start;
+
+		break;
+	case MLX7502X_HARDWARE:
+		ret = mlx7502x_write_regval(sd, mlx7502x_hw_trigger_cfg,
+					    ARRAY_SIZE(mlx7502x_hw_trigger_cfg));
+		sensor->trigger = mlx7502x_hw_trigger;
+		break;
+	case MLX7502X_CONTINUOUS:
+		ret = mlx7502x_check_frame_interval(sensor, sensor->frame_interval);
+		if (ret < 0)
+			break;
+
+		frame_interval = ret;
+		ret = mlx7502x_write_regval(sd, mlx7502x_sw_trigger_cfg,
+					    ARRAY_SIZE(mlx7502x_sw_trigger_cfg));
+		if (ret < 0)
+			break;
+
+		ret = mlx7502x_write8(sd, MLX7502X_SW_TRIGGER_REG, MLX7502X_SW_TRIGGER_CONT);
+		sensor->trigger = mlx7502x_cont_trigger;
+		break;
+	default:
+		/* should not be there */
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret < 0)
+		return ret;
+
+	return mlx7502x_write32(sd, MLX7502X_FRAME_TIME_REG, frame_interval);
+}
+
+static int mlx7502x_s_stream(struct v4l2_subdev *sd, int on)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	int ret;
+
+	mutex_lock(&sensor->lock);
+
+	if (on && !sensor->streaming) {
+		ret = pm_runtime_resume_and_get(sensor->dev);
+		if (ret < 0) {
+			mutex_unlock(&sensor->lock);
+			return ret;
+		}
+
+		ret = __v4l2_ctrl_handler_setup(sd->ctrl_handler);
+		if (ret < 0)
+			goto error_stream;
+		/* the registers below depends on hmax, which is configured in controls */
+		ret = mlx7502x_set_link_freq(sensor);
+		if (ret < 0)
+			goto error_stream;
+		ret = mlx7502x_update_output_format(sensor);
+		if (ret < 0)
+			goto error_stream;
+		ret = mlx7502x_set_trigger_mode(sensor);
+		if (ret < 0)
+			goto error_stream;
+
+		ret = mlx7502x_write8(sd, MLX7502X_STREAM_EN_REG, 1u);
+		if (ret < 0)
+			goto error_stream;
+
+		sensor->streaming = 1u;
+		dev_dbg(sensor->dev, "stream enabled\n");
+
+		/* we need to wait to stabilize the system after streaming on */
+		usleep_range(MLX7502X_STREAMING_DELAY_US, MLX7502X_STREAMING_DELAY_US + 10);
+
+		gpiod_set_value_cansleep(sensor->leden, 1);
+	} else if (!on && sensor->streaming) {
+		gpiod_set_value_cansleep(sensor->leden, 0);
+
+		sensor->streaming = 0u;
+		ret = mlx7502x_write8(sd, MLX7502X_STREAM_EN_REG, 0u);
+		dev_dbg(sensor->dev, "stream disabled\n");
+
+		pm_runtime_mark_last_busy(sensor->dev);
+		pm_runtime_put_autosuspend(sensor->dev);
+	}
+
+	mutex_unlock(&sensor->lock);
+	return ret;
+
+error_stream:
+	pm_runtime_put(sensor->dev);
+	mutex_unlock(&sensor->lock);
+	return ret;
+}
+
+static int mlx7502x_get_frame_interval(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_frame_interval *fi)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+
+	fi->interval = sensor->frame_interval;
+
+	return 0;
+}
+
+static int mlx7502x_set_frame_interval(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_frame_interval *frame_interval)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	int fi_register;
+	int ret = 0;
+
+	mutex_lock(&sensor->lock);
+
+	if (sensor->trigger_mode->val != MLX7502X_CONTINUOUS) {
+		dev_err(sensor->dev, "Can't change frame interval in non countinuous mode\n");
+		ret = -EINVAL;
+		goto unlock_mut;
+	}
+
+	fi_register = mlx7502x_check_frame_interval(sensor, frame_interval->interval);
+	if (fi_register < 0) {
+		ret = fi_register;
+		goto unlock_mut;
+	}
+
+	sensor->frame_interval = frame_interval->interval;
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		goto unlock_mut;
+
+	ret = mlx7502x_write32(sd, MLX7502X_FRAME_TIME_REG, fi_register);
+	pm_runtime_put_autosuspend(sensor->dev);
+
+unlock_mut:
+	mutex_unlock(&sensor->lock);
+	return ret;
+}
+
+static int mlx7502x_fill_format(struct mlx7502x *sensor,
+				struct v4l2_mbus_framefmt *format,
+				struct v4l2_rect *src_compose)
+{
+	int width_double;
+
+	mutex_lock(&sensor->lock);
+
+	width_double = sensor->output_mode->val == MLX7502X_RAW_ANB ? 2 : 1;
+
+	memset(format, 0, sizeof(*format));
+	format->code = MEDIA_BUS_FMT_Y12_1X12;
+	format->width = width_double * src_compose->width;
+	format->height = src_compose->height;
+	format->field = V4L2_FIELD_NONE;
+	format->colorspace = V4L2_COLORSPACE_RAW;
+	format->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	format->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	format->xfer_func = V4L2_XFER_FUNC_NONE;
+
+	mutex_unlock(&sensor->lock);
+
+	return 0;
+}
+
+static int mlx7502x_init_cfg(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+
+	/* copying active into try */
+	sd_state->pads->try_crop = sensor->crop;
+	sd_state->pads->try_compose = sensor->compose;
+	return mlx7502x_fill_format(sensor, &sd_state->pads->try_fmt, &sensor->compose);
+}
+
+static int mlx7502x_enum_mbus_code(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index > 0)
+		return -EINVAL;
+
+	code->code = MEDIA_BUS_FMT_Y12_1X12;
+
+	return 0;
+}
+
+static int mlx7502x_set_get_format(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_format *format)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	struct v4l2_rect *src_compose;
+
+	if (format->pad != 0)
+		return -EINVAL;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+		src_compose = &sd_state->pads->try_compose;
+	else
+		src_compose = &sensor->compose;
+
+	return mlx7502x_fill_format(sensor, &format->format, src_compose);
+}
+
+static int mlx7502x_round(u32 flags, u32 val, u32 max, u32 min, u32 step)
+{
+	u32 rounded;
+
+	/* using round_* as step is always power of 2 */
+	if (flags & V4L2_SEL_FLAG_LE)
+		rounded = round_down(val, step);
+	else
+		rounded = round_up(val, step);
+
+	return clamp(rounded, min, max);
+}
+
+static const struct mlx7502x_binning_mode *mlx7502x_find_binning_mode(struct v4l2_rect *dst_crop,
+								      struct v4l2_rect *dst_compose)
+{
+	int i;
+	const struct mlx7502x_binning_mode *bin_mode;
+	u8 ratio = dst_crop->width / dst_compose->width;
+
+	for (i = 0; i < ARRAY_SIZE(binning_mode); i++) {
+		bin_mode = &binning_mode[i];
+
+		if (bin_mode->ratio == ratio)
+			break;
+	}
+
+	return bin_mode;
+}
+
+static void mlx7502x_set_crop(struct mlx7502x *sensor,
+			      struct v4l2_subdev_selection *sel,
+			      struct v4l2_rect *dst_crop,
+			      struct v4l2_rect *dst_compose,
+			      const struct mlx7502x_binning_mode *bin_mode)
+{
+	u32 w, h;
+
+	/* cropping will always be in native size, even if binning was applied */
+	w = sensor->cur_desc->width;
+	h = sensor->cur_desc->height;
+
+	sel->r.width = mlx7502x_round(sel->flags, sel->r.width, w,
+				      bin_mode->width_min, bin_mode->width_step);
+	sel->r.left = mlx7502x_round(sel->flags, sel->r.left, w - sel->r.width,
+				     MLX7502X_LEFT_MIN, MLX7502X_LEFT_STEP);
+
+	sel->r.height = mlx7502x_round(sel->flags, sel->r.height, h,
+				       bin_mode->height_min, bin_mode->height_step);
+	sel->r.top = mlx7502x_round(sel->flags, sel->r.top, h - sel->r.height,
+				    MLX7502X_TOP_MIN, MLX7502X_TOP_STEP);
+	/* fill active or try */
+	*dst_crop = sel->r;
+	/* update binning w/h, as actual crop size could changed */
+	dst_compose->top = 0;
+	dst_compose->left = 0;
+	dst_compose->width = dst_crop->width / bin_mode->ratio;
+	dst_compose->height = dst_crop->height / bin_mode->ratio;
+}
+
+/*
+ * Binning is applied after cropping inside the sensor
+ */
+static void mlx7502x_set_compose(struct mlx7502x *sensor,
+				 struct v4l2_subdev_selection *sel,
+				 struct v4l2_rect *dst_crop,
+				 struct v4l2_rect *dst_compose)
+{
+	const struct mlx7502x_binning_mode *bin_mode;
+	u32 w, h, i, bin_w, bin_h;
+
+	w = dst_crop->width;
+	h = dst_crop->height;
+
+	/* select the best binning */
+	for (i = 0; i < ARRAY_SIZE(binning_mode); i++) {
+		bin_mode = &binning_mode[i];
+
+		if (sel->flags & V4L2_SEL_FLAG_LE) {
+			bin_w = w / bin_mode->ratio;
+			bin_h = h / bin_mode->ratio;
+		} else {/* for GE and KEEP choose lower bin_w/h */
+			bin_w = w / bin_mode[1].ratio + 1;
+			bin_h = h / bin_mode[1].ratio + 1;
+		}
+
+		if (sel->r.width >= bin_w && sel->r.height >= bin_h)
+			break;
+	}
+
+	/* save new binning config */
+	if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		sensor->binning_mode = bin_mode;
+
+	/* update crop step and min, based on new binning */
+	sel->r = *dst_crop;
+	mlx7502x_set_crop(sensor, sel, dst_crop, dst_compose, bin_mode);
+
+	/* resulted format after applying new binning */
+	sel->r = *dst_compose;
+}
+
+static int mlx7502x_set_selection(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state,
+				  struct v4l2_subdev_selection *sel)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	struct v4l2_rect *dst_crop, *dst_compose;
+	const struct mlx7502x_binning_mode *bin_mode;
+	int ret = 0;
+
+	if (sel->pad != 0)
+		return -EINVAL;
+
+	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+		dst_crop = v4l2_subdev_get_try_crop(sd, sd_state, 0);
+		dst_compose = v4l2_subdev_get_try_compose(sd, sd_state, 0);
+		bin_mode = mlx7502x_find_binning_mode(dst_crop, dst_compose);
+	} else {
+		dst_crop = &sensor->crop;
+		dst_compose = &sensor->compose;
+		bin_mode = sensor->binning_mode;
+	}
+
+	mutex_lock(&sensor->lock);
+
+	if (sel->target == V4L2_SEL_TGT_CROP)
+		mlx7502x_set_crop(sensor, sel, dst_crop, dst_compose, bin_mode);
+	else if (sel->target == V4L2_SEL_TGT_COMPOSE) /* actually this is  binning */
+		mlx7502x_set_compose(sensor, sel, dst_crop, dst_compose);
+	else
+		ret = -EINVAL;
+
+	mutex_unlock(&sensor->lock);
+
+	return ret;
+}
+
+static int mlx7502x_get_selection(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state,
+				  struct v4l2_subdev_selection *sel)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	struct v4l2_rect *src_crop, *src_compose;
+	int ret = 0;
+
+	if (sel->pad != 0)
+		return -EINVAL;
+
+	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+		src_crop = v4l2_subdev_get_try_crop(sd, sd_state, 0);
+		src_compose = v4l2_subdev_get_try_compose(sd, sd_state, 0);
+	} else {
+		src_crop = &sensor->crop;
+		src_compose = &sensor->compose;
+	}
+
+	mutex_lock(&sensor->lock);
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP:
+		sel->r = *src_crop;
+		break;
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = sensor->cur_desc->width;
+		sel->r.height = sensor->cur_desc->height;
+		break;
+	case V4L2_SEL_TGT_COMPOSE:
+		sel->r = *src_compose;
+		break;
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = src_crop->width;
+		sel->r.height = src_crop->height;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	mutex_unlock(&sensor->lock);
+
+	return ret;
+}
+
+static const struct v4l2_subdev_core_ops mlx7502x_subdev_core_ops = {
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+	.g_register = mlx7502x_get_register,
+	.s_register = mlx7502x_set_register,
+#endif
+};
+
+static const struct v4l2_subdev_video_ops mlx7502x_subdev_video_ops = {
+	.s_stream = mlx7502x_s_stream,
+	.g_frame_interval = mlx7502x_get_frame_interval,
+	.s_frame_interval = mlx7502x_set_frame_interval,
+};
+
+static const struct v4l2_subdev_pad_ops mlx7502x_subdev_pad_ops = {
+	.init_cfg = mlx7502x_init_cfg,
+	.enum_mbus_code = mlx7502x_enum_mbus_code,
+	.set_fmt = mlx7502x_set_get_format,
+	.get_fmt = mlx7502x_set_get_format,
+	.set_selection = mlx7502x_set_selection,
+	.get_selection = mlx7502x_get_selection,
+};
+
+static const struct v4l2_subdev_ops mlx7502x_subdev_ops = {
+	.core = &mlx7502x_subdev_core_ops,
+	.video = &mlx7502x_subdev_video_ops,
+	.pad = &mlx7502x_subdev_pad_ops,
+};
+
+/*
+ * *********************************************************************************
+ *  Controls
+ * *********************************************************************************
+ */
+static __always_inline u8 mlx7502x_phase_to_reg(u16 phase)
+{
+	/* Note:
+	 *  Sensor is working in mode when: MIX is a referance, and illumination is shifted
+	 *  To compensate it, we need to add 180 degrees
+	 */
+	if (phase != 0 && phase != 180)
+		phase = (phase + 180) % 360;
+
+	return phase / 45;
+}
+
+static int mlx7502x_set_phase_seq(struct mlx7502x *sensor, struct v4l2_ctrl *ctrl)
+{
+	int i, offset;
+	u16 *p_v;
+	u8 val;
+	u8 data[2 + (MLX7502X_PHASE_MAX_NUM >> 1)]; /* address + 4 bytes(2 phases per byte) */
+
+	/* the address */
+	put_unaligned_be16(MLX7502X_PX_PHASE_SHIFT_REG, data);
+
+	/* data itself */
+	offset = 2;
+	p_v = ctrl->p_new.p_u16;
+	for (i = 0; i < MLX7502X_PHASE_MAX_NUM; i += 2) {
+		val = FIELD_PREP(MLX7502X_NIBLE_LOW_MASK, mlx7502x_phase_to_reg(p_v[i]));
+		val |= FIELD_PREP(MLX7502X_NIBLE_HIGH_MASK, mlx7502x_phase_to_reg(p_v[i + 1]));
+
+		data[offset++] = val;
+	}
+
+	return mlx7502x_write(&sensor->sd, data, ARRAY_SIZE(data));
+}
+
+/* comparator for bsearch func */
+static int mlx7502x_fmod_cmp(const void *key, const void *elt)
+{
+	int ret = 1; /* lower then lowest */
+	u8 val = *((u8 *)key);
+	/* need prev elt for range checking, so key requires to be >= 4 */
+	struct mlx7502x_fmod_list *el = (struct mlx7502x_fmod_list *)elt - 1;
+	u8 high = el[0].fmod;
+	u8 low = el[1].fmod;
+
+	if (val > high) /* more then the highest, move to lower index */
+		ret = -1;
+	else if (val >= low) /* we are in a range */
+		ret = 0;
+
+	return ret;
+}
+
+static int mlx7502x_set_fmod(struct mlx7502x *sensor, u8 fmod)
+{
+	int ret;
+	u16 fmod_reg;
+	u8 pll_res_reg;
+	struct v4l2_subdev *sd = &sensor->sd;
+	struct mlx7502x_fmod_list *fc = bsearch(&fmod,
+						mlx7502x_fmod_cfg,
+						ARRAY_SIZE(mlx7502x_fmod_cfg),
+						sizeof(struct mlx7502x_fmod_list),
+						mlx7502x_fmod_cmp);
+
+	ret = mlx7502x_write8(sd, MLX7502X_DIVSELPRE_REG, fc->divselpre);
+	if (ret < 0)
+		return ret;
+
+	ret = mlx7502x_write8(sd, MLX7502X_DIVSEL_REG, fc->divsel);
+	if (ret < 0)
+		return ret;
+
+	fmod_reg = fmod << (fc->divselpre + fc->divsel);
+	ret = mlx7502x_write16(sd, MLX7502X_FMOD_REG, fmod_reg);
+	if (ret < 0)
+		return ret;
+
+	pll_res_reg = fmod_reg < MLX7502X_PLL_RES_THR ? 2 : 0;
+	return mlx7502x_write8(sd, MLX7502X_PLL_RES_REG, pll_res_reg);
+}
+
+static int mlx7502x_set_tint(struct mlx7502x *sensor, u16 tint)
+{
+	int i, offset;
+	u32 tint_reg = mlx7502x_from_us(sensor, tint) * sensor->hmax;
+	u8 data[2 + MLX7502X_PHASE_MAX_NUM * 4]; /* address + 32bytes(4 bytes per phase) */
+
+	/* the address */
+	put_unaligned_be16(MLX7502X_TINT0_REG, data);
+
+	/* data itself */
+	for (i = 0; i < MLX7502X_PHASE_MAX_NUM; i++) {
+		offset = 2 + i * 4;
+		put_unaligned_be32(tint_reg, data + offset);
+	}
+
+	return mlx7502x_write(&sensor->sd, data, ARRAY_SIZE(data));
+}
+
+static int mlx7502x_update_hmax(struct mlx7502x *sensor, int mipi_speed_ind, int output_mode)
+{
+	const struct mlx7502x_link_freq_list *lfc = sensor->cur_desc->link_freq_cfg;
+	int width_double = output_mode == MLX7502X_RAW_ANB ? 1 : 0;
+	u8 lane_n = sensor->ep.bus.mipi_csi2.num_data_lanes;
+	u32 tint_step;
+
+	sensor->hmax = lfc->hmax[lane_n >> 2][mipi_speed_ind][width_double];
+
+	tint_step = mlx7502x_to_us(sensor, 1);
+	return __v4l2_ctrl_modify_range(sensor->tint,
+					tint_step,
+					sensor->tint->maximum,
+					tint_step,
+					sensor->tint->default_value);
+}
+
+static int mlx7502x_hold(struct mlx7502x *sensor, u32 id, u8 hold)
+{
+	int ret = 0;
+
+	if (!sensor->streaming) /* only during streaming */
+		return 0;
+
+	switch (id) {
+	case V4L2_CID_MLX7502X_PHASE_NUMBER:
+	case V4L2_CID_MLX7502X_PHASE_SEQ:
+	case V4L2_CID_MLX7502X_FMOD:
+	case V4L2_CID_MLX7502X_TINT:
+		ret = mlx7502x_write8(&sensor->sd, MLX7502X_PARAM_HOLD_REG, hold);
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int mlx7502x_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+	int ret;
+	u8 val;
+	struct mlx7502x *sensor = container_of(ctrl->handler,
+					struct mlx7502x, ctrl_handler);
+	struct v4l2_subdev *sd = &sensor->sd;
+
+	if (ctrl->id != V4L2_CID_MLX7502X_TEMPERATURE)
+		return -EINVAL;
+
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		return 0;
+
+	ret = mlx7502x_read(sd, MLX7502X_TEMPERATURE_REG, &val, 1);
+	ctrl->val = val - 40;
+
+	pm_runtime_put_autosuspend(sensor->dev);
+
+	return ret;
+}
+
+static int mlx7502x_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct mlx7502x *sensor = container_of(ctrl->handler,
+					struct mlx7502x, ctrl_handler);
+	struct v4l2_subdev *sd = &sensor->sd;
+	int ret = 0;
+
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		return 0;
+
+	ret = mlx7502x_hold(sensor, ctrl->id, 1);
+	if (ret < 0)
+		return ret;
+
+	switch (ctrl->id) {
+	case V4L2_CID_MLX7502X_PHASE_NUMBER:
+		ret = mlx7502x_write8(sd, MLX7502X_PHASE_COUNT_REG, ctrl->val);
+		break;
+	case V4L2_CID_MLX7502X_PHASE_SEQ:
+		ret = mlx7502x_set_phase_seq(sensor, ctrl);
+		break;
+	case V4L2_CID_MLX7502X_FMOD:
+		ret = mlx7502x_set_fmod(sensor, ctrl->val);
+		break;
+	case V4L2_CID_MLX7502X_TINT:
+		ret = mlx7502x_set_tint(sensor, ctrl->val);
+		break;
+	case V4L2_CID_MLX7502X_TRIGGER_MODE:
+		if (ctrl->val == MLX7502X_CONTINUOUS)
+			ret = mlx7502x_check_frame_interval(sensor, sensor->frame_interval);
+		break;
+	case V4L2_CID_MLX7502X_TRIGGER:
+		ret = sensor->trigger(sensor);
+		break;
+	case V4L2_CID_MLX7502X_OUTPUT_MODE:
+		ret = mlx7502x_update_hmax(sensor,
+					   sensor->link_freq->val,
+					   ctrl->val);
+		break;
+	case V4L2_CID_LINK_FREQ:
+		ret = mlx7502x_update_hmax(sensor,
+					   ctrl->val,
+					   sensor->output_mode->val);
+		break;
+	case V4L2_CID_VFLIP:
+		ret = mlx7502x_write8(sd, MLX7502X_VFLIP_REG, ctrl->val);
+		break;
+	case V4L2_CID_HFLIP:
+		ret = mlx7502x_write8(sd, MLX7502X_HFLIP_REG, ctrl->val);
+		break;
+	default:
+		dev_err(sensor->dev, "Unknown id: %x\n", ctrl->id);
+		break;
+	}
+
+	mlx7502x_hold(sensor, ctrl->id, 0);
+	pm_runtime_put_autosuspend(sensor->dev);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops mlx7502x_ctrl_ops = {
+	.s_ctrl = mlx7502x_s_ctrl,
+	.g_volatile_ctrl = mlx7502x_g_volatile_ctrl,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_phase_number = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_PHASE_NUMBER,
+	.name = "Number of phases",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = 1,
+	.max = MLX7502X_PHASE_MAX_NUM,
+	.step = 1,
+	.def = 4,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_phase_sequence = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_PHASE_SEQ,
+	.name = "Sequence of phases",
+	.type = V4L2_CTRL_TYPE_U16,
+	.min = 0,
+	.max = 315,
+	.step = 45,
+	.def = 0,
+	.elem_size = sizeof(u16),
+	.dims = { 8 },
+};
+
+static const struct v4l2_ctrl_config mlx7502x_fmod = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_FMOD,
+	.name = "Modulation fraquency",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = 4,
+	.max = 100,
+	.step = 1, /* in MHz */
+	.def = 40,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_tint = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_TINT,
+	.name = "Time integration",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = 5,
+	.max = 1000,
+	.step = 1, /* in us */
+	.def = 198,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_trigger_mode = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_TRIGGER_MODE,
+	.name = "Trigger mode",
+	.type = V4L2_CTRL_TYPE_MENU,
+	.max = ARRAY_SIZE(mlx7502x_ctrl_trigger_mode) - 1,
+	.def = 2,
+	.qmenu = mlx7502x_ctrl_trigger_mode,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_trigger = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_TRIGGER,
+	.name = "Send trigger",
+	.type = V4L2_CTRL_TYPE_BUTTON,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_output_mode = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_OUTPUT_MODE,
+	.name = "Output mode",
+	.type = V4L2_CTRL_TYPE_MENU,
+	.max = ARRAY_SIZE(mlx7502x_ctrl_output) - 1,
+	.def = 0,
+	.qmenu = mlx7502x_ctrl_output,
+};
+
+static const struct v4l2_ctrl_config mlx7502x_temperature = {
+	.ops = &mlx7502x_ctrl_ops,
+	.id = V4L2_CID_MLX7502X_TEMPERATURE,
+	.name = "Temperature",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
+	.min = -40,
+	.max = 125,
+	.step = 1,
+};
+
+static int mlx7502x_link_freq_init(struct mlx7502x *sensor)
+{
+	struct v4l2_ctrl_handler *hdl = &sensor->ctrl_handler;
+	const s64 *link_freq = sensor->cur_desc->link_freq_cfg->link_freq;
+	int i, j, link_freq_mask;
+
+	link_freq_mask = 0;
+	for (i = 0; i < sensor->ep.nr_of_link_frequencies; i++) {
+		for (j = 0; j < MLX7502X_LINK_FREQ_N; j++) {
+			if (link_freq[j] == sensor->ep.link_frequencies[i])
+				break;
+		}
+
+		if (j == MLX7502X_LINK_FREQ_N) {
+			dev_err(sensor->dev, "no link frequency %lld supported\n", link_freq[j]);
+			return -EINVAL;
+		}
+
+		link_freq_mask |= (1 << j);
+	}
+
+	sensor->link_freq = v4l2_ctrl_new_int_menu(hdl, &mlx7502x_ctrl_ops,
+						   V4L2_CID_LINK_FREQ,
+						   MLX7502X_LINK_FREQ_N - 1,
+						   __fls(link_freq_mask),
+						   link_freq);
+
+	sensor->link_freq->menu_skip_mask = ~link_freq_mask;
+
+	return 0;
+}
+
+static int mlx7502x_ctrls_init(struct mlx7502x *sensor)
+{
+	struct v4l2_ctrl_handler *hdl;
+	struct v4l2_ctrl *ctrl;
+	int ret = 0;
+
+	hdl = &sensor->ctrl_handler;
+
+	ret = v4l2_ctrl_handler_init(hdl, 9);
+	if (ret) {
+		dev_err(sensor->dev, "Failed to init handler - %d\n", ret);
+		return ret;
+	}
+
+	ret = mlx7502x_link_freq_init(sensor);
+	if (ret)
+		goto error_ctrls;
+
+	v4l2_ctrl_new_std(hdl, &mlx7502x_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(hdl, &mlx7502x_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
+
+	v4l2_ctrl_new_custom(hdl, &mlx7502x_fmod, NULL);
+	v4l2_ctrl_new_custom(hdl, &mlx7502x_trigger, NULL);
+	v4l2_ctrl_new_custom(hdl, &mlx7502x_temperature, NULL);
+	ctrl = v4l2_ctrl_new_custom(hdl, &mlx7502x_phase_sequence, NULL);
+	/* init custom control(which has custom payload) */
+	memcpy(ctrl->p_cur.p_u16, mlx7502x_ctrl_phase_sequence,
+	       2 * ARRAY_SIZE(mlx7502x_ctrl_phase_sequence));
+
+	sensor->tint = v4l2_ctrl_new_custom(hdl, &mlx7502x_tint, NULL);
+	sensor->output_mode = v4l2_ctrl_new_custom(hdl, &mlx7502x_output_mode, NULL);
+	sensor->phase_number = v4l2_ctrl_new_custom(hdl, &mlx7502x_phase_number, NULL);
+	sensor->trigger_mode = v4l2_ctrl_new_custom(hdl, &mlx7502x_trigger_mode, NULL);
+	/* if hardware gpio is absent, then skip this config in ctrl */
+	if (!sensor->trig)
+		sensor->trigger_mode->menu_skip_mask = BIT(MLX7502X_HARDWARE);
+
+	if (hdl->error)	{
+		dev_err(sensor->dev, "Error %d while adding controls\n", hdl->error);
+		ret = hdl->error;
+		goto error_ctrls;
+	}
+
+	sensor->sd.ctrl_handler = hdl;
+	return 0;
+
+error_ctrls:
+	v4l2_ctrl_handler_free(hdl);
+	return ret;
+}
+
+/*
+ * *********************************************************************************
+ *  Probing
+ * *********************************************************************************
+ */
+static int mlx7502x_detect(struct v4l2_subdev *sd)
+{
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+	const struct mlx7502x_regval_list *reg;
+	int ret, i;
+	u8 val;
+
+	for (i = 0; i < ARRAY_SIZE(mlx7502x_detect_cfg); i++) {
+		reg = &mlx7502x_detect_cfg[i];
+
+		ret = mlx7502x_read(sd, reg->addr, &val, 1);
+		if (ret < 0) {
+			dev_err(sensor->dev, "Coudn't read any data\n");
+			return ret;
+		}
+
+		if (val != reg->data) {
+			dev_err(sensor->dev, "Coudn't find the sensor\n");
+			return -EIO;
+		}
+	}
+
+	ret = mlx7502x_read(sd, MLX7502X_SENSOR_ID_REG, &val, 1);
+	if (ret < 0) {
+		dev_err(sensor->dev, "Coudn't read any data\n");
+		return ret;
+	}
+
+	/* autodetect sensor type */
+	if ((val & MLX7502X_75026_ID) != 0)
+		sensor->cur_desc = &mlx75026;
+	else
+		sensor->cur_desc = &mlx75027;
+
+	return 0;
+}
+
+static int mlx7502x_parse_dt(struct mlx7502x *sensor, struct device *dev)
+{
+	struct fwnode_handle *endpoint;
+	int ret, num_data_lanes, i;
+	u32 xclk_rate;
+
+	for (i = 0; i < MLX7502X_NUM_SUPPLIES; i++)
+		sensor->supplies[i].supply = mlx7502x_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, MLX7502X_NUM_SUPPLIES,
+				      sensor->supplies);
+	if (ret)
+		return ret;
+
+	sensor->xclk = devm_clk_get(dev, NULL);
+	if (IS_ERR(sensor->xclk)) {
+		dev_err(dev, "failed to get xclk\n");
+		return PTR_ERR(sensor->xclk);
+	}
+
+	xclk_rate = clk_get_rate(sensor->xclk);
+	if (xclk_rate != MLX7502X_XCLK_RATE) {
+		dev_err(dev, "only 8MHz clock is supported\n");
+		return -EINVAL;
+	}
+
+	sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(sensor->reset)) {
+		dev_err(dev, "failed to get reset\n");
+		return PTR_ERR(sensor->reset);
+	}
+
+	sensor->leden = devm_gpiod_get_optional(dev, "melexis,leden", GPIOD_OUT_LOW);
+	if (IS_ERR(sensor->leden)) {
+		dev_err(dev, "failed to get leden\n");
+		return PTR_ERR(sensor->leden);
+	}
+
+	sensor->trig = devm_gpiod_get_optional(dev, "melexis,trig", GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->trig)) {
+		dev_err(dev, "failed to get trig\n");
+		return PTR_ERR(sensor->trig);
+	}
+
+	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+	if (!endpoint) {
+		dev_err(dev, "endpoint node not found\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &sensor->ep);
+	fwnode_handle_put(endpoint);
+	if (ret < 0) {
+		dev_err(dev, "parsing endpoint node failed\n");
+		return ret;
+	}
+
+	if (sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
+		dev_err(dev, "invalid bus type, must be MIPI CSI2\n");
+		ret = -EINVAL;
+		goto dt_ep_error;
+	}
+
+	num_data_lanes = sensor->ep.bus.mipi_csi2.num_data_lanes;
+	if (num_data_lanes != 2 && num_data_lanes != 4) {
+		dev_err(dev, "invalid num_data_lanes, must be 2 or 4. But it was %d\n",
+			num_data_lanes);
+		ret = -EINVAL;
+		goto dt_ep_error;
+	}
+
+	if (!sensor->ep.nr_of_link_frequencies) {
+		dev_err(dev, "missing link frequencies property\n");
+		ret = -EINVAL;
+		goto dt_ep_error;
+	}
+
+	return 0;
+
+dt_ep_error:
+	v4l2_fwnode_endpoint_free(&sensor->ep);
+	return ret;
+}
+
+static int mlx7502x_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct mlx7502x *sensor;
+	struct v4l2_subdev *sd;
+	int ret;
+	bool power_en;
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->dev = dev;
+	sd = &sensor->sd;
+
+	ret = mlx7502x_parse_dt(sensor, dev);
+	if (ret < 0)
+		return ret;
+
+	v4l2_i2c_subdev_init(sd, client, &mlx7502x_subdev_ops);
+
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
+	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
+	if (ret < 0)
+		goto error_ep;
+
+	mutex_init(&sensor->lock);
+
+	if (id->driver_data) {	/* Chip type is already known */
+		sensor->cur_desc = (void *)id->driver_data;
+		/*
+		 * the sensor will not be powered on in this case
+		 * CONFIG_PM is required to be able to use the driver then
+		 */
+		power_en = false;
+	} else {
+		power_en = true;
+		ret = mlx7502x_power_on(sensor);
+		if (ret < 0)
+			goto error_init;
+
+		ret = mlx7502x_detect(sd);
+		if (ret < 0)
+			goto error_init;
+
+		/* we need to write map into sensor, as we power on manually */
+		ret = mlx7502x_init(sd);
+		if (ret < 0)
+			goto error_init;
+
+		/* mark that device was power on */
+		pm_runtime_set_active(dev);
+	}
+
+	/* initial configurations and controls depends on sensor type */
+	sensor->frame_interval.numerator = 1;
+	sensor->frame_interval.denominator = MLX7502X_DEFAULT_FRAME_RATE;
+	sensor->crop.top = 0;
+	sensor->crop.left = 0;
+	sensor->crop.width = sensor->cur_desc->width;
+	sensor->crop.height = sensor->cur_desc->height;
+	sensor->compose = sensor->crop;
+	sensor->binning_mode = &binning_mode[0];
+
+	ret = mlx7502x_ctrls_init(sensor);
+	if (ret < 0)
+		goto error_init;
+
+	sensor->ctrl_handler.lock = &sensor->lock;
+
+	pm_runtime_enable(dev);
+	/* increase counter wo calling power on */
+	if (power_en)
+		pm_runtime_get_noresume(dev);
+
+	ret = v4l2_async_register_subdev(sd);
+	if (ret < 0)
+		goto error_pm;
+
+	/* configure to use autosuspend, delay 1s */
+	pm_runtime_set_autosuspend_delay(dev, 1000);
+	pm_runtime_use_autosuspend(dev);
+
+	if (power_en) {
+		/* autosuspend starting from now */
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+	}
+
+	dev_dbg(dev, "Melexis ToF camera driver probed\n");
+
+	return 0;
+
+error_pm:
+	if (power_en)
+		pm_runtime_put_noidle(dev);
+
+	pm_runtime_disable(dev);
+error_init:
+	if (power_en)
+		mlx7502x_power_off(sensor);
+
+	media_entity_cleanup(&sd->entity);
+	mutex_destroy(&sensor->lock);
+error_ep:
+	v4l2_fwnode_endpoint_free(&sensor->ep);
+	return ret;
+}
+
+static int mlx7502x_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct mlx7502x *sensor = to_mlx7502x(sd);
+
+	gpiod_set_value_cansleep(sensor->leden, 0);
+
+	pm_runtime_disable(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev))
+		mlx7502x_power_off(sensor);
+	pm_runtime_set_suspended(&client->dev);
+
+	v4l2_fwnode_endpoint_free(&sensor->ep);
+	v4l2_async_unregister_subdev(&sensor->sd);
+	v4l2_ctrl_handler_free(&sensor->ctrl_handler);
+
+	media_entity_cleanup(&sensor->sd.entity);
+	mutex_destroy(&sensor->lock);
+	return 0;
+}
+
+static const struct dev_pm_ops mlx7502x_pm_ops = {
+	SET_RUNTIME_PM_OPS(mlx7502x_runtime_suspend, mlx7502x_runtime_resume, NULL)
+};
+
+static const struct of_device_id mlx7502x_of_match[] = {
+	{ .compatible = "melexis,mlx7502x", .data = 0 }, /* auto-detection */
+	{ .compatible = "melexis,mlx75026", .data = &mlx75026 },
+	{ .compatible = "melexis,mlx75027", .data = &mlx75027 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mlx7502x_of_match);
+
+static const struct i2c_device_id mlx7502x_id[] = {
+	{ "mlx7502x", (kernel_ulong_t)0 }, /* auto-detection */
+	{ "mlx75026", (kernel_ulong_t)&mlx75026 },
+	{ "mlx75027", (kernel_ulong_t)&mlx75027 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mlx7502x_id);
+
+static struct i2c_driver mlx7502x_driver = {
+	.driver = {
+		.of_match_table = of_match_ptr(mlx7502x_of_match),
+		.name = "mlx7502x",
+		.pm = &mlx7502x_pm_ops,
+	},
+	.probe = mlx7502x_probe,
+	.remove = mlx7502x_remove,
+	.id_table = mlx7502x_id,
+};
+
+module_i2c_driver(mlx7502x_driver);
+
+MODULE_AUTHOR("Andrii Kyselov <ays@melexis.com>");
+MODULE_AUTHOR("Volodymyr Kharuk <vkh@melexis.com>");
+MODULE_DESCRIPTION("A low-level driver for Melexis TOF sensors");
+MODULE_LICENSE("GPL");
-- 
BR,
Volodymyr Kharuk

Re: [PATCH v2 6/6] media: i2c: Add driver for mlx7502x ToF sensor

[kernel test robot]

Hi Volodymyr,

I love your patch! Perhaps something to improve:

[auto build test WARNING on d8e8aa866ed8636fd6c1017c3d9453eab2922496]

url:    https://github.com/intel-lab-lkp/linux/commits/Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
base:   d8e8aa866ed8636fd6c1017c3d9453eab2922496
config: parisc-randconfig-s041-20220715 (https://download.01.org/0day-ci/archive/20220717/202207170928.qeUikXoT-lkp@intel.com/config)
compiler: hppa64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/59a691c2f449518699a328ea663098aa3ae038b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
        git checkout 59a691c2f449518699a328ea663098aa3ae038b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=parisc64 SHELL=/bin/bash drivers/media/i2c/ drivers/pci/ kernel/trace/ sound/soc/codecs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/media/i2c/mlx7502x.c:1989:53: sparse: sparse: Using plain integer as NULL pointer

vim +1989 drivers/media/i2c/mlx7502x.c

  1987	
  1988	static const struct of_device_id mlx7502x_of_match[] = {
> 1989		{ .compatible = "melexis,mlx7502x", .data = 0 }, /* auto-detection */
  1990		{ .compatible = "melexis,mlx75026", .data = &mlx75026 },
  1991		{ .compatible = "melexis,mlx75027", .data = &mlx75027 },
  1992		{ /* sentinel */ }
  1993	};
  1994	MODULE_DEVICE_TABLE(of, mlx7502x_of_match);
  1995	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 6/6] media: i2c: Add driver for mlx7502x ToF sensor

[kernel test robot]

Hi Volodymyr,

I love your patch! Perhaps something to improve:

[auto build test WARNING on d8e8aa866ed8636fd6c1017c3d9453eab2922496]

url:    https://github.com/intel-lab-lkp/linux/commits/Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
base:   d8e8aa866ed8636fd6c1017c3d9453eab2922496
config: parisc-randconfig-s041-20220715 (https://download.01.org/0day-ci/archive/20220718/202207180729.RLw6oeM7-lkp@intel.com/config)
compiler: hppa64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/59a691c2f449518699a328ea663098aa3ae038b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
        git checkout 59a691c2f449518699a328ea663098aa3ae038b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=parisc64 SHELL=/bin/bash drivers/media/i2c/ drivers/pci/ kernel/trace/ sound/soc/codecs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/media/i2c/mlx7502x.c:1989:53: sparse: sparse: Using plain integer as NULL pointer

vim +1989 drivers/media/i2c/mlx7502x.c

  1987	
  1988	static const struct of_device_id mlx7502x_of_match[] = {
> 1989		{ .compatible = "melexis,mlx7502x", .data = 0 }, /* auto-detection */
  1990		{ .compatible = "melexis,mlx75026", .data = &mlx75026 },
  1991		{ .compatible = "melexis,mlx75027", .data = &mlx75027 },
  1992		{ /* sentinel */ }
  1993	};
  1994	MODULE_DEVICE_TABLE(of, mlx7502x_of_match);
  1995	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 6/6] media: i2c: Add driver for mlx7502x ToF sensor

[kernel test robot]

Hi Volodymyr,

I love your patch! Perhaps something to improve:

[auto build test WARNING on d8e8aa866ed8636fd6c1017c3d9453eab2922496]

url:    https://github.com/intel-lab-lkp/linux/commits/Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
base:   d8e8aa866ed8636fd6c1017c3d9453eab2922496
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220718/202207181432.DwoL0GPb-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 07022e6cf9b5b3baa642be53d0b3c3f1c403dbfd)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/59a691c2f449518699a328ea663098aa3ae038b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
        git checkout 59a691c2f449518699a328ea663098aa3ae038b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/media/i2c/ drivers/net/pcs/ kernel/trace/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/mlx7502x.c:981:13: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           } else if (!on && sensor->streaming) {
                      ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/i2c/mlx7502x.c:993:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/media/i2c/mlx7502x.c:981:9: note: remove the 'if' if its condition is always true
           } else if (!on && sensor->streaming) {
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/media/i2c/mlx7502x.c:981:13: warning: variable 'ret' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized]
           } else if (!on && sensor->streaming) {
                      ^~~
   drivers/media/i2c/mlx7502x.c:993:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/media/i2c/mlx7502x.c:981:13: note: remove the '&&' if its condition is always true
           } else if (!on && sensor->streaming) {
                      ^~~~~~
   drivers/media/i2c/mlx7502x.c:945:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   2 warnings generated.


vim +981 drivers/media/i2c/mlx7502x.c

   941	
   942	static int mlx7502x_s_stream(struct v4l2_subdev *sd, int on)
   943	{
   944		struct mlx7502x *sensor = to_mlx7502x(sd);
   945		int ret;
   946	
   947		mutex_lock(&sensor->lock);
   948	
   949		if (on && !sensor->streaming) {
   950			ret = pm_runtime_resume_and_get(sensor->dev);
   951			if (ret < 0) {
   952				mutex_unlock(&sensor->lock);
   953				return ret;
   954			}
   955	
   956			ret = __v4l2_ctrl_handler_setup(sd->ctrl_handler);
   957			if (ret < 0)
   958				goto error_stream;
   959			/* the registers below depends on hmax, which is configured in controls */
   960			ret = mlx7502x_set_link_freq(sensor);
   961			if (ret < 0)
   962				goto error_stream;
   963			ret = mlx7502x_update_output_format(sensor);
   964			if (ret < 0)
   965				goto error_stream;
   966			ret = mlx7502x_set_trigger_mode(sensor);
   967			if (ret < 0)
   968				goto error_stream;
   969	
   970			ret = mlx7502x_write8(sd, MLX7502X_STREAM_EN_REG, 1u);
   971			if (ret < 0)
   972				goto error_stream;
   973	
   974			sensor->streaming = 1u;
   975			dev_dbg(sensor->dev, "stream enabled\n");
   976	
   977			/* we need to wait to stabilize the system after streaming on */
   978			usleep_range(MLX7502X_STREAMING_DELAY_US, MLX7502X_STREAMING_DELAY_US + 10);
   979	
   980			gpiod_set_value_cansleep(sensor->leden, 1);
 > 981		} else if (!on && sensor->streaming) {
   982			gpiod_set_value_cansleep(sensor->leden, 0);
   983	
   984			sensor->streaming = 0u;
   985			ret = mlx7502x_write8(sd, MLX7502X_STREAM_EN_REG, 0u);
   986			dev_dbg(sensor->dev, "stream disabled\n");
   987	
   988			pm_runtime_mark_last_busy(sensor->dev);
   989			pm_runtime_put_autosuspend(sensor->dev);
   990		}
   991	
   992		mutex_unlock(&sensor->lock);
   993		return ret;
   994	
   995	error_stream:
   996		pm_runtime_put(sensor->dev);
   997		mutex_unlock(&sensor->lock);
   998		return ret;
   999	}
  1000	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 6/6] media: i2c: Add driver for mlx7502x ToF sensor

[kernel test robot]

Hi Volodymyr,

I love your patch! Perhaps something to improve:

[auto build test WARNING on d8e8aa866ed8636fd6c1017c3d9453eab2922496]

url:    https://github.com/intel-lab-lkp/linux/commits/Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
base:   d8e8aa866ed8636fd6c1017c3d9453eab2922496
config: parisc-randconfig-s041-20220715 (https://download.01.org/0day-ci/archive/20220719/202207191647.RnqnKcMF-lkp@intel.com/config)
compiler: hppa64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/59a691c2f449518699a328ea663098aa3ae038b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Volodymyr-Kharuk/media-i2c-mlx7502x-ToF-camera-support/20220714-163709
        git checkout 59a691c2f449518699a328ea663098aa3ae038b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=parisc64 SHELL=/bin/bash drivers/media/i2c/ drivers/pci/ kernel/trace/ sound/soc/codecs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/media/i2c/mlx7502x.c:1989:53: sparse: sparse: Using plain integer as NULL pointer

vim +1989 drivers/media/i2c/mlx7502x.c

  1987	
  1988	static const struct of_device_id mlx7502x_of_match[] = {
> 1989		{ .compatible = "melexis,mlx7502x", .data = 0 }, /* auto-detection */
  1990		{ .compatible = "melexis,mlx75026", .data = &mlx75026 },
  1991		{ .compatible = "melexis,mlx75027", .data = &mlx75027 },
  1992		{ /* sentinel */ }
  1993	};
  1994	MODULE_DEVICE_TABLE(of, mlx7502x_of_match);
  1995	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp