linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] media: imx8qxp: add parallel camera support
@ 2025-07-08 17:48 Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Frank Li via B4 Relay @ 2025-07-08 17:48 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-media, imx, linux-arm-kernel, linux-kernel, devicetree,
	Frank Li, Alice Yuan, Robert Chiras, Zhipeng Wang

Add parallel camera support for i.MX8 chips.

The below patch to add new format support to test ov5640 sensor
   media: nxp: isi: add support for UYVY8_2X8 and YUYV8_2X8 bus codes

The bindings and driver for parallel CSI
   dt-bindings: media: add i.MX parallel csi support
   media: nxp: add V4L2 subdev driver for parallel CSI

DTS part need depend on previous MIPI CSI patches.
  https://lore.kernel.org/imx/20250522-8qxp_camera-v5-13-d4be869fdb7e@nxp.com/

  arm64: dts: imx8: add parellel csi nodes
  arm64: dts: imx8qxp-mek: add parallel ov5640 camera support

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v3:
- replace CSI with CPI.
- detail change see each patch's change logs
- Link to v2: https://lore.kernel.org/r/20250703-imx8qxp_pcam-v2-0-188be85f06f1@nxp.com

Changes in v2:
- remove patch media: nxp: isi: add support for UYVY8_2X8 and YUYV8_2X8 bus codes
  because pcif controller convert 2x8 to 1x16 to match isi's input
- rename comaptible string to fsl,imx8qxp-pcif
- See each patches's change log for detail
- Link to v1: https://lore.kernel.org/r/20250630-imx8qxp_pcam-v1-0-eccd38d99201@nxp.com

---
Alice Yuan (2):
      dt-bindings: media: add i.MX parallel CPI support
      media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)

Frank Li (2):
      arm64: dts: imx8: add camera parallel interface (CPI) node
      arm64: dts: imx8qxp-mek: add parallel ov5640 camera support

 .../devicetree/bindings/media/fsl,imx93-pcif.yaml  | 126 +++
 MAINTAINERS                                        |   2 +
 arch/arm64/boot/dts/freescale/Makefile             |   3 +
 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi     |  13 +
 .../boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso |  82 ++
 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi  |  27 +
 drivers/media/platform/nxp/Kconfig                 |  11 +
 drivers/media/platform/nxp/Makefile                |   1 +
 drivers/media/platform/nxp/imx-parallel-cpi.c      | 920 +++++++++++++++++++++
 9 files changed, 1185 insertions(+)
---
base-commit: 93355cfe8aec9e47fc93fbc940f1bbeedd62e249
change-id: 20250626-imx8qxp_pcam-d851238343c3

Best regards,
--
Frank Li <Frank.Li@nxp.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support
  2025-07-08 17:48 [PATCH v3 0/4] media: imx8qxp: add parallel camera support Frank Li via B4 Relay
@ 2025-07-08 17:48 ` Frank Li via B4 Relay
  2025-07-09  8:22   ` Krzysztof Kozlowski
  2025-07-24 22:15   ` Laurent Pinchart
  2025-07-08 17:48 ` [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI) Frank Li via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Frank Li via B4 Relay @ 2025-07-08 17:48 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-media, imx, linux-arm-kernel, linux-kernel, devicetree,
	Frank Li, Alice Yuan

From: Alice Yuan <alice.yuan@nxp.com>

Document the binding for parallel CPI controller found in i.MX8QXP, i.MX93
and i.MX91 SoCs.

Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change in v3:
- use enum at compatible string
- add ref to video-interfaces.yaml#
- use cpi as node name in examples.
- replace csi (Camera Serial Interface) with CPI (Camera Parallel Interface)
in commit message.

Change in v2:
- use pcif surfix as Laurent Pinchart's suggest.
- put power-domains into required list
---
 .../devicetree/bindings/media/fsl,imx93-pcif.yaml  | 126 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 127 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..f92e9af371c91636cef42e48573198d4ea5e15f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/fsl,imx93-pcif.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX8/9 Parallel Camera Interface
