[PATCH v5 0/8] media: add new API simple 1to1 subdev register and add imx parallel camera support

[PATCH v5 0/8] media: add new API simple 1to1 subdev register and add imx parallel camera support

[Frank.Li]

This patches base on previous' thread "media: imx8qxp: add parallel camera
support".

Add new API media_async_register_subdev_1to1() to simple 1to1 subdev
register.

Many V4L2 subdev drivers implement the same registration and media pad
setup logic for simple pipelines consisting of a single sink pad and a
single source pad. As a result, the same boilerplate code is duplicated
across multiple drivers.

Introduce a common helper library for 1-to-1 subdevs to encapsulate the
registration, media entity initialization, and cleanup paths. Drivers
can embed a struct v4l2_subdev_1to1 instance and use the provided helper
APIs instead of open-coding the setup sequence.

This reduces code duplication and simplifies the implementation of
simple bridge and converter drivers.

    In 1TO1 subdev driver:

    struct your_device {
            v4l2_subdev_1to1 sd_1to1; // instead of v4l2_subdev sd;
            ...
    }
    ...
    your_device_probe()
    {
            v4l2_subdev_init(&sd_1to1->sd, &dw_mipi_csi2rx_ops);
            ...
            return media_async_register_subdev_1to1(sd_1to1);
    }

    ...
    your_device_remove()
    {
            media_async_subdev_1to1_cleanup();
    }

This API help reduce over line duplcated code in synopsys/dw-mipi-csi2rx.c.
And use this API at imx8's parallel CPI driver, which over 90% code now
hardware related.

And also benefit on going pix format patch
https://lore.kernel.org/imx/20260525-csi_formatter-v8-0-6b646231224b@oss.nxp.com/

It will also reduce missed media_entity_cleanup() problem at some error path
https://lore.kernel.org/linux-media/20260614202835.11977-15-birenpandya@gmail.com/

Previous do partial simpilfy at
https://lore.kernel.org/imx/aaisdJSsFE5-PLx1@lizhi-Precision-Tower-5810/

To: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Michael Riesch <michael.riesch@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Frank Li <Frank.Li@nxp.com>
To: Martin Kepplinger-Novakovic <martink@posteo.de>
To: Rui Miguel Silva <rmfrfs@gmail.com>
To: Purism Kernel Team <kernel@puri.sm>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v5:
- Add media_async_register_subdev_1to1() to simple code.
- Link to v4: https://lore.kernel.org/r/20250729-imx8qxp_pcam-v4-0-4dfca4ed2f87@nxp.com

Changes in v4:
- remove imx93 driver support since have not camera sensor module to do test now.
  Add it later
- Add new patch
  media: v4l2-common: Add helper function v4l_get_required_align_by_bpp()
- See each patche's change log for detail.
- Link to v3: https://lore.kernel.org/r/20250708-imx8qxp_pcam-v3-0-c8533e405df1@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 (6):
      media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()
      media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration
      media: synopsys: Use v4l2_subdev_get_frame_desc_passthrough()
      media: synopsys: Use V4L2 1-to-1 subdev helpers
      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 |  83 +++
 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi  |  27 +
 drivers/media/platform/nxp/Kconfig                 |  12 +
 drivers/media/platform/nxp/Makefile                |   1 +
 drivers/media/platform/nxp/imx-parallel-cpi.c      | 614 +++++++++++++++++++++
 drivers/media/platform/synopsys/Kconfig            |   1 +
 drivers/media/platform/synopsys/dw-mipi-csi2rx.c   | 194 +------
 drivers/media/v4l2-core/Kconfig                    |   3 +
 drivers/media/v4l2-core/Makefile                   |   1 +
 drivers/media/v4l2-core/v4l2-1to1.c                | 117 ++++
 drivers/media/v4l2-core/v4l2-fwnode.c              |  17 +-
 include/media/v4l2-async.h                         |   6 +
 include/media/v4l2-device-1to1.h                   |  72 +++
 17 files changed, 1123 insertions(+), 169 deletions(-)
---
base-commit: 7193e493653c9c91b4be159cd919924ec6ad2392
change-id: 20250626-imx8qxp_pcam-d851238343c3

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

[PATCH v5 1/8] media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()

[Frank.Li]

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

Extract __v4l2_async_register_subdev_fwnode() from
__media_async_register_subdev_1to1() and make the notifier parsing
function a parameter.

This prepares for future support of v4l2_async_register_subdev_1to1() by
allowing different fwnode parsing implementations to reuse the common
registration logic.

No functional change intended.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v5:
- new patch
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 17 ++++++++++++++---
 include/media/v4l2-async.h            |  6 ++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 62a3a452f7884..f32dbf5bc8344 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -1256,7 +1256,11 @@ v4l2_async_nf_parse_fwnode_sensor(struct device *dev,
 	return 0;
 }
 
-int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *module)
+int
+__v4l2_async_register_subdev_fwnode(struct v4l2_subdev *sd,
+				    int (*parse_fwnode)(struct device *dev,
+							struct v4l2_async_notifier *notifier),
+				    struct module *module)
 {
 	struct v4l2_async_notifier *notifier;
 	int ret;
@@ -1274,7 +1278,7 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
 	if (ret < 0)
 		goto out_cleanup;
 
-	ret = v4l2_async_nf_parse_fwnode_sensor(sd->dev, notifier);
+	ret = parse_fwnode(sd->dev, notifier);
 	if (ret < 0)
 		goto out_cleanup;
 
@@ -1300,8 +1304,15 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
+EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_fwnode);
 
+int
+__v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *module)
+{
+	return __v4l2_async_register_subdev_fwnode(sd, v4l2_async_nf_parse_fwnode_sensor,
+						   module);
+}
+EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
 MODULE_DESCRIPTION("V4L2 fwnode binding parsing library");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 54a2d9620ed5b..a9345fb921b43 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -338,6 +338,12 @@ int __v4l2_async_register_subdev(struct v4l2_subdev *sd, struct module *module);
 int __must_check
 __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *module);
 