+
+maintainers:
+  - Frank Li <Frank.Li@nxp.com>
+
+description: |
+  This is device node for the Parallel Camera Interface which enables the
+  chip to connect directly to external Parallel CMOS image sensors.
+  Supports up to 80MHz input clock from sensor.
+  Supports the following input data formats
+    - 8-bit/10-bit Camera Sensor Interface (CSI)
+    - 8-bit data port for RGB, YCbCr, and YUV data input
+    - 8-bit/10-bit data ports for Bayer data input
+  Parallel Camera Interface is hooked to the Imaging subsystem via the
+  Pixel Link.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - fsl,imx8qxp-pcif
+          - fsl,imx93-pcif
+      - items:
+          - enum:
+              - fsl,imx91-pcif
+          - const: fsl,imx93-pcif
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: pixel
+      - const: ipg
+
+  power-domains:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        unevaluatedProperties: false
+        description: Input port node.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              bus-type:
+                const: 5
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        unevaluatedProperties: false
+        description: Output port node.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              bus-type:
+                const: 5
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx93-clock.h>
+    #include <dt-bindings/power/fsl,imx93-power.h>
+
+    cpi@4ac10070 {
+        compatible = "fsl,imx93-pcif";
+        reg = <0x4ac10070 0x10>;
+        clocks = <&clk IMX93_CLK_MIPI_CSI_GATE>,
+                 <&clk IMX93_CLK_MEDIA_APB>;
+        clock-names = "pixel", "ipg";
+        assigned-clocks = <&clk IMX93_CLK_CAM_PIX>;
+        assigned-clock-parents = <&clk IMX93_CLK_VIDEO_PLL>;
+        assigned-clock-rates = <140000000>;
+        power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_MIPI_CSI>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+
+                endpoint {
+                    remote-endpoint = <&mt9m114_ep>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                endpoint {
+                    remote-endpoint = <&isi_in>;
+                };
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 8dc0f6609d1fe2e3eefd50088dbe566d9e107bfa..8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15107,6 +15107,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media.git
 F:	Documentation/admin-guide/media/imx7.rst
+F:	Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
  2025-07-08 17:48 [PATCH v3 0/4] media: imx8qxp: add parallel camera support Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
@ 2025-07-08 17:48 ` Frank Li via B4 Relay
  2025-07-18 16:44   ` Frank Li
  2025-07-25  0:04   ` Laurent Pinchart
  2025-07-08 17:48 ` [PATCH v3 3/4] arm64: dts: imx8: add camera parallel interface (CPI) node Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 4/4] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support Frank Li via B4 Relay
  3 siblings, 2 replies; 11+ messages in thread
From: Frank Li via B4 Relay @ 2025-07-08 17:48 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-media, imx, linux-arm-kernel, linux-kernel, devicetree,
	Frank Li, Alice Yuan, Robert Chiras, Zhipeng Wang

From: Alice Yuan <alice.yuan@nxp.com>

Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
and enables image data capture through a parallel interface.

Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- replace csi with cpi
- use __free(fwnode_handle) to simpilfy code
- remove imx91 driver data, which is the same as imx93

change in v2
- remove MODULE_ALIAS
- use devm_pm_runtime_enable() and cleanup remove function
- change output format to 1x16. controller convert 2x8 to 1x16 format
---
 MAINTAINERS                                   |   1 +
 drivers/media/platform/nxp/Kconfig            |  11 +
 drivers/media/platform/nxp/Makefile           |   1 +
 drivers/media/platform/nxp/imx-parallel-cpi.c | 920 ++++++++++++++++++++++++++
 4 files changed, 933 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303..14842a3b860a6f23846f12a684eedcbb9eb69e19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15112,6 +15112,7 @@ F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
 F:	drivers/media/platform/nxp/imx-mipi-csis.c
+F:	drivers/media/platform/nxp/imx-parallel-cpi.c
 F:	drivers/media/platform/nxp/imx7-media-csi.c
 F:	drivers/media/platform/nxp/imx8mq-mipi-csi2.c
 
diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
index 40e3436669e213fdc5da70821dc0b420e1821f4f..504ae1c6494f331c16124a224421ac7acd433ba5 100644
--- a/drivers/media/platform/nxp/Kconfig
+++ b/drivers/media/platform/nxp/Kconfig
@@ -39,6 +39,17 @@ config VIDEO_IMX_MIPI_CSIS
 	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
 	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
 
+config VIDEO_IMX_PARALLEL_CPI
+	tristate "NXP i.MX9/i.MX8 Parallel CPI Driver"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on VIDEO_DEV
+	select MEDIA_CONTROLLER
+	select V4L2_FWNODE
+	select VIDEO_V4L2_SUBDEV_API
+	help
+	  Video4Linux2 sub-device driver for PARALLEL CPI receiver found
+	  on some iMX8 and iMX9 SoCs.
+
 source "drivers/media/platform/nxp/imx8-isi/Kconfig"
 
 # mem2mem drivers
diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
index 4d90eb71365259ebdda84ea58483e1c4131d3ac7..5346919d2f1083b51ec99b66981c5d38b3df960c 100644
--- a/drivers/media/platform/nxp/Makefile
+++ b/drivers/media/platform/nxp/Makefile
@@ -7,5 +7,6 @@ obj-y += imx8-isi/
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
 obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
 obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
+obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
 obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
 obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
new file mode 100644
index 0000000000000000000000000000000000000000..718f02bf70c4d0ae74ecf842c1ecb1a1afbcdd45
--- /dev/null
+++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
@@ -0,0 +1,920 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX Parallel CPI receiver driver.
+ *
+ * Copyright 2019-2025 NXP
+ *
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+#define IMX_CPI_DEF_MBUS_CODE			MEDIA_BUS_FMT_UYVY8_2X8
+#define IMX_CPI_DEF_PIX_WIDTH			1920
+#define IMX_CPI_DEF_PIX_HEIGHT			1080
+
+#define IMX_CPI_MAX_PIX_WIDTH			0xffff
+#define IMX_CPI_MAX_PIX_HEIGHT			0xffff
+
+#define IMX_CPI_PAD_SINK			0
+#define IMX_CPI_PAD_SOURCE			1
+#define IMX_CPI_PADS_NUM			2
+
+/* CI_PI INTERFACE CONTROL */
+#define IF_CTRL_REG_PL_ENABLE			BIT(0)
+#define IF_CTRL_REG_PL_VALID			BIT(1)
+#define IF_CTRL_REG_DATA_TYPE_SEL		BIT(8)
+#define IF_CTRL_REG_DATA_TYPE(x)		FIELD_PREP(GENMASK(13, 9), (x))
+
+#define DATA_TYPE_OUT_NULL			0x00
+#define DATA_TYPE_OUT_RGB			0x04
+#define DATA_TYPE_OUT_YUV444			0x08
+#define DATA_TYPE_OUT_YYU420_ODD		0x10
+#define DATA_TYPE_OUT_YYU420_EVEN		0x12
+#define DATA_TYPE_OUT_YYY_ODD			0x18
+#define DATA_TYPE_OUT_UYVY_EVEN			0x1a
+#define DATA_TYPE_OUT_RAW			0x1c
+
+#define IF_CTRL_REG_IF_FORCE_HSYNV_OVERRIDE	0x4
+#define IF_CTRL_REG_IF_FORCE_VSYNV_OVERRIDE	0x2
+#define IF_CTRL_REG_IF_FORCE_DATA_ENABLE_OVERRIDE	0x1
+
+/* CPI INTERFACE CONTROL REG */
+#define CPI_CTRL_REG_CPI_EN			BIT(0)
+#define CPI_CTRL_REG_PIXEL_CLK_POL		BIT(1)
+#define CPI_CTRL_REG_HSYNC_POL			BIT(2)
+#define CPI_CTRL_REG_VSYNC_POL			BIT(3)
+#define CPI_CTRL_REG_DE_POL			BIT(4)
+#define CPI_CTRL_REG_PIXEL_DATA_POL		BIT(5)
+#define CPI_CTRL_REG_CCIR_EXT_VSYNC_EN		BIT(6)
+#define CPI_CTRL_REG_CCIR_EN			BIT(7)
+#define CPI_CTRL_REG_CCIR_VIDEO_MODE		BIT(8)
+#define CPI_CTRL_REG_CCIR_NTSC_EN		BIT(9)
+#define CPI_CTRL_REG_CCIR_VSYNC_RESET_EN	BIT(10)
+#define CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN	BIT(11)
+#define CPI_CTRL_REG_HSYNC_FORCE_EN		BIT(12)
+#define CPI_CTRL_REG_VSYNC_FORCE_EN		BIT(13)
+#define CPI_CTRL_REG_GCLK_MODE_EN		BIT(14)
+#define CPI_CTRL_REG_VALID_SEL			BIT(15)
+#define CPI_CTRL_REG_RAW_OUT_SEL		BIT(16)
+#define CPI_CTRL_REG_HSYNC_OUT_SEL		BIT(17)
+#define CPI_CTRL_REG_HSYNC_PULSE(x)		FIELD_PREP(GENMASK(21, 19), (x))
+#define CPI_CTRL_REG_UV_SWAP_EN			BIT(22)
+#define CPI_CTRL_REG_DATA_TYPE_IN(x)		FIELD_PREP(GENMASK(26, 23), (x))
+#define CPI_CTRL_REG_MASK_VSYNC_COUNTER(x)	FIELD_PREP(GENMASK(28, 27), (x))
+#define CPI_CTRL_REG_SOFTRST			BIT(31)
+
+/* CPI INTERFACE STATUS */
+#define CPI_STATUS_FIELD_TOGGLE			BIT(0)
+#define CPI_STATUS_ECC_ERROR			BIT(1)
+
+/* CPI INTERFACE CONTROL REG1 */
+#define CPI_CTRL_REG1_PIXEL_WIDTH(v)		FIELD_PREP(GENMASK(15, 0), (v))
+#define CPI_CTRL_REG1_VSYNC_PULSE(v)		FIELD_PREP(GENMASK(31, 16), (v))
+
+/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
+enum cpi_in_data_type {
+	CPI_IN_DT_UYVY_BT656_8 = 0x0,
+	CPI_IN_DT_UYVY_BT656_10,
+	CPI_IN_DT_RGB_8,
+	CPI_IN_DT_BGR_8,
+	CPI_IN_DT_YVYU_8 = 0x5,
+	CPI_IN_DT_YUV_8,
+	CPI_IN_DT_RAW_8 = 0x9,
+	CPI_IN_DT_RAW_10,
+};
+
+enum {
+	PI_MODE_INIT,
+	PI_GATE_CLOCK_MODE,
+	PI_CCIR_MODE,
+};
+
+enum {
+	PI_V1,
+	PI_V2,
+};
+
+static const char *const imx_cpi_clk_id[] = {
+	"pixel",
+	"ipg",
+};
+
+#define PCPIDEV_NUM_CLKS   ARRAY_SIZE(imx_cpi_clk_id)
+
+struct imx_cpi_plat_data {
+	u32 version;
+	u32 if_ctrl_reg;
+	u32 interface_status;
+	u32 interface_ctrl_reg;
+	u32 interface_ctrl_reg1;
+	u8 def_hsync_pol;
+	u8 def_vsync_pol;
+	u8 def_pixel_clk_pol;
+	u8 def_cpi_in_data_type;
+};
+
+struct cpi_pm_domain {
+	struct device *dev;
+	struct device_link *link;
+};
+
+struct imx_cpi_device {
+	struct device *dev;
+	void __iomem *regs;
+	struct reset_control *mrst;
+	struct regulator *pcpi_phy_regulator;
+	struct clk_bulk_data clks[PCPIDEV_NUM_CLKS];
+
+	struct v4l2_subdev sd;
+	struct media_pad pads[IMX_CPI_PADS_NUM];
+	struct v4l2_async_notifier notifier;
+
+	struct v4l2_mbus_framefmt format;
+	const struct imx_cpi_plat_data *pdata;
+	struct imx_cpi_pix_format const *pcpidev_fmt;
+
+	struct {
+		struct v4l2_subdev *sd;
+		const struct media_pad *pad;
+	} source;
+
+	struct cpi_pm_domain pm_domains[2];
+
+	u8 mode;
+	u8 uv_swap;
+};
+
+struct imx_cpi_pix_format {
+	u32 code;
+	u32 output;
+	u32 data_type;
+	u8 width;
+};
+
+static const struct imx_cpi_pix_format imx_cpi_formats[] = {
+	/* YUV formats. */
+	{
+		.code = MEDIA_BUS_FMT_UYVY8_2X8,
+		.output = MEDIA_BUS_FMT_UYVY8_1X16,
+		.data_type = CPI_IN_DT_YVYU_8,
+		.width = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_2X8,
+		.output = MEDIA_BUS_FMT_YUYV8_1X16,
+		.data_type = CPI_IN_DT_YVYU_8,
+		.width = 16,
+	},
+};
+
+static const struct imx_cpi_plat_data imx8qxp_pdata = {
+	.version = PI_V1,
+	.if_ctrl_reg = 0x0,
+	.interface_status = 0x20,
+	.interface_ctrl_reg = 0x10,
+	.interface_ctrl_reg1 = 0x30,
+	.def_hsync_pol = 1,
+	.def_vsync_pol = 0,
+	.def_pixel_clk_pol = 0,
+	.def_cpi_in_data_type = CPI_IN_DT_UYVY_BT656_8,
+};
+
+static const struct imx_cpi_plat_data imx93_pdata = {
+	.version = PI_V2,
+	.if_ctrl_reg = 0x0,
+	.interface_status = 0x4,
+	.interface_ctrl_reg = 0x8,
+	.interface_ctrl_reg1 = 0xc,
+	.def_hsync_pol = 0,
+	.def_vsync_pol = 1,
+	.def_pixel_clk_pol = 0,
+	.def_cpi_in_data_type = CPI_IN_DT_YVYU_8,
+};
+
+static void imx_cpi_regs_dump(struct imx_cpi_device *pcpidev)
+{
+	struct device *dev = pcpidev->dev;
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 i;
+
+	struct {
+		u32 offset;
+		const char *const name;
+	} registers[] = {
+		{ pdata->if_ctrl_reg, "HW_IF_CTRL_REG" },
+		{ pdata->interface_ctrl_reg, "HW_CPI_CTRL_REG" },
+		{ pdata->interface_status, "HW_CPI_STATUS" },
+		{ pdata->interface_ctrl_reg1, "HW_CPI_CTRL_REG1" },
+
+	};
+
+	for (i = 0; i < ARRAY_SIZE(registers); i++) {
+		u32 reg = readl(pcpidev->regs + registers[i].offset);
+
+		dev_dbg(dev, "%20s[0x%.2x]: 0x%.8x\n",
+			registers[i].name, registers[i].offset, reg);
+	}
+}
+
+static const struct imx_cpi_pix_format *find_imx_cpi_format(u32 code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(imx_cpi_formats); i++)
+		if (code == imx_cpi_formats[i].code)
+			return &imx_cpi_formats[i];
+
+	return NULL;
+}
+
+static void imx_cpi_sw_reset(struct imx_cpi_device *pcpidev)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 val;
+
+	/* Softwaret Reset */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val |= CPI_CTRL_REG_SOFTRST;
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+	usleep_range(500, 1000);
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val &= ~CPI_CTRL_REG_SOFTRST;
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 val;
+
+	/* Software Reset */
+	imx_cpi_sw_reset(pcpidev);
+
+	/* Config PL Data Type */
+	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
+	val |= IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
+	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+
+	/* Enable sync Force */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+	/* Enable Pixel Link */
+	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
+	val |= IF_CTRL_REG_PL_ENABLE;
+	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+
+	/* Enable Pixel Link */
+	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
+	val |= IF_CTRL_REG_PL_VALID;
+	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+
+	/* Config CTRL REG */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+
+	val |= (CPI_CTRL_REG_DATA_TYPE_IN(pdata->def_cpi_in_data_type) |
+		FIELD_PREP(CPI_CTRL_REG_HSYNC_POL, pdata->def_hsync_pol) |
+		FIELD_PREP(CPI_CTRL_REG_VSYNC_POL, pdata->def_vsync_pol) |
+		FIELD_PREP(CPI_CTRL_REG_PIXEL_CLK_POL, pdata->def_pixel_clk_pol) |
+		CPI_CTRL_REG_MASK_VSYNC_COUNTER(3) |
+		CPI_CTRL_REG_HSYNC_PULSE(2));
+
+	if (pcpidev->uv_swap)
+		val |= CPI_CTRL_REG_UV_SWAP_EN;
+
+	if (pcpidev->mode & PI_GATE_CLOCK_MODE) {
+		val |= CPI_CTRL_REG_GCLK_MODE_EN;
+	} else if (pcpidev->mode & PI_CCIR_MODE) {
+		val |= (CPI_CTRL_REG_CCIR_EN |
+			CPI_CTRL_REG_CCIR_VSYNC_RESET_EN |
+			CPI_CTRL_REG_CCIR_EXT_VSYNC_EN |
+			CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN);
+	}
+
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static int get_interface_ctrl_reg1_param(struct imx_cpi_device *pcpidev,
+					 u32 *pixel_width, u32 *vsync_pulse,
+					 const struct v4l2_mbus_framefmt *format)
+{
+	u32 version = pcpidev->pdata->version;
+
+	switch (version) {
+	case PI_V1:
+		*pixel_width = format->width - 1;
+		*vsync_pulse = format->width << 1;
+		break;
+	case PI_V2:
+		*pixel_width = format->width << 3;
+		*vsync_pulse = format->width - 1;
+		break;
+	default:
+		dev_err(pcpidev->dev, "Not support PI version %d\n", version);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void imx_cpi_config_ctrl_reg1(struct imx_cpi_device *pcpidev,
+				     const struct v4l2_mbus_framefmt *format)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	struct device *dev = pcpidev->dev;
+	u32 pixel_width;
+	u32 vsync_pulse;
+	u32 val;
+	int ret;
+
+	dev_dbg(dev, "%s %dx%d, fmt->code:0x%0x\n", __func__,
+		format->width, format->height, format->code);
+
+	if (format->width <= 0 || format->height <= 0) {
+		dev_err(dev, "%s width/height invalid\n", __func__);
+		return;
+	}
+
+	ret = get_interface_ctrl_reg1_param(pcpidev, &pixel_width,
+					    &vsync_pulse, format);
+	if (ret < 0)
+		return;
+
+	val = (CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
+	       CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse));
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
+}
+
+static void imx_cpi_enable(struct imx_cpi_device *pcpidev)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 val;
+
+	/* Enable CPI */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val |= CPI_CTRL_REG_CPI_EN;
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+	/* Disable SYNC Force */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val &= ~(CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static void imx_cpi_disable(struct imx_cpi_device *pcpidev)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 val;
+
+	/* Enable Sync Force */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+	/* Disable CPI */
+	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+	val &= ~CPI_CTRL_REG_CPI_EN;
+	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+	/* Disable Pixel Link */
+	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
+	val &= ~(IF_CTRL_REG_PL_VALID | IF_CTRL_REG_PL_ENABLE);
+	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+}
+
+static void imx_cpi_start_stream(struct imx_cpi_device *pcpidev,
+				 const struct v4l2_mbus_framefmt *format,
+				 const struct imx_cpi_pix_format *pcpidev_fmt)
+{
+	if (pcpidev_fmt->code == MEDIA_BUS_FMT_YUYV8_2X8 ||
+	    pcpidev_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8)
+		pcpidev->uv_swap = 1;
+
+	imx_cpi_hw_config(pcpidev);
+	imx_cpi_config_ctrl_reg1(pcpidev, format);
+	imx_cpi_enable(pcpidev);
+	imx_cpi_regs_dump(pcpidev);
+}
+
+static void imx_cpi_stop_stream(struct imx_cpi_device *pcpidev)
+{
+	imx_cpi_regs_dump(pcpidev);
+	imx_cpi_disable(pcpidev);
+}
+
+/* -----------------------------------------------------------------------------
+ * Async subdev notifier
+ */
+
+static struct imx_cpi_device *
+notifier_to_imx_cpi_device(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct imx_cpi_device, notifier);
+}
+
+static struct imx_cpi_device *
+sd_to_imx_cpi_device(struct v4l2_subdev *sdev)
+{
+	return container_of(sdev, struct imx_cpi_device, sd);
+}
+
+static int imx_cpi_notify_bound(struct v4l2_async_notifier *notifier,
+				struct v4l2_subdev *sd,
+				struct v4l2_async_connection *asd)
+{
+	struct imx_cpi_device *pcpidev = notifier_to_imx_cpi_device(notifier);
+	struct media_pad *sink = &pcpidev->sd.entity.pads[IMX_CPI_PAD_SINK];
+
+	return v4l2_create_fwnode_links_to_pad(sd, sink, 0);
+}
+
+static const struct v4l2_async_notifier_operations imx_cpi_notify_ops = {
+	.bound = imx_cpi_notify_bound,
+};
+
+static int imx_cpi_async_register(struct imx_cpi_device *pcpidev)
+{
+	struct v4l2_fwnode_endpoint vep = {
+		.bus_type = V4L2_MBUS_PARALLEL,
+	};
+	struct v4l2_async_connection *asd;
+	struct fwnode_handle *ep __free(fwnode_handle) = NULL;
+	int ret;
+
+	v4l2_async_subdev_nf_init(&pcpidev->notifier, &pcpidev->sd);
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(pcpidev->dev), 0, 0,
+					     FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (!ep)
+		return -ENOTCONN;
+
+	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+	if (ret)
+		return ret;
+
+	asd = v4l2_async_nf_add_fwnode_remote(&pcpidev->notifier, ep,
+					      struct v4l2_async_connection);
+	if (IS_ERR(asd))
+		return PTR_ERR(asd);
+
+	pcpidev->notifier.ops = &imx_cpi_notify_ops;
+	ret = v4l2_async_nf_register(&pcpidev->notifier);
+	if (ret)
+		return ret;
+
+	return v4l2_async_register_subdev(&pcpidev->sd);
+}
+
+/* -----------------------------------------------------------------------------
+ * Media entity operations
+ */
+
+static int imx_cpi_link_setup(struct media_entity *entity,
+			      const struct media_pad *local_pad,
+			      const struct media_pad *remote_pad,
+			      u32 flags)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+	struct v4l2_subdev *remote_sd;
+
+	dev_dbg(pcpidev->dev, "link setup %s -> %s", remote_pad->entity->name,
+		local_pad->entity->name);
+
+	/* We only care about the link to the source. */
+	if (!(local_pad->flags & MEDIA_PAD_FL_SINK))
+		return 0;
+
+	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+	if (flags & MEDIA_LNK_FL_ENABLED) {
+		if (pcpidev->source.sd)
+			return -EBUSY;
+
+		pcpidev->source.sd = remote_sd;
+		pcpidev->source.pad = remote_pad;
+	} else {
+		pcpidev->source.sd = NULL;
+		pcpidev->source.pad = NULL;
+	}
+
+	return 0;
+}
+
+static const struct media_entity_operations imx_cpi_entity_ops = {
+	.link_setup = imx_cpi_link_setup,
+	.link_validate = v4l2_subdev_link_validate,
+	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
+};
+
+static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *sd_state,
+			   struct v4l2_subdev_format *sdformat)
+{
+	struct imx_cpi_pix_format const *pcpidev_fmt;
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+	struct device *dev = pcpidev->dev;
+	struct v4l2_mbus_framefmt *fmt;
+	unsigned int align;
+
+	/*
+	 * The Parallel cpi can't transcode in any way, the source format
+	 * can't be modified.
+	 */
+	if (sdformat->pad == IMX_CPI_PAD_SOURCE)
+		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
+
+	/*
+	 * Validate the media bus code and clamp and align the size.
+	 *
+	 * The total number of bits per line must be a multiple of 8. We thus
+	 * need to align the width for formats that are not multiples of 8
+	 * bits.
+	 */
+	pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
+	if (!pcpidev_fmt)
+		pcpidev_fmt = &imx_cpi_formats[0];
+
+	switch (pcpidev_fmt->width % 8) {
+	case 0:
+		align = 0;
+		break;
+	case 4:
+		align = 1;
+		break;
+	case 2:
+	case 6:
+		align = 2;
+		break;
+	default:
+		/* 1, 3, 5, 7 */
+		align = 3;
+		break;
+	}
+
+	v4l_bound_align_image(&sdformat->format.width, 1,
+			      IMX_CPI_MAX_PIX_WIDTH, align,
+			      &sdformat->format.height, 1,
+			      IMX_CPI_MAX_PIX_HEIGHT, 0, 0);
+
+	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
+	if (!fmt)
+		return -EINVAL;
+
+	fmt->code = pcpidev_fmt->code;
+	fmt->width = sdformat->format.width;
+	fmt->height = sdformat->format.height;
+	fmt->field = V4L2_FIELD_NONE;
+	fmt->colorspace = sdformat->format.colorspace;
+	fmt->quantization = sdformat->format.quantization;
+	fmt->xfer_func = sdformat->format.xfer_func;
+	fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
+
+	sdformat->format = *fmt;
+
+	/* Propagate the format from sink to source. */
+	fmt = v4l2_subdev_state_get_format(sd_state, IMX_CPI_PAD_SOURCE);
+	*fmt = sdformat->format;
+
+	/* The format on the source pad might change due to unpacking. */
+	fmt->code = pcpidev_fmt->output;
+
+	dev_dbg(dev, "%s: fmt_code:0x%0x, %dx%d\n", __func__,
+		fmt->code, fmt->width, fmt->height);
+	return 0;
+}
+
+static const struct v4l2_mbus_framefmt imx_cpi_default_fmt = {
+	.code = IMX_CPI_DEF_MBUS_CODE,
+	.width = IMX_CPI_DEF_PIX_WIDTH,
+	.height = IMX_CPI_DEF_PIX_HEIGHT,
+	.field = V4L2_FIELD_NONE,
+	.colorspace = V4L2_COLORSPACE_SMPTE170M,
+	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
+	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
+	.quantization = V4L2_QUANTIZATION_LIM_RANGE,
+};
+
+static int imx_cpi_init_state(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state)
+{
+	struct v4l2_subdev_format fmt = {
+		.pad = IMX_CPI_PAD_SINK,
+	};
+
+	fmt.format.code = imx_cpi_formats[0].code;
+	fmt.format.width = IMX_CPI_DEF_PIX_WIDTH;
+	fmt.format.height = IMX_CPI_DEF_PIX_HEIGHT;
+
+	fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
+	fmt.format.xfer_func =
+	    V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
+	fmt.format.ycbcr_enc =
+	    V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
+	fmt.format.quantization =
+	    V4L2_MAP_QUANTIZATION_DEFAULT(false,
+					  fmt.format.colorspace,
+					  fmt.format.ycbcr_enc);
+
+	return imx_cpi_set_fmt(sd, sd_state, &fmt);
+}
+
+static int imx_cpi_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+	const struct v4l2_mbus_framefmt *format;
+	const struct imx_cpi_pix_format *pcpidev_fmt;
+	struct v4l2_subdev_state *state;
+	int ret;
+
+	if (!enable) {
+		v4l2_subdev_disable_streams(pcpidev->source.sd,
+					    pcpidev->source.pad->index, BIT(0));
+
+		imx_cpi_stop_stream(pcpidev);
+
+		pm_runtime_put(pcpidev->dev);
+
+		return 0;
+	}
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+	format = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SINK);
+	pcpidev_fmt = find_imx_cpi_format(format->code);
+
+	ret = pm_runtime_resume_and_get(pcpidev->dev);
+	if (ret < 0)
+		goto err_unlock;
+
+	imx_cpi_start_stream(pcpidev, format, pcpidev_fmt);
+
+	ret = v4l2_subdev_enable_streams(pcpidev->source.sd,
+					 pcpidev->source.pad->index, BIT(0));
+	if (ret < 0)
+		goto err_stop;
+
+	v4l2_subdev_unlock_state(state);
+
+	return 0;
+
+err_stop:
+	imx_cpi_stop_stream(pcpidev);
+	pm_runtime_put(pcpidev->dev);
+err_unlock:
+	v4l2_subdev_unlock_state(state);
+	return ret;
+}
+
+static int imx_cpi_enum_mbus_code(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state,
+				  struct v4l2_subdev_mbus_code_enum *code)
+{
+	/*
+	 * The PARALLEL CPI can't transcode in any way, the source format
+	 * is identical to the sink format.
+	 */
+	if (code->pad == IMX_CPI_PAD_SOURCE) {
+		struct v4l2_mbus_framefmt *fmt;
+
+		if (code->index > 0)
+			return -EINVAL;
+
+		fmt = v4l2_subdev_state_get_format(sd_state, code->pad);
+		code->code = fmt->code;
+		return 0;
+	}
+
+	if (code->pad != IMX_CPI_PAD_SINK)
+		return -EINVAL;
+
+	if (code->index >= ARRAY_SIZE(imx_cpi_formats))
+		return -EINVAL;
+
+	code->code = imx_cpi_formats[code->index].code;
+
+	return 0;
+}
+
+static int imx_cpi_get_frame_desc(struct v4l2_subdev *sd,
+				  unsigned int pad,
+				  struct v4l2_mbus_frame_desc *fd)
+{
+	struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[0];
+	const struct imx_cpi_pix_format *pcpidev_fmt;
+	const struct v4l2_mbus_framefmt *fmt;
+	struct v4l2_subdev_state *state;
+
+	if (pad != IMX_CPI_PAD_SOURCE)
+		return -EINVAL;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+	fmt = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SOURCE);
+	pcpidev_fmt = find_imx_cpi_format(fmt->code);
+	v4l2_subdev_unlock_state(state);
+
+	if (!pcpidev_fmt)
+		return -EPIPE;
+
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
+	fd->num_entries = 1;
+
+	entry->flags = 0;
+	entry->pixelcode = pcpidev_fmt->code;
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops imx_cpi_video_ops = {
+	.s_stream = imx_cpi_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops imx_cpi_pad_ops = {
+	.enum_mbus_code = imx_cpi_enum_mbus_code,
+	.get_fmt = v4l2_subdev_get_fmt,
+	.set_fmt = imx_cpi_set_fmt,
+	.get_frame_desc = imx_cpi_get_frame_desc,
+};
+
+static const struct v4l2_subdev_ops imx_cpi_subdev_ops = {
+	.pad = &imx_cpi_pad_ops,
+	.video = &imx_cpi_video_ops,
+};
+
+static const struct v4l2_subdev_internal_ops imx_cpi_internal_ops = {
+	.init_state = imx_cpi_init_state,
+};
+
+static int imx_cpi_clk_get(struct imx_cpi_device *pcpidev)
+{
+	unsigned int i;
+	int ret = 0;
+
+	for (i = 0; i < PCPIDEV_NUM_CLKS; i++)
+		pcpidev->clks[i].id = imx_cpi_clk_id[i];
+
+	ret = devm_clk_bulk_get(pcpidev->dev, PCPIDEV_NUM_CLKS, pcpidev->clks);
+
+	return ret;
+}
+
+/* ----------------------------------------------------------------------
+ * Suspend/resume
+ */
+static int __maybe_unused imx_cpi_runtime_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+
+	clk_bulk_disable_unprepare(PCPIDEV_NUM_CLKS, pcpidev->clks);
+
+	return 0;
+}
+
+static int __maybe_unused imx_cpi_runtime_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+
+	return clk_bulk_prepare_enable(PCPIDEV_NUM_CLKS, pcpidev->clks);
+}
+
+static const struct dev_pm_ops imx_cpi_pm_ops = {
+	RUNTIME_PM_OPS(imx_cpi_runtime_suspend, imx_cpi_runtime_resume, NULL)
+};
+
+static int imx_cpi_subdev_init(struct imx_cpi_device *pcpidev)
+{
+	struct v4l2_subdev *sd = &pcpidev->sd;
+	int ret;
+
+	v4l2_subdev_init(sd, &imx_cpi_subdev_ops);
+
+	sd->internal_ops = &imx_cpi_internal_ops;
+	sd->owner = THIS_MODULE;
+	snprintf(sd->name, sizeof(sd->name), "parallel-%s",
+		 dev_name(pcpidev->dev));
+
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->ctrl_handler = NULL;
+
+	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	sd->entity.ops = &imx_cpi_entity_ops;
+
+	sd->dev = pcpidev->dev;
+
+	pcpidev->pads[IMX_CPI_PAD_SINK].flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
+	pcpidev->pads[IMX_CPI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE |
+						       MEDIA_PAD_FL_MUST_CONNECT;
+
+	ret = media_entity_pads_init(&sd->entity, IMX_CPI_PADS_NUM, pcpidev->pads);
+	if (ret)
+		return ret;
+
+	ret = v4l2_subdev_init_finalize(sd);
+	if (ret)
+		media_entity_cleanup(&sd->entity);
+
+	return ret;
+}
+
+static void imx_cpi_cleanup(void *data)
+{
+	struct imx_cpi_device *pcpidev = data;
+
+	v4l2_subdev_cleanup(&pcpidev->sd);
+	media_entity_cleanup(&pcpidev->sd.entity);
+	v4l2_async_nf_unregister(&pcpidev->notifier);
+	v4l2_async_nf_cleanup(&pcpidev->notifier);
+	v4l2_async_unregister_subdev(&pcpidev->sd);
+}
+
+static int imx_cpi_probe(struct platform_device *pdev)
+{
+	struct imx_cpi_device *pcpidev;
+	struct device *dev = &pdev->dev;
+	int ret = 0;
+
+	pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
+	if (!pcpidev)
+		return -ENOMEM;
+
+	pcpidev->dev = dev;
+	platform_set_drvdata(pdev, pcpidev);
+
+	pcpidev->pdata = of_device_get_match_data(dev);
+
+	pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pcpidev->regs))
+		return dev_err_probe(dev, PTR_ERR(pcpidev->regs),
+				     "Failed to get regs\n");
+
+	ret = imx_cpi_clk_get(pcpidev);
+	if (ret < 0)
+		return ret;
+
+	ret = imx_cpi_subdev_init(pcpidev);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, imx_cpi_cleanup, pcpidev);
+	if (ret)
+		return ret;
+
+	pcpidev->mode = PI_GATE_CLOCK_MODE;
+
+	platform_set_drvdata(pdev, &pcpidev->sd);
+
+	ret = imx_cpi_async_register(pcpidev);
+	if (ret < 0)
+		return ret;
+
+	devm_pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static const struct of_device_id imx_cpi_of_match[] = {
+	{.compatible = "fsl,imx8qxp-pcif", .data = &imx8qxp_pdata },
+	{.compatible = "fsl,imx93-pcif", .data = &imx93_pdata },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, imx_cpi_of_match);
+
+static struct platform_driver _driver = {
+	.probe = imx_cpi_probe,
+	.driver = {
+		.of_match_table = imx_cpi_of_match,
+		.name = "imx-parallel-cpi",
+		.pm = pm_ptr(&imx_cpi_pm_ops),
+	},
+};
+
+module_platform_driver(_driver);
+
+MODULE_DESCRIPTION("i.MX9 Parallel CPI receiver driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/4] arm64: dts: imx8: add camera parallel interface (CPI) node
  2025-07-08 17:48 [PATCH v3 0/4] media: imx8qxp: add parallel camera support Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI) Frank Li via B4 Relay
@ 2025-07-08 17:48 ` Frank Li via B4 Relay
  2025-07-08 17:48 ` [PATCH v3 4/4] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support Frank Li via B4 Relay
  3 siblings, 0 replies; 11+ messages in thread
From: Frank Li via B4 Relay @ 2025-07-08 17:48 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-media, imx, linux-arm-kernel, linux-kernel, devicetree,
	Frank Li

From: Frank Li <Frank.Li@nxp.com>

Add camera parallel interface (CPI) node.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v4
- replace csi with cpi.

changes in v2
- update compatible string to match binding's change
---
 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi    | 13 +++++++++++
 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi | 27 +++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index 2cf0f7208350a416d77b11140279d2f66f41498f..e321b0d62cce13ef999085fce7aff9ea2fd7fc21 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -224,6 +224,19 @@ irqsteer_parallel: irqsteer@58260000 {
 		status = "disabled";
 	};
 
+	parallel_cpi: cpi@58261000 {
+		compatible = "fsl,imx8qxp-pcif";
+		reg = <0x58261000 0x1000>;
+		clocks = <&pi0_pxl_lpcg IMX_LPCG_CLK_0>,
+			 <&pi0_ipg_lpcg IMX_LPCG_CLK_4>;
+		clock-names = "pixel", "ipg";
+		assigned-clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
+		assigned-clock-parents = <&clk IMX_SC_R_PI_0_PLL IMX_SC_PM_CLK_PLL>;
+		assigned-clock-rates = <160000000>;
+		power-domains = <&pd IMX_SC_R_PI_0>;
+		status = "disabled";
+	};
+
 	pi0_ipg_lpcg: clock-controller@58263004 {
 		compatible = "fsl,imx8qxp-lpcg";
 		reg = <0x58263004 0x4>;
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
index 50015f8dd4e43b507c879479e7320cb58ec8bf74..5b5efaedd403c040f1231ead02834bb3d16a9dd2 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
@@ -62,6 +62,14 @@ isi_in_2: endpoint {
 				remote-endpoint = <&mipi_csi0_out>;
 			};
 		};
+
+		port@4 {
+			reg = <4>;
+
+			isi_in_4: endpoint {
+				remote-endpoint = <&parallel_cpi_out>;
+			};
+		};
 	};
 };
 
@@ -94,3 +102,22 @@ &jpegenc {
 &mipi_csi_1 {
 	status = "disabled";
 };
+
+&parallel_cpi {
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+		};
+
+		port@1 {
+			reg = <1>;
+
+			parallel_cpi_out: endpoint {
+				remote-endpoint = <&isi_in_4>;
+			};
+		};
+	};
+};

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support
  2025-07-08 17:48 [PATCH v3 0/4] media: imx8qxp: add parallel camera support Frank Li via B4 Relay
                   ` (2 preceding siblings ...)
  2025-07-08 17:48 ` [PATCH v3 3/4] arm64: dts: imx8: add camera parallel interface (CPI) node Frank Li via B4 Relay
@ 2025-07-08 17:48 ` Frank Li via B4 Relay
  3 siblings, 0 replies; 11+ messages in thread
From: Frank Li via B4 Relay @ 2025-07-08 17:48 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-media, imx, linux-arm-kernel, linux-kernel, devicetree,
	Frank Li

From: Frank Li <Frank.Li@nxp.com>

Add parallel ov5640 nodes in imx8qxp-mek and create overlay file to enable
it because it can work at two mode: MIPI CSI and parallel mode.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v3
- replace csi with cpi.
- use imx8qxp-mek-ov5640-cpi.dtso since csi use imx8qxp-mek-ov5640-csi.dtso

change in v2
- move ov5640 part to overlay file
- rename to imx8qxp-mek-ov5640-parallel.dtso
- remove data-lanes
---
 arch/arm64/boot/dts/freescale/Makefile             |  3 +
 .../boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso | 82 ++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 02ef35578dbc7e05b35b781dbfca0f0bc124ead1..d5b6cbdc144bb00963d93c92dd493513d1f00540 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -330,6 +330,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek-pcie-ep.dtb
 imx8qxp-mek-ov5640-csi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-csi.dtbo
 dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-csi.dtb
 
+imx8qxp-mek-ov5640-cpi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-cpi.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-cpi.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqp-mba8xx.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqps-mb-smarc-2.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..01f6333f507286133000eb0401a50043ca49a5df
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2025 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8-lpcg.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+#include <dt-bindings/pinctrl/pads-imx8qxp.h>
+
+&cm40_i2c {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ov5640_pi: camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
+		clock-names = "xclk";
+		assigned-clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
+		assigned-clock-rates = <24000000>;
+		AVDD-supply = <&reg_2v8>;
+		DOVDD-supply = <&reg_1v8>;
+		DVDD-supply = <&reg_1v5>;
+		pinctrl-0 = <&pinctrl_parallel_cpi>;
+		pinctrl-names = "default";
+		powerdown-gpios = <&lsio_gpio3 2 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&lsio_gpio3 3 GPIO_ACTIVE_LOW>;
+
+		port {
+			ov5640_pi_ep: endpoint {
+				bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+				bus-width = <8>;
+				hsync-active = <1>;
+				pclk-sample = <1>;
+				remote-endpoint = <&parallel_cpi_in>;
+				vsync-active = <0>;
+			};
+		};
+	};
+};
+
+&iomuxc {
+	pinctrl_parallel_cpi: parallelcpigrp {
+		fsl,pins = <
+			IMX8QXP_CSI_D00_CI_PI_D02		0xc0000041
+			IMX8QXP_CSI_D01_CI_PI_D03		0xc0000041
+			IMX8QXP_CSI_D02_CI_PI_D04		0xc0000041
+			IMX8QXP_CSI_D03_CI_PI_D05		0xc0000041
+			IMX8QXP_CSI_D04_CI_PI_D06		0xc0000041
+			IMX8QXP_CSI_D05_CI_PI_D07		0xc0000041
+			IMX8QXP_CSI_D06_CI_PI_D08		0xc0000041
+			IMX8QXP_CSI_D07_CI_PI_D09		0xc0000041
+
+			IMX8QXP_CSI_MCLK_CI_PI_MCLK		0xc0000041
+			IMX8QXP_CSI_PCLK_CI_PI_PCLK		0xc0000041
+			IMX8QXP_CSI_HSYNC_CI_PI_HSYNC		0xc0000041
+			IMX8QXP_CSI_VSYNC_CI_PI_VSYNC		0xc0000041
+			IMX8QXP_CSI_EN_LSIO_GPIO3_IO02		0xc0000041
+			IMX8QXP_CSI_RESET_LSIO_GPIO3_IO03	0xc0000041
+		>;
+	};
+};
+
+&isi {
+	status = "okay";
+};
+
+&parallel_cpi {
+	status = "okay";
+
+	ports {
+		port@0 {
+			parallel_cpi_in: endpoint {
+				remote-endpoint = <&ov5640_pi_ep>;
+			};
+		};
+	};
+};

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support
  2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
@ 2025-07-09  8:22   ` Krzysztof Kozlowski
  2025-07-24 22:15   ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-09  8:22 UTC (permalink / raw)
  To: Frank Li
  Cc: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan

On Tue, Jul 08, 2025 at 01:48:42PM -0400, Frank Li wrote:
> From: Alice Yuan <alice.yuan@nxp.com>
> 
> Document the binding for parallel CPI controller found in i.MX8QXP, i.MX93
> and i.MX91 SoCs.
> 
> Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change in v3:
> - use enum at compatible string
> - add ref to video-interfaces.yaml#
> - use cpi as node name in examples.
> - replace csi (Camera Serial Interface) with CPI (Camera Parallel Interface)
> in commit message.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
  2025-07-08 17:48 ` [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI) Frank Li via B4 Relay
@ 2025-07-18 16:44   ` Frank Li
  2025-07-25  0:04   ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Frank Li @ 2025-07-18 16:44 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan,
	Robert Chiras, Zhipeng Wang

On Tue, Jul 08, 2025 at 01:48:43PM -0400, Frank Li via B4 Relay wrote:
> From: Alice Yuan <alice.yuan@nxp.com>

Laurent Pinchart:

	Do you have chance to review this patch ?

Frank