+int __must_check
+__v4l2_async_register_subdev_fwnode(struct v4l2_subdev *sd,
+				    int (*parse_fwnode)(struct device *dev,
+							struct v4l2_async_notifier *notifier),
+				    struct module *module);
+
 /**
  * v4l2_async_unregister_subdev - unregisters a sub-device to the asynchronous
  *	subdevice framework

-- 
2.43.0

Re: [PATCH v5 1/8] media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly states the function from which the common helper is extracted.
--

commit 1d02bf2bbb834a499240f695cd83dc1f1f8db5fb
Author: Frank Li <Frank.Li@nxp.com>

media: v4l2-fwnode: Extract common helper __v4l2_async_register_subdev_fwnode()

This commit refactors the subdev registration logic to extract a common
helper, __v4l2_async_register_subdev_fwnode(). It allows the notifier
parsing function to be passed as a parameter so that different fwnode
parsing implementations can reuse the same registration code.

> Extract __v4l2_async_register_subdev_fwnode() from
> __media_async_register_subdev_1to1() and make the notifier parsing
> function a parameter.

[Severity: Medium]
Is this function name correct? The commit message states the helper is
extracted from __media_async_register_subdev_1to1(), but the diff shows
the logic is actually being extracted from
__v4l2_async_register_subdev_sensor(). 

Should the commit message be updated to match the actual function being
modified in the code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=1

[PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

[Frank.Li]

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

Many V4L2 subdev drivers implement the same registration and media pad
setup logic for simple pipelines consisting of a single sink pad and a
single source pad. As a result, the same boilerplate code is duplicated
across multiple drivers.

Introduce a common helper library for 1-to-1 subdevs to encapsulate the
registration, media entity initialization, and cleanup paths. Drivers
can embed a struct v4l2_subdev_1to1 instance and use the provided helper
APIs instead of open-coding the setup sequence.

This reduces code duplication and simplifies the implementation of
simple bridge and converter drivers.

In 1TO1 subdev driver:

struct your_device {
	v4l2_subdev_1to1 sd_1to1; // instead of v4l2_subdev sd;
	...
}
...
your_device_probe()
{
	v4l2_subdev_init(&sd_1to1->sd, &dw_mipi_csi2rx_ops);
	...
	media_async_register_subdev_1to1(sd_1to1);
}

...
your_device_remove()
{
	media_async_subdev_1to1_cleanup();
}

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v5
- new patch
---
 drivers/media/v4l2-core/Kconfig     |   3 +
 drivers/media/v4l2-core/Makefile    |   1 +
 drivers/media/v4l2-core/v4l2-1to1.c | 117 ++++++++++++++++++++++++++++++++++++
 include/media/v4l2-device-1to1.h    |  72 ++++++++++++++++++++++
 4 files changed, 193 insertions(+)

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index d50ccac9733cc..532375cae7947 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -74,6 +74,9 @@ config V4L2_FWNODE
 config V4L2_ASYNC
 	tristate
 
+config V4L2_1TO1
+	tristate
+
 config V4L2_CCI
 	tristate
 
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 329f0eadce994..55bf0e6bf2e33 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -24,6 +24,7 @@ videodev-$(CONFIG_VIDEO_V4L2_I2C) += v4l2-i2c.o
 # Please keep it alphabetically sorted by Kconfig name
 # (e. g. LC_ALL=C sort Makefile)
 
+obj-$(CONFIG_V4L2_1TO1) += v4l2-1to1.o
 obj-$(CONFIG_V4L2_ASYNC) += v4l2-async.o
 obj-$(CONFIG_V4L2_CCI) += v4l2-cci.o
 obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o
diff --git a/drivers/media/v4l2-core/v4l2-1to1.c b/drivers/media/v4l2-core/v4l2-1to1.c
new file mode 100644
index 0000000000000..9f23dccece704
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-1to1.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/property.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-device-1to1.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+static int v4l2_1to1_notifier_bound(struct v4l2_async_notifier *notifier,
+				    struct v4l2_subdev *sd,
+				    struct v4l2_async_connection *asd)
+{
+	struct v4l2_subdev_1to1 *sd_1to1 = v4l2_sd_to_1to1_device(notifier->sd);
+	struct media_pad *sink_pad = &sd_1to1->pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+	int ret;
+
+	ret = v4l2_create_fwnode_links_to_pad(sd, sink_pad, MEDIA_LNK_FL_ENABLED);
+	if (ret) {
+		dev_err(sd_1to1->sd.dev, "failed to link source pad of %s\n", sd->name);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_async_notifier_operations v4l2_1to1_notifier_ops = {
+	.bound = v4l2_1to1_notifier_bound,
+};
+
+static int
+v4l2_async_nf_parse_fwnode_1to1(struct device *dev, struct v4l2_async_notifier *notifier)
+{
+	struct v4l2_subdev *sd = notifier->sd;
+	struct v4l2_subdev_1to1 *sd_1to1 = v4l2_sd_to_1to1_device(sd);
+	struct v4l2_fwnode_endpoint *vep = &sd_1to1->vep;
+	struct v4l2_async_connection *asd;
+	int ret;
+
+	struct fwnode_handle *ep __free(fwnode_handle) =
+		fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
+	if (!ep)
+		return dev_err_probe(dev, -ENODEV, "failed to get endpoint\n");
+
+	ret = v4l2_fwnode_endpoint_parse(ep, vep);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse endpoint\n");
+
+	if (!(BIT(vep->bus_type) & sd_1to1->remote_bustype_cap_mask))
+		return dev_err_probe(dev, -EINVAL,
+				     "invalid bus type %d of endpoint\n",
+				     vep->bus_type);
+
+	notifier->ops = &v4l2_1to1_notifier_ops;
+
+	asd = v4l2_async_nf_add_fwnode_remote(notifier, ep,
+					      struct v4l2_async_connection);
+	if (IS_ERR(asd))
+		return dev_err_probe(dev, PTR_ERR(asd),
+				     "failed to add notifier\n");
+
+	return 0;
+}
+
+void media_async_subdev_1to1_cleanup(struct v4l2_subdev_1to1 *sd_1to1)
+{
+	struct v4l2_subdev *sd = &sd_1to1->sd;
+
+	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_async_nf_unregister(sd->subdev_notifier);
+	v4l2_async_nf_cleanup(sd->subdev_notifier);
+
+	kfree(sd->subdev_notifier);
+}
+EXPORT_SYMBOL_GPL(media_async_subdev_1to1_cleanup);
+
+int __media_async_register_subdev_1to1(struct v4l2_subdev_1to1 *sd_1to1, struct module *module)
+{
+	struct media_pad *pads = sd_1to1->pads;
+	int ret;
+
+	pads[V4L2_SUBDEV_1TO1_PADS_SINK].flags = MEDIA_PAD_FL_SINK |
+					       MEDIA_PAD_FL_MUST_CONNECT;
+	pads[V4L2_SUBDEV_1TO1_PADS_SOURCE].flags = MEDIA_PAD_FL_SOURCE |
+						 MEDIA_PAD_FL_MUST_CONNECT;
+
+	ret = media_entity_pads_init(&sd_1to1->sd.entity, V4L2_SUBDEV_1TO1_PADS_TOTAL, pads);
+	if (ret)
+		return ret;
+
+	ret = v4l2_subdev_init_finalize(&sd_1to1->sd);
+	if (ret)
+		goto err_entity_cleanup;
+
+	ret = __v4l2_async_register_subdev_fwnode(&sd_1to1->sd,
+						  v4l2_async_nf_parse_fwnode_1to1,
+						  module);
+	if (ret)
+		goto err_subdev_cleanup;
+
+	return 0;
+
+err_subdev_cleanup:
+	v4l2_subdev_cleanup(&sd_1to1->sd);
+err_entity_cleanup:
+	media_entity_cleanup(&sd_1to1->sd.entity);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__media_async_register_subdev_1to1);
+
+MODULE_DESCRIPTION("V4L2 subdev 1to1 helper library");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Frank.Li@kernel.org");
diff --git a/include/media/v4l2-device-1to1.h b/include/media/v4l2-device-1to1.h
new file mode 100644
index 0000000000000..a1256767b4d4c
--- /dev/null
+++ b/include/media/v4l2-device-1to1.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __V4L2_SUBDEV_1TO1__
+#define __V4L2_SUBDEV_1TO1__
+
+#include <media/media-entity.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+enum v4l2_subdev_1to1_pads {
+	V4L2_SUBDEV_1TO1_PADS_SINK,
+	V4L2_SUBDEV_1TO1_PADS_SOURCE,
+	V4L2_SUBDEV_1TO1_PADS_TOTAL,
+};
+
+/**
+ * struct v4l2_subdev_1to1 - 1to1 sub-device
+ *
+ * @sd:         sub-device that registered the notifier, NULL otherwise
+ * @pads:	media pads(the first one is sink, the second one is source)
+ * @vep:	The V4L2 fwnode data structure for remote node.
+ * @remote_bustype_cap_mask:  Bit mask for required remote node v4l2_mbus_type.
+ */
+struct v4l2_subdev_1to1 {
+	struct v4l2_subdev sd;
+	struct media_pad pads[V4L2_SUBDEV_1TO1_PADS_TOTAL];
+	struct v4l2_fwnode_endpoint vep;
+	/* bit masks for enum v4l2_mbus_type*/
+	u32 remote_bustype_cap_mask;
+};
+
+static inline struct v4l2_subdev_1to1 *
+v4l2_sd_to_1to1_device(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct v4l2_subdev_1to1, sd);
+}
+
+/**
+ * media_async_register_subdev_1to1 - registers a 1to1 sub-device to the
+ *                                    asynchronous sub-device framework and
+ *                                    parse set up common 1to1 related
+ *                                    devices
+ *
+ * @sd_1to1: pointer to struct &v4l2_subdev_1to1
+ *
+ * This function is just like v4l2_async_register_subdev() with the exception
+ * that calling it will also parse firmware interfaces for remote references
+ * using v4l2_async_nf_parse_fwnode_sensor() and registers the
+ * async sub-devices.
+ *
+ * This function also init media_pads.
+ *
+ * The sub-device is similarly unregistered and cleanup by
+ * media_async_subdev_1to1_cleanup()
+ *
+ * While registered, the subdev module is marked as in-use.
+ *
+ * An error is returned if the module is no longer loaded on any attempts
+ * to register it.
+ */
+#define media_async_register_subdev_1to1(sd_1to1) \
+	__media_async_register_subdev_1to1(sd_1to1, THIS_MODULE)
+
+int __media_async_register_subdev_1to1(struct v4l2_subdev_1to1 *sd_1to1, struct module *module);
+
+/**
+ * media_async_subdev_1to1_cleanup - unregistered and cleanup subdev and media
+ *				     pads
+ * @sd_1to1: pointer to struct &v4l2_subdev_1to1
+ */
+void media_async_subdev_1to1_cleanup(struct v4l2_subdev_1to1 *sd_1to1);
+
+#endif