>
> Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
> i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
> and enables image data capture through a parallel interface.
>
> Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v3
> - replace csi with cpi
> - use __free(fwnode_handle) to simpilfy code
> - remove imx91 driver data, which is the same as imx93
>
> change in v2
> - remove MODULE_ALIAS
> - use devm_pm_runtime_enable() and cleanup remove function
> - change output format to 1x16. controller convert 2x8 to 1x16 format
> ---
>  MAINTAINERS                                   |   1 +
>  drivers/media/platform/nxp/Kconfig            |  11 +
>  drivers/media/platform/nxp/Makefile           |   1 +
>  drivers/media/platform/nxp/imx-parallel-cpi.c | 920 ++++++++++++++++++++++++++
>  4 files changed, 933 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303..14842a3b860a6f23846f12a684eedcbb9eb69e19 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15112,6 +15112,7 @@ F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
>  F:	drivers/media/platform/nxp/imx-mipi-csis.c
> +F:	drivers/media/platform/nxp/imx-parallel-cpi.c
>  F:	drivers/media/platform/nxp/imx7-media-csi.c
>  F:	drivers/media/platform/nxp/imx8mq-mipi-csi2.c
>
> diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e213fdc5da70821dc0b420e1821f4f..504ae1c6494f331c16124a224421ac7acd433ba5 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -39,6 +39,17 @@ config VIDEO_IMX_MIPI_CSIS
>  	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
>  	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
>
> +config VIDEO_IMX_PARALLEL_CPI
> +	tristate "NXP i.MX9/i.MX8 Parallel CPI Driver"
> +	depends on ARCH_MXC || COMPILE_TEST
> +	depends on VIDEO_DEV
> +	select MEDIA_CONTROLLER
> +	select V4L2_FWNODE
> +	select VIDEO_V4L2_SUBDEV_API
> +	help
> +	  Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> +	  on some iMX8 and iMX9 SoCs.
> +
>  source "drivers/media/platform/nxp/imx8-isi/Kconfig"
>
>  # mem2mem drivers
> diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
> index 4d90eb71365259ebdda84ea58483e1c4131d3ac7..5346919d2f1083b51ec99b66981c5d38b3df960c 100644
> --- a/drivers/media/platform/nxp/Makefile
> +++ b/drivers/media/platform/nxp/Makefile
> @@ -7,5 +7,6 @@ obj-y += imx8-isi/
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
>  obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> +obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
>  obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
>  obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..718f02bf70c4d0ae74ecf842c1ecb1a1afbcdd45
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,920 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX Parallel CPI receiver driver.
> + *
> + * Copyright 2019-2025 NXP
> + *
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define IMX_CPI_DEF_MBUS_CODE			MEDIA_BUS_FMT_UYVY8_2X8
> +#define IMX_CPI_DEF_PIX_WIDTH			1920
> +#define IMX_CPI_DEF_PIX_HEIGHT			1080
> +
> +#define IMX_CPI_MAX_PIX_WIDTH			0xffff
> +#define IMX_CPI_MAX_PIX_HEIGHT			0xffff
> +
> +#define IMX_CPI_PAD_SINK			0
> +#define IMX_CPI_PAD_SOURCE			1
> +#define IMX_CPI_PADS_NUM			2
> +
> +/* CI_PI INTERFACE CONTROL */
> +#define IF_CTRL_REG_PL_ENABLE			BIT(0)
> +#define IF_CTRL_REG_PL_VALID			BIT(1)
> +#define IF_CTRL_REG_DATA_TYPE_SEL		BIT(8)
> +#define IF_CTRL_REG_DATA_TYPE(x)		FIELD_PREP(GENMASK(13, 9), (x))
> +
> +#define DATA_TYPE_OUT_NULL			0x00
> +#define DATA_TYPE_OUT_RGB			0x04
> +#define DATA_TYPE_OUT_YUV444			0x08
> +#define DATA_TYPE_OUT_YYU420_ODD		0x10
> +#define DATA_TYPE_OUT_YYU420_EVEN		0x12
> +#define DATA_TYPE_OUT_YYY_ODD			0x18
> +#define DATA_TYPE_OUT_UYVY_EVEN			0x1a
> +#define DATA_TYPE_OUT_RAW			0x1c
> +
> +#define IF_CTRL_REG_IF_FORCE_HSYNV_OVERRIDE	0x4
> +#define IF_CTRL_REG_IF_FORCE_VSYNV_OVERRIDE	0x2
> +#define IF_CTRL_REG_IF_FORCE_DATA_ENABLE_OVERRIDE	0x1
> +
> +/* CPI INTERFACE CONTROL REG */
> +#define CPI_CTRL_REG_CPI_EN			BIT(0)
> +#define CPI_CTRL_REG_PIXEL_CLK_POL		BIT(1)
> +#define CPI_CTRL_REG_HSYNC_POL			BIT(2)
> +#define CPI_CTRL_REG_VSYNC_POL			BIT(3)
> +#define CPI_CTRL_REG_DE_POL			BIT(4)
> +#define CPI_CTRL_REG_PIXEL_DATA_POL		BIT(5)
> +#define CPI_CTRL_REG_CCIR_EXT_VSYNC_EN		BIT(6)
> +#define CPI_CTRL_REG_CCIR_EN			BIT(7)
> +#define CPI_CTRL_REG_CCIR_VIDEO_MODE		BIT(8)
> +#define CPI_CTRL_REG_CCIR_NTSC_EN		BIT(9)
> +#define CPI_CTRL_REG_CCIR_VSYNC_RESET_EN	BIT(10)
> +#define CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN	BIT(11)
> +#define CPI_CTRL_REG_HSYNC_FORCE_EN		BIT(12)
> +#define CPI_CTRL_REG_VSYNC_FORCE_EN		BIT(13)
> +#define CPI_CTRL_REG_GCLK_MODE_EN		BIT(14)
> +#define CPI_CTRL_REG_VALID_SEL			BIT(15)
> +#define CPI_CTRL_REG_RAW_OUT_SEL		BIT(16)
> +#define CPI_CTRL_REG_HSYNC_OUT_SEL		BIT(17)
> +#define CPI_CTRL_REG_HSYNC_PULSE(x)		FIELD_PREP(GENMASK(21, 19), (x))
> +#define CPI_CTRL_REG_UV_SWAP_EN			BIT(22)
> +#define CPI_CTRL_REG_DATA_TYPE_IN(x)		FIELD_PREP(GENMASK(26, 23), (x))
> +#define CPI_CTRL_REG_MASK_VSYNC_COUNTER(x)	FIELD_PREP(GENMASK(28, 27), (x))
> +#define CPI_CTRL_REG_SOFTRST			BIT(31)
> +
> +/* CPI INTERFACE STATUS */
> +#define CPI_STATUS_FIELD_TOGGLE			BIT(0)
> +#define CPI_STATUS_ECC_ERROR			BIT(1)
> +
> +/* CPI INTERFACE CONTROL REG1 */
> +#define CPI_CTRL_REG1_PIXEL_WIDTH(v)		FIELD_PREP(GENMASK(15, 0), (v))
> +#define CPI_CTRL_REG1_VSYNC_PULSE(v)		FIELD_PREP(GENMASK(31, 16), (v))
> +
> +/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
> +enum cpi_in_data_type {
> +	CPI_IN_DT_UYVY_BT656_8 = 0x0,
> +	CPI_IN_DT_UYVY_BT656_10,
> +	CPI_IN_DT_RGB_8,
> +	CPI_IN_DT_BGR_8,
> +	CPI_IN_DT_YVYU_8 = 0x5,
> +	CPI_IN_DT_YUV_8,
> +	CPI_IN_DT_RAW_8 = 0x9,
> +	CPI_IN_DT_RAW_10,
> +};
> +
> +enum {
> +	PI_MODE_INIT,
> +	PI_GATE_CLOCK_MODE,
> +	PI_CCIR_MODE,
> +};
> +
> +enum {
> +	PI_V1,
> +	PI_V2,
> +};
> +
> +static const char *const imx_cpi_clk_id[] = {
> +	"pixel",
> +	"ipg",
> +};
> +
> +#define PCPIDEV_NUM_CLKS   ARRAY_SIZE(imx_cpi_clk_id)
> +
> +struct imx_cpi_plat_data {
> +	u32 version;
> +	u32 if_ctrl_reg;
> +	u32 interface_status;
> +	u32 interface_ctrl_reg;
> +	u32 interface_ctrl_reg1;
> +	u8 def_hsync_pol;
> +	u8 def_vsync_pol;
> +	u8 def_pixel_clk_pol;
> +	u8 def_cpi_in_data_type;
> +};
> +
> +struct cpi_pm_domain {
> +	struct device *dev;
> +	struct device_link *link;
> +};
> +
> +struct imx_cpi_device {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct reset_control *mrst;
> +	struct regulator *pcpi_phy_regulator;
> +	struct clk_bulk_data clks[PCPIDEV_NUM_CLKS];
> +
> +	struct v4l2_subdev sd;
> +	struct media_pad pads[IMX_CPI_PADS_NUM];
> +	struct v4l2_async_notifier notifier;
> +
> +	struct v4l2_mbus_framefmt format;
> +	const struct imx_cpi_plat_data *pdata;
> +	struct imx_cpi_pix_format const *pcpidev_fmt;
> +
> +	struct {
> +		struct v4l2_subdev *sd;
> +		const struct media_pad *pad;
> +	} source;
> +
> +	struct cpi_pm_domain pm_domains[2];
> +
> +	u8 mode;
> +	u8 uv_swap;
> +};
> +
> +struct imx_cpi_pix_format {
> +	u32 code;
> +	u32 output;
> +	u32 data_type;
> +	u8 width;
> +};
> +
> +static const struct imx_cpi_pix_format imx_cpi_formats[] = {
> +	/* YUV formats. */
> +	{
> +		.code = MEDIA_BUS_FMT_UYVY8_2X8,
> +		.output = MEDIA_BUS_FMT_UYVY8_1X16,
> +		.data_type = CPI_IN_DT_YVYU_8,
> +		.width = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV8_2X8,
> +		.output = MEDIA_BUS_FMT_YUYV8_1X16,
> +		.data_type = CPI_IN_DT_YVYU_8,
> +		.width = 16,
> +	},
> +};
> +
> +static const struct imx_cpi_plat_data imx8qxp_pdata = {
> +	.version = PI_V1,
> +	.if_ctrl_reg = 0x0,
> +	.interface_status = 0x20,
> +	.interface_ctrl_reg = 0x10,
> +	.interface_ctrl_reg1 = 0x30,
> +	.def_hsync_pol = 1,
> +	.def_vsync_pol = 0,
> +	.def_pixel_clk_pol = 0,
> +	.def_cpi_in_data_type = CPI_IN_DT_UYVY_BT656_8,
> +};
> +
> +static const struct imx_cpi_plat_data imx93_pdata = {
> +	.version = PI_V2,
> +	.if_ctrl_reg = 0x0,
> +	.interface_status = 0x4,
> +	.interface_ctrl_reg = 0x8,
> +	.interface_ctrl_reg1 = 0xc,
> +	.def_hsync_pol = 0,
> +	.def_vsync_pol = 1,
> +	.def_pixel_clk_pol = 0,
> +	.def_cpi_in_data_type = CPI_IN_DT_YVYU_8,
> +};
> +
> +static void imx_cpi_regs_dump(struct imx_cpi_device *pcpidev)
> +{
> +	struct device *dev = pcpidev->dev;
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 i;
> +
> +	struct {
> +		u32 offset;
> +		const char *const name;
> +	} registers[] = {
> +		{ pdata->if_ctrl_reg, "HW_IF_CTRL_REG" },
> +		{ pdata->interface_ctrl_reg, "HW_CPI_CTRL_REG" },
> +		{ pdata->interface_status, "HW_CPI_STATUS" },
> +		{ pdata->interface_ctrl_reg1, "HW_CPI_CTRL_REG1" },
> +
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> +		u32 reg = readl(pcpidev->regs + registers[i].offset);
> +
> +		dev_dbg(dev, "%20s[0x%.2x]: 0x%.8x\n",
> +			registers[i].name, registers[i].offset, reg);
> +	}
> +}
> +
> +static const struct imx_cpi_pix_format *find_imx_cpi_format(u32 code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(imx_cpi_formats); i++)
> +		if (code == imx_cpi_formats[i].code)
> +			return &imx_cpi_formats[i];
> +
> +	return NULL;
> +}
> +
> +static void imx_cpi_sw_reset(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Softwaret Reset */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= CPI_CTRL_REG_SOFTRST;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	usleep_range(500, 1000);
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~CPI_CTRL_REG_SOFTRST;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Software Reset */
> +	imx_cpi_sw_reset(pcpidev);
> +
> +	/* Config PL Data Type */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Enable sync Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Enable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_PL_ENABLE;
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Enable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_PL_VALID;
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Config CTRL REG */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	val |= (CPI_CTRL_REG_DATA_TYPE_IN(pdata->def_cpi_in_data_type) |
> +		FIELD_PREP(CPI_CTRL_REG_HSYNC_POL, pdata->def_hsync_pol) |
> +		FIELD_PREP(CPI_CTRL_REG_VSYNC_POL, pdata->def_vsync_pol) |
> +		FIELD_PREP(CPI_CTRL_REG_PIXEL_CLK_POL, pdata->def_pixel_clk_pol) |
> +		CPI_CTRL_REG_MASK_VSYNC_COUNTER(3) |
> +		CPI_CTRL_REG_HSYNC_PULSE(2));
> +
> +	if (pcpidev->uv_swap)
> +		val |= CPI_CTRL_REG_UV_SWAP_EN;
> +
> +	if (pcpidev->mode & PI_GATE_CLOCK_MODE) {
> +		val |= CPI_CTRL_REG_GCLK_MODE_EN;
> +	} else if (pcpidev->mode & PI_CCIR_MODE) {
> +		val |= (CPI_CTRL_REG_CCIR_EN |
> +			CPI_CTRL_REG_CCIR_VSYNC_RESET_EN |
> +			CPI_CTRL_REG_CCIR_EXT_VSYNC_EN |
> +			CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN);
> +	}
> +
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static int get_interface_ctrl_reg1_param(struct imx_cpi_device *pcpidev,
> +					 u32 *pixel_width, u32 *vsync_pulse,
> +					 const struct v4l2_mbus_framefmt *format)
> +{
> +	u32 version = pcpidev->pdata->version;
> +
> +	switch (version) {
> +	case PI_V1:
> +		*pixel_width = format->width - 1;
> +		*vsync_pulse = format->width << 1;
> +		break;
> +	case PI_V2:
> +		*pixel_width = format->width << 3;
> +		*vsync_pulse = format->width - 1;
> +		break;
> +	default:
> +		dev_err(pcpidev->dev, "Not support PI version %d\n", version);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void imx_cpi_config_ctrl_reg1(struct imx_cpi_device *pcpidev,
> +				     const struct v4l2_mbus_framefmt *format)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	struct device *dev = pcpidev->dev;
> +	u32 pixel_width;
> +	u32 vsync_pulse;
> +	u32 val;
> +	int ret;
> +
> +	dev_dbg(dev, "%s %dx%d, fmt->code:0x%0x\n", __func__,
> +		format->width, format->height, format->code);
> +
> +	if (format->width <= 0 || format->height <= 0) {
> +		dev_err(dev, "%s width/height invalid\n", __func__);
> +		return;
> +	}
> +
> +	ret = get_interface_ctrl_reg1_param(pcpidev, &pixel_width,
> +					    &vsync_pulse, format);
> +	if (ret < 0)
> +		return;
> +
> +	val = (CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> +	       CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse));
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
> +}
> +
> +static void imx_cpi_enable(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Enable CPI */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= CPI_CTRL_REG_CPI_EN;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable SYNC Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~(CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static void imx_cpi_disable(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Enable Sync Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable CPI */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~CPI_CTRL_REG_CPI_EN;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val &= ~(IF_CTRL_REG_PL_VALID | IF_CTRL_REG_PL_ENABLE);
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +}
> +
> +static void imx_cpi_start_stream(struct imx_cpi_device *pcpidev,
> +				 const struct v4l2_mbus_framefmt *format,
> +				 const struct imx_cpi_pix_format *pcpidev_fmt)
> +{
> +	if (pcpidev_fmt->code == MEDIA_BUS_FMT_YUYV8_2X8 ||
> +	    pcpidev_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8)
> +		pcpidev->uv_swap = 1;
> +
> +	imx_cpi_hw_config(pcpidev);
> +	imx_cpi_config_ctrl_reg1(pcpidev, format);
> +	imx_cpi_enable(pcpidev);
> +	imx_cpi_regs_dump(pcpidev);
> +}
> +
> +static void imx_cpi_stop_stream(struct imx_cpi_device *pcpidev)
> +{
> +	imx_cpi_regs_dump(pcpidev);
> +	imx_cpi_disable(pcpidev);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Async subdev notifier
> + */
> +
> +static struct imx_cpi_device *
> +notifier_to_imx_cpi_device(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct imx_cpi_device, notifier);
> +}
> +
> +static struct imx_cpi_device *
> +sd_to_imx_cpi_device(struct v4l2_subdev *sdev)
> +{
> +	return container_of(sdev, struct imx_cpi_device, sd);
> +}
> +
> +static int imx_cpi_notify_bound(struct v4l2_async_notifier *notifier,
> +				struct v4l2_subdev *sd,
> +				struct v4l2_async_connection *asd)
> +{
> +	struct imx_cpi_device *pcpidev = notifier_to_imx_cpi_device(notifier);
> +	struct media_pad *sink = &pcpidev->sd.entity.pads[IMX_CPI_PAD_SINK];
> +
> +	return v4l2_create_fwnode_links_to_pad(sd, sink, 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx_cpi_notify_ops = {
> +	.bound = imx_cpi_notify_bound,
> +};
> +
> +static int imx_cpi_async_register(struct imx_cpi_device *pcpidev)
> +{
> +	struct v4l2_fwnode_endpoint vep = {
> +		.bus_type = V4L2_MBUS_PARALLEL,
> +	};
> +	struct v4l2_async_connection *asd;
> +	struct fwnode_handle *ep __free(fwnode_handle) = NULL;
> +	int ret;
> +
> +	v4l2_async_subdev_nf_init(&pcpidev->notifier, &pcpidev->sd);
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(pcpidev->dev), 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep)
> +		return -ENOTCONN;
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +	if (ret)
> +		return ret;
> +
> +	asd = v4l2_async_nf_add_fwnode_remote(&pcpidev->notifier, ep,
> +					      struct v4l2_async_connection);
> +	if (IS_ERR(asd))
> +		return PTR_ERR(asd);
> +
> +	pcpidev->notifier.ops = &imx_cpi_notify_ops;
> +	ret = v4l2_async_nf_register(&pcpidev->notifier);
> +	if (ret)
> +		return ret;
> +
> +	return v4l2_async_register_subdev(&pcpidev->sd);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Media entity operations
> + */
> +
> +static int imx_cpi_link_setup(struct media_entity *entity,
> +			      const struct media_pad *local_pad,
> +			      const struct media_pad *remote_pad,
> +			      u32 flags)
> +{
> +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	struct v4l2_subdev *remote_sd;
> +
> +	dev_dbg(pcpidev->dev, "link setup %s -> %s", remote_pad->entity->name,
> +		local_pad->entity->name);
> +
> +	/* We only care about the link to the source. */
> +	if (!(local_pad->flags & MEDIA_PAD_FL_SINK))
> +		return 0;
> +
> +	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
> +	if (flags & MEDIA_LNK_FL_ENABLED) {
> +		if (pcpidev->source.sd)
> +			return -EBUSY;
> +
> +		pcpidev->source.sd = remote_sd;
> +		pcpidev->source.pad = remote_pad;
> +	} else {
> +		pcpidev->source.sd = NULL;
> +		pcpidev->source.pad = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct media_entity_operations imx_cpi_entity_ops = {
> +	.link_setup = imx_cpi_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
> +	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> +};
> +
> +static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *sd_state,
> +			   struct v4l2_subdev_format *sdformat)
> +{
> +	struct imx_cpi_pix_format const *pcpidev_fmt;
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	struct device *dev = pcpidev->dev;
> +	struct v4l2_mbus_framefmt *fmt;
> +	unsigned int align;
> +
> +	/*
> +	 * The Parallel cpi can't transcode in any way, the source format
> +	 * can't be modified.
> +	 */
> +	if (sdformat->pad == IMX_CPI_PAD_SOURCE)
> +		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> +
> +	/*
> +	 * Validate the media bus code and clamp and align the size.
> +	 *
> +	 * The total number of bits per line must be a multiple of 8. We thus
> +	 * need to align the width for formats that are not multiples of 8
> +	 * bits.
> +	 */
> +	pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
> +	if (!pcpidev_fmt)
> +		pcpidev_fmt = &imx_cpi_formats[0];
> +
> +	switch (pcpidev_fmt->width % 8) {
> +	case 0:
> +		align = 0;
> +		break;
> +	case 4:
> +		align = 1;
> +		break;
> +	case 2:
> +	case 6:
> +		align = 2;
> +		break;
> +	default:
> +		/* 1, 3, 5, 7 */
> +		align = 3;
> +		break;
> +	}
> +
> +	v4l_bound_align_image(&sdformat->format.width, 1,
> +			      IMX_CPI_MAX_PIX_WIDTH, align,
> +			      &sdformat->format.height, 1,
> +			      IMX_CPI_MAX_PIX_HEIGHT, 0, 0);
> +
> +	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	fmt->code = pcpidev_fmt->code;
> +	fmt->width = sdformat->format.width;
> +	fmt->height = sdformat->format.height;
> +	fmt->field = V4L2_FIELD_NONE;
> +	fmt->colorspace = sdformat->format.colorspace;
> +	fmt->quantization = sdformat->format.quantization;
> +	fmt->xfer_func = sdformat->format.xfer_func;
> +	fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
> +
> +	sdformat->format = *fmt;
> +
> +	/* Propagate the format from sink to source. */
> +	fmt = v4l2_subdev_state_get_format(sd_state, IMX_CPI_PAD_SOURCE);
> +	*fmt = sdformat->format;
> +
> +	/* The format on the source pad might change due to unpacking. */
> +	fmt->code = pcpidev_fmt->output;
> +
> +	dev_dbg(dev, "%s: fmt_code:0x%0x, %dx%d\n", __func__,
> +		fmt->code, fmt->width, fmt->height);
> +	return 0;
> +}
> +
> +static const struct v4l2_mbus_framefmt imx_cpi_default_fmt = {
> +	.code = IMX_CPI_DEF_MBUS_CODE,
> +	.width = IMX_CPI_DEF_PIX_WIDTH,
> +	.height = IMX_CPI_DEF_PIX_HEIGHT,
> +	.field = V4L2_FIELD_NONE,
> +	.colorspace = V4L2_COLORSPACE_SMPTE170M,
> +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> +	.quantization = V4L2_QUANTIZATION_LIM_RANGE,
> +};
> +
> +static int imx_cpi_init_state(struct v4l2_subdev *sd,
> +			      struct v4l2_subdev_state *sd_state)
> +{
> +	struct v4l2_subdev_format fmt = {
> +		.pad = IMX_CPI_PAD_SINK,
> +	};
> +
> +	fmt.format.code = imx_cpi_formats[0].code;
> +	fmt.format.width = IMX_CPI_DEF_PIX_WIDTH;
> +	fmt.format.height = IMX_CPI_DEF_PIX_HEIGHT;
> +
> +	fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
> +	fmt.format.xfer_func =
> +	    V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
> +	fmt.format.ycbcr_enc =
> +	    V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
> +	fmt.format.quantization =
> +	    V4L2_MAP_QUANTIZATION_DEFAULT(false,
> +					  fmt.format.colorspace,
> +					  fmt.format.ycbcr_enc);
> +
> +	return imx_cpi_set_fmt(sd, sd_state, &fmt);
> +}
> +
> +static int imx_cpi_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	const struct v4l2_mbus_framefmt *format;
> +	const struct imx_cpi_pix_format *pcpidev_fmt;
> +	struct v4l2_subdev_state *state;
> +	int ret;
> +
> +	if (!enable) {
> +		v4l2_subdev_disable_streams(pcpidev->source.sd,
> +					    pcpidev->source.pad->index, BIT(0));
> +
> +		imx_cpi_stop_stream(pcpidev);
> +
> +		pm_runtime_put(pcpidev->dev);
> +
> +		return 0;
> +	}
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +	format = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SINK);
> +	pcpidev_fmt = find_imx_cpi_format(format->code);
> +
> +	ret = pm_runtime_resume_and_get(pcpidev->dev);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	imx_cpi_start_stream(pcpidev, format, pcpidev_fmt);
> +
> +	ret = v4l2_subdev_enable_streams(pcpidev->source.sd,
> +					 pcpidev->source.pad->index, BIT(0));
> +	if (ret < 0)
> +		goto err_stop;
> +
> +	v4l2_subdev_unlock_state(state);
> +
> +	return 0;
> +
> +err_stop:
> +	imx_cpi_stop_stream(pcpidev);
> +	pm_runtime_put(pcpidev->dev);
> +err_unlock:
> +	v4l2_subdev_unlock_state(state);
> +	return ret;
> +}
> +
> +static int imx_cpi_enum_mbus_code(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *sd_state,
> +				  struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	/*
> +	 * The PARALLEL CPI can't transcode in any way, the source format
> +	 * is identical to the sink format.
> +	 */
> +	if (code->pad == IMX_CPI_PAD_SOURCE) {
> +		struct v4l2_mbus_framefmt *fmt;
> +
> +		if (code->index > 0)
> +			return -EINVAL;
> +
> +		fmt = v4l2_subdev_state_get_format(sd_state, code->pad);
> +		code->code = fmt->code;
> +		return 0;
> +	}
> +
> +	if (code->pad != IMX_CPI_PAD_SINK)
> +		return -EINVAL;
> +
> +	if (code->index >= ARRAY_SIZE(imx_cpi_formats))
> +		return -EINVAL;
> +
> +	code->code = imx_cpi_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int imx_cpi_get_frame_desc(struct v4l2_subdev *sd,
> +				  unsigned int pad,
> +				  struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[0];
> +	const struct imx_cpi_pix_format *pcpidev_fmt;
> +	const struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +
> +	if (pad != IMX_CPI_PAD_SOURCE)
> +		return -EINVAL;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +	fmt = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SOURCE);
> +	pcpidev_fmt = find_imx_cpi_format(fmt->code);
> +	v4l2_subdev_unlock_state(state);
> +
> +	if (!pcpidev_fmt)
> +		return -EPIPE;
> +
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
> +	fd->num_entries = 1;
> +
> +	entry->flags = 0;
> +	entry->pixelcode = pcpidev_fmt->code;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops imx_cpi_video_ops = {
> +	.s_stream = imx_cpi_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx_cpi_pad_ops = {
> +	.enum_mbus_code = imx_cpi_enum_mbus_code,
> +	.get_fmt = v4l2_subdev_get_fmt,
> +	.set_fmt = imx_cpi_set_fmt,
> +	.get_frame_desc = imx_cpi_get_frame_desc,
> +};
> +
> +static const struct v4l2_subdev_ops imx_cpi_subdev_ops = {
> +	.pad = &imx_cpi_pad_ops,
> +	.video = &imx_cpi_video_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops imx_cpi_internal_ops = {
> +	.init_state = imx_cpi_init_state,
> +};
> +
> +static int imx_cpi_clk_get(struct imx_cpi_device *pcpidev)
> +{
> +	unsigned int i;
> +	int ret = 0;
> +
> +	for (i = 0; i < PCPIDEV_NUM_CLKS; i++)
> +		pcpidev->clks[i].id = imx_cpi_clk_id[i];
> +
> +	ret = devm_clk_bulk_get(pcpidev->dev, PCPIDEV_NUM_CLKS, pcpidev->clks);
> +
> +	return ret;
> +}
> +
> +/* ----------------------------------------------------------------------
> + * Suspend/resume
> + */
> +static int __maybe_unused imx_cpi_runtime_suspend(struct device *dev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +
> +	clk_bulk_disable_unprepare(PCPIDEV_NUM_CLKS, pcpidev->clks);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused imx_cpi_runtime_resume(struct device *dev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +
> +	return clk_bulk_prepare_enable(PCPIDEV_NUM_CLKS, pcpidev->clks);
> +}
> +
> +static const struct dev_pm_ops imx_cpi_pm_ops = {
> +	RUNTIME_PM_OPS(imx_cpi_runtime_suspend, imx_cpi_runtime_resume, NULL)
> +};
> +
> +static int imx_cpi_subdev_init(struct imx_cpi_device *pcpidev)
> +{
> +	struct v4l2_subdev *sd = &pcpidev->sd;
> +	int ret;
> +
> +	v4l2_subdev_init(sd, &imx_cpi_subdev_ops);
> +
> +	sd->internal_ops = &imx_cpi_internal_ops;
> +	sd->owner = THIS_MODULE;
> +	snprintf(sd->name, sizeof(sd->name), "parallel-%s",
> +		 dev_name(pcpidev->dev));
> +
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	sd->ctrl_handler = NULL;
> +
> +	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	sd->entity.ops = &imx_cpi_entity_ops;
> +
> +	sd->dev = pcpidev->dev;
> +
> +	pcpidev->pads[IMX_CPI_PAD_SINK].flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> +	pcpidev->pads[IMX_CPI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE |
> +						       MEDIA_PAD_FL_MUST_CONNECT;
> +
> +	ret = media_entity_pads_init(&sd->entity, IMX_CPI_PADS_NUM, pcpidev->pads);
> +	if (ret)
> +		return ret;
> +
> +	ret = v4l2_subdev_init_finalize(sd);
> +	if (ret)
> +		media_entity_cleanup(&sd->entity);
> +
> +	return ret;
> +}
> +
> +static void imx_cpi_cleanup(void *data)
> +{
> +	struct imx_cpi_device *pcpidev = data;
> +
> +	v4l2_subdev_cleanup(&pcpidev->sd);
> +	media_entity_cleanup(&pcpidev->sd.entity);
> +	v4l2_async_nf_unregister(&pcpidev->notifier);
> +	v4l2_async_nf_cleanup(&pcpidev->notifier);
> +	v4l2_async_unregister_subdev(&pcpidev->sd);
> +}
> +
> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> +	struct imx_cpi_device *pcpidev;
> +	struct device *dev = &pdev->dev;
> +	int ret = 0;
> +
> +	pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
> +	if (!pcpidev)
> +		return -ENOMEM;
> +
> +	pcpidev->dev = dev;
> +	platform_set_drvdata(pdev, pcpidev);
> +
> +	pcpidev->pdata = of_device_get_match_data(dev);
> +
> +	pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pcpidev->regs))
> +		return dev_err_probe(dev, PTR_ERR(pcpidev->regs),
> +				     "Failed to get regs\n");
> +
> +	ret = imx_cpi_clk_get(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = imx_cpi_subdev_init(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, imx_cpi_cleanup, pcpidev);
> +	if (ret)
> +		return ret;
> +
> +	pcpidev->mode = PI_GATE_CLOCK_MODE;
> +
> +	platform_set_drvdata(pdev, &pcpidev->sd);
> +
> +	ret = imx_cpi_async_register(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	devm_pm_runtime_enable(dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx_cpi_of_match[] = {
> +	{.compatible = "fsl,imx8qxp-pcif", .data = &imx8qxp_pdata },
> +	{.compatible = "fsl,imx93-pcif", .data = &imx93_pdata },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, imx_cpi_of_match);
> +
> +static struct platform_driver _driver = {
> +	.probe = imx_cpi_probe,
> +	.driver = {
> +		.of_match_table = imx_cpi_of_match,
> +		.name = "imx-parallel-cpi",
> +		.pm = pm_ptr(&imx_cpi_pm_ops),
> +	},
> +};
> +
> +module_platform_driver(_driver);
> +
> +MODULE_DESCRIPTION("i.MX9 Parallel CPI receiver driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support
  2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
  2025-07-09  8:22   ` Krzysztof Kozlowski
@ 2025-07-24 22:15   ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2025-07-24 22:15 UTC (permalink / raw)
  To: Frank.Li
  Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan

Hi Frank,

On Tue, Jul 08, 2025 at 01:48:42PM -0400, Frank Li via B4 Relay wrote:
> From: Alice Yuan <alice.yuan@nxp.com>
> 
> Document the binding for parallel CPI controller found in i.MX8QXP, i.MX93
> and i.MX91 SoCs.
> 
> Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

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

> ---
> Change in v3:
> - use enum at compatible string
> - add ref to video-interfaces.yaml#
> - use cpi as node name in examples.
> - replace csi (Camera Serial Interface) with CPI (Camera Parallel Interface)
> in commit message.
> 
> Change in v2:
> - use pcif surfix as Laurent Pinchart's suggest.
> - put power-domains into required list
> ---
>  .../devicetree/bindings/media/fsl,imx93-pcif.yaml  | 126 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 127 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..f92e9af371c91636cef42e48573198d4ea5e15f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/fsl,imx93-pcif.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX8/9 Parallel Camera Interface
> +
> +maintainers:
> +  - Frank Li <Frank.Li@nxp.com>
> +
> +description: |
> +  This is device node for the Parallel Camera Interface which enables the
> +  chip to connect directly to external Parallel CMOS image sensors.
> +  Supports up to 80MHz input clock from sensor.
> +  Supports the following input data formats
> +    - 8-bit/10-bit Camera Sensor Interface (CSI)
> +    - 8-bit data port for RGB, YCbCr, and YUV data input
> +    - 8-bit/10-bit data ports for Bayer data input
> +  Parallel Camera Interface is hooked to the Imaging subsystem via the
> +  Pixel Link.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - fsl,imx8qxp-pcif
> +          - fsl,imx93-pcif
> +      - items:
> +          - enum:
> +              - fsl,imx91-pcif
> +          - const: fsl,imx93-pcif
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: pixel
> +      - const: ipg
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        unevaluatedProperties: false
> +        description: Input port node.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              bus-type:
> +                const: 5
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        unevaluatedProperties: false
> +        description: Output port node.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              bus-type:
> +                const: 5
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx93-clock.h>
> +    #include <dt-bindings/power/fsl,imx93-power.h>
> +
> +    cpi@4ac10070 {
> +        compatible = "fsl,imx93-pcif";
> +        reg = <0x4ac10070 0x10>;
> +        clocks = <&clk IMX93_CLK_MIPI_CSI_GATE>,
> +                 <&clk IMX93_CLK_MEDIA_APB>;
> +        clock-names = "pixel", "ipg";
> +        assigned-clocks = <&clk IMX93_CLK_CAM_PIX>;
> +        assigned-clock-parents = <&clk IMX93_CLK_VIDEO_PLL>;
> +        assigned-clock-rates = <140000000>;
> +        power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_MIPI_CSI>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +
> +                endpoint {
> +                    remote-endpoint = <&mt9m114_ep>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                endpoint {
> +                    remote-endpoint = <&isi_in>;
> +                };
> +            };
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8dc0f6609d1fe2e3eefd50088dbe566d9e107bfa..8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15107,6 +15107,7 @@ L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media.git
>  F:	Documentation/admin-guide/media/imx7.rst
> +F:	Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
  2025-07-08 17:48 ` [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI) Frank Li via B4 Relay
  2025-07-18 16:44   ` Frank Li
@ 2025-07-25  0:04   ` Laurent Pinchart
  2025-07-25 15:38     ` Frank Li
  1 sibling, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2025-07-25  0:04 UTC (permalink / raw)
  To: Frank.Li
  Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan,
	Robert Chiras, Zhipeng Wang

Hi Frank,

Thank you for the patch.

On Tue, Jul 08, 2025 at 01:48:43PM -0400, Frank Li via B4 Relay wrote:
> From: Alice Yuan <alice.yuan@nxp.com>
> 
> Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
> i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
> and enables image data capture through a parallel interface.
> 
> Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v3
> - replace csi with cpi
> - use __free(fwnode_handle) to simpilfy code
> - remove imx91 driver data, which is the same as imx93
> 
> change in v2
> - remove MODULE_ALIAS
> - use devm_pm_runtime_enable() and cleanup remove function
> - change output format to 1x16. controller convert 2x8 to 1x16 format
> ---
>  MAINTAINERS                                   |   1 +
>  drivers/media/platform/nxp/Kconfig            |  11 +
>  drivers/media/platform/nxp/Makefile           |   1 +
>  drivers/media/platform/nxp/imx-parallel-cpi.c | 920 ++++++++++++++++++++++++++
>  4 files changed, 933 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303..14842a3b860a6f23846f12a684eedcbb9eb69e19 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15112,6 +15112,7 @@ F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
>  F:	drivers/media/platform/nxp/imx-mipi-csis.c
> +F:	drivers/media/platform/nxp/imx-parallel-cpi.c
>  F:	drivers/media/platform/nxp/imx7-media-csi.c
>  F:	drivers/media/platform/nxp/imx8mq-mipi-csi2.c
>  
> diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e213fdc5da70821dc0b420e1821f4f..504ae1c6494f331c16124a224421ac7acd433ba5 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -39,6 +39,17 @@ config VIDEO_IMX_MIPI_CSIS
>  	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
>  	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
>  
> +config VIDEO_IMX_PARALLEL_CPI
> +	tristate "NXP i.MX9/i.MX8 Parallel CPI Driver"

I'd write it in numerical order, "i.MX8/i.MX9"

> +	depends on ARCH_MXC || COMPILE_TEST
> +	depends on VIDEO_DEV
> +	select MEDIA_CONTROLLER
> +	select V4L2_FWNODE
> +	select VIDEO_V4L2_SUBDEV_API
> +	help
> +	  Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> +	  on some iMX8 and iMX9 SoCs.
> +
>  source "drivers/media/platform/nxp/imx8-isi/Kconfig"
>  
>  # mem2mem drivers
> diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
> index 4d90eb71365259ebdda84ea58483e1c4131d3ac7..5346919d2f1083b51ec99b66981c5d38b3df960c 100644
> --- a/drivers/media/platform/nxp/Makefile
> +++ b/drivers/media/platform/nxp/Makefile
> @@ -7,5 +7,6 @@ obj-y += imx8-isi/
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
>  obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> +obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
>  obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
>  obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..718f02bf70c4d0ae74ecf842c1ecb1a1afbcdd45
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,920 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX Parallel CPI receiver driver.
> + *
> + * Copyright 2019-2025 NXP
> + *
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/debugfs.h>

I don't think you use this.

> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>

I don't think you use this either.

> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/mfd/syscon.h>

Alphabetical order please.

> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>

This doesn't seem needed.

> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>

Same here.

> +#include <linux/spinlock.h>

And here. Please verify all headers, drop the ones you don't need, and
add any you may have forgotten. That includes the media/ headers.

> +
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define IMX_CPI_DEF_MBUS_CODE			MEDIA_BUS_FMT_UYVY8_2X8
> +#define IMX_CPI_DEF_PIX_WIDTH			1920
> +#define IMX_CPI_DEF_PIX_HEIGHT			1080

Those macros are used in a single location only (or will be in the next
version), drop them and use the values directly in imx_cpi_default_fmt.

> +
> +#define IMX_CPI_MAX_PIX_WIDTH			0xffff
> +#define IMX_CPI_MAX_PIX_HEIGHT			0xffff
> +
> +#define IMX_CPI_PAD_SINK			0
> +#define IMX_CPI_PAD_SOURCE			1
> +#define IMX_CPI_PADS_NUM			2
> +
> +/* CI_PI INTERFACE CONTROL */
> +#define IF_CTRL_REG_PL_ENABLE			BIT(0)
> +#define IF_CTRL_REG_PL_VALID			BIT(1)
> +#define IF_CTRL_REG_DATA_TYPE_SEL		BIT(8)
> +#define IF_CTRL_REG_DATA_TYPE(x)		FIELD_PREP(GENMASK(13, 9), (x))
> +
> +#define DATA_TYPE_OUT_NULL			0x00
> +#define DATA_TYPE_OUT_RGB			0x04
> +#define DATA_TYPE_OUT_YUV444			0x08
> +#define DATA_TYPE_OUT_YYU420_ODD		0x10
> +#define DATA_TYPE_OUT_YYU420_EVEN		0x12
> +#define DATA_TYPE_OUT_YYY_ODD			0x18
> +#define DATA_TYPE_OUT_UYVY_EVEN			0x1a
> +#define DATA_TYPE_OUT_RAW			0x1c
> +
> +#define IF_CTRL_REG_IF_FORCE_HSYNV_OVERRIDE	0x4
> +#define IF_CTRL_REG_IF_FORCE_VSYNV_OVERRIDE	0x2
> +#define IF_CTRL_REG_IF_FORCE_DATA_ENABLE_OVERRIDE	0x1
> +
> +/* CPI INTERFACE CONTROL REG */
> +#define CPI_CTRL_REG_CPI_EN			BIT(0)
> +#define CPI_CTRL_REG_PIXEL_CLK_POL		BIT(1)
> +#define CPI_CTRL_REG_HSYNC_POL			BIT(2)
> +#define CPI_CTRL_REG_VSYNC_POL			BIT(3)
> +#define CPI_CTRL_REG_DE_POL			BIT(4)
> +#define CPI_CTRL_REG_PIXEL_DATA_POL		BIT(5)
> +#define CPI_CTRL_REG_CCIR_EXT_VSYNC_EN		BIT(6)
> +#define CPI_CTRL_REG_CCIR_EN			BIT(7)
> +#define CPI_CTRL_REG_CCIR_VIDEO_MODE		BIT(8)
> +#define CPI_CTRL_REG_CCIR_NTSC_EN		BIT(9)
> +#define CPI_CTRL_REG_CCIR_VSYNC_RESET_EN	BIT(10)
> +#define CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN	BIT(11)
> +#define CPI_CTRL_REG_HSYNC_FORCE_EN		BIT(12)
> +#define CPI_CTRL_REG_VSYNC_FORCE_EN		BIT(13)
> +#define CPI_CTRL_REG_GCLK_MODE_EN		BIT(14)
> +#define CPI_CTRL_REG_VALID_SEL			BIT(15)
> +#define CPI_CTRL_REG_RAW_OUT_SEL		BIT(16)
> +#define CPI_CTRL_REG_HSYNC_OUT_SEL		BIT(17)
> +#define CPI_CTRL_REG_HSYNC_PULSE(x)		FIELD_PREP(GENMASK(21, 19), (x))
> +#define CPI_CTRL_REG_UV_SWAP_EN			BIT(22)
> +#define CPI_CTRL_REG_DATA_TYPE_IN(x)		FIELD_PREP(GENMASK(26, 23), (x))
> +#define CPI_CTRL_REG_MASK_VSYNC_COUNTER(x)	FIELD_PREP(GENMASK(28, 27), (x))
> +#define CPI_CTRL_REG_SOFTRST			BIT(31)
> +
> +/* CPI INTERFACE STATUS */
> +#define CPI_STATUS_FIELD_TOGGLE			BIT(0)
> +#define CPI_STATUS_ECC_ERROR			BIT(1)
> +
> +/* CPI INTERFACE CONTROL REG1 */
> +#define CPI_CTRL_REG1_PIXEL_WIDTH(v)		FIELD_PREP(GENMASK(15, 0), (v))
> +#define CPI_CTRL_REG1_VSYNC_PULSE(v)		FIELD_PREP(GENMASK(31, 16), (v))
> +
> +/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
> +enum cpi_in_data_type {
> +	CPI_IN_DT_UYVY_BT656_8 = 0x0,
> +	CPI_IN_DT_UYVY_BT656_10,
> +	CPI_IN_DT_RGB_8,
> +	CPI_IN_DT_BGR_8,
> +	CPI_IN_DT_YVYU_8 = 0x5,
> +	CPI_IN_DT_YUV_8,
> +	CPI_IN_DT_RAW_8 = 0x9,
> +	CPI_IN_DT_RAW_10,
> +};
> +
> +enum {
> +	PI_MODE_INIT,

Not used.

> +	PI_GATE_CLOCK_MODE,
> +	PI_CCIR_MODE,
> +};
> +
> +enum {
> +	PI_V1,
> +	PI_V2,
> +};
> +
> +static const char *const imx_cpi_clk_id[] = {
> +	"pixel",
> +	"ipg",
> +};
> +
> +#define PCPIDEV_NUM_CLKS   ARRAY_SIZE(imx_cpi_clk_id)

Drop this macro and use ARRAY_SIZE() in the single location below.

> +
> +struct imx_cpi_plat_data {
> +	u32 version;
> +	u32 if_ctrl_reg;
> +	u32 interface_status;
> +	u32 interface_ctrl_reg;
> +	u32 interface_ctrl_reg1;
> +	u8 def_hsync_pol;
> +	u8 def_vsync_pol;
> +	u8 def_pixel_clk_pol;
> +	u8 def_cpi_in_data_type;
> +};
> +
> +struct cpi_pm_domain {
> +	struct device *dev;
> +	struct device_link *link;
> +};
> +
> +struct imx_cpi_device {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct reset_control *mrst;
> +	struct regulator *pcpi_phy_regulator;

Those two fields are unusued.

> +	struct clk_bulk_data clks[PCPIDEV_NUM_CLKS];
> +
> +	struct v4l2_subdev sd;
> +	struct media_pad pads[IMX_CPI_PADS_NUM];
> +	struct v4l2_async_notifier notifier;
> +
> +	struct v4l2_mbus_framefmt format;

Unused.

> +	const struct imx_cpi_plat_data *pdata;
> +	struct imx_cpi_pix_format const *pcpidev_fmt;

Unused.

> +
> +	struct {
> +		struct v4l2_subdev *sd;
> +		const struct media_pad *pad;
> +	} source;
> +
> +	struct cpi_pm_domain pm_domains[2];

Unused.

> +
> +	u8 mode;
> +	u8 uv_swap;
> +};
> +
> +struct imx_cpi_pix_format {
> +	u32 code;
> +	u32 output;
> +	u32 data_type;

This isn't used. It seems a bit weird though that .def_cpi_in_data_type
is used, but not the per-format data type.

> +	u8 width;
> +};
> +
> +static const struct imx_cpi_pix_format imx_cpi_formats[] = {
> +	/* YUV formats. */
> +	{
> +		.code = MEDIA_BUS_FMT_UYVY8_2X8,
> +		.output = MEDIA_BUS_FMT_UYVY8_1X16,
> +		.data_type = CPI_IN_DT_YVYU_8,
> +		.width = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV8_2X8,
> +		.output = MEDIA_BUS_FMT_YUYV8_1X16,
> +		.data_type = CPI_IN_DT_YVYU_8,
> +		.width = 16,
> +	},
> +};
> +
> +static const struct imx_cpi_plat_data imx8qxp_pdata = {
> +	.version = PI_V1,
> +	.if_ctrl_reg = 0x0,
> +	.interface_status = 0x20,
> +	.interface_ctrl_reg = 0x10,
> +	.interface_ctrl_reg1 = 0x30,
> +	.def_hsync_pol = 1,
> +	.def_vsync_pol = 0,
> +	.def_pixel_clk_pol = 0,
> +	.def_cpi_in_data_type = CPI_IN_DT_UYVY_BT656_8,
> +};
> +
> +static const struct imx_cpi_plat_data imx93_pdata = {
> +	.version = PI_V2,
> +	.if_ctrl_reg = 0x0,
> +	.interface_status = 0x4,
> +	.interface_ctrl_reg = 0x8,
> +	.interface_ctrl_reg1 = 0xc,
> +	.def_hsync_pol = 0,
> +	.def_vsync_pol = 1,
> +	.def_pixel_clk_pol = 0,

This is the same for the two variants. Do you plan to support other
variants that will have default clock polarities ? If not, drop this.

> +	.def_cpi_in_data_type = CPI_IN_DT_YVYU_8,
> +};
> +
> +static void imx_cpi_regs_dump(struct imx_cpi_device *pcpidev)
> +{
> +	struct device *dev = pcpidev->dev;
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 i;
> +
> +	struct {
> +		u32 offset;
> +		const char *const name;
> +	} registers[] = {
> +		{ pdata->if_ctrl_reg, "HW_IF_CTRL_REG" },
> +		{ pdata->interface_ctrl_reg, "HW_CPI_CTRL_REG" },
> +		{ pdata->interface_status, "HW_CPI_STATUS" },
> +		{ pdata->interface_ctrl_reg1, "HW_CPI_CTRL_REG1" },
> +
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> +		u32 reg = readl(pcpidev->regs + registers[i].offset);
> +
> +		dev_dbg(dev, "%20s[0x%.2x]: 0x%.8x\n",
> +			registers[i].name, registers[i].offset, reg);
> +	}
> +}
> +
> +static const struct imx_cpi_pix_format *find_imx_cpi_format(u32 code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(imx_cpi_formats); i++)
> +		if (code == imx_cpi_formats[i].code)
> +			return &imx_cpi_formats[i];
> +
> +	return NULL;
> +}
> +
> +static void imx_cpi_sw_reset(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Softwaret Reset */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= CPI_CTRL_REG_SOFTRST;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	usleep_range(500, 1000);
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~CPI_CTRL_REG_SOFTRST;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Software Reset */
> +	imx_cpi_sw_reset(pcpidev);
> +
> +	/* Config PL Data Type */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);

Don't you need to clear the field first ? Same everywhere else where
applicable.

> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Enable sync Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Enable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_PL_ENABLE;
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Enable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val |= IF_CTRL_REG_PL_VALID;
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +
> +	/* Config CTRL REG */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	val |= (CPI_CTRL_REG_DATA_TYPE_IN(pdata->def_cpi_in_data_type) |
> +		FIELD_PREP(CPI_CTRL_REG_HSYNC_POL, pdata->def_hsync_pol) |
> +		FIELD_PREP(CPI_CTRL_REG_VSYNC_POL, pdata->def_vsync_pol) |
> +		FIELD_PREP(CPI_CTRL_REG_PIXEL_CLK_POL, pdata->def_pixel_clk_pol) |

You should parse the polarities from the device tree.

> +		CPI_CTRL_REG_MASK_VSYNC_COUNTER(3) |
> +		CPI_CTRL_REG_HSYNC_PULSE(2));
> +
> +	if (pcpidev->uv_swap)
> +		val |= CPI_CTRL_REG_UV_SWAP_EN;
> +
> +	if (pcpidev->mode & PI_GATE_CLOCK_MODE) {

mode is hardcoded to PI_GATE_CLOCK_MODE. And the mode seem exclusive, so
they shouldn't be handled as a bitmask, especially given that the
PI_GATE_CLOCK_MODE and PI_CCIR_MODE enumerators have a single bit set
only by accident.

> +		val |= CPI_CTRL_REG_GCLK_MODE_EN;
> +	} else if (pcpidev->mode & PI_CCIR_MODE) {
> +		val |= (CPI_CTRL_REG_CCIR_EN |
> +			CPI_CTRL_REG_CCIR_VSYNC_RESET_EN |
> +			CPI_CTRL_REG_CCIR_EXT_VSYNC_EN |
> +			CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN);
> +	}
> +
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static int get_interface_ctrl_reg1_param(struct imx_cpi_device *pcpidev,

imx_cpi_ prefix please.

> +					 u32 *pixel_width, u32 *vsync_pulse,
> +					 const struct v4l2_mbus_framefmt *format)
> +{
> +	u32 version = pcpidev->pdata->version;

No need for the variable.

> +
> +	switch (version) {
> +	case PI_V1:
> +		*pixel_width = format->width - 1;
> +		*vsync_pulse = format->width << 1;
> +		break;
> +	case PI_V2:
> +		*pixel_width = format->width << 3;
> +		*vsync_pulse = format->width - 1;
> +		break;
> +	default:
> +		dev_err(pcpidev->dev, "Not support PI version %d\n", version);
> +		return -EINVAL;

This can't happen. You can make this function void and drop the error
check in the caller. Actually, as the caller is rather simple, and this
function is very simple too, you can just inline it in the caller.

> +	}
> +
> +	return 0;
> +}
> +
> +static void imx_cpi_config_ctrl_reg1(struct imx_cpi_device *pcpidev,
> +				     const struct v4l2_mbus_framefmt *format)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	struct device *dev = pcpidev->dev;
> +	u32 pixel_width;
> +	u32 vsync_pulse;
> +	u32 val;
> +	int ret;
> +
> +	dev_dbg(dev, "%s %dx%d, fmt->code:0x%0x\n", __func__,
> +		format->width, format->height, format->code);
> +
> +	if (format->width <= 0 || format->height <= 0) {
> +		dev_err(dev, "%s width/height invalid\n", __func__);
> +		return;
> +	}

This can't happen.

> +
> +	ret = get_interface_ctrl_reg1_param(pcpidev, &pixel_width,
> +					    &vsync_pulse, format);
> +	if (ret < 0)
> +		return;
> +
> +	val = (CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> +	       CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse));

No need for parentheses.

> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
> +}
> +
> +static void imx_cpi_enable(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Enable CPI */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= CPI_CTRL_REG_CPI_EN;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable SYNC Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~(CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +}
> +
> +static void imx_cpi_disable(struct imx_cpi_device *pcpidev)
> +{
> +	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> +	u32 val;
> +
> +	/* Enable Sync Force */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);

No need for parentheses.

> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable CPI */
> +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> +	val &= ~CPI_CTRL_REG_CPI_EN;
> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> +
> +	/* Disable Pixel Link */
> +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> +	val &= ~(IF_CTRL_REG_PL_VALID | IF_CTRL_REG_PL_ENABLE);
> +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> +}
> +
> +static void imx_cpi_start_stream(struct imx_cpi_device *pcpidev,
> +				 const struct v4l2_mbus_framefmt *format,
> +				 const struct imx_cpi_pix_format *pcpidev_fmt)
> +{
> +	if (pcpidev_fmt->code == MEDIA_BUS_FMT_YUYV8_2X8 ||
> +	    pcpidev_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8)
> +		pcpidev->uv_swap = 1;

Pass the information required to calculate this to the
imx_cpi_hw_config() function and drop the uv_swap field.

> +
> +	imx_cpi_hw_config(pcpidev);
> +	imx_cpi_config_ctrl_reg1(pcpidev, format);
> +	imx_cpi_enable(pcpidev);
> +	imx_cpi_regs_dump(pcpidev);
> +}
> +
> +static void imx_cpi_stop_stream(struct imx_cpi_device *pcpidev)
> +{
> +	imx_cpi_regs_dump(pcpidev);
> +	imx_cpi_disable(pcpidev);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Async subdev notifier
> + */
> +
> +static struct imx_cpi_device *
> +notifier_to_imx_cpi_device(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct imx_cpi_device, notifier);
> +}
> +
> +static struct imx_cpi_device *
> +sd_to_imx_cpi_device(struct v4l2_subdev *sdev)
> +{
> +	return container_of(sdev, struct imx_cpi_device, sd);
> +}
> +
> +static int imx_cpi_notify_bound(struct v4l2_async_notifier *notifier,
> +				struct v4l2_subdev *sd,
> +				struct v4l2_async_connection *asd)
> +{
> +	struct imx_cpi_device *pcpidev = notifier_to_imx_cpi_device(notifier);
> +	struct media_pad *sink = &pcpidev->sd.entity.pads[IMX_CPI_PAD_SINK];
> +
> +	return v4l2_create_fwnode_links_to_pad(sd, sink, 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx_cpi_notify_ops = {
> +	.bound = imx_cpi_notify_bound,
> +};
> +
> +static int imx_cpi_async_register(struct imx_cpi_device *pcpidev)
> +{
> +	struct v4l2_fwnode_endpoint vep = {
> +		.bus_type = V4L2_MBUS_PARALLEL,
> +	};
> +	struct v4l2_async_connection *asd;
> +	struct fwnode_handle *ep __free(fwnode_handle) = NULL;
> +	int ret;
> +
> +	v4l2_async_subdev_nf_init(&pcpidev->notifier, &pcpidev->sd);

Move this after v4l2_fwnode_endpoint_parse() so you won't need to
cleanup if DT parsing fails.

> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(pcpidev->dev), 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep)
> +		return -ENOTCONN;
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +	if (ret)
> +		return ret;
> +
> +	asd = v4l2_async_nf_add_fwnode_remote(&pcpidev->notifier, ep,
> +					      struct v4l2_async_connection);
> +	if (IS_ERR(asd))
> +		return PTR_ERR(asd);
> +
> +	pcpidev->notifier.ops = &imx_cpi_notify_ops;

Move this just after v4l2_async_subdev_nf_init().

> +	ret = v4l2_async_nf_register(&pcpidev->notifier);
> +	if (ret)
> +		return ret;
> +
> +	return v4l2_async_register_subdev(&pcpidev->sd);

This will need to move to the probe function, see below.

> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Media entity operations
> + */
> +
> +static int imx_cpi_link_setup(struct media_entity *entity,
> +			      const struct media_pad *local_pad,
> +			      const struct media_pad *remote_pad,
> +			      u32 flags)
> +{
> +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	struct v4l2_subdev *remote_sd;
> +
> +	dev_dbg(pcpidev->dev, "link setup %s -> %s", remote_pad->entity->name,
> +		local_pad->entity->name);
> +
> +	/* We only care about the link to the source. */
> +	if (!(local_pad->flags & MEDIA_PAD_FL_SINK))
> +		return 0;
> +
> +	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
> +	if (flags & MEDIA_LNK_FL_ENABLED) {
> +		if (pcpidev->source.sd)
> +			return -EBUSY;
> +
> +		pcpidev->source.sd = remote_sd;
> +		pcpidev->source.pad = remote_pad;
> +	} else {
> +		pcpidev->source.sd = NULL;
> +		pcpidev->source.pad = NULL;
> +	}

You only support one connected subdev, so you can make the sink link
immutable and avoid the complexity here. source.sd and source.pad can be
assigned in the notifier .bound() handler.

> +
> +	return 0;
> +}
> +
> +static const struct media_entity_operations imx_cpi_entity_ops = {
> +	.link_setup = imx_cpi_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
> +	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> +};
> +
> +static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *sd_state,
> +			   struct v4l2_subdev_format *sdformat)
> +{
> +	struct imx_cpi_pix_format const *pcpidev_fmt;
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	struct device *dev = pcpidev->dev;
> +	struct v4l2_mbus_framefmt *fmt;
> +	unsigned int align;
> +
> +	/*
> +	 * The Parallel cpi can't transcode in any way, the source format
> +	 * can't be modified.
> +	 */
> +	if (sdformat->pad == IMX_CPI_PAD_SOURCE)
> +		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> +
> +	/*
> +	 * Validate the media bus code and clamp and align the size.
> +	 *
> +	 * The total number of bits per line must be a multiple of 8. We thus
> +	 * need to align the width for formats that are not multiples of 8
> +	 * bits.

All formats are multiples of 16 bits.

> +	 */
> +	pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
> +	if (!pcpidev_fmt)
> +		pcpidev_fmt = &imx_cpi_formats[0];
> +
> +	switch (pcpidev_fmt->width % 8) {
> +	case 0:
> +		align = 0;
> +		break;
> +	case 4:
> +		align = 1;
> +		break;
> +	case 2:
> +	case 6:
> +		align = 2;
> +		break;
> +	default:
> +		/* 1, 3, 5, 7 */
> +		align = 3;
> +		break;
> +	}
> +
> +	v4l_bound_align_image(&sdformat->format.width, 1,
> +			      IMX_CPI_MAX_PIX_WIDTH, align,
> +			      &sdformat->format.height, 1,
> +			      IMX_CPI_MAX_PIX_HEIGHT, 0, 0);
> +
> +	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
> +	if (!fmt)
> +		return -EINVAL;

I don't think this can return an error.

> +
> +	fmt->code = pcpidev_fmt->code;
> +	fmt->width = sdformat->format.width;
> +	fmt->height = sdformat->format.height;
> +	fmt->field = V4L2_FIELD_NONE;
> +	fmt->colorspace = sdformat->format.colorspace;
> +	fmt->quantization = sdformat->format.quantization;
> +	fmt->xfer_func = sdformat->format.xfer_func;
> +	fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
> +
> +	sdformat->format = *fmt;
> +
> +	/* Propagate the format from sink to source. */
> +	fmt = v4l2_subdev_state_get_format(sd_state, IMX_CPI_PAD_SOURCE);
> +	*fmt = sdformat->format;
> +
> +	/* The format on the source pad might change due to unpacking. */
> +	fmt->code = pcpidev_fmt->output;
> +
> +	dev_dbg(dev, "%s: fmt_code:0x%0x, %dx%d\n", __func__,
> +		fmt->code, fmt->width, fmt->height);

I think you can drop this, it's useful for development, but less so in
upstream.

> +	return 0;
> +}
> +
> +static const struct v4l2_mbus_framefmt imx_cpi_default_fmt = {

This isn't used.

> +	.code = IMX_CPI_DEF_MBUS_CODE,
> +	.width = IMX_CPI_DEF_PIX_WIDTH,
> +	.height = IMX_CPI_DEF_PIX_HEIGHT,
> +	.field = V4L2_FIELD_NONE,
> +	.colorspace = V4L2_COLORSPACE_SMPTE170M,
> +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SMPTE170M),
> +	.quantization = V4L2_QUANTIZATION_LIM_RANGE,
> +};
> +
> +static int imx_cpi_init_state(struct v4l2_subdev *sd,
> +			      struct v4l2_subdev_state *sd_state)
> +{
> +	struct v4l2_subdev_format fmt = {
> +		.pad = IMX_CPI_PAD_SINK,
> +	};
> +
> +	fmt.format.code = imx_cpi_formats[0].code;
> +	fmt.format.width = IMX_CPI_DEF_PIX_WIDTH;
> +	fmt.format.height = IMX_CPI_DEF_PIX_HEIGHT;
> +
> +	fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
> +	fmt.format.xfer_func =
> +	    V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
> +	fmt.format.ycbcr_enc =
> +	    V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
> +	fmt.format.quantization =
> +	    V4L2_MAP_QUANTIZATION_DEFAULT(false,
> +					  fmt.format.colorspace,
> +					  fmt.format.ycbcr_enc);
> +
> +	return imx_cpi_set_fmt(sd, sd_state, &fmt);

If possible, I'd just get the sink and source pad formats with
v4l2_subdev_get_fmt() here and set them to imx_cpi_default_fmt. That
will be simpler.

> +}
> +
> +static int imx_cpi_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	const struct v4l2_mbus_framefmt *format;
> +	const struct imx_cpi_pix_format *pcpidev_fmt;
> +	struct v4l2_subdev_state *state;
> +	int ret;
> +
> +	if (!enable) {
> +		v4l2_subdev_disable_streams(pcpidev->source.sd,
> +					    pcpidev->source.pad->index, BIT(0));
> +
> +		imx_cpi_stop_stream(pcpidev);
> +
> +		pm_runtime_put(pcpidev->dev);
> +
> +		return 0;
> +	}
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +	format = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SINK);
> +	pcpidev_fmt = find_imx_cpi_format(format->code);
> +
> +	ret = pm_runtime_resume_and_get(pcpidev->dev);
> +	if (ret < 0)
> +		goto err_unlock;
> +
> +	imx_cpi_start_stream(pcpidev, format, pcpidev_fmt);
> +
> +	ret = v4l2_subdev_enable_streams(pcpidev->source.sd,
> +					 pcpidev->source.pad->index, BIT(0));
> +	if (ret < 0)
> +		goto err_stop;
> +
> +	v4l2_subdev_unlock_state(state);
> +
> +	return 0;
> +
> +err_stop:
> +	imx_cpi_stop_stream(pcpidev);
> +	pm_runtime_put(pcpidev->dev);
> +err_unlock:
> +	v4l2_subdev_unlock_state(state);
> +	return ret;
> +}
> +
> +static int imx_cpi_enum_mbus_code(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *sd_state,
> +				  struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	/*
> +	 * The PARALLEL CPI can't transcode in any way, the source format
> +	 * is identical to the sink format.
> +	 */
> +	if (code->pad == IMX_CPI_PAD_SOURCE) {
> +		struct v4l2_mbus_framefmt *fmt;
> +
> +		if (code->index > 0)
> +			return -EINVAL;
> +
> +		fmt = v4l2_subdev_state_get_format(sd_state, code->pad);
> +		code->code = fmt->code;
> +		return 0;
> +	}
> +
> +	if (code->pad != IMX_CPI_PAD_SINK)
> +		return -EINVAL;
> +
> +	if (code->index >= ARRAY_SIZE(imx_cpi_formats))
> +		return -EINVAL;
> +
> +	code->code = imx_cpi_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int imx_cpi_get_frame_desc(struct v4l2_subdev *sd,
> +				  unsigned int pad,
> +				  struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[0];
> +	const struct imx_cpi_pix_format *pcpidev_fmt;
> +	const struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +
> +	if (pad != IMX_CPI_PAD_SOURCE)
> +		return -EINVAL;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +	fmt = v4l2_subdev_state_get_format(state, IMX_CPI_PAD_SOURCE);
> +	pcpidev_fmt = find_imx_cpi_format(fmt->code);
> +	v4l2_subdev_unlock_state(state);
> +
> +	if (!pcpidev_fmt)
> +		return -EPIPE;

This can't happen, as the format on the source pad is guaranteed to be
valid at all times.

> +
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
> +	fd->num_entries = 1;
> +
> +	entry->flags = 0;
> +	entry->pixelcode = pcpidev_fmt->code;

Use fmt->code, and drop the pcpidev_fmt lookup.

> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops imx_cpi_video_ops = {
> +	.s_stream = imx_cpi_s_stream,

.s_stream() is deprecated, please implement .enable_streams() and
.disable_streams(), and set .s_stream to v4l2_subdev_s_stream_helper.

> +};
> +
> +static const struct v4l2_subdev_pad_ops imx_cpi_pad_ops = {
> +	.enum_mbus_code = imx_cpi_enum_mbus_code,
> +	.get_fmt = v4l2_subdev_get_fmt,
> +	.set_fmt = imx_cpi_set_fmt,
> +	.get_frame_desc = imx_cpi_get_frame_desc,
> +};
> +
> +static const struct v4l2_subdev_ops imx_cpi_subdev_ops = {
> +	.pad = &imx_cpi_pad_ops,
> +	.video = &imx_cpi_video_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops imx_cpi_internal_ops = {
> +	.init_state = imx_cpi_init_state,
> +};
> +
> +static int imx_cpi_clk_get(struct imx_cpi_device *pcpidev)
> +{
> +	unsigned int i;
> +	int ret = 0;

Drop ret.

> +
> +	for (i = 0; i < PCPIDEV_NUM_CLKS; i++)

Use ARRAY_SIZE(pcpidev->clks) instead of PCPIDEV_NUM_CLKS. Same below.

> +		pcpidev->clks[i].id = imx_cpi_clk_id[i];
> +
> +	ret = devm_clk_bulk_get(pcpidev->dev, PCPIDEV_NUM_CLKS, pcpidev->clks);

Return directly here.

> +
> +	return ret;
> +}
> +
> +/* ----------------------------------------------------------------------
> + * Suspend/resume
> + */

Add a blank line.

> +static int __maybe_unused imx_cpi_runtime_suspend(struct device *dev)

Drop __maybe_unused, it's not needed when using RUNTIME_PM_OPS() and
pm_ptr().

> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +
> +	clk_bulk_disable_unprepare(PCPIDEV_NUM_CLKS, pcpidev->clks);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused imx_cpi_runtime_resume(struct device *dev)

Here too.

> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +
> +	return clk_bulk_prepare_enable(PCPIDEV_NUM_CLKS, pcpidev->clks);
> +}
> +
> +static const struct dev_pm_ops imx_cpi_pm_ops = {
> +	RUNTIME_PM_OPS(imx_cpi_runtime_suspend, imx_cpi_runtime_resume, NULL)


> +};
> +
> +static int imx_cpi_subdev_init(struct imx_cpi_device *pcpidev)
> +{
> +	struct v4l2_subdev *sd = &pcpidev->sd;
> +	int ret;
> +
> +	v4l2_subdev_init(sd, &imx_cpi_subdev_ops);
> +
> +	sd->internal_ops = &imx_cpi_internal_ops;
> +	sd->owner = THIS_MODULE;

Not needed. Does this come from copying code from an existing driver ?
If so, please send a patch to fix that driver and lower the amount of
cargo-cult.

> +	snprintf(sd->name, sizeof(sd->name), "parallel-%s",
> +		 dev_name(pcpidev->dev));
> +
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	sd->ctrl_handler = NULL;

Not needed.

> +
> +	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	sd->entity.ops = &imx_cpi_entity_ops;
> +
> +	sd->dev = pcpidev->dev;
> +
> +	pcpidev->pads[IMX_CPI_PAD_SINK].flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;

The media subsystem wraps lines to 80 columns, with exceptions when
doing so would significantly decrease readability. Please check through
the driver.

> +	pcpidev->pads[IMX_CPI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE |
> +						       MEDIA_PAD_FL_MUST_CONNECT;

Weird indentation. Use either

	pcpidev->pads[IMX_CPI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE |
						  MEDIA_PAD_FL_MUST_CONNECT;

or

	pcpidev->pads[IMX_CPI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
						| MEDIA_PAD_FL_MUST_CONNECT;

> +
> +	ret = media_entity_pads_init(&sd->entity, IMX_CPI_PADS_NUM, pcpidev->pads);
> +	if (ret)
> +		return ret;
> +
> +	ret = v4l2_subdev_init_finalize(sd);
> +	if (ret)
> +		media_entity_cleanup(&sd->entity);
> +
> +	return ret;
> +}
> +
> +static void imx_cpi_cleanup(void *data)
> +{
> +	struct imx_cpi_device *pcpidev = data;
> +
> +	v4l2_subdev_cleanup(&pcpidev->sd);
> +	media_entity_cleanup(&pcpidev->sd.entity);
> +	v4l2_async_nf_unregister(&pcpidev->notifier);
> +	v4l2_async_nf_cleanup(&pcpidev->notifier);
> +	v4l2_async_unregister_subdev(&pcpidev->sd);
> +}
> +
> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> +	struct imx_cpi_device *pcpidev;
> +	struct device *dev = &pdev->dev;
> +	int ret = 0;

No need to initialize this to 0.

> +
> +	pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
> +	if (!pcpidev)
> +		return -ENOMEM;
> +
> +	pcpidev->dev = dev;
> +	platform_set_drvdata(pdev, pcpidev);
> +
> +	pcpidev->pdata = of_device_get_match_data(dev);
> +
> +	pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pcpidev->regs))
> +		return dev_err_probe(dev, PTR_ERR(pcpidev->regs),
> +				     "Failed to get regs\n");
> +
> +	ret = imx_cpi_clk_get(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = imx_cpi_subdev_init(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, imx_cpi_cleanup, pcpidev);
> +	if (ret)
> +		return ret;

Are you sure that all the cleanup calls in imx_cpi_cleanup() are safe if
imx_cpi_async_register() fails below ? I think manual error handling is
better, with a .remove() function.

> +
> +	pcpidev->mode = PI_GATE_CLOCK_MODE;

I'd move this just after assigning pdata to group all static
initialization together.

> +
> +	platform_set_drvdata(pdev, &pcpidev->sd);

That's the second platform_set_drvdata() call. Drop it, keep the first
one, and turn

	struct v4l2_subdev *sd = dev_get_drvdata(dev);
	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);

above into

	struct imx_cpi_device *pcpidev = dev_get_drvdata(dev);

> +
> +	ret = imx_cpi_async_register(pcpidev);
> +	if (ret < 0)
> +		return ret;
> +
> +	devm_pm_runtime_enable(dev);

Please use runtime PM autosuspend. This also needs to be enabled before
calling v4l2_async_register_subdev(), as the subdev could get used as
soon as it gets registered.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx_cpi_of_match[] = {
> +	{.compatible = "fsl,imx8qxp-pcif", .data = &imx8qxp_pdata },

Missing space after the curly brace.

> +	{.compatible = "fsl,imx93-pcif", .data = &imx93_pdata },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, imx_cpi_of_match);
> +
> +static struct platform_driver _driver = {

_driver is quite uncommon. You can use the same prefix as in the rest of
the driver and name this imx_cpi_driver.

> +	.probe = imx_cpi_probe,
> +	.driver = {
> +		.of_match_table = imx_cpi_of_match,
> +		.name = "imx-parallel-cpi",
> +		.pm = pm_ptr(&imx_cpi_pm_ops),
> +	},
> +};
> +
> +module_platform_driver(_driver);
> +
> +MODULE_DESCRIPTION("i.MX9 Parallel CPI receiver driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
  2025-07-25  0:04   ` Laurent Pinchart
@ 2025-07-25 15:38     ` Frank Li
  2025-07-25 17:57       ` Laurent Pinchart
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2025-07-25 15:38 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan,
	Robert Chiras, Zhipeng Wang

On Fri, Jul 25, 2025 at 03:04:04AM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> Thank you for the patch.
>
> On Tue, Jul 08, 2025 at 01:48:43PM -0400, Frank Li via B4 Relay wrote:
> > From: Alice Yuan <alice.yuan@nxp.com>
> >
> > Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
> > i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
> > and enables image data capture through a parallel interface.
> >
> > Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change in v3
> > - replace csi with cpi
> > - use __free(fwnode_handle) to simpilfy code
> > - remove imx91 driver data, which is the same as imx93
> >
> > change in v2
> > - remove MODULE_ALIAS
> > - use devm_pm_runtime_enable() and cleanup remove function
> > - change output format to 1x16. controller convert 2x8 to 1x16 format
> > ---
> >  MAINTAINERS                                   |   1 +
> >  drivers/media/platform/nxp/Kconfig            |  11 +
> >  drivers/media/platform/nxp/Makefile           |   1 +
> >  drivers/media/platform/nxp/imx-parallel-cpi.c | 920 ++++++++++++++++++++++++++
> >  4 files changed, 933 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303..14842a3b860a6f23846f12a684eedcbb9eb69e19 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15112,6 +15112,7 @@ F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> >  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> >  F:	drivers/media/platform/nxp/imx-mipi-csis.c
> > +F:	drivers/media/platform/nxp/imx-parallel-cpi.c
> >  F:	drivers/media/platform/nxp/imx7-media-csi.c
> >  F:	drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> >
> > diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> > index 40e3436669e213fdc5da70821dc0b420e1821f4f..504ae1c6494f331c16124a224421ac7acd433ba5 100644
> > --- a/drivers/media/platform/nxp/Kconfig
> > +++ b/drivers/media/platform/nxp/Kconfig
> > @@ -39,6 +39,17 @@ config VIDEO_IMX_MIPI_CSIS
> >  	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
> >  	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
> >
> > +config VIDEO_IMX_PARALLEL_CPI
> > +	tristate "NXP i.MX9/i.MX8 Parallel CPI Driver"
>
> I'd write it in numerical order, "i.MX8/i.MX9"
>
> > +	depends on ARCH_MXC || COMPILE_TEST
> > +	depends on VIDEO_DEV
> > +	select MEDIA_CONTROLLER
> > +	select V4L2_FWNODE
> > +	select VIDEO_V4L2_SUBDEV_API
> > +	help
> > +	  Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> > +	  on some iMX8 and iMX9 SoCs.
> > +
> >  source "drivers/media/platform/nxp/imx8-isi/Kconfig"
> >
> >  # mem2mem drivers
> > diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
> > index 4d90eb71365259ebdda84ea58483e1c4131d3ac7..5346919d2f1083b51ec99b66981c5d38b3df960c 100644
> > --- a/drivers/media/platform/nxp/Makefile
> > +++ b/drivers/media/platform/nxp/Makefile
> > @@ -7,5 +7,6 @@ obj-y += imx8-isi/
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> >  obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
> >  obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> > +obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
> >  obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
> >  obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> > diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..718f02bf70c4d0ae74ecf842c1ecb1a1afbcdd45
> > --- /dev/null
> > +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> > @@ -0,0 +1,920 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * i.MX Parallel CPI receiver driver.
> > + *
> > + * Copyright 2019-2025 NXP
> > + *
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/clk.h>
> > +#include <linux/debugfs.h>
>
> I don't think you use this.
>
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/interrupt.h>
>
> I don't think you use this either.
>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/mfd/syscon.h>
>
> Alphabetical order please.
>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
>
> This doesn't seem needed.
>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/reset.h>
>
> Same here.
>
> > +#include <linux/spinlock.h>
>
> And here. Please verify all headers, drop the ones you don't need, and
> add any you may have forgotten. That includes the media/ headers.

Do you have good method to check it?

>
> > +
> > +#include <media/v4l2-common.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>

...

> > +
> > +	/* Software Reset */
> > +	imx_cpi_sw_reset(pcpidev);
> > +
> > +	/* Config PL Data Type */
> > +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> > +	val |= IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
>
> Don't you need to clear the field first ? Same everywhere else where
> applicable.

imx_cpi_sw_reset() clean all register.

Frank

>
> > +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> > +
> > +	/* Enable sync Force */
> > +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> > +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> > +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);

....

> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
  2025-07-25 15:38     ` Frank Li
@ 2025-07-25 17:57       ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2025-07-25 17:57 UTC (permalink / raw)
  To: Frank Li
  Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-media,
	imx, linux-arm-kernel, linux-kernel, devicetree, Alice Yuan,
	Robert Chiras, Zhipeng Wang

On Fri, Jul 25, 2025 at 11:38:29AM -0400, Frank Li wrote:
> On Fri, Jul 25, 2025 at 03:04:04AM +0300, Laurent Pinchart wrote:
> > Hi Frank,
> >
> > Thank you for the patch.
> >
> > On Tue, Jul 08, 2025 at 01:48:43PM -0400, Frank Li via B4 Relay wrote:
> > > From: Alice Yuan <alice.yuan@nxp.com>
> > >
> > > Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
> > > i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
> > > and enables image data capture through a parallel interface.
> > >
> > > Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
> > > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > change in v3
> > > - replace csi with cpi
> > > - use __free(fwnode_handle) to simpilfy code
> > > - remove imx91 driver data, which is the same as imx93
> > >
> > > change in v2
> > > - remove MODULE_ALIAS
> > > - use devm_pm_runtime_enable() and cleanup remove function
> > > - change output format to 1x16. controller convert 2x8 to 1x16 format
> > > ---
> > >  MAINTAINERS                                   |   1 +
> > >  drivers/media/platform/nxp/Kconfig            |  11 +
> > >  drivers/media/platform/nxp/Makefile           |   1 +
> > >  drivers/media/platform/nxp/imx-parallel-cpi.c | 920 ++++++++++++++++++++++++++
> > >  4 files changed, 933 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 8ae0667d2bb41fb6a1549bd3b2b33f326cbd1303..14842a3b860a6f23846f12a684eedcbb9eb69e19 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -15112,6 +15112,7 @@ F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > >  F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > >  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > >  F:	drivers/media/platform/nxp/imx-mipi-csis.c
> > > +F:	drivers/media/platform/nxp/imx-parallel-cpi.c
> > >  F:	drivers/media/platform/nxp/imx7-media-csi.c
> > >  F:	drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > >
> > > diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> > > index 40e3436669e213fdc5da70821dc0b420e1821f4f..504ae1c6494f331c16124a224421ac7acd433ba5 100644
> > > --- a/drivers/media/platform/nxp/Kconfig
> > > +++ b/drivers/media/platform/nxp/Kconfig
> > > @@ -39,6 +39,17 @@ config VIDEO_IMX_MIPI_CSIS
> > >  	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
> > >  	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
> > >
> > > +config VIDEO_IMX_PARALLEL_CPI
> > > +	tristate "NXP i.MX9/i.MX8 Parallel CPI Driver"
> >
> > I'd write it in numerical order, "i.MX8/i.MX9"
> >
> > > +	depends on ARCH_MXC || COMPILE_TEST
> > > +	depends on VIDEO_DEV
> > > +	select MEDIA_CONTROLLER
> > > +	select V4L2_FWNODE
> > > +	select VIDEO_V4L2_SUBDEV_API
> > > +	help
> > > +	  Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> > > +	  on some iMX8 and iMX9 SoCs.
> > > +
> > >  source "drivers/media/platform/nxp/imx8-isi/Kconfig"
> > >
> > >  # mem2mem drivers
> > > diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
> > > index 4d90eb71365259ebdda84ea58483e1c4131d3ac7..5346919d2f1083b51ec99b66981c5d38b3df960c 100644
> > > --- a/drivers/media/platform/nxp/Makefile
> > > +++ b/drivers/media/platform/nxp/Makefile
> > > @@ -7,5 +7,6 @@ obj-y += imx8-isi/
> > >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> > >  obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
> > >  obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
> > > +obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
> > >  obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
> > >  obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> > > diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..718f02bf70c4d0ae74ecf842c1ecb1a1afbcdd45
> > > --- /dev/null
> > > +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> > > @@ -0,0 +1,920 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * i.MX Parallel CPI receiver driver.
> > > + *
> > > + * Copyright 2019-2025 NXP
> > > + *
> > > + */
> > > +
> > > +#include <linux/bits.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/debugfs.h>
> >
> > I don't think you use this.
> >
> > > +#include <linux/delay.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/interrupt.h>
> >
> > I don't think you use this either.
> >
> > > +#include <linux/io.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/mfd/syscon.h>
> >
> > Alphabetical order please.
> >
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_domain.h>
> >
> > This doesn't seem needed.
> >
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/reset.h>
> >
> > Same here.
> >
> > > +#include <linux/spinlock.h>
> >
> > And here. Please verify all headers, drop the ones you don't need, and
> > add any you may have forgotten. That includes the media/ headers.
> 
> Do you have good method to check it?

There are tools such as include-what-you-use, but I haven't tested that
one with the kernel.

> > > +
> > > +#include <media/v4l2-common.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-event.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-mc.h>
> > > +#include <media/v4l2-subdev.h>
> 
> ...
> 
> > > +
> > > +	/* Software Reset */
> > > +	imx_cpi_sw_reset(pcpidev);
> > > +
> > > +	/* Config PL Data Type */
> > > +	val = readl(pcpidev->regs + pdata->if_ctrl_reg);
> > > +	val |= IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
> >
> > Don't you need to clear the field first ? Same everywhere else where
> > applicable.
> 
> imx_cpi_sw_reset() clean all register.

OK. I wonder if you could then replace some (most ? all ?) of the
read-update-write sequences with plain writes.

> > > +	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
> > > +
> > > +	/* Enable sync Force */
> > > +	val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
> > > +	val |= (CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
> > > +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
> 
> ....

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-07-25 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-08 17:48 [PATCH v3 0/4] media: imx8qxp: add parallel camera support Frank Li via B4 Relay
2025-07-08 17:48 ` [PATCH v3 1/4] dt-bindings: media: add i.MX parallel CPI support Frank Li via B4 Relay
2025-07-09  8:22   ` Krzysztof Kozlowski
2025-07-24 22:15   ` Laurent Pinchart
2025-07-08 17:48 ` [PATCH v3 2/4] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI) Frank Li via B4 Relay
2025-07-18 16:44   ` Frank Li
2025-07-25  0:04   ` Laurent Pinchart
2025-07-25 15:38     ` Frank Li
2025-07-25 17:57       ` Laurent Pinchart
2025-07-08 17:48 ` [PATCH v3 3/4] arm64: dts: imx8: add camera parallel interface (CPI) node Frank Li via B4 Relay
2025-07-08 17:48 ` [PATCH v3 4/4] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support Frank Li via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).