-- 
2.43.0

Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant cleanup and misguided teardown of `sd->subdev_notifier`.
--

commit ebb79cbbc440da4b8d1a35f25ce9168deff7467d
Author: Frank Li <Frank.Li@nxp.com>

media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

This commit introduces a common helper library to encapsulate registration,
media entity initialization, and cleanup for 1-to-1 V4L2 subdevices,
reducing boilerplate in simple bridge and converter drivers.

> diff --git a/drivers/media/v4l2-core/v4l2-1to1.c b/drivers/media/v4l2-core/v4l2-1to1.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-1to1.c

[ ... ]

> +void media_async_subdev_1to1_cleanup(struct v4l2_subdev_1to1 *sd_1to1)
> +{
> +	struct v4l2_subdev *sd = &sd_1to1->sd;
> +
> +	v4l2_async_unregister_subdev(sd);
> +	v4l2_subdev_cleanup(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_async_nf_unregister(sd->subdev_notifier);
> +	v4l2_async_nf_cleanup(sd->subdev_notifier);
> +
> +	kfree(sd->subdev_notifier);

[Severity: Low]
Does this code perform redundant cleanups?

Looking at v4l2_async_unregister_subdev(), it already handles unregistering,
cleaning up, freeing, and setting sd->subdev_notifier to NULL:

v4l2_async_unregister_subdev(struct v4l2_subdev *sd) {
    ...
    __v4l2_async_nf_unregister(sd->subdev_notifier);
    __v4l2_async_nf_cleanup(sd->subdev_notifier);
    kfree(sd->subdev_notifier);
    sd->subdev_notifier = NULL;
    ...
}

Since sd->subdev_notifier is NULL after v4l2_async_unregister_subdev()
returns, are the subsequent calls to v4l2_async_nf_unregister(),
v4l2_async_nf_cleanup(), and kfree() necessary here in
media_async_subdev_1to1_cleanup()?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=2

Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

[Sakari Ailus]

Hi Frank,

Thanks for the patch.

On Wed, Jun 17, 2026 at 03:50:12PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Many V4L2 subdev drivers implement the same registration and media pad
> setup logic for simple pipelines consisting of a single sink pad and a
> single source pad. As a result, the same boilerplate code is duplicated
> across multiple drivers.
> 
> Introduce a common helper library for 1-to-1 subdevs to encapsulate the
> registration, media entity initialization, and cleanup paths. Drivers
> can embed a struct v4l2_subdev_1to1 instance and use the provided helper
> APIs instead of open-coding the setup sequence.

I appreciate your efforts in trying to reduce the amount of code drivers
need simply to get things done but I think there are a few issues with the
approach taken in this patch:

- The new helpers aren't generic enough, but require two pads; one sink,
  one source. You could provide special helpers for just this case, but
  right now it looks like that if there's something you need that the
  helper assumes you don't, you can't use the helper at all. In other
  words, more modularity would be nice.

- The new helper should work with the existing types and not add new types
  (struct v4l2_subdev_1to1).

- There should be a way to provide default V4L2 fwnode endpoint
  configuration as well as to validate the obtained configuration.

I don't have a good proposal to address the above but at least one way I
can think of making error handling easier would be to use devm_() for
teardown in more places we to today. That certainly does have its own
issues though.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

[Frank Li]

On Thu, Jun 18, 2026 at 01:36:20AM +0300, Sakari Ailus wrote:
> Hi Frank,
>
> Thanks for the patch.

Thanks for you quick reply.

>
> On Wed, Jun 17, 2026 at 03:50:12PM -0400, Frank.Li@oss.nxp.com wrote:
> > From: Frank Li <Frank.Li@nxp.com>
> >
> > Many V4L2 subdev drivers implement the same registration and media pad
> > setup logic for simple pipelines consisting of a single sink pad and a
> > single source pad. As a result, the same boilerplate code is duplicated
> > across multiple drivers.
> >
> > Introduce a common helper library for 1-to-1 subdevs to encapsulate the
> > registration, media entity initialization, and cleanup paths. Drivers
> > can embed a struct v4l2_subdev_1to1 instance and use the provided helper
> > APIs instead of open-coding the setup sequence.
>
> I appreciate your efforts in trying to reduce the amount of code drivers
> need simply to get things done but I think there are a few issues with the
> approach taken in this patch:
>
> - The new helpers aren't generic enough, but require two pads; one sink,
>   one source.

It can cover many case already, there are many bridge type subdev. after
glace of all code, many CSI2RX is type device. It should one kind important
type/case, like sensors.

And I plan do 1 TO N replicator driver, which duplicate 1 sink pad to N
source pad (with/without register config), plus exist video-mux driver,

It think It can cover more than 80% cases.

> You could provide special helpers for just this case, but
>   right now it looks like that if there's something you need that the
>   helper assumes you don't, you can't use the helper at all. In other
>   words, more modularity would be nice.

We can add it later if need, which easy to replace 1to1 API, like I did
for sensor one.

>
> - The new helper should work with the existing types and not add new types
>   (struct v4l2_subdev_1to1).

May be save vep data into v4l2_subdev to avoid parse it every time. and
enhence media_entity_pads_init() to avoid refer caller data.

>
> - There should be a way to provide default V4L2 fwnode endpoint
>   configuration as well as to validate the obtained configuration.

Do you means remote_bustype_cap_mask information get from a callback?

>
> I don't have a good proposal to address the above but at least one way I
> can think of making error handling easier would be to use devm_() for
> teardown in more places we to today. That certainly does have its own
> issues though.

I tried it before, media and v4l2's clean up is not revised order of init.
Sorry, I can't find original thread. I remember laurnet pinchart said there
are order problem.

1  v4l2_subdev_init()
2. v4l2_async_subdev_nf_init()
3. v4l2_async_nf_register()
4. media_entity_pads_init()
5  v4l2_async_register_subdev()


v4l2_async_unregister_subdev(sd);
v4l2_subdev_cleanup(sd);        // Not sure if it save to move to last step
media_entity_cleanup(&sd->entity);
v4l2_async_nf_unregister(&csi2->notifier);
v4l2_async_nf_cleanup(&csi2->notifier);

Frank
>
> --
> Kind regards,
>
> Sakari Ailus

[PATCH v5 3/8] media: synopsys: Use v4l2_subdev_get_frame_desc_passthrough()

[Frank.Li]

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

Replace the local frame descriptor callback implementation with
v4l2_subdev_get_frame_desc_passthrough().

This helper provides the same functionality while avoiding duplicate
code and simplifying the driver implementation.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v5
- new patch
---
 drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
index 41e48365167e5..f51367409ff46 100644
--- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
+++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
@@ -630,31 +630,11 @@ static int dw_mipi_csi2rx_disable_streams(struct v4l2_subdev *sd,
 	return ret;
 }
 
-static int
-dw_mipi_csi2rx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
-			      struct v4l2_mbus_frame_desc *fd)
-{
-	struct dw_mipi_csi2rx_device *csi2 = to_csi2(sd);
-	struct v4l2_subdev *remote_sd;
-	struct media_pad *remote_pad;
-
-	remote_pad = media_pad_remote_pad_unique(&csi2->pads[DW_MIPI_CSI2RX_PAD_SINK]);
-	if (IS_ERR(remote_pad)) {
-		dev_err(csi2->dev, "can't get remote source pad\n");
-		return PTR_ERR(remote_pad);
-	}
-
-	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
-
-	return v4l2_subdev_call(remote_sd, pad, get_frame_desc,
-				remote_pad->index, fd);
-}
-
 static const struct v4l2_subdev_pad_ops dw_mipi_csi2rx_pad_ops = {
 	.enum_mbus_code = dw_mipi_csi2rx_enum_mbus_code,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = dw_mipi_csi2rx_set_fmt,
-	.get_frame_desc = dw_mipi_csi2rx_get_frame_desc,
+	.get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
 	.set_routing = dw_mipi_csi2rx_set_routing,
 	.enable_streams = dw_mipi_csi2rx_enable_streams,
 	.disable_streams = dw_mipi_csi2rx_disable_streams,

-- 
2.43.0

[PATCH v5 4/8] media: synopsys: Use V4L2 1-to-1 subdev helpers

[Frank.Li]

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

Use the V4L2 1-to-1 subdev infrastructure to simplify the driver.

Replace the local subdev registration and media pad setup code with
media_async_register_subdev_1to1() and struct v4l2_subdev_1to1. Reduce
boilerplate code and aligns the driver with the common pattern used by
simple subdevices that have a single sink and a single source pad.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v5
new patch

previous method:
https://lore.kernel.org/imx/20260226-v4l2_init_register-v2-2-902d7140f9fa@nxp.com/
---
 drivers/media/platform/synopsys/Kconfig          |   1 +
 drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 172 ++++-------------------
 2 files changed, 28 insertions(+), 145 deletions(-)

diff --git a/drivers/media/platform/synopsys/Kconfig b/drivers/media/platform/synopsys/Kconfig
index b109de2c8111c..8d7aabf93af34 100644
--- a/drivers/media/platform/synopsys/Kconfig
+++ b/drivers/media/platform/synopsys/Kconfig
@@ -10,6 +10,7 @@ config VIDEO_DW_MIPI_CSI2RX
 	depends on PM && COMMON_CLK
 	select GENERIC_PHY_MIPI_DPHY
 	select MEDIA_CONTROLLER
+	select V4L2_1TO1
 	select V4L2_FWNODE
 	select VIDEO_V4L2_SUBDEV_API
 	help
diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
index f51367409ff46..b70e3783adcd3 100644
--- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
+++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
@@ -22,6 +22,7 @@
 
 #include <media/mipi-csi2.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-device-1to1.h>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-mc.h>
 #include <media/v4l2-subdev.h>
@@ -78,12 +79,6 @@ enum dw_mipi_csi2rx_regs_index {
 	DW_MIPI_CSI2RX_MAX,
 };
 
-enum {
-	DW_MIPI_CSI2RX_PAD_SINK,
-	DW_MIPI_CSI2RX_PAD_SRC,
-	DW_MIPI_CSI2RX_PAD_MAX,
-};
-
 struct dw_mipi_csi2rx_device;
 
 struct dw_mipi_csi2rx_drvdata {
@@ -112,12 +107,8 @@ struct dw_mipi_csi2rx_device {
 	const struct dw_mipi_csi2rx_format *formats;
 	unsigned int formats_num;
 
-	struct media_pad pads[DW_MIPI_CSI2RX_PAD_MAX];
-	struct v4l2_async_notifier notifier;
-	struct v4l2_subdev sd;
+	struct v4l2_subdev_1to1 sd_1to1;
 
-	enum v4l2_mbus_type bus_type;
-	u32 lanes_num;
 	u64 enabled_streams;
 
 	const struct dw_mipi_csi2rx_drvdata *drvdata;
@@ -294,7 +285,7 @@ static const struct dw_mipi_csi2rx_format formats[] = {
 
 static inline struct dw_mipi_csi2rx_device *to_csi2(struct v4l2_subdev *sd)
 {
-	return container_of(sd, struct dw_mipi_csi2rx_device, sd);
+	return container_of(sd, struct dw_mipi_csi2rx_device, sd_1to1.sd);
 }
 
 static bool dw_mipi_csi2rx_has_reg(struct dw_mipi_csi2rx_device *csi2,
@@ -360,9 +351,9 @@ dw_mipi_csi2rx_find_format(struct dw_mipi_csi2rx_device *csi2, u32 mbus_code)
 
 static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
 {
+	u32 lanes = csi2->sd_1to1.vep.bus.mipi_csi2.num_data_lanes;
 	struct media_pad *source_pad;
 	union phy_configure_opts opts;
-	u32 lanes = csi2->lanes_num;
 	u32 control = 0;
 	s64 link_freq;
 	int ret;
@@ -371,7 +362,7 @@ static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
 		return -EINVAL;
 
 	source_pad = media_pad_remote_pad_unique(
-		&csi2->pads[DW_MIPI_CSI2RX_PAD_SINK]);
+		&csi2->sd_1to1.pads[V4L2_SUBDEV_1TO1_PADS_SINK]);
 	if (IS_ERR(source_pad))
 		return PTR_ERR(source_pad);
 
@@ -380,7 +371,7 @@ static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
 	if (link_freq < 0)
 		return link_freq;
 
-	switch (csi2->bus_type) {
+	switch (csi2->sd_1to1.vep.bus_type) {
 	case V4L2_MBUS_CSI2_DPHY:
 		ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq * 2,
 								 lanes, &opts.mipi_dphy);
@@ -458,16 +449,16 @@ dw_mipi_csi2rx_enum_mbus_code(struct v4l2_subdev *sd,
 	struct dw_mipi_csi2rx_device *csi2 = to_csi2(sd);
 
 	switch (code->pad) {
-	case DW_MIPI_CSI2RX_PAD_SRC:
+	case V4L2_SUBDEV_1TO1_PADS_SOURCE:
 		if (code->index)
 			return -EINVAL;
 
 		code->code =
 			v4l2_subdev_state_get_format(sd_state,
-						     DW_MIPI_CSI2RX_PAD_SINK)->code;
+						     V4L2_SUBDEV_1TO1_PADS_SINK)->code;
 
 		return 0;
-	case DW_MIPI_CSI2RX_PAD_SINK:
+	case V4L2_SUBDEV_1TO1_PADS_SINK:
 		if (code->index >= csi2->formats_num)
 			return -EINVAL;
 
@@ -487,7 +478,7 @@ static int dw_mipi_csi2rx_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_mbus_framefmt *sink, *src;
 
 	/* the format on the source pad always matches the sink pad */
-	if (format->pad == DW_MIPI_CSI2RX_PAD_SRC)
+	if (format->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
 		return v4l2_subdev_get_fmt(sd, state, format);
 
 	sink = v4l2_subdev_state_get_format(state, format->pad, format->stream);
@@ -549,12 +540,12 @@ static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
 	u64 mask;
 	int ret;
 
-	sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
+	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
 	remote_pad = media_pad_remote_pad_first(sink_pad);
 	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
 
-	mask = v4l2_subdev_state_xlate_streams(state, DW_MIPI_CSI2RX_PAD_SINK,
-					       DW_MIPI_CSI2RX_PAD_SRC,
+	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
 					       &streams_mask);
 
 	if (!csi2->enabled_streams) {
@@ -608,12 +599,12 @@ static int dw_mipi_csi2rx_disable_streams(struct v4l2_subdev *sd,
 	u64 mask;
 	int ret;
 
-	sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
+	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
 	remote_pad = media_pad_remote_pad_first(sink_pad);
 	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
 
-	mask = v4l2_subdev_state_xlate_streams(state, DW_MIPI_CSI2RX_PAD_SINK,
-					       DW_MIPI_CSI2RX_PAD_SRC,
+	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
 					       &streams_mask);
 
 	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
@@ -649,9 +640,9 @@ static int dw_mipi_csi2rx_init_state(struct v4l2_subdev *sd,
 {
 	struct v4l2_subdev_route routes[] = {
 		{
-			.sink_pad = DW_MIPI_CSI2RX_PAD_SINK,
+			.sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
 			.sink_stream = 0,
-			.source_pad = DW_MIPI_CSI2RX_PAD_SRC,
+			.source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
 			.source_stream = 0,
 			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
 		},
@@ -670,91 +661,11 @@ static const struct v4l2_subdev_internal_ops dw_mipi_csi2rx_internal_ops = {
 	.init_state = dw_mipi_csi2rx_init_state,
 };
 
-static int dw_mipi_csi2rx_notifier_bound(struct v4l2_async_notifier *notifier,
-					 struct v4l2_subdev *sd,
-					 struct v4l2_async_connection *asd)
-{
-	struct dw_mipi_csi2rx_device *csi2 =
-		container_of(notifier, struct dw_mipi_csi2rx_device, notifier);
-	struct media_pad *sink_pad = &csi2->pads[DW_MIPI_CSI2RX_PAD_SINK];
-	int ret;
-
-	ret = v4l2_create_fwnode_links_to_pad(sd, sink_pad,
-					      MEDIA_LNK_FL_ENABLED);
-	if (ret) {
-		dev_err(csi2->dev, "failed to link source pad of %s\n",
-			sd->name);
-		return ret;
-	}
-
-	return 0;
-}
-
-static const struct v4l2_async_notifier_operations dw_mipi_csi2rx_notifier_ops = {
-	.bound = dw_mipi_csi2rx_notifier_bound,
-};
-
-static int dw_mipi_csi2rx_register_notifier(struct dw_mipi_csi2rx_device *csi2)
-{
-	struct v4l2_async_connection *asd;
-	struct v4l2_async_notifier *ntf = &csi2->notifier;
-	struct v4l2_fwnode_endpoint vep;
-	struct v4l2_subdev *sd = &csi2->sd;
-	struct device *dev = csi2->dev;
-	int ret;
-
-	struct fwnode_handle *ep __free(fwnode_handle) =
-		fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
-	if (!ep)
-		return dev_err_probe(dev, -ENODEV, "failed to get endpoint\n");
-
-	vep.bus_type = V4L2_MBUS_UNKNOWN;
-	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to parse endpoint\n");
-
-	if (vep.bus_type != V4L2_MBUS_CSI2_DPHY &&
-	    vep.bus_type != V4L2_MBUS_CSI2_CPHY)
-		return dev_err_probe(dev, -EINVAL,
-				     "invalid bus type of endpoint\n");
-
-	csi2->bus_type = vep.bus_type;
-	csi2->lanes_num = vep.bus.mipi_csi2.num_data_lanes;
-
-	v4l2_async_subdev_nf_init(ntf, sd);
-	ntf->ops = &dw_mipi_csi2rx_notifier_ops;
-
-	asd = v4l2_async_nf_add_fwnode_remote(ntf, ep,
-					      struct v4l2_async_connection);
-	if (IS_ERR(asd)) {
-		ret = PTR_ERR(asd);
-		goto err_nf_cleanup;
-	}
-
-	ret = v4l2_async_nf_register(ntf);
-	if (ret) {
-		ret = dev_err_probe(dev, ret, "failed to register notifier\n");
-		goto err_nf_cleanup;
-	}
-
-	return 0;
-
-err_nf_cleanup:
-	v4l2_async_nf_cleanup(ntf);
-
-	return ret;
-}
-
 static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
 {
-	struct media_pad *pads = csi2->pads;
-	struct v4l2_subdev *sd = &csi2->sd;
+	struct v4l2_subdev *sd = &csi2->sd_1to1.sd;
 	int ret;
 
-	ret = dw_mipi_csi2rx_register_notifier(csi2);
-	if (ret)
-		goto err;
-
 	v4l2_subdev_init(sd, &dw_mipi_csi2rx_ops);
 	sd->dev = csi2->dev;
 	sd->entity.ops = &dw_mipi_csi2rx_media_ops;
@@ -764,45 +675,15 @@ static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
 	snprintf(sd->name, sizeof(sd->name), "dw-mipi-csi2rx %s",
 		 dev_name(csi2->dev));
 
-	pads[DW_MIPI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
-					      MEDIA_PAD_FL_MUST_CONNECT;
-	pads[DW_MIPI_CSI2RX_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
-	ret = media_entity_pads_init(&sd->entity, DW_MIPI_CSI2RX_PAD_MAX, pads);
-	if (ret)
-		goto err_notifier_unregister;
+	csi2->sd_1to1.remote_bustype_cap_mask = BIT(V4L2_MBUS_CSI2_DPHY) |
+						BIT(V4L2_MBUS_CSI2_CPHY);
 
-	ret = v4l2_subdev_init_finalize(sd);
+	ret = media_async_register_subdev_1to1(&csi2->sd_1to1);
 	if (ret)
-		goto err_entity_cleanup;
-
-	ret = v4l2_async_register_subdev(sd);
-	if (ret) {
-		dev_err(sd->dev, "failed to register CSI-2 subdev\n");
-		goto err_subdev_cleanup;
-	}
+		return dev_err_probe(sd->dev, ret,
+				     "failed to register CSI-2 subdev\n");
 
 	return 0;
-
-err_subdev_cleanup:
-	v4l2_subdev_cleanup(sd);
-err_entity_cleanup:
-	media_entity_cleanup(&sd->entity);
-err_notifier_unregister:
-	v4l2_async_nf_unregister(&csi2->notifier);
-	v4l2_async_nf_cleanup(&csi2->notifier);
-err:
-	return ret;
-}
-
-static void dw_mipi_csi2rx_unregister(struct dw_mipi_csi2rx_device *csi2)
-{
-	struct v4l2_subdev *sd = &csi2->sd;
-
-	v4l2_async_unregister_subdev(sd);
-	v4l2_subdev_cleanup(sd);
-	media_entity_cleanup(&sd->entity);
-	v4l2_async_nf_unregister(&csi2->notifier);
-	v4l2_async_nf_cleanup(&csi2->notifier);
 }
 
 static void imx93_csi2rx_dphy_assert_reset(struct dw_mipi_csi2rx_device *csi2)
@@ -879,12 +760,13 @@ static void imx93_csi2rx_dphy_ipi_enable(struct dw_mipi_csi2rx_device *csi2)
 
 static int imx93_csi2rx_wait_for_phy_stopstate(struct dw_mipi_csi2rx_device *csi2)
 {
+	u32 num_lanes = csi2->sd_1to1.vep.bus.mipi_csi2.num_data_lanes;
 	struct device *dev = csi2->dev;
 	u32 stopstate_mask;
 	u32 val;
 	int ret;
 
-	stopstate_mask = DPHY_STOPSTATE_CLK_LANE | GENMASK(csi2->lanes_num - 1, 0);
+	stopstate_mask = DPHY_STOPSTATE_CLK_LANE | GENMASK(num_lanes - 1, 0);
 
 	ret = read_poll_timeout(dw_mipi_csi2rx_read, val,
 				(val & stopstate_mask) == stopstate_mask,
@@ -993,7 +875,7 @@ static void dw_mipi_csi2rx_remove(struct platform_device *pdev)
 {
 	struct dw_mipi_csi2rx_device *csi2 = platform_get_drvdata(pdev);
 
-	dw_mipi_csi2rx_unregister(csi2);
+	media_async_subdev_1to1_cleanup(&csi2->sd_1to1);
 	phy_exit(csi2->phy);
 }
 

-- 
2.43.0

[PATCH v5 5/8] dt-bindings: media: add i.MX parallel CPI support

[Frank.Li]

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>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Chagnes in v4
- add Laurent Pinchart's review by tag
- fix $ref: /schemas/graph.yaml#/$defs/port-base, original is
$ref: /schemas/graph.yaml#/properties/port-base

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 0000000000000..9dd0331f6ef75
--- /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#/$defs/port-base
+        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#/$defs/port-base
+        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 825e7cd2d6739..deeec900ad071 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16258,6 +16258,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.43.0

[PATCH v5 6/8] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)

[Frank.Li]

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 v5
- Use subdev_1to1 register function
- Use v4l2_subdev_get_frame_desc_passthrough
- Use dwc csi2 similar logic enable/disable stream
- Add route settup at imx_cpi_init_state()
- Remove V2 register layout support, add it later

change in v4
- remove unnecesary header file.
- use devm_bulk_clk_get().
- update kConfig i.MX8/i.MX9
- Remove define IMX_CPI_DEF_PIX_WIDTH ..., which used once only
- drop get_interface_ctrl_reg1_param
- drop uv-swap
- drop imx_cpi_link_setup by use immutable link.
- use enable/disable_stream() replace depericated .s_stream.
- remove dbg print and reg dump functions.
- use goto/.remove() to do manual cleanup.
- remove imx93 support. Add it later.

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            |  12 +
 drivers/media/platform/nxp/Makefile           |   1 +
 drivers/media/platform/nxp/imx-parallel-cpi.c | 614 ++++++++++++++++++++++++++
 4 files changed, 628 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index deeec900ad071..e28b395734ef5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16263,6 +16263,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 40e3436669e21..90f7c792003f2 100644
--- a/drivers/media/platform/nxp/Kconfig
+++ b/drivers/media/platform/nxp/Kconfig
@@ -39,6 +39,18 @@ 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.MX8/i.MX9 Parallel CPI Driver"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on VIDEO_DEV
+	select MEDIA_CONTROLLER
+	select V4L2_1TO1
+	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 4d90eb7136525..5346919d2f108 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 0000000000000..00f5d5f47644b
--- /dev/null
+++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
@@ -0,0 +1,614 @@
+// 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/delay.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-device-1to1.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+/* 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))
+
+#define CPI_CTRL_V2_REG1_PIXEL_WIDTH(v)		FIELD_PREP(GENMASK(16, 0), (v))
+#define CPI_CTRL_V2_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_GATE_CLOCK_MODE,
+	PI_CCIR_MODE,
+};
+
+enum {
+	PI_V1,
+};
+
+struct imx_cpi_plat_data {
+	u32 version;
+	u32 if_ctrl_reg;
+	u32 interface_status;
+	u32 interface_ctrl_reg;
+	u32 interface_ctrl_reg1;
+};
+
+struct imx_cpi_device {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk_bulk_data *clks;
+	int num_clks;
+
+	struct v4l2_subdev_1to1 sd_1to1;
+
+	const struct imx_cpi_plat_data *pdata;
+
+	u32 enabled_streams;
+	u8 mode;
+};
+
+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_UYVY_BT656_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,
+};
+
+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);
+
+	fsleep(500);
+	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_pix_format *pcpidev_fmt)
+{
+	const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+	u32 flags = pcpidev->sd_1to1.vep.bus.parallel.flags;
+	bool hsync_pol = flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH;
+	bool vsync_pol = flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH;
+	u32 val;
+
+	/* Software Reset */
+	imx_cpi_sw_reset(pcpidev);
+
+	/* Config PL Data Type */
+	val = IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
+	val |= IF_CTRL_REG_PL_ENABLE | IF_CTRL_REG_PL_VALID;
+	writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+
+	/* Config CTRL REG */
+	val = CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN;
+
+	val |= CPI_CTRL_REG_DATA_TYPE_IN(pcpidev_fmt->data_type) |
+	       FIELD_PREP(CPI_CTRL_REG_HSYNC_POL, hsync_pol) |
+	       FIELD_PREP(CPI_CTRL_REG_VSYNC_POL, vsync_pol) |
+	       FIELD_PREP(CPI_CTRL_REG_PIXEL_CLK_POL, 0) |
+	       CPI_CTRL_REG_MASK_VSYNC_COUNTER(3) |
+	       CPI_CTRL_REG_HSYNC_PULSE(2);
+
+	if (pcpidev_fmt->code == MEDIA_BUS_FMT_YUYV8_2X8 ||
+	    pcpidev_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8)
+		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 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;
+	u32 pixel_width;
+	u32 vsync_pulse;
+	u32 val;
+
+	pixel_width = format->width - 1;
+	vsync_pulse = format->width << 1;
+
+	switch (pcpidev->pdata->version) {
+	case PI_V1:
+		val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
+		      CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
+		break;
+	default:
+		val = 0; /* Never happen */
+	}
+
+	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 struct imx_cpi_device *sd_to_imx_cpi_device(struct v4l2_subdev *sdev)
+{
+	return container_of(sdev, struct imx_cpi_device, sd_1to1.sd);
+}
+
+static const struct media_entity_operations imx_cpi_entity_ops = {
+	.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 v4l2_mbus_framefmt *fmt;
+
+	/*
+	 * The Parallel cpi can't transcode in any way, the source format
+	 * can't be modified.
+	 */
+	if (sdformat->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
+		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
+
+	pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
+	if (!pcpidev_fmt)
+		pcpidev_fmt = &imx_cpi_formats[0];
+
+	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
+
+	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, V4L2_SUBDEV_1TO1_PADS_SOURCE);
+	*fmt = sdformat->format;
+
+	/* The format on the source pad might change due to unpacking. */
+	fmt->code = pcpidev_fmt->output;
+
+	return 0;
+}
+
+static int imx_cpi_init_state(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *state)
+{
+	struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
+			.sink_stream = 0,
+			.source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+	struct v4l2_subdev_krouting routing = {
+		.len_routes = ARRAY_SIZE(routes),
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
+	struct v4l2_mbus_framefmt *fmt;
+
+	fmt = v4l2_subdev_state_get_format(state, 0);
+
+	fmt->code = imx_cpi_formats[0].code;
+	fmt->width = 1920;
+	fmt->height = 1080;
+
+	fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
+	fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+	fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+	fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(false,
+							  fmt->colorspace,
+							  fmt->ycbcr_enc);
+
+	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing, fmt);
+}
+
+static int imx_cpi_disable_streams(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state, u32 pad,
+				   u64 streams_mask)
+{
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+	struct media_pad *sink_pad, *remote_pad;
+	struct device *dev = pcpidev->dev;
+	struct v4l2_subdev *remote_sd;
+	u64 mask;
+	int ret;
+
+	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+	remote_pad = media_pad_remote_pad_first(sink_pad);
+	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
+					       &streams_mask);
+
+	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
+	if (ret)
+		dev_err(dev, "failed to disable streams on remote subdev: %d\n", ret);
+
+	pcpidev->enabled_streams &= ~streams_mask;
+
+	if (!pcpidev->enabled_streams) {
+		imx_cpi_disable(pcpidev);
+		pm_runtime_put_autosuspend(dev);
+	}
+
+	return 0;
+}
+
+static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state, u32 pad,
+				  u64 streams_mask)
+{
+	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+	const struct imx_cpi_pix_format *pcpidev_fmt;
+	const struct v4l2_mbus_framefmt *format;
+	struct media_pad *sink_pad, *remote_pad;
+	struct device *dev = pcpidev->dev;
+	struct v4l2_subdev *remote_sd;
+	u64 mask;
+	int ret;
+
+	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+	remote_pad = media_pad_remote_pad_first(sink_pad);
+	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
+					       &streams_mask);
+
+	format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);
+	pcpidev_fmt = find_imx_cpi_format(format->code);
+
+	if (!pcpidev->enabled_streams) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret)
+			return ret;
+
+		imx_cpi_hw_config(pcpidev, pcpidev_fmt);
+		imx_cpi_config_ctrl_reg1(pcpidev, format);
+		imx_cpi_enable(pcpidev);
+	}
+
+	ret = v4l2_subdev_enable_streams(remote_sd, remote_pad->index, mask);
+	if (ret)
+		goto err_cpi_stop;
+
+	pcpidev->enabled_streams |= streams_mask;
+
+	return 0;
+
+err_cpi_stop:
+	/* Stop CSI hardware if no streams are enabled */
+	if (!pcpidev->enabled_streams)
+		imx_cpi_disable(pcpidev);
+
+	pm_runtime_put_autosuspend(dev);
+
+	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 == V4L2_SUBDEV_1TO1_PADS_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 != V4L2_SUBDEV_1TO1_PADS_SINK)
+		return -EINVAL;
+
+	if (code->index >= ARRAY_SIZE(imx_cpi_formats))
+		return -EINVAL;
+
+	code->code = imx_cpi_formats[code->index].code;
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops imx_cpi_video_ops = {
+	.s_stream = 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 = v4l2_subdev_get_frame_desc_passthrough,
+	.enable_streams = imx_cpi_enable_streams,
+	.disable_streams = imx_cpi_disable_streams,
+};
+
+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,
+};
+
+/* ----------------------------------------------------------------------
+ * Suspend/resume
+ */
+
+static int imx_cpi_runtime_suspend(struct device *dev)
+{
+	struct imx_cpi_device *pcpidev = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(pcpidev->num_clks, pcpidev->clks);
+
+	return 0;
+}
+
+static int imx_cpi_runtime_resume(struct device *dev)
+{
+	struct imx_cpi_device *pcpidev = dev_get_drvdata(dev);
+
+	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 void imx_cpi_remove(struct platform_device *pdev)
+{
+	struct imx_cpi_device *pcpidev = platform_get_drvdata(pdev);
+
+	media_async_subdev_1to1_cleanup(&pcpidev->sd_1to1);
+}
+
+static int imx_cpi_probe(struct platform_device *pdev)
+{
+	struct imx_cpi_device *pcpidev;
+	struct device *dev = &pdev->dev;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	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->mode = PI_GATE_CLOCK_MODE;
+
+	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");
+
+	pcpidev->num_clks = devm_clk_bulk_get_all(dev, &pcpidev->clks);
+	if (pcpidev->num_clks < 0)
+		return pcpidev->num_clks;
+
+	sd = &pcpidev->sd_1to1.sd;
+
+	v4l2_subdev_init(sd, &imx_cpi_subdev_ops);
+
+	sd->internal_ops = &imx_cpi_internal_ops;
+	snprintf(sd->name, sizeof(sd->name), "parallel-%s",
+		 dev_name(pcpidev->dev));
+
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	sd->entity.ops = &imx_cpi_entity_ops;
+
+	sd->dev = pcpidev->dev;
+	pcpidev->sd_1to1.remote_bustype_cap_mask = BIT(V4L2_MBUS_PARALLEL);
+
+	pm_runtime_use_autosuspend(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	return media_async_register_subdev_1to1(&pcpidev->sd_1to1);
+}
+
+static const struct of_device_id imx_cpi_of_match[] = {
+	{ .compatible = "fsl,imx8qxp-pcif", .data = &imx8qxp_pdata },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, imx_cpi_of_match);
+
+static struct platform_driver imx_cpi_driver = {
+	.probe = imx_cpi_probe,
+	.remove = imx_cpi_remove,
+	.driver = {
+		.of_match_table = imx_cpi_of_match,
+		.name = "imx-parallel-cpi",
+		.pm = pm_ptr(&imx_cpi_pm_ops),
+	},
+};
+
+module_platform_driver(imx_cpi_driver);
+
+MODULE_DESCRIPTION("i.MX9 Parallel CPI receiver driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH v5 6/8] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] NULL pointer dereference when enabling or disabling streams on an unconnected sink pad.
- [High] Missing format bounds clamping causes integer underflow and massive hardware width configuration.
- [Medium] Missing `V4L2_SUBDEV_FL_STREAMS` flag breaks V4L2 format and routing state initialization.
- [Medium] Reversed pad direction arguments in `v4l2_subdev_state_xlate_streams` leads to improper stream mask mapping.
- [Low] Unconditional overwrite of hardware configuration variable `val` renders `switch` statement dead code.
--

commit 6eecb6f34887a526e9a659dcd6d0a03e320f98ab
Author: Alice Yuan <alice.yuan@nxp.com>

media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)

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.

> 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 0000000000000..00f5d5f47644b
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,614 @@

[ ... ]

> +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;
> +	u32 pixel_width;
> +	u32 vsync_pulse;
> +	u32 val;
> +
> +	pixel_width = format->width - 1;
> +	vsync_pulse = format->width << 1;
> +
> +	switch (pcpidev->pdata->version) {
> +	case PI_V1:
> +		val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> +		      CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
> +		break;
> +	default:
> +		val = 0; /* Never happen */
> +	}
> +
> +	val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
> +	      CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);

[Severity: Low]
Is the unconditional assignment to val here intentional? It immediately
overwrites the val computed inside the switch statement above, effectively
rendering the switch block as dead code and dropping any future hardware
version configurations.

> +	writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
> +}

[ ... ]

> +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 v4l2_mbus_framefmt *fmt;
> +
> +	/*
> +	 * The Parallel cpi can't transcode in any way, the source format
> +	 * can't be modified.
> +	 */
> +	if (sdformat->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
> +		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
> +
> +	pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
> +	if (!pcpidev_fmt)
> +		pcpidev_fmt = &imx_cpi_formats[0];
> +
> +	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
> +
> +	fmt->code = pcpidev_fmt->code;
> +	fmt->width = sdformat->format.width;
> +	fmt->height = sdformat->format.height;

[Severity: High]
Does this code need bounds checking for width and height? If a malicious
application provides a width of 0, it is copied directly to the format
structure here.

Later in imx_cpi_config_ctrl_reg1(), the width is used like this:

    pixel_width = format->width - 1;

which would underflow to 0xffffffff, causing an extremely large dimension to
be written to the hardware configuration.

> +	fmt->field = V4L2_FIELD_NONE;
> +	fmt->colorspace = sdformat->format.colorspace;

[ ... ]

> +static int imx_cpi_disable_streams(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *state, u32 pad,
> +				   u64 streams_mask)
> +{
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	struct media_pad *sink_pad, *remote_pad;
> +	struct device *dev = pcpidev->dev;
> +	struct v4l2_subdev *remote_sd;
> +	u64 mask;
> +	int ret;
> +
> +	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> +	remote_pad = media_pad_remote_pad_first(sink_pad);
> +	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);

[Severity: High]
Could this cause a NULL pointer dereference if the subdevice is not connected?

media_pad_remote_pad_first() returns NULL if no link is enabled on the sink
pad. If userspace sends VIDIOC_SUBDEV_S_STREAM directly to the unconnected
device node, remote_pad->entity would trigger a panic.

> +
> +	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> +					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
> +					       &streams_mask);

[Severity: Medium]
Are the pad arguments reversed here? 

When disable_streams is called, streams_mask originates from the pad passed
to the operation (the SOURCE pad). To find the remote mask, it should
translate from SOURCE to SINK. 

Should this be passing V4L2_SUBDEV_1TO1_PADS_SOURCE as the second argument
and V4L2_SUBDEV_1TO1_PADS_SINK as the third?

> +
> +	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
> +	if (ret)

[ ... ]

> +static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *state, u32 pad,
> +				  u64 streams_mask)
> +{
> +	struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> +	const struct imx_cpi_pix_format *pcpidev_fmt;
> +	const struct v4l2_mbus_framefmt *format;
> +	struct media_pad *sink_pad, *remote_pad;
> +	struct device *dev = pcpidev->dev;
> +	struct v4l2_subdev *remote_sd;
> +	u64 mask;
> +	int ret;
> +
> +	sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> +	remote_pad = media_pad_remote_pad_first(sink_pad);
> +	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);

[Severity: High]
Does this have the same NULL pointer dereference issue on unconnected links as
imx_cpi_disable_streams()?

> +
> +	mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> +					       V4L2_SUBDEV_1TO1_PADS_SOURCE,
> +					       &streams_mask);

[Severity: Medium]
Similarly, do the pad arguments need to be swapped here to correctly
translate from SOURCE to SINK?

> +
> +	format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);

[ ... ]

> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> +	struct imx_cpi_device *pcpidev;
> +	struct device *dev = &pdev->dev;
> +	struct v4l2_subdev *sd;
> +	int ret;

[ ... ]

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

[Severity: Medium]
Should V4L2_SUBDEV_FL_STREAMS also be set here?

Without this flag, the V4L2 core falls back to allocating the legacy
state->pads array. This causes the routing and stream configuration
set up by v4l2_subdev_set_routing_with_fmt() in imx_cpi_init_state()
to be ignored, which leaves the initial format reporting uninitialized.

> +
> +	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	sd->entity.ops = &imx_cpi_entity_ops;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=6

[PATCH v5 7/8] arm64: dts: imx8: add camera parallel interface (CPI) node

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

changes in v3
- 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 a72b2f1c4a1b2..b504f99f6acdb 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -222,6 +222,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 232cf25dadfcd..5aae15540d6cb 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>;
+			};
+		};
 	};
 };
 
@@ -95,3 +103,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.43.0

Re: [PATCH v5 7/8] arm64: dts: imx8: add camera parallel interface (CPI) node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The devicetree routes the parallel CPI to `port@4`, contradicting the `fsl,imx8qxp-isi` binding which explicitly documents `port@6`.
- [Low] Devicetree validation failure (dtbs_check) due to missing `ports` property in `parallel_cpi` base node for imx8qm.
--

commit 0f41e0622cccbe3c369609c51df0eb689a3de5d4
Author: Frank Li <Frank.Li@nxp.com>

arm64: dts: imx8: add camera parallel interface (CPI) node

Add the camera parallel interface (CPI) base node to imx8-ss-img.dtsi and
extend it with routing in imx8qxp-ss-img.dtsi.

> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> index a72b2f1c4a1b2..b504f99f6acdb 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
[ ... ]
> @@ -222,6 +222,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";
> +	};

[Severity: Low]
Will this cause dtbs_check failures for other platforms that include this
file?

Since this base node definition lacks a ports property, validating platforms
like imx8qm that include this file without appending ports might trigger a
dt-schema warning. The dt-schema tool unconditionally validates disabled
nodes, and fsl,imx93-pcif.yaml requires the ports property.

[ ... ]
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> index 232cf25dadfcd..5aae15540d6cb 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>;
> +			};
> +		};

[Severity: Medium]
Does connecting this to port 4 conflict with the documented binding?

Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml explicitly
documents port 6 as the CSI-2 Parallel RX. While port 4 is technically
correct for the imx8-isi crossbar driver (which only supports 5 ports),
this setup directly contradicts the documented schema.

Should the binding be updated or the devicetree corrected to maintain
consistency?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx8qxp_pcam-v5-0-7fa6c8e7fba7@nxp.com?part=7

[PATCH v5 8/8] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support

[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 v4
- add hsync-active = <1>

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 | 83 ++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 001ca3a12c0ae..3b9e9844f11ef 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -554,6 +554,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-9x9-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 0000000000000..9fbdd798f17d6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
@@ -0,0 +1,83 @@
+// 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 {
+				hsync-active = <1>;
+				remote-endpoint = <&ov5640_pi_ep>;
+			};
+		};
+	};
+};

-- 
2.43.0