[PATCH 0/8] media: imx-mipi-csis: Cleanups and debugging improvements

[PATCH 0/8] media: imx-mipi-csis: Cleanups and debugging improvements

[Laurent Pinchart]

Hello,

This patch series bring a few miscellaneous improvements to the
imx-mipi-csis driver, and in particular improves the debugging
infrastructure.

Patch 1/8 starts by aligning the code with the reference manual for
register field names, increasing readability of the driver when read
alongside the hardware documentation. Patch 2/8 then fixes a small
alignment issue in register dumps. Patch 3/8 logs per-lane start of
transmission error instead of supporting the first data lane only,
easing debugging of D-PHY issues.

The next two patches deprecate the clock-frequency DT property, which
shouldn't have been added in the first place. Patch 4/8 improves
handling of the clock frequency in the driver, and patch 5/8 deprecates
the property in the DT bindings. The driver still supports the property
to ensure backward compatibility.

The last three patches introduce support for multiple output channels
and wire it up in the debugging infrastructure. The CSIS IP core
supports up to 4 output channels, with the number of instantiated
channels being a property of the SoC integration. So far, only the
i.MX8MP is known to have multiple output channels. Patch 6/8 adds a
corresponding DT property, and patch 7/8 sets it in the i.MX8MP DT.
Patch 8/8 then adds initial support for that property in the driver, and
uses it to dump per-channel registers and event counters.

Laurent Pinchart (8):
  media: imx-mipi-csis: Rename register macros to match reference manual
  media: imx-mipi-csis: Fix field alignment in register dump
  media: imx-mipi-csis: Log per-lane start of transmission errors
  media: imx-mipi-csis: Only set clock rate when specified in DT
  dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as
    deprecated
  dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
  arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers
  media: imx-mipi-csis: Initial support for multiple output channels

 .../bindings/media/nxp,imx-mipi-csi2.yaml     |  18 +-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |   2 +
 drivers/media/platform/nxp/imx-mipi-csis.c    | 311 +++++++++++-------
 3 files changed, 211 insertions(+), 120 deletions(-)


base-commit: 3c699df678515355e871141e142adae3bbf44216
prerequisite-patch-id: 7200af6e6d693b425b5cdb9ae6be1c55e23e2a45
-- 
Regards,

Laurent Pinchart

[PATCH 1/8] media: imx-mipi-csis: Rename register macros to match reference manual

[Laurent Pinchart]

The CSIS driver uses register macro names that do not match the
reference manual of the i.MX7[DS] and i.MX8M[MNP] SoCs in which the CSIS
is integrated. Rename them to match the documentation, making the code
easier to read alongside the reference manuals.

One of the misnamed register fields is MIPI_CSIS_INT_SRC_ERR_UNKNOWN,
which led to the corresponding event being logged as "Unknown Error".
The correct register field name is MIPI_CSIS_INT_SRC_ERR_ID, documented
as "Unknown ID error". Update the event description accordingly.

While at it, also replace a few *_OFFSET macros with parametric macros
for consistency, and add the missing MIPI_CSIS_ISP_RESOL_VRESOL and
MIPI_CSIS_ISP_RESOL_HRESOL register field macros.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 69 +++++++++++-----------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..d59666ef7545 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -55,13 +55,13 @@
 /* CSIS common control */
 #define MIPI_CSIS_CMN_CTRL			0x04
 #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
-#define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE	(0 << 10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	(1 << 10)
+#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
+#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	(3 << 8)
 #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
-#define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
-#define MIPI_CSIS_CMN_CTRL_ENABLE		BIT(0)
-
-#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET	8
-#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK		(3 << 8)
+#define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
+#define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
 
 /* CSIS clock control */
 #define MIPI_CSIS_CLK_CTRL			0x08
@@ -87,7 +87,7 @@
 #define MIPI_CSIS_INT_MSK_ERR_WRONG_CFG		BIT(3)
 #define MIPI_CSIS_INT_MSK_ERR_ECC		BIT(2)
 #define MIPI_CSIS_INT_MSK_ERR_CRC		BIT(1)
-#define MIPI_CSIS_INT_MSK_ERR_UNKNOWN		BIT(0)
+#define MIPI_CSIS_INT_MSK_ERR_ID		BIT(0)
 
 /* CSIS Interrupt source */
 #define MIPI_CSIS_INT_SRC			0x14
@@ -107,7 +107,7 @@
 #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
 #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
 #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
-#define MIPI_CSIS_INT_SRC_ERR_UNKNOWN		BIT(0)
+#define MIPI_CSIS_INT_SRC_ERR_ID		BIT(0)
 #define MIPI_CSIS_INT_SRC_ERRORS		0xfffff
 
 /* D-PHY status control */
@@ -123,8 +123,8 @@
 #define MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE_MASK	GENMASK(31, 24)
 #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(n)	((n) << 22)
 #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE_MASK	GENMASK(23, 22)
-#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_CLK	BIT(6)
-#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_DAT	BIT(5)
+#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_CLK	BIT(6)
+#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_DAT	BIT(5)
 #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_DAT	BIT(1)
 #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_CLK	BIT(0)
 #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE		(0x1f << 0)
@@ -179,21 +179,23 @@
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
-#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
-#define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
-#define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
-#define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
+#define MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK	(3 << 12)
+#define MIPI_CSIS_ISPCFG_PARALLEL		BIT(11)
+#define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt)	((fmt) << 2)
+#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	(0x3f << 2)
 
 /* ISP Image Resolution register */
 #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
+#define MIPI_CSIS_ISP_RESOL_VRESOL(n)		((n) << 16)
+#define MIPI_CSIS_ISP_RESOL_HRESOL(n)		((n) << 0)
 #define CSIS_MAX_PIX_WIDTH			0xffff
 #define CSIS_MAX_PIX_HEIGHT			0xffff
 
 /* ISP SYNC register */
 #define MIPI_CSIS_ISP_SYNC_CH(n)		(0x48 + (n) * 0x10)
-#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET	18
-#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET	12
-#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET	0
+#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(n)	((n) << 18)
+#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(n)	((n) << 12)
+#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(n)	((n) << 0)
 
 /* ISP shadow registers */
 #define MIPI_CSIS_SDW_CONFIG_CH(n)		(0x80 + (n) * 0x10)
@@ -246,7 +248,7 @@ static const struct mipi_csis_event mipi_csis_events[] = {
 	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
 	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
 	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN,		"Unknown Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
 	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
 	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
 	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
@@ -517,7 +519,7 @@ static void mipi_csis_sw_reset(struct mipi_csis_device *csis)
 	u32 val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
 
 	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
-			val | MIPI_CSIS_CMN_CTRL_RESET);
+			val | MIPI_CSIS_CMN_CTRL_SW_RESET);
 	usleep_range(10, 20);
 }
 
@@ -527,9 +529,9 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
 	if (on)
-		val |= MIPI_CSIS_CMN_CTRL_ENABLE;
+		val |= MIPI_CSIS_CMN_CTRL_CSI_EN;
 	else
-		val &= ~MIPI_CSIS_CMN_CTRL_ENABLE;
+		val &= ~MIPI_CSIS_CMN_CTRL_CSI_EN;
 	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
 
 	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
@@ -549,8 +551,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
 
 	/* Color format */
 	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
-	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
-		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
+	val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
+		 MIPI_CSIS_ISPCFG_DATAFORMAT_MASK);
 
 	/*
 	 * YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
@@ -568,12 +570,13 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
 	if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
 		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
 
-	val |= MIPI_CSIS_ISPCFG_FMT(csis_fmt->data_type);
+	val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
 	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
 
 	/* Pixel resolution */
-	val = format->width | (format->height << 16);
-	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0), val);
+	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
+			MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
+			MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
 }
 
 static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
@@ -635,10 +638,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 	u32 val;
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
-	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK;
-	val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET;
+	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
+	val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
 	if (csis->info->version == MIPI_CSIS_V3_3)
-		val |= MIPI_CSIS_CMN_CTRL_INTER_MODE;
+		val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
 	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
 
 	__mipi_csis_set_format(csis, format, csis_fmt);
@@ -647,10 +650,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
 			MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
 
-	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
-	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
-	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET);
-	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0), val);
+	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
+			MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
+			MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
+			MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
 	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
-- 
Regards,

Laurent Pinchart

[PATCH 2/8] media: imx-mipi-csis: Fix field alignment in register dump

[Laurent Pinchart]

Commit 95a1379004cb ("media: staging: media: imx: imx7-mipi-csis: Dump
MIPI_CSIS_FRAME_COUNTER_CH0 register") forgot to increase the maximum
register name length, resulting in misalignment of names printed in the
kernel log. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index d59666ef7545..b652d59851c2 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -895,7 +895,7 @@ static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
 
 	for (i = 0; i < ARRAY_SIZE(registers); i++) {
 		cfg = mipi_csis_read(csis, registers[i].offset);
-		dev_info(csis->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
+		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
 	}
 
 	pm_runtime_put(csis->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 3/8] media: imx-mipi-csis: Log per-lane start of transmission errors

[Laurent Pinchart]

The CSIS has per-line start of transmission error interrupts. Log them
all, instead of only the first data lane.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index b652d59851c2..e27467e6372f 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -100,7 +100,7 @@
 #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
 #define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
 #define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
-#define MIPI_CSIS_INT_SRC_ERR_SOT_HS		BIT(16)
+#define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
 #define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
 #define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
 #define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
@@ -241,7 +241,10 @@ struct mipi_csis_event {
 
 static const struct mipi_csis_event mipi_csis_events[] = {
 	/* Errors */
-	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS,		"SOT Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),	"SOT 0 Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),	"SOT 1 Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),	"SOT 2 Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),	"SOT 3 Error" },
 	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FS,		"Lost Frame Start Error" },
 	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FE,		"Lost Frame End Error" },
 	{ false, MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
-- 
Regards,

Laurent Pinchart

[PATCH 4/8] media: imx-mipi-csis: Only set clock rate when specified in DT

[Laurent Pinchart]

The imx-mipi-csis driver sets the rate of the wrap clock to the value
specified in the device tree's "clock-frequency" property, and defaults
to 166 MHz otherwise. This is a historical mistake, as clock rate
selection should have been left to the assigned-clock-rates property.

Honouring the clock-frequency property can't be removed without breaking
backwards compatibility, and the corresponding code isn't very
intrusive. The 166 MHz default, on the other hand, prevents
configuration of the clock rate through assigned-clock-rates, as the
driver immediately overwrites the rate. This behaviour is confusing and
has cost debugging time.

There is little value in a 166 MHz default. All mainline device tree
sources that enable the CSIS specify a clock-frequency explicitly, and
the default wrap clock configuration on supported platforms is at least
as high as 166 MHz. Drop the default, and only set the clock rate
manually when the clock-frequency property is specified.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index e27467e6372f..080e40837463 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -230,8 +230,6 @@
 #define MIPI_CSIS_PKTDATA_EVEN			0x3000
 #define MIPI_CSIS_PKTDATA_SIZE			SZ_4K
 
-#define DEFAULT_SCLK_CSIS_FREQ			166000000UL
-
 struct mipi_csis_event {
 	bool debug;
 	u32 mask;
@@ -710,12 +708,17 @@ static int mipi_csis_clk_get(struct mipi_csis_device *csis)
 	if (ret < 0)
 		return ret;
 
-	/* Set clock rate */
-	ret = clk_set_rate(csis->clks[MIPI_CSIS_CLK_WRAP].clk,
-			   csis->clk_frequency);
-	if (ret < 0)
-		dev_err(csis->dev, "set rate=%d failed: %d\n",
-			csis->clk_frequency, ret);
+	if (csis->clk_frequency) {
+		/*
+		 * Set the clock rate. This is deprecated, for backward
+		 * compatibility with old device trees.
+		 */
+		ret = clk_set_rate(csis->clks[MIPI_CSIS_CLK_WRAP].clk,
+				   csis->clk_frequency);
+		if (ret < 0)
+			dev_err(csis->dev, "set rate=%d failed: %d\n",
+				csis->clk_frequency, ret);
+	}
 
 	return ret;
 }
@@ -1419,9 +1422,7 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
 {
 	struct device_node *node = csis->dev->of_node;
 
-	if (of_property_read_u32(node, "clock-frequency",
-				 &csis->clk_frequency))
-		csis->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
+	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
 
 	return 0;
 }
-- 
Regards,

Laurent Pinchart

[PATCH 5/8] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated

[Laurent Pinchart]

Usage of the clock-frequency property, which is already optional, is
discouraged in favour of using assigned-clock-rates (and
assigned-clock-parents where needed). Mark the property as deprecated,
and update the examples accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml  | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
index 03a23a26c4f3..db4889bf881e 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
@@ -66,6 +66,7 @@ properties:
   clock-frequency:
     description: The desired external clock ("wrap") frequency, in Hz
     default: 166000000
+    deprecated: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -147,7 +148,9 @@ examples:
                  <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
                  <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
         clock-names = "pclk", "wrap", "phy";
-        clock-frequency = <166000000>;
+
+        assigned-clocks = <&clks IMX7D_MIPI_CSI_ROOT_CLK>;
+        assigned-clock-rates = <166000000>;
 
         power-domains = <&pgc_mipi_phy>;
         phy-supply = <&reg_1p0d>;
@@ -185,12 +188,16 @@ examples:
         compatible = "fsl,imx8mm-mipi-csi2";
         reg = <0x32e30000 0x1000>;
         interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
-        clock-frequency = <333000000>;
+
         clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
                  <&clk IMX8MM_CLK_CSI1_ROOT>,
                  <&clk IMX8MM_CLK_CSI1_PHY_REF>,
                  <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
         clock-names = "pclk", "wrap", "phy", "axi";
+
+        assigned-clocks = <&clk IMX8MM_CLK_CSI1_ROOT>;
+        assigned-clock-rates = <250000000>;
+
         power-domains = <&mipi_pd>;
 
         ports {
-- 
Regards,

Laurent Pinchart

[PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Laurent Pinchart]

The CSI-2 receiver can be instantiated with up to four output channels.
This is an integration-specific property, specify the number of
instantiated channels through a new fsl,num-channels property. The
property is optional, and defaults to 1 as only one channel is currently
supported by drivers.

The only known SoC to have more than one channel is the i.MX8MP. As the
binding examples do not cover that SoC, don't update them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
index db4889bf881e..41ad5b84eaeb 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
@@ -68,6 +68,13 @@ properties:
     default: 166000000
     deprecated: true
 
+  fsl,num-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of output channels
+    minimum: 1
+    maximum: 4
+    default: 1
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
Regards,

Laurent Pinchart

[PATCH 7/8] arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers

[Laurent Pinchart]

The CSI-2 receivers in the i.MX8MP have 3 output channels. Specify this
in the device tree, to enable support for more than one channel in
drivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 95b3f250d363..c409a1d1e851 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1767,6 +1767,7 @@ mipi_csi_0: csi@32e40000 {
 				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_250M>,
 							 <&clk IMX8MP_CLK_24M>;
 				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
+				fsl,num-channels = <3>;
 				status = "disabled";
 
 				ports {
@@ -1802,6 +1803,7 @@ mipi_csi_1: csi@32e50000 {
 				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_250M>,
 							 <&clk IMX8MP_CLK_24M>;
 				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
+				fsl,num-channels = <3>;
 				status = "disabled";
 
 				ports {
-- 
Regards,

Laurent Pinchart

[PATCH 8/8] media: imx-mipi-csis: Initial support for multiple output channels

[Laurent Pinchart]

Some CSIS instances feature more than one output channel. Parse the
number of channels from the device tree, and update register dumps and
event counters accordingly. Support for routing virtual channels and
data types to output channels through the subdev internal routing API
will come later.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 224 ++++++++++++++-------
 1 file changed, 146 insertions(+), 78 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 080e40837463..4cc358d93187 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -98,12 +98,12 @@
 #define MIPI_CSIS_INT_SRC_ODD_AFTER		BIT(28)
 #define MIPI_CSIS_INT_SRC_ODD			(0x3 << 28)
 #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
-#define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
-#define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
+#define MIPI_CSIS_INT_SRC_FRAME_START(n)	BIT((n) + 24)
+#define MIPI_CSIS_INT_SRC_FRAME_END(n)		BIT((n) + 20)
 #define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
-#define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
-#define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
-#define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
+#define MIPI_CSIS_INT_SRC_ERR_LOST_FS(n)	BIT((n) + 12)
+#define MIPI_CSIS_INT_SRC_ERR_LOST_FE(n)	BIT((n) + 8)
+#define MIPI_CSIS_INT_SRC_ERR_OVER(n)		BIT((n) + 4)
 #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
 #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
 #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
@@ -205,23 +205,23 @@
 /* Debug control register */
 #define MIPI_CSIS_DBG_CTRL			0xc0
 #define MIPI_CSIS_DBG_INTR_MSK			0xc4
-#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT	BIT(25)
-#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE	BIT(24)
-#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE	BIT(20)
-#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME	BIT(16)
-#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE		BIT(12)
-#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS		BIT(8)
-#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
-#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)
+#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
+#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)
+#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE(n)	BIT((n) + 20)
+#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME(n)	BIT((n) + 16)
+#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE(n)		BIT((n) + 12)
+#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS(n)		BIT((n) + 8)
+#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL(n)	BIT((n) + 4)
+#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE(n)	BIT((n) + 0)
 #define MIPI_CSIS_DBG_INTR_SRC			0xc8
-#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT	BIT(25)
-#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE	BIT(24)
-#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE	BIT(20)
-#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME	BIT(16)
-#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE		BIT(12)
-#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS		BIT(8)
-#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
-#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
+#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT		BIT(25)
+#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE		BIT(24)
+#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(n)	BIT((n) + 20)
+#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(n)	BIT((n) + 16)
+#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(n)		BIT((n) + 12)
+#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(n)		BIT((n) + 8)
+#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(n)	BIT((n) + 4)
+#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(n)	BIT((n) + 0)
 
 #define MIPI_CSIS_FRAME_COUNTER_CH(n)		(0x0100 + (n) * 4)
 
@@ -230,8 +230,11 @@
 #define MIPI_CSIS_PKTDATA_EVEN			0x3000
 #define MIPI_CSIS_PKTDATA_SIZE			SZ_4K
 
+#define MIPI_CSIS_MAX_CHANNELS			4
+
 struct mipi_csis_event {
 	bool debug;
+	unsigned int channel;
 	u32 mask;
 	const char * const name;
 	unsigned int counter;
@@ -239,36 +242,70 @@ struct mipi_csis_event {
 
 static const struct mipi_csis_event mipi_csis_events[] = {
 	/* Errors */
-	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),	"SOT 0 Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),	"SOT 1 Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),	"SOT 2 Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),	"SOT 3 Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FS,		"Lost Frame Start Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FE,		"Lost Frame End Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
-	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME,	"Truncated Frame" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE,	"Early Frame End" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS,	"Early Frame Start" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),		"SOT 0 Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),		"SOT 1 Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),		"SOT 2 Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),		"SOT 3 Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS(0),		"Lost Frame Start Error 0" },
+	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FS(1),		"Lost Frame Start Error 1" },
+	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FS(2),		"Lost Frame Start Error 2" },
+	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FS(3),		"Lost Frame Start Error 3" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE(0),		"Lost Frame End Error 0" },
+	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FE(1),		"Lost Frame End Error 1" },
+	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FE(2),		"Lost Frame End Error 2" },
+	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FE(3),		"Lost Frame End Error 3" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_OVER(0),		"FIFO Overflow Error 0" },
+	{ false, 1, MIPI_CSIS_INT_SRC_ERR_OVER(1),		"FIFO Overflow Error 1" },
+	{ false, 2, MIPI_CSIS_INT_SRC_ERR_OVER(2),		"FIFO Overflow Error 2" },
+	{ false, 3, MIPI_CSIS_INT_SRC_ERR_OVER(3),		"FIFO Overflow Error 3" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,		"Wrong Configuration Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ECC,			"ECC Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_CRC,			"CRC Error" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ID,			"Unknown ID Error" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,		"Data Type Ignored" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(0),	"Frame Size Error 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(1),	"Frame Size Error 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(2),	"Frame Size Error 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(3),	"Frame Size Error 3" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(0),	"Truncated Frame 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(1),	"Truncated Frame 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(2),	"Truncated Frame 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(3),	"Truncated Frame 3" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(0),		"Early Frame End 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(1),		"Early Frame End 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(2),		"Early Frame End 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(3),		"Early Frame End 3" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(0),		"Early Frame Start 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(1),		"Early Frame Start 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(2),		"Early Frame Start 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(3),		"Early Frame Start 3" },
 	/* Non-image data receive events */
-	{ false, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
-	{ false, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
-	{ false, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
-	{ false, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
+	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
+	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
+	{ false, 0, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
 	/* Frame start/end */
-	{ false, MIPI_CSIS_INT_SRC_FRAME_START,		"Frame Start" },
-	{ false, MIPI_CSIS_INT_SRC_FRAME_END,		"Frame End" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL,	"VSYNC Falling Edge" },
-	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE,	"VSYNC Rising Edge" },
+	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_START(0),		"Frame Start 0" },
+	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_START(1),		"Frame Start 1" },
+	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_START(2),		"Frame Start 2" },
+	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_START(3),		"Frame Start 3" },
+	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_END(0),		"Frame End 0" },
+	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_END(1),		"Frame End 1" },
+	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_END(2),		"Frame End 2" },
+	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_END(3),		"Frame End 3" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(0),	"VSYNC Falling Edge 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(1),	"VSYNC Falling Edge 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(2),	"VSYNC Falling Edge 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(3),	"VSYNC Falling Edge 3" },
+	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(0),	"VSYNC Rising Edge 0" },
+	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(1),	"VSYNC Rising Edge 1" },
+	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(2),	"VSYNC Rising Edge 2" },
+	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(3),	"VSYNC Rising Edge 3" },
 };
 
-#define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events)
+#define MIPI_CSIS_NUM_EVENTS		ARRAY_SIZE(mipi_csis_events)
+#define MIPI_CSIS_NUM_ERROR_EVENTS	(MIPI_CSIS_NUM_EVENTS - 20)
 
 enum mipi_csis_clk {
 	MIPI_CSIS_CLK_PCLK,
@@ -300,7 +337,9 @@ struct mipi_csis_device {
 	struct clk_bulk_data *clks;
 	struct reset_control *mrst;
 	struct regulator *mipi_phy_regulator;
+
 	const struct mipi_csis_info *info;
+	unsigned int num_channels;
 
 	struct v4l2_subdev sd;
 	struct media_pad pads[CSIS_PADS_NUM];
@@ -766,16 +805,19 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
 
 	/* Update the event/error counters */
 	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) {
-		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
+		for (i = 0; i < ARRAY_SIZE(csis->events); i++) {
 			struct mipi_csis_event *event = &csis->events[i];
 
+			if (event->channel >= csis->num_channels)
+				continue;
+
 			if ((!event->debug && (status & event->mask)) ||
 			    (event->debug && (dbg_status & event->mask)))
 				event->counter++;
 		}
 	}
 
-	if (status & MIPI_CSIS_INT_SRC_FRAME_START)
+	if (status & MIPI_CSIS_INT_SRC_FRAME_START(0))
 		mipi_csis_queue_event_sof(csis);
 
 	spin_unlock_irqrestore(&csis->slock, flags);
@@ -852,7 +894,7 @@ static void mipi_csis_clear_counters(struct mipi_csis_device *csis)
 static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_errors)
 {
 	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
-				: MIPI_CSIS_NUM_EVENTS - 8;
+				: MIPI_CSIS_NUM_ERROR_EVENTS;
 	unsigned int counters[MIPI_CSIS_NUM_EVENTS];
 	unsigned long flags;
 	unsigned int i;
@@ -863,45 +905,67 @@ static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_error
 	spin_unlock_irqrestore(&csis->slock, flags);
 
 	for (i = 0; i < num_events; ++i) {
+		const struct mipi_csis_event *event = &csis->events[i];
+
+		if (event->channel >= csis->num_channels)
+			continue;
+
 		if (counters[i] > 0 || csis->debug.enable)
 			dev_info(csis->dev, "%s events: %d\n",
-				 csis->events[i].name,
-				 counters[i]);
+				 event->name, counters[i]);
 	}
 }
 
+struct mipi_csis_reg_info {
+	u32 addr;
+	unsigned int offset;
+	const char * const name;
+};
+
+static void mipi_csis_dump_channel_reg(struct mipi_csis_device *csis,
+				       const struct mipi_csis_reg_info *reg,
+				       unsigned int channel)
+{
+	dev_info(csis->dev, "%16s%u: 0x%08x\n", reg->name, channel,
+		 mipi_csis_read(csis, reg->addr + channel * reg->offset));
+}
+
 static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
 {
-	static const struct {
-		u32 offset;
-		const char * const name;
-	} registers[] = {
-		{ MIPI_CSIS_CMN_CTRL, "CMN_CTRL" },
-		{ MIPI_CSIS_CLK_CTRL, "CLK_CTRL" },
-		{ MIPI_CSIS_INT_MSK, "INT_MSK" },
-		{ MIPI_CSIS_DPHY_STATUS, "DPHY_STATUS" },
-		{ MIPI_CSIS_DPHY_CMN_CTRL, "DPHY_CMN_CTRL" },
-		{ MIPI_CSIS_DPHY_SCTRL_L, "DPHY_SCTRL_L" },
-		{ MIPI_CSIS_DPHY_SCTRL_H, "DPHY_SCTRL_H" },
-		{ MIPI_CSIS_ISP_CONFIG_CH(0), "ISP_CONFIG_CH0" },
-		{ MIPI_CSIS_ISP_RESOL_CH(0), "ISP_RESOL_CH0" },
-		{ MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" },
-		{ MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" },
-		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
-		{ MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" },
+	static const struct mipi_csis_reg_info common_registers[] = {
+		{ MIPI_CSIS_CMN_CTRL, 0, "CMN_CTRL" },
+		{ MIPI_CSIS_CLK_CTRL, 0, "CLK_CTRL" },
+		{ MIPI_CSIS_INT_MSK, 0, "INT_MSK" },
+		{ MIPI_CSIS_DPHY_STATUS, 0, "DPHY_STATUS" },
+		{ MIPI_CSIS_DPHY_CMN_CTRL, 0, "DPHY_CMN_CTRL" },
+		{ MIPI_CSIS_DPHY_SCTRL_L, 0, "DPHY_SCTRL_L" },
+		{ MIPI_CSIS_DPHY_SCTRL_H, 0, "DPHY_SCTRL_H" },
+		{ MIPI_CSIS_DBG_CTRL, 0, "DBG_CTRL" },
+	};
+	static const struct mipi_csis_reg_info channel_registers[] = {
+		{ MIPI_CSIS_ISP_CONFIG_CH(0), 0x10, "ISP_CONFIG_CH" },
+		{ MIPI_CSIS_ISP_RESOL_CH(0), 0x10, "ISP_RESOL_CH" },
+		{ MIPI_CSIS_SDW_CONFIG_CH(0), 0x10, "SDW_CONFIG_CH" },
+		{ MIPI_CSIS_SDW_RESOL_CH(0), 0x10, "SDW_RESOL_CH" },
+		{ MIPI_CSIS_FRAME_COUNTER_CH(0), 4, "FRAME_COUNTER_CH" },
 	};
-
-	unsigned int i;
-	u32 cfg;
 
 	if (!pm_runtime_get_if_in_use(csis->dev))
 		return 0;
 
 	dev_info(csis->dev, "--- REGISTERS ---\n");
 
-	for (i = 0; i < ARRAY_SIZE(registers); i++) {
-		cfg = mipi_csis_read(csis, registers[i].offset);
-		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
+	for (unsigned int i = 0; i < ARRAY_SIZE(common_registers); i++) {
+		const struct mipi_csis_reg_info *reg = &common_registers[i];
+
+		dev_info(csis->dev, "%17s: 0x%08x\n", reg->name,
+			 mipi_csis_read(csis, reg->addr));
+	}
+
+	for (unsigned int chan = 0; chan < csis->num_channels; chan++) {
+		for (unsigned int i = 0; i < ARRAY_SIZE(channel_registers); ++i)
+			mipi_csis_dump_channel_reg(csis, &channel_registers[i],
+						   chan);
 	}
 
 	pm_runtime_put(csis->dev);
@@ -1424,6 +1488,12 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
 
 	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
 
+	csis->num_channels = 1;
+	of_property_read_u32(node, "fsl,num-channels", &csis->num_channels);
+	if (csis->num_channels < 1 || csis->num_channels > MIPI_CSIS_MAX_CHANNELS)
+		return dev_err_probe(csis->dev, -EINVAL,
+				     "Invalid fsl,num-channels value\n");
+
 	return 0;
 }
 
@@ -1447,10 +1517,8 @@ static int mipi_csis_probe(struct platform_device *pdev)
 
 	/* Parse DT properties. */
 	ret = mipi_csis_parse_dt(csis);
-	if (ret < 0) {
-		dev_err(dev, "Failed to parse device tree: %d\n", ret);
+	if (ret < 0)
 		return ret;
-	}
 
 	/* Acquire resources. */
 	csis->regs = devm_platform_ioremap_resource(pdev, 0);
-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Frank Li]

On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> The CSI-2 receiver can be instantiated with up to four output channels.
> This is an integration-specific property, specify the number of
> instantiated channels through a new fsl,num-channels property. The
> property is optional, and defaults to 1 as only one channel is currently
> supported by drivers.
>
> The only known SoC to have more than one channel is the i.MX8MP. As the
> binding examples do not cover that SoC, don't update them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> index db4889bf881e..41ad5b84eaeb 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> @@ -68,6 +68,13 @@ properties:
>      default: 166000000
>      deprecated: true
>
> +  fsl,num-channels:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of output channels
> +    minimum: 1
> +    maximum: 4
> +    default: 1
> +

Look like it is fixed value for each compabiable string, So it is not
suitable for adding new property. It should be in driver data of each
compatible strings.

I met similar case before. DT team generally don't agree on add such
property, unless there are two instances in the same chip, which have
difference channel number.

Frank

>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 5/8] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated

[Frank Li]

On Mon, Jun 09, 2025 at 02:58:37AM +0300, Laurent Pinchart wrote:
> Usage of the clock-frequency property, which is already optional, is
> discouraged in favour of using assigned-clock-rates (and
> assigned-clock-parents where needed). Mark the property as deprecated,
> and update the examples accordingly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml  | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> index 03a23a26c4f3..db4889bf881e 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> @@ -66,6 +66,7 @@ properties:
>    clock-frequency:
>      description: The desired external clock ("wrap") frequency, in Hz
>      default: 166000000
> +    deprecated: true
>
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -147,7 +148,9 @@ examples:
>                   <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
>                   <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
>          clock-names = "pclk", "wrap", "phy";
> -        clock-frequency = <166000000>;
> +
> +        assigned-clocks = <&clks IMX7D_MIPI_CSI_ROOT_CLK>;
> +        assigned-clock-rates = <166000000>;
>
>          power-domains = <&pgc_mipi_phy>;
>          phy-supply = <&reg_1p0d>;
> @@ -185,12 +188,16 @@ examples:
>          compatible = "fsl,imx8mm-mipi-csi2";
>          reg = <0x32e30000 0x1000>;
>          interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> -        clock-frequency = <333000000>;
> +
>          clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
>                   <&clk IMX8MM_CLK_CSI1_ROOT>,
>                   <&clk IMX8MM_CLK_CSI1_PHY_REF>,
>                   <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
>          clock-names = "pclk", "wrap", "phy", "axi";
> +
> +        assigned-clocks = <&clk IMX8MM_CLK_CSI1_ROOT>;
> +        assigned-clock-rates = <250000000>;
> +
>          power-domains = <&mipi_pd>;
>
>          ports {
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 8/8] media: imx-mipi-csis: Initial support for multiple output channels

[Frank Li]

On Mon, Jun 09, 2025 at 02:58:40AM +0300, Laurent Pinchart wrote:
> Some CSIS instances feature more than one output channel. Parse the
> number of channels from the device tree, and update register dumps and
> event counters accordingly. Support for routing virtual channels and
> data types to output channels through the subdev internal routing API
> will come later.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 224 ++++++++++++++-------
>  1 file changed, 146 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 080e40837463..4cc358d93187 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -98,12 +98,12 @@
>  #define MIPI_CSIS_INT_SRC_ODD_AFTER		BIT(28)
>  #define MIPI_CSIS_INT_SRC_ODD			(0x3 << 28)
>  #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
> -#define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
> -#define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
> +#define MIPI_CSIS_INT_SRC_FRAME_START(n)	BIT((n) + 24)
> +#define MIPI_CSIS_INT_SRC_FRAME_END(n)		BIT((n) + 20)
>  #define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
> -#define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
> -#define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
> -#define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
> +#define MIPI_CSIS_INT_SRC_ERR_LOST_FS(n)	BIT((n) + 12)
> +#define MIPI_CSIS_INT_SRC_ERR_LOST_FE(n)	BIT((n) + 8)
> +#define MIPI_CSIS_INT_SRC_ERR_OVER(n)		BIT((n) + 4)
>  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
>  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
>  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> @@ -205,23 +205,23 @@
>  /* Debug control register */
>  #define MIPI_CSIS_DBG_CTRL			0xc0
>  #define MIPI_CSIS_DBG_INTR_MSK			0xc4
> -#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT	BIT(25)
> -#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE	BIT(24)
> -#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE	BIT(20)
> -#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME	BIT(16)
> -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE		BIT(12)
> -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS		BIT(8)
> -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
> -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)
> +#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
> +#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)

not sure why cause above two line changes.

> +#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> +#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME(n)	BIT((n) + 16)
> +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE(n)		BIT((n) + 12)
> +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS(n)		BIT((n) + 8)
> +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE(n)	BIT((n) + 0)
>  #define MIPI_CSIS_DBG_INTR_SRC			0xc8
> -#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT	BIT(25)
> -#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE	BIT(24)
> -#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE	BIT(20)
> -#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME	BIT(16)
> -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE		BIT(12)
> -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS		BIT(8)
> -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
> -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
> +#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT		BIT(25)
> +#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE		BIT(24)

the same here.

> +#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> +#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(n)	BIT((n) + 16)
> +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(n)		BIT((n) + 12)
> +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(n)		BIT((n) + 8)
> +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(n)	BIT((n) + 0)
>
>  #define MIPI_CSIS_FRAME_COUNTER_CH(n)		(0x0100 + (n) * 4)
>
> @@ -230,8 +230,11 @@
>  #define MIPI_CSIS_PKTDATA_EVEN			0x3000
>  #define MIPI_CSIS_PKTDATA_SIZE			SZ_4K
>
> +#define MIPI_CSIS_MAX_CHANNELS			4
> +
>  struct mipi_csis_event {
>  	bool debug;
> +	unsigned int channel;
>  	u32 mask;
>  	const char * const name;
>  	unsigned int counter;
> @@ -239,36 +242,70 @@ struct mipi_csis_event {
>
>  static const struct mipi_csis_event mipi_csis_events[] = {
>  	/* Errors */
> -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),	"SOT 0 Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),	"SOT 1 Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),	"SOT 2 Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),	"SOT 3 Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FS,		"Lost Frame Start Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FE,		"Lost Frame End Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME,	"Truncated Frame" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE,	"Early Frame End" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS,	"Early Frame Start" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),		"SOT 0 Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),		"SOT 1 Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),		"SOT 2 Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),		"SOT 3 Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS(0),		"Lost Frame Start Error 0" },
> +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FS(1),		"Lost Frame Start Error 1" },
> +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FS(2),		"Lost Frame Start Error 2" },
> +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FS(3),		"Lost Frame Start Error 3" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE(0),		"Lost Frame End Error 0" },
> +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FE(1),		"Lost Frame End Error 1" },
> +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FE(2),		"Lost Frame End Error 2" },
> +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FE(3),		"Lost Frame End Error 3" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_OVER(0),		"FIFO Overflow Error 0" },
> +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_OVER(1),		"FIFO Overflow Error 1" },
> +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_OVER(2),		"FIFO Overflow Error 2" },
> +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_OVER(3),		"FIFO Overflow Error 3" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,		"Wrong Configuration Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ECC,			"ECC Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_CRC,			"CRC Error" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ID,			"Unknown ID Error" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,		"Data Type Ignored" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(0),	"Frame Size Error 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(1),	"Frame Size Error 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(2),	"Frame Size Error 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(3),	"Frame Size Error 3" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(0),	"Truncated Frame 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(1),	"Truncated Frame 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(2),	"Truncated Frame 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(3),	"Truncated Frame 3" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(0),		"Early Frame End 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(1),		"Early Frame End 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(2),		"Early Frame End 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(3),		"Early Frame End 3" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(0),		"Early Frame Start 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(1),		"Early Frame Start 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(2),		"Early Frame Start 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(3),		"Early Frame Start 3" },
>  	/* Non-image data receive events */
> -	{ false, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
> -	{ false, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
> -	{ false, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
> -	{ false, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
>  	/* Frame start/end */
> -	{ false, MIPI_CSIS_INT_SRC_FRAME_START,		"Frame Start" },
> -	{ false, MIPI_CSIS_INT_SRC_FRAME_END,		"Frame End" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL,	"VSYNC Falling Edge" },
> -	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE,	"VSYNC Rising Edge" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_START(0),		"Frame Start 0" },
> +	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_START(1),		"Frame Start 1" },
> +	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_START(2),		"Frame Start 2" },
> +	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_START(3),		"Frame Start 3" },
> +	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_END(0),		"Frame End 0" },
> +	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_END(1),		"Frame End 1" },
> +	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_END(2),		"Frame End 2" },
> +	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_END(3),		"Frame End 3" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(0),	"VSYNC Falling Edge 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(1),	"VSYNC Falling Edge 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(2),	"VSYNC Falling Edge 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(3),	"VSYNC Falling Edge 3" },
> +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(0),	"VSYNC Rising Edge 0" },
> +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(1),	"VSYNC Rising Edge 1" },
> +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(2),	"VSYNC Rising Edge 2" },
> +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(3),	"VSYNC Rising Edge 3" },
>  };
>
> -#define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events)
> +#define MIPI_CSIS_NUM_EVENTS		ARRAY_SIZE(mipi_csis_events)
> +#define MIPI_CSIS_NUM_ERROR_EVENTS	(MIPI_CSIS_NUM_EVENTS - 20)
>
>  enum mipi_csis_clk {
>  	MIPI_CSIS_CLK_PCLK,
> @@ -300,7 +337,9 @@ struct mipi_csis_device {
>  	struct clk_bulk_data *clks;
>  	struct reset_control *mrst;
>  	struct regulator *mipi_phy_regulator;
> +
>  	const struct mipi_csis_info *info;
> +	unsigned int num_channels;
>
>  	struct v4l2_subdev sd;
>  	struct media_pad pads[CSIS_PADS_NUM];
> @@ -766,16 +805,19 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
>
>  	/* Update the event/error counters */
>  	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) {
> -		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
> +		for (i = 0; i < ARRAY_SIZE(csis->events); i++) {
>  			struct mipi_csis_event *event = &csis->events[i];
>
> +			if (event->channel >= csis->num_channels)
> +				continue;
> +
>  			if ((!event->debug && (status & event->mask)) ||
>  			    (event->debug && (dbg_status & event->mask)))
>  				event->counter++;
>  		}
>  	}
>
> -	if (status & MIPI_CSIS_INT_SRC_FRAME_START)
> +	if (status & MIPI_CSIS_INT_SRC_FRAME_START(0))
>  		mipi_csis_queue_event_sof(csis);
>
>  	spin_unlock_irqrestore(&csis->slock, flags);
> @@ -852,7 +894,7 @@ static void mipi_csis_clear_counters(struct mipi_csis_device *csis)
>  static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_errors)
>  {
>  	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
> -				: MIPI_CSIS_NUM_EVENTS - 8;
> +				: MIPI_CSIS_NUM_ERROR_EVENTS;
>  	unsigned int counters[MIPI_CSIS_NUM_EVENTS];
>  	unsigned long flags;
>  	unsigned int i;
> @@ -863,45 +905,67 @@ static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_error
>  	spin_unlock_irqrestore(&csis->slock, flags);
>
>  	for (i = 0; i < num_events; ++i) {
> +		const struct mipi_csis_event *event = &csis->events[i];
> +
> +		if (event->channel >= csis->num_channels)
> +			continue;
> +
>  		if (counters[i] > 0 || csis->debug.enable)
>  			dev_info(csis->dev, "%s events: %d\n",
> -				 csis->events[i].name,
> -				 counters[i]);
> +				 event->name, counters[i]);
>  	}
>  }
>
> +struct mipi_csis_reg_info {
> +	u32 addr;
> +	unsigned int offset;
> +	const char * const name;
> +};
> +
> +static void mipi_csis_dump_channel_reg(struct mipi_csis_device *csis,
> +				       const struct mipi_csis_reg_info *reg,
> +				       unsigned int channel)
> +{
> +	dev_info(csis->dev, "%16s%u: 0x%08x\n", reg->name, channel,
> +		 mipi_csis_read(csis, reg->addr + channel * reg->offset));
> +}
> +
>  static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
>  {
> -	static const struct {
> -		u32 offset;
> -		const char * const name;
> -	} registers[] = {
> -		{ MIPI_CSIS_CMN_CTRL, "CMN_CTRL" },
> -		{ MIPI_CSIS_CLK_CTRL, "CLK_CTRL" },
> -		{ MIPI_CSIS_INT_MSK, "INT_MSK" },
> -		{ MIPI_CSIS_DPHY_STATUS, "DPHY_STATUS" },
> -		{ MIPI_CSIS_DPHY_CMN_CTRL, "DPHY_CMN_CTRL" },
> -		{ MIPI_CSIS_DPHY_SCTRL_L, "DPHY_SCTRL_L" },
> -		{ MIPI_CSIS_DPHY_SCTRL_H, "DPHY_SCTRL_H" },
> -		{ MIPI_CSIS_ISP_CONFIG_CH(0), "ISP_CONFIG_CH0" },
> -		{ MIPI_CSIS_ISP_RESOL_CH(0), "ISP_RESOL_CH0" },
> -		{ MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" },
> -		{ MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" },
> -		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
> -		{ MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" },
> +	static const struct mipi_csis_reg_info common_registers[] = {
> +		{ MIPI_CSIS_CMN_CTRL, 0, "CMN_CTRL" },
> +		{ MIPI_CSIS_CLK_CTRL, 0, "CLK_CTRL" },
> +		{ MIPI_CSIS_INT_MSK, 0, "INT_MSK" },
> +		{ MIPI_CSIS_DPHY_STATUS, 0, "DPHY_STATUS" },
> +		{ MIPI_CSIS_DPHY_CMN_CTRL, 0, "DPHY_CMN_CTRL" },
> +		{ MIPI_CSIS_DPHY_SCTRL_L, 0, "DPHY_SCTRL_L" },
> +		{ MIPI_CSIS_DPHY_SCTRL_H, 0, "DPHY_SCTRL_H" },
> +		{ MIPI_CSIS_DBG_CTRL, 0, "DBG_CTRL" },
> +	};
> +	static const struct mipi_csis_reg_info channel_registers[] = {
> +		{ MIPI_CSIS_ISP_CONFIG_CH(0), 0x10, "ISP_CONFIG_CH" },
> +		{ MIPI_CSIS_ISP_RESOL_CH(0), 0x10, "ISP_RESOL_CH" },
> +		{ MIPI_CSIS_SDW_CONFIG_CH(0), 0x10, "SDW_CONFIG_CH" },
> +		{ MIPI_CSIS_SDW_RESOL_CH(0), 0x10, "SDW_RESOL_CH" },
> +		{ MIPI_CSIS_FRAME_COUNTER_CH(0), 4, "FRAME_COUNTER_CH" },
>  	};
> -
> -	unsigned int i;
> -	u32 cfg;
>
>  	if (!pm_runtime_get_if_in_use(csis->dev))
>  		return 0;
>
>  	dev_info(csis->dev, "--- REGISTERS ---\n");
>
> -	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> -		cfg = mipi_csis_read(csis, registers[i].offset);
> -		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
> +	for (unsigned int i = 0; i < ARRAY_SIZE(common_registers); i++) {
> +		const struct mipi_csis_reg_info *reg = &common_registers[i];
> +
> +		dev_info(csis->dev, "%17s: 0x%08x\n", reg->name,
> +			 mipi_csis_read(csis, reg->addr));
> +	}
> +
> +	for (unsigned int chan = 0; chan < csis->num_channels; chan++) {
> +		for (unsigned int i = 0; i < ARRAY_SIZE(channel_registers); ++i)
> +			mipi_csis_dump_channel_reg(csis, &channel_registers[i],
> +						   chan);
>  	}
>
>  	pm_runtime_put(csis->dev);
> @@ -1424,6 +1488,12 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
>
>  	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
>
> +	csis->num_channels = 1;
> +	of_property_read_u32(node, "fsl,num-channels", &csis->num_channels);

Wait for dt team comments, most likely they do not agree add fsl,num-channels.

Frank

> +	if (csis->num_channels < 1 || csis->num_channels > MIPI_CSIS_MAX_CHANNELS)
> +		return dev_err_probe(csis->dev, -EINVAL,
> +				     "Invalid fsl,num-channels value\n");
> +
>  	return 0;
>  }
>
> @@ -1447,10 +1517,8 @@ static int mipi_csis_probe(struct platform_device *pdev)
>
>  	/* Parse DT properties. */
>  	ret = mipi_csis_parse_dt(csis);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to parse device tree: %d\n", ret);
> +	if (ret < 0)
>  		return ret;
> -	}
>
>  	/* Acquire resources. */
>  	csis->regs = devm_platform_ioremap_resource(pdev, 0);
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Adam Ford]

On Mon, Jun 9, 2025 at 10:32 AM Frank Li <Frank.li@nxp.com> wrote:
>
> On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > The CSI-2 receiver can be instantiated with up to four output channels.
> > This is an integration-specific property, specify the number of
> > instantiated channels through a new fsl,num-channels property. The
> > property is optional, and defaults to 1 as only one channel is currently
> > supported by drivers.
> >
> > The only known SoC to have more than one channel is the i.MX8MP. As the
> > binding examples do not cover that SoC, don't update them.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > index db4889bf881e..41ad5b84eaeb 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > @@ -68,6 +68,13 @@ properties:
> >      default: 166000000
> >      deprecated: true
> >
> > +  fsl,num-channels:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Number of output channels
> > +    minimum: 1
> > +    maximum: 4
> > +    default: 1
> > +
>
> Look like it is fixed value for each compabiable string, So it is not
> suitable for adding new property. It should be in driver data of each
> compatible strings.
>
> I met similar case before. DT team generally don't agree on add such
> property, unless there are two instances in the same chip, which have
> difference channel number.

If the DT changes are rejected, can the number of channels be added to
the data structure inside mipi_csis_of_match?  We have compatibles for
8mm and imx7.  If we add an imx8mp compatible we could add a reference
to the number of channels.

adam
>
> Frank
>
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
>

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Frank Li]

On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> On Mon, Jun 9, 2025 at 10:32 AM Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > This is an integration-specific property, specify the number of
> > > instantiated channels through a new fsl,num-channels property. The
> > > property is optional, and defaults to 1 as only one channel is currently
> > > supported by drivers.
> > >
> > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > binding examples do not cover that SoC, don't update them.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > index db4889bf881e..41ad5b84eaeb 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > @@ -68,6 +68,13 @@ properties:
> > >      default: 166000000
> > >      deprecated: true
> > >
> > > +  fsl,num-channels:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Number of output channels
> > > +    minimum: 1
> > > +    maximum: 4
> > > +    default: 1
> > > +
> >
> > Look like it is fixed value for each compabiable string, So it is not
> > suitable for adding new property. It should be in driver data of each
> > compatible strings.
> >
> > I met similar case before. DT team generally don't agree on add such
> > property, unless there are two instances in the same chip, which have
> > difference channel number.
>
> If the DT changes are rejected, can the number of channels be added to
> the data structure inside mipi_csis_of_match?  We have compatibles for
> 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> to the number of channels.

Yes, that's prefer method.

Frank

>
> adam
> >
> > Frank
> >
> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
> >

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Laurent Pinchart]

On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > This is an integration-specific property, specify the number of
> > > instantiated channels through a new fsl,num-channels property. The
> > > property is optional, and defaults to 1 as only one channel is currently
> > > supported by drivers.
> > >
> > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > binding examples do not cover that SoC, don't update them.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > index db4889bf881e..41ad5b84eaeb 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > @@ -68,6 +68,13 @@ properties:
> > >      default: 166000000
> > >      deprecated: true
> > >
> > > +  fsl,num-channels:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Number of output channels
> > > +    minimum: 1
> > > +    maximum: 4
> > > +    default: 1
> > > +
> >
> > Look like it is fixed value for each compabiable string, So it is not
> > suitable for adding new property. It should be in driver data of each
> > compatible strings.
> >
> > I met similar case before. DT team generally don't agree on add such
> > property, unless there are two instances in the same chip, which have
> > difference channel number.
> 
> If the DT changes are rejected, can the number of channels be added to
> the data structure inside mipi_csis_of_match?  We have compatibles for
> 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> to the number of channels.

I thought about it, and decided to add a new property because the number
of channels is really a synthesis time configuration parameter, and
could differ between different CSIS instances in the same SoC.

> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Frank Li]

On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > This is an integration-specific property, specify the number of
> > > > instantiated channels through a new fsl,num-channels property. The
> > > > property is optional, and defaults to 1 as only one channel is currently
> > > > supported by drivers.
> > > >
> > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > binding examples do not cover that SoC, don't update them.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > @@ -68,6 +68,13 @@ properties:
> > > >      default: 166000000
> > > >      deprecated: true
> > > >
> > > > +  fsl,num-channels:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: Number of output channels
> > > > +    minimum: 1
> > > > +    maximum: 4
> > > > +    default: 1
> > > > +
> > >
> > > Look like it is fixed value for each compabiable string, So it is not
> > > suitable for adding new property. It should be in driver data of each
> > > compatible strings.
> > >
> > > I met similar case before. DT team generally don't agree on add such
> > > property, unless there are two instances in the same chip, which have
> > > difference channel number.
> >
> > If the DT changes are rejected, can the number of channels be added to
> > the data structure inside mipi_csis_of_match?  We have compatibles for
> > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > to the number of channels.
>
> I thought about it, and decided to add a new property because the number
> of channels is really a synthesis time configuration parameter, and
> could differ between different CSIS instances in the same SoC.

Need add such information at binding doc's commit message, ideally provide
an example for it.

Frank
>
> > > >    ports:
> > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > >
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH 2/8] media: imx-mipi-csis: Fix field alignment in register dump

[Alexander Stein]

Hi Laurent,

thanks for the patch.

Am Montag, 9. Juni 2025, 01:58:34 CEST schrieb Laurent Pinchart:
> Commit 95a1379004cb ("media: staging: media: imx: imx7-mipi-csis: Dump
> MIPI_CSIS_FRAME_COUNTER_CH0 register") forgot to increase the maximum
> register name length, resulting in misalignment of names printed in the
> kernel log. Fix it.

Does this warrant a Fixes tag? Anyway
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index d59666ef7545..b652d59851c2 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -895,7 +895,7 @@ static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
>  
>  	for (i = 0; i < ARRAY_SIZE(registers); i++) {
>  		cfg = mipi_csis_read(csis, registers[i].offset);
> -		dev_info(csis->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
> +		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
>  	}
>  
>  	pm_runtime_put(csis->dev);
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 3/8] media: imx-mipi-csis: Log per-lane start of transmission errors

[Alexander Stein]

Hi Laurent,

thanks for the patch.

Am Montag, 9. Juni 2025, 01:58:35 CEST schrieb Laurent Pinchart:
> The CSIS has per-line start of transmission error interrupts. Log them
> all, instead of only the first data lane.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index b652d59851c2..e27467e6372f 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -100,7 +100,7 @@
>  #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
>  #define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
>  #define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
> -#define MIPI_CSIS_INT_SRC_ERR_SOT_HS		BIT(16)
> +#define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
>  #define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
>  #define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
>  #define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
> @@ -241,7 +241,10 @@ struct mipi_csis_event {
>  
>  static const struct mipi_csis_event mipi_csis_events[] = {
>  	/* Errors */
> -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS,		"SOT Error" },
> +	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),	"SOT 0 Error" },
> +	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),	"SOT 1 Error" },
> +	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),	"SOT 2 Error" },
> +	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),	"SOT 3 Error" },
>  	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FS,		"Lost Frame Start Error" },
>  	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FE,		"Lost Frame End Error" },
>  	{ false, MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 2/8] media: imx-mipi-csis: Fix field alignment in register dump

[Laurent Pinchart]

On Tue, Jun 10, 2025 at 09:12:26AM +0200, Alexander Stein wrote:
> Am Montag, 9. Juni 2025, 01:58:34 CEST schrieb Laurent Pinchart:
> > Commit 95a1379004cb ("media: staging: media: imx: imx7-mipi-csis: Dump
> > MIPI_CSIS_FRAME_COUNTER_CH0 register") forgot to increase the maximum
> > register name length, resulting in misalignment of names printed in the
> > kernel log. Fix it.
> 
> Does this warrant a Fixes tag? Anyway

I would then have to Cc stable as per the media subsystem rules, and I
really don't think this patch warrants being backported to stable
kernels.

> Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index d59666ef7545..b652d59851c2 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -895,7 +895,7 @@ static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
> >  
> >  	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> >  		cfg = mipi_csis_read(csis, registers[i].offset);
> > -		dev_info(csis->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
> > +		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
> >  	}
> >  
> >  	pm_runtime_put(csis->dev);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Laurent Pinchart]

Hi Frank,

On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > This is an integration-specific property, specify the number of
> > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > supported by drivers.
> > > > >
> > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > binding examples do not cover that SoC, don't update them.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > ---
> > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > @@ -68,6 +68,13 @@ properties:
> > > > >      default: 166000000
> > > > >      deprecated: true
> > > > >
> > > > > +  fsl,num-channels:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    description: Number of output channels
> > > > > +    minimum: 1
> > > > > +    maximum: 4
> > > > > +    default: 1
> > > > > +
> > > >
> > > > Look like it is fixed value for each compabiable string, So it is not
> > > > suitable for adding new property. It should be in driver data of each
> > > > compatible strings.
> > > >
> > > > I met similar case before. DT team generally don't agree on add such
> > > > property, unless there are two instances in the same chip, which have
> > > > difference channel number.
> > >
> > > If the DT changes are rejected, can the number of channels be added to
> > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > to the number of channels.
> >
> > I thought about it, and decided to add a new property because the number
> > of channels is really a synthesis time configuration parameter, and
> > could differ between different CSIS instances in the same SoC.
> 
> Need add such information at binding doc's commit message,

I'll update the commit message.

> ideally provide an example for it.

That I can't provide because the few SoCs I'm working with do not
integrate multiple CSIS instances with different parameters.

> > > > >    ports:
> > > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > > >

-- 
Regards,

Laurent Pinchart

Re: [PATCH 8/8] media: imx-mipi-csis: Initial support for multiple output channels

[Alexander Stein]

Hi Laurent,

thanks for the patch.

Am Montag, 9. Juni 2025, 01:58:40 CEST schrieb Laurent Pinchart:
> Some CSIS instances feature more than one output channel. Parse the
> number of channels from the device tree, and update register dumps and
> event counters accordingly. Support for routing virtual channels and
> data types to output channels through the subdev internal routing API
> will come later.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 224 ++++++++++++++-------
>  1 file changed, 146 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 080e40837463..4cc358d93187 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -98,12 +98,12 @@
>  #define MIPI_CSIS_INT_SRC_ODD_AFTER		BIT(28)
>  #define MIPI_CSIS_INT_SRC_ODD			(0x3 << 28)
>  #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)

As a side note: I just noticed Bits 28-31 are only defined on i.MX7. They
are reserved on i.MX8M (Mini, Nano, Plus).

> -#define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
> -#define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
> +#define MIPI_CSIS_INT_SRC_FRAME_START(n)	BIT((n) + 24)
> +#define MIPI_CSIS_INT_SRC_FRAME_END(n)		BIT((n) + 20)
>  #define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
> -#define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
> -#define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
> -#define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
> +#define MIPI_CSIS_INT_SRC_ERR_LOST_FS(n)	BIT((n) + 12)
> +#define MIPI_CSIS_INT_SRC_ERR_LOST_FE(n)	BIT((n) + 8)
> +#define MIPI_CSIS_INT_SRC_ERR_OVER(n)		BIT((n) + 4)

Similar here. Only i.MX7 has the bits for CH1, CH2 and CH3 defined.

>  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
>  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
>  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> @@ -205,23 +205,23 @@
>  /* Debug control register */
>  #define MIPI_CSIS_DBG_CTRL			0xc0
>  #define MIPI_CSIS_DBG_INTR_MSK			0xc4
> -#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT	BIT(25)
> -#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE	BIT(24)
> -#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE	BIT(20)
> -#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME	BIT(16)
> -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE		BIT(12)
> -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS		BIT(8)
> -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
> -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)
> +#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
> +#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)
> +#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> +#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME(n)	BIT((n) + 16)
> +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE(n)		BIT((n) + 12)
> +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS(n)		BIT((n) + 8)
> +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE(n)	BIT((n) + 0)
>  #define MIPI_CSIS_DBG_INTR_SRC			0xc8
> -#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT	BIT(25)
> -#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE	BIT(24)
> -#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE	BIT(20)
> -#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME	BIT(16)
> -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE		BIT(12)
> -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS		BIT(8)
> -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
> -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
> +#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT		BIT(25)
> +#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE		BIT(24)
> +#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> +#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(n)	BIT((n) + 16)
> +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(n)		BIT((n) + 12)
> +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(n)		BIT((n) + 8)
> +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(n)	BIT((n) + 0)

Out of curiosity: Where do these bits come from? I can't find them in RM.

Best regards,
Alexander

> [snip]
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 1/8] media: imx-mipi-csis: Rename register macros to match reference manual

[Alexander Stein]

Hi Laurent,

thanks for the patch.

Am Montag, 9. Juni 2025, 01:58:33 CEST schrieb Laurent Pinchart:
> The CSIS driver uses register macro names that do not match the
> reference manual of the i.MX7[DS] and i.MX8M[MNP] SoCs in which the CSIS
> is integrated. Rename them to match the documentation, making the code
> easier to read alongside the reference manuals.
> 
> One of the misnamed register fields is MIPI_CSIS_INT_SRC_ERR_UNKNOWN,
> which led to the corresponding event being logged as "Unknown Error".
> The correct register field name is MIPI_CSIS_INT_SRC_ERR_ID, documented
> as "Unknown ID error". Update the event description accordingly.
> 
> While at it, also replace a few *_OFFSET macros with parametric macros
> for consistency, and add the missing MIPI_CSIS_ISP_RESOL_VRESOL and
> MIPI_CSIS_ISP_RESOL_HRESOL register field macros.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 69 +++++++++++-----------
>  1 file changed, 36 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 2beb5f43c2c0..d59666ef7545 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -55,13 +55,13 @@
>  /* CSIS common control */
>  #define MIPI_CSIS_CMN_CTRL			0x04
>  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
> -#define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
> +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE	(0 << 10)
> +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	(1 << 10)
> +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
> +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	(3 << 8)
>  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
> -#define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
> -#define MIPI_CSIS_CMN_CTRL_ENABLE		BIT(0)
> -
> -#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET	8
> -#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK		(3 << 8)
> +#define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
> +#define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
>  
>  /* CSIS clock control */
>  #define MIPI_CSIS_CLK_CTRL			0x08
> @@ -87,7 +87,7 @@
>  #define MIPI_CSIS_INT_MSK_ERR_WRONG_CFG		BIT(3)
>  #define MIPI_CSIS_INT_MSK_ERR_ECC		BIT(2)
>  #define MIPI_CSIS_INT_MSK_ERR_CRC		BIT(1)
> -#define MIPI_CSIS_INT_MSK_ERR_UNKNOWN		BIT(0)
> +#define MIPI_CSIS_INT_MSK_ERR_ID		BIT(0)
>  
>  /* CSIS Interrupt source */
>  #define MIPI_CSIS_INT_SRC			0x14
> @@ -107,7 +107,7 @@
>  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
>  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
>  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> -#define MIPI_CSIS_INT_SRC_ERR_UNKNOWN		BIT(0)
> +#define MIPI_CSIS_INT_SRC_ERR_ID		BIT(0)
>  #define MIPI_CSIS_INT_SRC_ERRORS		0xfffff
>  
>  /* D-PHY status control */
> @@ -123,8 +123,8 @@
>  #define MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE_MASK	GENMASK(31, 24)
>  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(n)	((n) << 22)
>  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE_MASK	GENMASK(23, 22)
> -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_CLK	BIT(6)
> -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_DAT	BIT(5)
> +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_CLK	BIT(6)
> +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_DAT	BIT(5)
>  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_DAT	BIT(1)
>  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_CLK	BIT(0)
>  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE		(0x1f << 0)
> @@ -179,21 +179,23 @@
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
> -#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
> -#define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
> -#define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
> -#define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
> +#define MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK	(3 << 12)
> +#define MIPI_CSIS_ISPCFG_PARALLEL		BIT(11)
> +#define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt)	((fmt) << 2)
> +#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	(0x3f << 2)
>  
>  /* ISP Image Resolution register */
>  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> +#define MIPI_CSIS_ISP_RESOL_VRESOL(n)		((n) << 16)
> +#define MIPI_CSIS_ISP_RESOL_HRESOL(n)		((n) << 0)
>  #define CSIS_MAX_PIX_WIDTH			0xffff
>  #define CSIS_MAX_PIX_HEIGHT			0xffff
>  
>  /* ISP SYNC register */
>  #define MIPI_CSIS_ISP_SYNC_CH(n)		(0x48 + (n) * 0x10)
> -#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET	18
> -#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET	12
> -#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET	0
> +#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(n)	((n) << 18)
> +#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(n)	((n) << 12)
> +#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(n)	((n) << 0)
>  
>  /* ISP shadow registers */
>  #define MIPI_CSIS_SDW_CONFIG_CH(n)		(0x80 + (n) * 0x10)
> @@ -246,7 +248,7 @@ static const struct mipi_csis_event mipi_csis_events[] = {
>  	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
>  	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
>  	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
> -	{ false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN,		"Unknown Error" },
> +	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
>  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
>  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
>  	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
> @@ -517,7 +519,7 @@ static void mipi_csis_sw_reset(struct mipi_csis_device *csis)
>  	u32 val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
>  
>  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> -			val | MIPI_CSIS_CMN_CTRL_RESET);
> +			val | MIPI_CSIS_CMN_CTRL_SW_RESET);
>  	usleep_range(10, 20);
>  }
>  
> @@ -527,9 +529,9 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
>  	if (on)
> -		val |= MIPI_CSIS_CMN_CTRL_ENABLE;
> +		val |= MIPI_CSIS_CMN_CTRL_CSI_EN;
>  	else
> -		val &= ~MIPI_CSIS_CMN_CTRL_ENABLE;
> +		val &= ~MIPI_CSIS_CMN_CTRL_CSI_EN;
>  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
> @@ -549,8 +551,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
>  
>  	/* Color format */
>  	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
> -	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
> -		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
> +	val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
> +		 MIPI_CSIS_ISPCFG_DATAFORMAT_MASK);
>  
>  	/*
>  	 * YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
> @@ -568,12 +570,13 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
>  	if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
>  		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
>  
> -	val |= MIPI_CSIS_ISPCFG_FMT(csis_fmt->data_type);
> +	val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
>  	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
>  
>  	/* Pixel resolution */
> -	val = format->width | (format->height << 16);
> -	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0), val);
> +	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
> +			MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
> +			MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
>  }
>  
>  static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> @@ -635,10 +638,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  	u32 val;
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> -	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK;
> -	val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET;
> +	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> +	val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
>  	if (csis->info->version == MIPI_CSIS_V3_3)
> -		val |= MIPI_CSIS_CMN_CTRL_INTER_MODE;
> +		val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;

Mh, what about i.MX8MP which also has these bitfield defined, but is
not a MIPI_CSIS_V3_3 core?

Best regards,
Alexander

>  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>  
>  	__mipi_csis_set_format(csis, format, csis_fmt);
> @@ -647,10 +650,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
>  			MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
>  
> -	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
> -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
> -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET);
> -	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0), val);
> +	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
> +			MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> +			MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
> +			MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
>  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 1/8] media: imx-mipi-csis: Rename register macros to match reference manual

[Laurent Pinchart]

On Tue, Jun 10, 2025 at 11:10:54AM +0200, Alexander Stein wrote:
> Am Montag, 9. Juni 2025, 01:58:33 CEST schrieb Laurent Pinchart:
> > The CSIS driver uses register macro names that do not match the
> > reference manual of the i.MX7[DS] and i.MX8M[MNP] SoCs in which the CSIS
> > is integrated. Rename them to match the documentation, making the code
> > easier to read alongside the reference manuals.
> > 
> > One of the misnamed register fields is MIPI_CSIS_INT_SRC_ERR_UNKNOWN,
> > which led to the corresponding event being logged as "Unknown Error".
> > The correct register field name is MIPI_CSIS_INT_SRC_ERR_ID, documented
> > as "Unknown ID error". Update the event description accordingly.
> > 
> > While at it, also replace a few *_OFFSET macros with parametric macros
> > for consistency, and add the missing MIPI_CSIS_ISP_RESOL_VRESOL and
> > MIPI_CSIS_ISP_RESOL_HRESOL register field macros.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 69 +++++++++++-----------
> >  1 file changed, 36 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 2beb5f43c2c0..d59666ef7545 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -55,13 +55,13 @@
> >  /* CSIS common control */
> >  #define MIPI_CSIS_CMN_CTRL			0x04
> >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
> > -#define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
> > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE	(0 << 10)
> > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	(1 << 10)
> > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
> > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	(3 << 8)
> >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
> > -#define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
> > -#define MIPI_CSIS_CMN_CTRL_ENABLE		BIT(0)
> > -
> > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET	8
> > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK		(3 << 8)
> > +#define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
> > +#define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
> >  
> >  /* CSIS clock control */
> >  #define MIPI_CSIS_CLK_CTRL			0x08
> > @@ -87,7 +87,7 @@
> >  #define MIPI_CSIS_INT_MSK_ERR_WRONG_CFG		BIT(3)
> >  #define MIPI_CSIS_INT_MSK_ERR_ECC		BIT(2)
> >  #define MIPI_CSIS_INT_MSK_ERR_CRC		BIT(1)
> > -#define MIPI_CSIS_INT_MSK_ERR_UNKNOWN		BIT(0)
> > +#define MIPI_CSIS_INT_MSK_ERR_ID		BIT(0)
> >  
> >  /* CSIS Interrupt source */
> >  #define MIPI_CSIS_INT_SRC			0x14
> > @@ -107,7 +107,7 @@
> >  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
> >  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
> >  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> > -#define MIPI_CSIS_INT_SRC_ERR_UNKNOWN		BIT(0)
> > +#define MIPI_CSIS_INT_SRC_ERR_ID		BIT(0)
> >  #define MIPI_CSIS_INT_SRC_ERRORS		0xfffff
> >  
> >  /* D-PHY status control */
> > @@ -123,8 +123,8 @@
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE_MASK	GENMASK(31, 24)
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(n)	((n) << 22)
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE_MASK	GENMASK(23, 22)
> > -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_CLK	BIT(6)
> > -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_DAT	BIT(5)
> > +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_CLK	BIT(6)
> > +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_DAT	BIT(5)
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_DAT	BIT(1)
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_CLK	BIT(0)
> >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE		(0x1f << 0)
> > @@ -179,21 +179,23 @@
> >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
> >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
> >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
> > -#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
> > -#define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
> > -#define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
> > -#define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
> > +#define MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK	(3 << 12)
> > +#define MIPI_CSIS_ISPCFG_PARALLEL		BIT(11)
> > +#define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt)	((fmt) << 2)
> > +#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	(0x3f << 2)
> >  
> >  /* ISP Image Resolution register */
> >  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> > +#define MIPI_CSIS_ISP_RESOL_VRESOL(n)		((n) << 16)
> > +#define MIPI_CSIS_ISP_RESOL_HRESOL(n)		((n) << 0)
> >  #define CSIS_MAX_PIX_WIDTH			0xffff
> >  #define CSIS_MAX_PIX_HEIGHT			0xffff
> >  
> >  /* ISP SYNC register */
> >  #define MIPI_CSIS_ISP_SYNC_CH(n)		(0x48 + (n) * 0x10)
> > -#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET	18
> > -#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET	12
> > -#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET	0
> > +#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(n)	((n) << 18)
> > +#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(n)	((n) << 12)
> > +#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(n)	((n) << 0)
> >  
> >  /* ISP shadow registers */
> >  #define MIPI_CSIS_SDW_CONFIG_CH(n)		(0x80 + (n) * 0x10)
> > @@ -246,7 +248,7 @@ static const struct mipi_csis_event mipi_csis_events[] = {
> >  	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
> >  	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
> >  	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN,		"Unknown Error" },
> > +	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
> >  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> >  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
> >  	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
> > @@ -517,7 +519,7 @@ static void mipi_csis_sw_reset(struct mipi_csis_device *csis)
> >  	u32 val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> >  
> >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > -			val | MIPI_CSIS_CMN_CTRL_RESET);
> > +			val | MIPI_CSIS_CMN_CTRL_SW_RESET);
> >  	usleep_range(10, 20);
> >  }
> >  
> > @@ -527,9 +529,9 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> >  	if (on)
> > -		val |= MIPI_CSIS_CMN_CTRL_ENABLE;
> > +		val |= MIPI_CSIS_CMN_CTRL_CSI_EN;
> >  	else
> > -		val &= ~MIPI_CSIS_CMN_CTRL_ENABLE;
> > +		val &= ~MIPI_CSIS_CMN_CTRL_CSI_EN;
> >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
> > @@ -549,8 +551,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> >  
> >  	/* Color format */
> >  	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
> > -	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
> > -		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
> > +	val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
> > +		 MIPI_CSIS_ISPCFG_DATAFORMAT_MASK);
> >  
> >  	/*
> >  	 * YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
> > @@ -568,12 +570,13 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> >  	if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
> >  		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> >  
> > -	val |= MIPI_CSIS_ISPCFG_FMT(csis_fmt->data_type);
> > +	val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
> >  	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
> >  
> >  	/* Pixel resolution */
> > -	val = format->width | (format->height << 16);
> > -	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0), val);
> > +	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
> > +			MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
> > +			MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
> >  }
> >  
> >  static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> > @@ -635,10 +638,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >  	u32 val;
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > -	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK;
> > -	val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET;
> > +	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> > +	val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
> >  	if (csis->info->version == MIPI_CSIS_V3_3)
> > -		val |= MIPI_CSIS_CMN_CTRL_INTER_MODE;
> > +		val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> 
> Mh, what about i.MX8MP which also has these bitfield defined, but is
> not a MIPI_CSIS_V3_3 core?

Short answer: no idea yet. Has anyone been able to capture embedded data
through the ISI on the i.MX8MP ?

> >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> >  
> >  	__mipi_csis_set_format(csis, format, csis_fmt);
> > @@ -647,10 +650,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >  			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
> >  			MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
> >  
> > -	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
> > -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
> > -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET);
> > -	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0), val);
> > +	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
> > +			MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> > +			MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
> > +			MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> >  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 8/8] media: imx-mipi-csis: Initial support for multiple output channels

[Laurent Pinchart]

On Tue, Jun 10, 2025 at 11:01:20AM +0200, Alexander Stein wrote:
> Am Montag, 9. Juni 2025, 01:58:40 CEST schrieb Laurent Pinchart:
> > Some CSIS instances feature more than one output channel. Parse the
> > number of channels from the device tree, and update register dumps and
> > event counters accordingly. Support for routing virtual channels and
> > data types to output channels through the subdev internal routing API
> > will come later.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 224 ++++++++++++++-------
> >  1 file changed, 146 insertions(+), 78 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 080e40837463..4cc358d93187 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -98,12 +98,12 @@
> >  #define MIPI_CSIS_INT_SRC_ODD_AFTER		BIT(28)
> >  #define MIPI_CSIS_INT_SRC_ODD			(0x3 << 28)
> >  #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
> 
> As a side note: I just noticed Bits 28-31 are only defined on i.MX7. They
> are reserved on i.MX8M (Mini, Nano, Plus).

They are many bit marked as reserved that are actually implemented. The
CSIS is a big pain point :-(

> > -#define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
> > -#define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
> > +#define MIPI_CSIS_INT_SRC_FRAME_START(n)	BIT((n) + 24)
> > +#define MIPI_CSIS_INT_SRC_FRAME_END(n)		BIT((n) + 20)
> >  #define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
> > -#define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
> > -#define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
> > -#define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
> > +#define MIPI_CSIS_INT_SRC_ERR_LOST_FS(n)	BIT((n) + 12)
> > +#define MIPI_CSIS_INT_SRC_ERR_LOST_FE(n)	BIT((n) + 8)
> > +#define MIPI_CSIS_INT_SRC_ERR_OVER(n)		BIT((n) + 4)
> 
> Similar here. Only i.MX7 has the bits for CH1, CH2 and CH3 defined.
> 
> >  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
> >  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
> >  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> > @@ -205,23 +205,23 @@
> >  /* Debug control register */
> >  #define MIPI_CSIS_DBG_CTRL			0xc0
> >  #define MIPI_CSIS_DBG_INTR_MSK			0xc4
> > -#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT	BIT(25)
> > -#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE	BIT(24)
> > -#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE	BIT(20)
> > -#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME	BIT(16)
> > -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE		BIT(12)
> > -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS		BIT(8)
> > -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
> > -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)
> > +#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
> > +#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)
> > +#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> > +#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME(n)	BIT((n) + 16)
> > +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE(n)		BIT((n) + 12)
> > +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS(n)		BIT((n) + 8)
> > +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> > +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE(n)	BIT((n) + 0)
> >  #define MIPI_CSIS_DBG_INTR_SRC			0xc8
> > -#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT	BIT(25)
> > -#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE	BIT(24)
> > -#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE	BIT(20)
> > -#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME	BIT(16)
> > -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE		BIT(12)
> > -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS		BIT(8)
> > -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
> > -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
> > +#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT		BIT(25)
> > +#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE		BIT(24)
> > +#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> > +#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(n)	BIT((n) + 16)
> > +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(n)		BIT((n) + 12)
> > +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(n)		BIT((n) + 8)
> > +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> > +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(n)	BIT((n) + 0)
> 
> Out of curiosity: Where do these bits come from? I can't find them in RM.

They are documented in the i.MX7D RM, and they appear to be implemented
on the i.MX8MP as the interrupts fire in the expected way.

> > [snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH 8/8] media: imx-mipi-csis: Initial support for multiple output channels

[Laurent Pinchart]

On Mon, Jun 09, 2025 at 11:39:53AM -0400, Frank Li wrote:
> On Mon, Jun 09, 2025 at 02:58:40AM +0300, Laurent Pinchart wrote:
> > Some CSIS instances feature more than one output channel. Parse the
> > number of channels from the device tree, and update register dumps and
> > event counters accordingly. Support for routing virtual channels and
> > data types to output channels through the subdev internal routing API
> > will come later.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 224 ++++++++++++++-------
> >  1 file changed, 146 insertions(+), 78 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 080e40837463..4cc358d93187 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -98,12 +98,12 @@
> >  #define MIPI_CSIS_INT_SRC_ODD_AFTER		BIT(28)
> >  #define MIPI_CSIS_INT_SRC_ODD			(0x3 << 28)
> >  #define MIPI_CSIS_INT_SRC_NON_IMAGE_DATA	(0xf << 28)
> > -#define MIPI_CSIS_INT_SRC_FRAME_START		BIT(24)
> > -#define MIPI_CSIS_INT_SRC_FRAME_END		BIT(20)
> > +#define MIPI_CSIS_INT_SRC_FRAME_START(n)	BIT((n) + 24)
> > +#define MIPI_CSIS_INT_SRC_FRAME_END(n)		BIT((n) + 20)
> >  #define MIPI_CSIS_INT_SRC_ERR_SOT_HS(n)		BIT((n) + 16)
> > -#define MIPI_CSIS_INT_SRC_ERR_LOST_FS		BIT(12)
> > -#define MIPI_CSIS_INT_SRC_ERR_LOST_FE		BIT(8)
> > -#define MIPI_CSIS_INT_SRC_ERR_OVER		BIT(4)
> > +#define MIPI_CSIS_INT_SRC_ERR_LOST_FS(n)	BIT((n) + 12)
> > +#define MIPI_CSIS_INT_SRC_ERR_LOST_FE(n)	BIT((n) + 8)
> > +#define MIPI_CSIS_INT_SRC_ERR_OVER(n)		BIT((n) + 4)
> >  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
> >  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
> >  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> > @@ -205,23 +205,23 @@
> >  /* Debug control register */
> >  #define MIPI_CSIS_DBG_CTRL			0xc0
> >  #define MIPI_CSIS_DBG_INTR_MSK			0xc4
> > -#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT	BIT(25)
> > -#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE	BIT(24)
> > -#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE	BIT(20)
> > -#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME	BIT(16)
> > -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE		BIT(12)
> > -#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS		BIT(8)
> > -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
> > -#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)
> > +#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
> > +#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)
> 
> not sure why cause above two line changes.

Because the whole block is reindented due to longer names. Those two
lines change from

#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL	BIT(4)
#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE	BIT(0)

to

#define MIPI_CSIS_DBG_INTR_MSK_DT_NOT_SUPPORT		BIT(25)
#define MIPI_CSIS_DBG_INTR_MSK_DT_IGNORE		BIT(24)

(one extra tab between the macro name and the value)

> > +#define MIPI_CSIS_DBG_INTR_MSK_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> > +#define MIPI_CSIS_DBG_INTR_MSK_TRUNCATED_FRAME(n)	BIT((n) + 16)
> > +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FE(n)		BIT((n) + 12)
> > +#define MIPI_CSIS_DBG_INTR_MSK_EARLY_FS(n)		BIT((n) + 8)
> > +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> > +#define MIPI_CSIS_DBG_INTR_MSK_CAM_VSYNC_RISE(n)	BIT((n) + 0)
> >  #define MIPI_CSIS_DBG_INTR_SRC			0xc8
> > -#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT	BIT(25)
> > -#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE	BIT(24)
> > -#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE	BIT(20)
> > -#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME	BIT(16)
> > -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE		BIT(12)
> > -#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS		BIT(8)
> > -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
> > -#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
> > +#define MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT		BIT(25)
> > +#define MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE		BIT(24)
> 
> the same here.
> 
> > +#define MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(n)	BIT((n) + 20)
> > +#define MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(n)	BIT((n) + 16)
> > +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(n)		BIT((n) + 12)
> > +#define MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(n)		BIT((n) + 8)
> > +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(n)	BIT((n) + 4)
> > +#define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(n)	BIT((n) + 0)
> >
> >  #define MIPI_CSIS_FRAME_COUNTER_CH(n)		(0x0100 + (n) * 4)
> >
> > @@ -230,8 +230,11 @@
> >  #define MIPI_CSIS_PKTDATA_EVEN			0x3000
> >  #define MIPI_CSIS_PKTDATA_SIZE			SZ_4K
> >
> > +#define MIPI_CSIS_MAX_CHANNELS			4
> > +
> >  struct mipi_csis_event {
> >  	bool debug;
> > +	unsigned int channel;
> >  	u32 mask;
> >  	const char * const name;
> >  	unsigned int counter;
> > @@ -239,36 +242,70 @@ struct mipi_csis_event {
> >
> >  static const struct mipi_csis_event mipi_csis_events[] = {
> >  	/* Errors */
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),	"SOT 0 Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),	"SOT 1 Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),	"SOT 2 Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),	"SOT 3 Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FS,		"Lost Frame Start Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_LOST_FE,		"Lost Frame End Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
> > -	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME,	"Truncated Frame" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE,	"Early Frame End" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS,	"Early Frame Start" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(0),		"SOT 0 Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(1),		"SOT 1 Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(2),		"SOT 2 Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS(3),		"SOT 3 Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS(0),		"Lost Frame Start Error 0" },
> > +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FS(1),		"Lost Frame Start Error 1" },
> > +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FS(2),		"Lost Frame Start Error 2" },
> > +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FS(3),		"Lost Frame Start Error 3" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE(0),		"Lost Frame End Error 0" },
> > +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_LOST_FE(1),		"Lost Frame End Error 1" },
> > +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_LOST_FE(2),		"Lost Frame End Error 2" },
> > +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_LOST_FE(3),		"Lost Frame End Error 3" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_OVER(0),		"FIFO Overflow Error 0" },
> > +	{ false, 1, MIPI_CSIS_INT_SRC_ERR_OVER(1),		"FIFO Overflow Error 1" },
> > +	{ false, 2, MIPI_CSIS_INT_SRC_ERR_OVER(2),		"FIFO Overflow Error 2" },
> > +	{ false, 3, MIPI_CSIS_INT_SRC_ERR_OVER(3),		"FIFO Overflow Error 3" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,		"Wrong Configuration Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ECC,			"ECC Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_CRC,			"CRC Error" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ERR_ID,			"Unknown ID Error" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,		"Data Type Ignored" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(0),	"Frame Size Error 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(1),	"Frame Size Error 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(2),	"Frame Size Error 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE(3),	"Frame Size Error 3" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(0),	"Truncated Frame 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(1),	"Truncated Frame 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(2),	"Truncated Frame 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME(3),	"Truncated Frame 3" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(0),		"Early Frame End 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(1),		"Early Frame End 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(2),		"Early Frame End 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE(3),		"Early Frame End 3" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(0),		"Early Frame Start 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(1),		"Early Frame Start 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(2),		"Early Frame Start 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS(3),		"Early Frame Start 3" },
> >  	/* Non-image data receive events */
> > -	{ false, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
> > -	{ false, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
> > -	{ false, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
> > -	{ false, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE,		"Non-image data before even frame" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
> >  	/* Frame start/end */
> > -	{ false, MIPI_CSIS_INT_SRC_FRAME_START,		"Frame Start" },
> > -	{ false, MIPI_CSIS_INT_SRC_FRAME_END,		"Frame End" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL,	"VSYNC Falling Edge" },
> > -	{ true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE,	"VSYNC Rising Edge" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_START(0),		"Frame Start 0" },
> > +	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_START(1),		"Frame Start 1" },
> > +	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_START(2),		"Frame Start 2" },
> > +	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_START(3),		"Frame Start 3" },
> > +	{ false, 0, MIPI_CSIS_INT_SRC_FRAME_END(0),		"Frame End 0" },
> > +	{ false, 1, MIPI_CSIS_INT_SRC_FRAME_END(1),		"Frame End 1" },
> > +	{ false, 2, MIPI_CSIS_INT_SRC_FRAME_END(2),		"Frame End 2" },
> > +	{ false, 3, MIPI_CSIS_INT_SRC_FRAME_END(3),		"Frame End 3" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(0),	"VSYNC Falling Edge 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(1),	"VSYNC Falling Edge 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(2),	"VSYNC Falling Edge 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL(3),	"VSYNC Falling Edge 3" },
> > +	{ true, 0, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(0),	"VSYNC Rising Edge 0" },
> > +	{ true, 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(1),	"VSYNC Rising Edge 1" },
> > +	{ true, 2, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(2),	"VSYNC Rising Edge 2" },
> > +	{ true, 3, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE(3),	"VSYNC Rising Edge 3" },
> >  };
> >
> > -#define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events)
> > +#define MIPI_CSIS_NUM_EVENTS		ARRAY_SIZE(mipi_csis_events)
> > +#define MIPI_CSIS_NUM_ERROR_EVENTS	(MIPI_CSIS_NUM_EVENTS - 20)
> >
> >  enum mipi_csis_clk {
> >  	MIPI_CSIS_CLK_PCLK,
> > @@ -300,7 +337,9 @@ struct mipi_csis_device {
> >  	struct clk_bulk_data *clks;
> >  	struct reset_control *mrst;
> >  	struct regulator *mipi_phy_regulator;
> > +
> >  	const struct mipi_csis_info *info;
> > +	unsigned int num_channels;
> >
> >  	struct v4l2_subdev sd;
> >  	struct media_pad pads[CSIS_PADS_NUM];
> > @@ -766,16 +805,19 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
> >
> >  	/* Update the event/error counters */
> >  	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) {
> > -		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
> > +		for (i = 0; i < ARRAY_SIZE(csis->events); i++) {
> >  			struct mipi_csis_event *event = &csis->events[i];
> >
> > +			if (event->channel >= csis->num_channels)
> > +				continue;
> > +
> >  			if ((!event->debug && (status & event->mask)) ||
> >  			    (event->debug && (dbg_status & event->mask)))
> >  				event->counter++;
> >  		}
> >  	}
> >
> > -	if (status & MIPI_CSIS_INT_SRC_FRAME_START)
> > +	if (status & MIPI_CSIS_INT_SRC_FRAME_START(0))
> >  		mipi_csis_queue_event_sof(csis);
> >
> >  	spin_unlock_irqrestore(&csis->slock, flags);
> > @@ -852,7 +894,7 @@ static void mipi_csis_clear_counters(struct mipi_csis_device *csis)
> >  static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_errors)
> >  {
> >  	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
> > -				: MIPI_CSIS_NUM_EVENTS - 8;
> > +				: MIPI_CSIS_NUM_ERROR_EVENTS;
> >  	unsigned int counters[MIPI_CSIS_NUM_EVENTS];
> >  	unsigned long flags;
> >  	unsigned int i;
> > @@ -863,45 +905,67 @@ static void mipi_csis_log_counters(struct mipi_csis_device *csis, bool non_error
> >  	spin_unlock_irqrestore(&csis->slock, flags);
> >
> >  	for (i = 0; i < num_events; ++i) {
> > +		const struct mipi_csis_event *event = &csis->events[i];
> > +
> > +		if (event->channel >= csis->num_channels)
> > +			continue;
> > +
> >  		if (counters[i] > 0 || csis->debug.enable)
> >  			dev_info(csis->dev, "%s events: %d\n",
> > -				 csis->events[i].name,
> > -				 counters[i]);
> > +				 event->name, counters[i]);
> >  	}
> >  }
> >
> > +struct mipi_csis_reg_info {
> > +	u32 addr;
> > +	unsigned int offset;
> > +	const char * const name;
> > +};
> > +
> > +static void mipi_csis_dump_channel_reg(struct mipi_csis_device *csis,
> > +				       const struct mipi_csis_reg_info *reg,
> > +				       unsigned int channel)
> > +{
> > +	dev_info(csis->dev, "%16s%u: 0x%08x\n", reg->name, channel,
> > +		 mipi_csis_read(csis, reg->addr + channel * reg->offset));
> > +}
> > +
> >  static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
> >  {
> > -	static const struct {
> > -		u32 offset;
> > -		const char * const name;
> > -	} registers[] = {
> > -		{ MIPI_CSIS_CMN_CTRL, "CMN_CTRL" },
> > -		{ MIPI_CSIS_CLK_CTRL, "CLK_CTRL" },
> > -		{ MIPI_CSIS_INT_MSK, "INT_MSK" },
> > -		{ MIPI_CSIS_DPHY_STATUS, "DPHY_STATUS" },
> > -		{ MIPI_CSIS_DPHY_CMN_CTRL, "DPHY_CMN_CTRL" },
> > -		{ MIPI_CSIS_DPHY_SCTRL_L, "DPHY_SCTRL_L" },
> > -		{ MIPI_CSIS_DPHY_SCTRL_H, "DPHY_SCTRL_H" },
> > -		{ MIPI_CSIS_ISP_CONFIG_CH(0), "ISP_CONFIG_CH0" },
> > -		{ MIPI_CSIS_ISP_RESOL_CH(0), "ISP_RESOL_CH0" },
> > -		{ MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" },
> > -		{ MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" },
> > -		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
> > -		{ MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" },
> > +	static const struct mipi_csis_reg_info common_registers[] = {
> > +		{ MIPI_CSIS_CMN_CTRL, 0, "CMN_CTRL" },
> > +		{ MIPI_CSIS_CLK_CTRL, 0, "CLK_CTRL" },
> > +		{ MIPI_CSIS_INT_MSK, 0, "INT_MSK" },
> > +		{ MIPI_CSIS_DPHY_STATUS, 0, "DPHY_STATUS" },
> > +		{ MIPI_CSIS_DPHY_CMN_CTRL, 0, "DPHY_CMN_CTRL" },
> > +		{ MIPI_CSIS_DPHY_SCTRL_L, 0, "DPHY_SCTRL_L" },
> > +		{ MIPI_CSIS_DPHY_SCTRL_H, 0, "DPHY_SCTRL_H" },
> > +		{ MIPI_CSIS_DBG_CTRL, 0, "DBG_CTRL" },
> > +	};
> > +	static const struct mipi_csis_reg_info channel_registers[] = {
> > +		{ MIPI_CSIS_ISP_CONFIG_CH(0), 0x10, "ISP_CONFIG_CH" },
> > +		{ MIPI_CSIS_ISP_RESOL_CH(0), 0x10, "ISP_RESOL_CH" },
> > +		{ MIPI_CSIS_SDW_CONFIG_CH(0), 0x10, "SDW_CONFIG_CH" },
> > +		{ MIPI_CSIS_SDW_RESOL_CH(0), 0x10, "SDW_RESOL_CH" },
> > +		{ MIPI_CSIS_FRAME_COUNTER_CH(0), 4, "FRAME_COUNTER_CH" },
> >  	};
> > -
> > -	unsigned int i;
> > -	u32 cfg;
> >
> >  	if (!pm_runtime_get_if_in_use(csis->dev))
> >  		return 0;
> >
> >  	dev_info(csis->dev, "--- REGISTERS ---\n");
> >
> > -	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> > -		cfg = mipi_csis_read(csis, registers[i].offset);
> > -		dev_info(csis->dev, "%17s: 0x%08x\n", registers[i].name, cfg);
> > +	for (unsigned int i = 0; i < ARRAY_SIZE(common_registers); i++) {
> > +		const struct mipi_csis_reg_info *reg = &common_registers[i];
> > +
> > +		dev_info(csis->dev, "%17s: 0x%08x\n", reg->name,
> > +			 mipi_csis_read(csis, reg->addr));
> > +	}
> > +
> > +	for (unsigned int chan = 0; chan < csis->num_channels; chan++) {
> > +		for (unsigned int i = 0; i < ARRAY_SIZE(channel_registers); ++i)
> > +			mipi_csis_dump_channel_reg(csis, &channel_registers[i],
> > +						   chan);
> >  	}
> >
> >  	pm_runtime_put(csis->dev);
> > @@ -1424,6 +1488,12 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
> >
> >  	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
> >
> > +	csis->num_channels = 1;
> > +	of_property_read_u32(node, "fsl,num-channels", &csis->num_channels);
> 
> Wait for dt team comments, most likely they do not agree add fsl,num-channels.
> 
> Frank
> 
> > +	if (csis->num_channels < 1 || csis->num_channels > MIPI_CSIS_MAX_CHANNELS)
> > +		return dev_err_probe(csis->dev, -EINVAL,
> > +				     "Invalid fsl,num-channels value\n");
> > +
> >  	return 0;
> >  }
> >
> > @@ -1447,10 +1517,8 @@ static int mipi_csis_probe(struct platform_device *pdev)
> >
> >  	/* Parse DT properties. */
> >  	ret = mipi_csis_parse_dt(csis);
> > -	if (ret < 0) {
> > -		dev_err(dev, "Failed to parse device tree: %d\n", ret);
> > +	if (ret < 0)
> >  		return ret;
> > -	}
> >
> >  	/* Acquire resources. */
> >  	csis->regs = devm_platform_ioremap_resource(pdev, 0);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/8] media: imx-mipi-csis: Rename register macros to match reference manual

[Laurent Pinchart]

On Tue, Jun 10, 2025 at 12:16:34PM +0300, Laurent Pinchart wrote:
> On Tue, Jun 10, 2025 at 11:10:54AM +0200, Alexander Stein wrote:
> > Am Montag, 9. Juni 2025, 01:58:33 CEST schrieb Laurent Pinchart:
> > > The CSIS driver uses register macro names that do not match the
> > > reference manual of the i.MX7[DS] and i.MX8M[MNP] SoCs in which the CSIS
> > > is integrated. Rename them to match the documentation, making the code
> > > easier to read alongside the reference manuals.
> > > 
> > > One of the misnamed register fields is MIPI_CSIS_INT_SRC_ERR_UNKNOWN,
> > > which led to the corresponding event being logged as "Unknown Error".
> > > The correct register field name is MIPI_CSIS_INT_SRC_ERR_ID, documented
> > > as "Unknown ID error". Update the event description accordingly.
> > > 
> > > While at it, also replace a few *_OFFSET macros with parametric macros
> > > for consistency, and add the missing MIPI_CSIS_ISP_RESOL_VRESOL and
> > > MIPI_CSIS_ISP_RESOL_HRESOL register field macros.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/nxp/imx-mipi-csis.c | 69 +++++++++++-----------
> > >  1 file changed, 36 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index 2beb5f43c2c0..d59666ef7545 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -55,13 +55,13 @@
> > >  /* CSIS common control */
> > >  #define MIPI_CSIS_CMN_CTRL			0x04
> > >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
> > > -#define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
> > > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE	(0 << 10)
> > > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	(1 << 10)
> > > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
> > > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	(3 << 8)
> > >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
> > > -#define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
> > > -#define MIPI_CSIS_CMN_CTRL_ENABLE		BIT(0)
> > > -
> > > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET	8
> > > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK		(3 << 8)
> > > +#define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
> > > +#define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
> > >  
> > >  /* CSIS clock control */
> > >  #define MIPI_CSIS_CLK_CTRL			0x08
> > > @@ -87,7 +87,7 @@
> > >  #define MIPI_CSIS_INT_MSK_ERR_WRONG_CFG		BIT(3)
> > >  #define MIPI_CSIS_INT_MSK_ERR_ECC		BIT(2)
> > >  #define MIPI_CSIS_INT_MSK_ERR_CRC		BIT(1)
> > > -#define MIPI_CSIS_INT_MSK_ERR_UNKNOWN		BIT(0)
> > > +#define MIPI_CSIS_INT_MSK_ERR_ID		BIT(0)
> > >  
> > >  /* CSIS Interrupt source */
> > >  #define MIPI_CSIS_INT_SRC			0x14
> > > @@ -107,7 +107,7 @@
> > >  #define MIPI_CSIS_INT_SRC_ERR_WRONG_CFG		BIT(3)
> > >  #define MIPI_CSIS_INT_SRC_ERR_ECC		BIT(2)
> > >  #define MIPI_CSIS_INT_SRC_ERR_CRC		BIT(1)
> > > -#define MIPI_CSIS_INT_SRC_ERR_UNKNOWN		BIT(0)
> > > +#define MIPI_CSIS_INT_SRC_ERR_ID		BIT(0)
> > >  #define MIPI_CSIS_INT_SRC_ERRORS		0xfffff
> > >  
> > >  /* D-PHY status control */
> > > @@ -123,8 +123,8 @@
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE_MASK	GENMASK(31, 24)
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(n)	((n) << 22)
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE_MASK	GENMASK(23, 22)
> > > -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_CLK	BIT(6)
> > > -#define MIPI_CSIS_DPHY_CMN_CTRL_DPDN_SWAP_DAT	BIT(5)
> > > +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_CLK	BIT(6)
> > > +#define MIPI_CSIS_DPHY_CMN_CTRL_S_DPDN_SWAP_DAT	BIT(5)
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_DAT	BIT(1)
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE_CLK	BIT(0)
> > >  #define MIPI_CSIS_DPHY_CMN_CTRL_ENABLE		(0x1f << 0)
> > > @@ -179,21 +179,23 @@
> > >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
> > >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
> > >  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
> > > -#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
> > > -#define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
> > > -#define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
> > > -#define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
> > > +#define MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK	(3 << 12)
> > > +#define MIPI_CSIS_ISPCFG_PARALLEL		BIT(11)
> > > +#define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt)	((fmt) << 2)
> > > +#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	(0x3f << 2)
> > >  
> > >  /* ISP Image Resolution register */
> > >  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> > > +#define MIPI_CSIS_ISP_RESOL_VRESOL(n)		((n) << 16)
> > > +#define MIPI_CSIS_ISP_RESOL_HRESOL(n)		((n) << 0)
> > >  #define CSIS_MAX_PIX_WIDTH			0xffff
> > >  #define CSIS_MAX_PIX_HEIGHT			0xffff
> > >  
> > >  /* ISP SYNC register */
> > >  #define MIPI_CSIS_ISP_SYNC_CH(n)		(0x48 + (n) * 0x10)
> > > -#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET	18
> > > -#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET	12
> > > -#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET	0
> > > +#define MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(n)	((n) << 18)
> > > +#define MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(n)	((n) << 12)
> > > +#define MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(n)	((n) << 0)
> > >  
> > >  /* ISP shadow registers */
> > >  #define MIPI_CSIS_SDW_CONFIG_CH(n)		(0x80 + (n) * 0x10)
> > > @@ -246,7 +248,7 @@ static const struct mipi_csis_event mipi_csis_events[] = {
> > >  	{ false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
> > >  	{ false, MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
> > >  	{ false, MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
> > > -	{ false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN,		"Unknown Error" },
> > > +	{ false, MIPI_CSIS_INT_SRC_ERR_ID,		"Unknown ID Error" },
> > >  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT,	"Data Type Not Supported" },
> > >  	{ true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE,	"Data Type Ignored" },
> > >  	{ true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE,	"Frame Size Error" },
> > > @@ -517,7 +519,7 @@ static void mipi_csis_sw_reset(struct mipi_csis_device *csis)
> > >  	u32 val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > >  
> > >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > > -			val | MIPI_CSIS_CMN_CTRL_RESET);
> > > +			val | MIPI_CSIS_CMN_CTRL_SW_RESET);
> > >  	usleep_range(10, 20);
> > >  }
> > >  
> > > @@ -527,9 +529,9 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> > >  
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > >  	if (on)
> > > -		val |= MIPI_CSIS_CMN_CTRL_ENABLE;
> > > +		val |= MIPI_CSIS_CMN_CTRL_CSI_EN;
> > >  	else
> > > -		val &= ~MIPI_CSIS_CMN_CTRL_ENABLE;
> > > +		val &= ~MIPI_CSIS_CMN_CTRL_CSI_EN;
> > >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> > >  
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
> > > @@ -549,8 +551,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> > >  
> > >  	/* Color format */
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
> > > -	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
> > > -		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
> > > +	val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
> > > +		 MIPI_CSIS_ISPCFG_DATAFORMAT_MASK);
> > >  
> > >  	/*
> > >  	 * YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
> > > @@ -568,12 +570,13 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> > >  	if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
> > >  		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> > >  
> > > -	val |= MIPI_CSIS_ISPCFG_FMT(csis_fmt->data_type);
> > > +	val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
> > >  	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
> > >  
> > >  	/* Pixel resolution */
> > > -	val = format->width | (format->height << 16);
> > > -	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0), val);
> > > +	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
> > > +			MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
> > > +			MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
> > >  }
> > >  
> > >  static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> > > @@ -635,10 +638,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >  	u32 val;
> > >  
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > > -	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK;
> > > -	val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET;
> > > +	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> > > +	val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
> > >  	if (csis->info->version == MIPI_CSIS_V3_3)
> > > -		val |= MIPI_CSIS_CMN_CTRL_INTER_MODE;
> > > +		val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > 
> > Mh, what about i.MX8MP which also has these bitfield defined, but is
> > not a MIPI_CSIS_V3_3 core?
> 
> Short answer: no idea yet. Has anyone been able to capture embedded data
> through the ISI on the i.MX8MP ?

My current understanding is that the i.MX8MP hardware integration
doesn't allow capturing DT=EMBEDDED_8B when sent through the same VC as
image data. I would love to be proven wrong.

> > >  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> > >  
> > >  	__mipi_csis_set_format(csis, format, csis_fmt);
> > > @@ -647,10 +650,10 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >  			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
> > >  			MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
> > >  
> > > -	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
> > > -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
> > > -	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET);
> > > -	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0), val);
> > > +	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
> > > +			MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> > > +			MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
> > > +			MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
> > >  
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > >  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Laurent Pinchart]

On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > This is an integration-specific property, specify the number of
> > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > supported by drivers.
> > > > > >
> > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > binding examples do not cover that SoC, don't update them.
> > > > > >
> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > >  1 file changed, 7 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > >      default: 166000000
> > > > > >      deprecated: true
> > > > > >
> > > > > > +  fsl,num-channels:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    description: Number of output channels
> > > > > > +    minimum: 1
> > > > > > +    maximum: 4
> > > > > > +    default: 1
> > > > > > +
> > > > >
> > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > suitable for adding new property. It should be in driver data of each
> > > > > compatible strings.
> > > > >
> > > > > I met similar case before. DT team generally don't agree on add such
> > > > > property, unless there are two instances in the same chip, which have
> > > > > difference channel number.
> > > >
> > > > If the DT changes are rejected, can the number of channels be added to
> > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > to the number of channels.
> > >
> > > I thought about it, and decided to add a new property because the number
> > > of channels is really a synthesis time configuration parameter, and
> > > could differ between different CSIS instances in the same SoC.
> > 
> > Need add such information at binding doc's commit message,
> 
> I'll update the commit message.

The commit message in v2 will state

    dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

    The CSI-2 receiver can be instantiated with up to four output channels.
    This is an integration-specific property, specify the number of
    instantiated channels through a new fsl,num-channels property. The
    property is optional, and defaults to 1 as only one channel is currently
    supported by drivers.

    Using the compatible string to infer the number of channels has been
    considered, but multiple instances of the same CSIS in the same SoC
    could conceptually be synthesized with a different number of channels.
    An explicit property is therefore more appropriate.

    The only known SoC to have more than one channel is the i.MX8MP. As the
    binding examples do not cover that SoC, don't update them.

Rob, Krzysztof, Conor, are you fine with adding this property ?

> > ideally provide an example for it.
> 
> That I can't provide because the few SoCs I'm working with do not
> integrate multiple CSIS instances with different parameters.
> 
> > > > > >    ports:
> > > > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > > > >

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/8] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated

[Rob Herring (Arm)]

On Mon, 09 Jun 2025 02:58:37 +0300, Laurent Pinchart wrote:
> Usage of the clock-frequency property, which is already optional, is
> discouraged in favour of using assigned-clock-rates (and
> assigned-clock-parents where needed). Mark the property as deprecated,
> and update the examples accordingly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml  | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Rob Herring]

On Fri, Jun 20, 2025 at 12:02:37AM +0300, Laurent Pinchart wrote:
> On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > > This is an integration-specific property, specify the number of
> > > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > > supported by drivers.
> > > > > > >
> > > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > > binding examples do not cover that SoC, don't update them.
> > > > > > >
> > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > > >  1 file changed, 7 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > > >      default: 166000000
> > > > > > >      deprecated: true
> > > > > > >
> > > > > > > +  fsl,num-channels:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    description: Number of output channels
> > > > > > > +    minimum: 1
> > > > > > > +    maximum: 4
> > > > > > > +    default: 1
> > > > > > > +
> > > > > >
> > > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > > suitable for adding new property. It should be in driver data of each
> > > > > > compatible strings.
> > > > > >
> > > > > > I met similar case before. DT team generally don't agree on add such
> > > > > > property, unless there are two instances in the same chip, which have
> > > > > > difference channel number.
> > > > >
> > > > > If the DT changes are rejected, can the number of channels be added to
> > > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > > to the number of channels.
> > > >
> > > > I thought about it, and decided to add a new property because the number
> > > > of channels is really a synthesis time configuration parameter, and
> > > > could differ between different CSIS instances in the same SoC.
> > > 
> > > Need add such information at binding doc's commit message,
> > 
> > I'll update the commit message.
> 
> The commit message in v2 will state
> 
>     dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
> 
>     The CSI-2 receiver can be instantiated with up to four output channels.
>     This is an integration-specific property, specify the number of
>     instantiated channels through a new fsl,num-channels property. The
>     property is optional, and defaults to 1 as only one channel is currently
>     supported by drivers.
> 
>     Using the compatible string to infer the number of channels has been
>     considered, but multiple instances of the same CSIS in the same SoC
>     could conceptually be synthesized with a different number of channels.
>     An explicit property is therefore more appropriate.
> 
>     The only known SoC to have more than one channel is the i.MX8MP. As the
>     binding examples do not cover that SoC, don't update them.

So how many channels does i.MX8MP have in a DT without this property? 
It's either 1 or the driver overrides it to 2.

> Rob, Krzysztof, Conor, are you fine with adding this property ?

Yes, but seems a little late.

Rob

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Laurent Pinchart]

On Wed, Jun 25, 2025 at 02:27:28PM -0500, Rob Herring wrote:
> On Fri, Jun 20, 2025 at 12:02:37AM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > > > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > > > This is an integration-specific property, specify the number of
> > > > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > > > supported by drivers.
> > > > > > > >
> > > > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > > > binding examples do not cover that SoC, don't update them.
> > > > > > > >
> > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > > > ---
> > > > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > > > >  1 file changed, 7 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > > > >      default: 166000000
> > > > > > > >      deprecated: true
> > > > > > > >
> > > > > > > > +  fsl,num-channels:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    description: Number of output channels
> > > > > > > > +    minimum: 1
> > > > > > > > +    maximum: 4
> > > > > > > > +    default: 1
> > > > > > > > +
> > > > > > >
> > > > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > > > suitable for adding new property. It should be in driver data of each
> > > > > > > compatible strings.
> > > > > > >
> > > > > > > I met similar case before. DT team generally don't agree on add such
> > > > > > > property, unless there are two instances in the same chip, which have
> > > > > > > difference channel number.
> > > > > >
> > > > > > If the DT changes are rejected, can the number of channels be added to
> > > > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > > > to the number of channels.
> > > > >
> > > > > I thought about it, and decided to add a new property because the number
> > > > > of channels is really a synthesis time configuration parameter, and
> > > > > could differ between different CSIS instances in the same SoC.
> > > > 
> > > > Need add such information at binding doc's commit message,
> > > 
> > > I'll update the commit message.
> > 
> > The commit message in v2 will state
> > 
> >     dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
> > 
> >     The CSI-2 receiver can be instantiated with up to four output channels.
> >     This is an integration-specific property, specify the number of
> >     instantiated channels through a new fsl,num-channels property. The
> >     property is optional, and defaults to 1 as only one channel is currently
> >     supported by drivers.
> > 
> >     Using the compatible string to infer the number of channels has been
> >     considered, but multiple instances of the same CSIS in the same SoC
> >     could conceptually be synthesized with a different number of channels.
> >     An explicit property is therefore more appropriate.
> > 
> >     The only known SoC to have more than one channel is the i.MX8MP. As the
> >     binding examples do not cover that SoC, don't update them.
> 
> So how many channels does i.MX8MP have in a DT without this property? 
> It's either 1 or the driver overrides it to 2.

Only 1. I only realized a few weeks ago that the CSIS has multiple
output channels on i.MX8MP. This wasn't documented anywhere, and
understanding how all of this works took lots of trial-and-error.

> > Rob, Krzysztof, Conor, are you fine with adding this property ?
> 
> Yes, but seems a little late.

It is, I wish I had known about this hardware feature years ago. Now I
need to figure out how to properly support it in the driver and expose
it to userspace through V4L2 without breaking backward compatibility
with existing applications. The DT binding is the easy part :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

[Rob Herring (Arm)]

On Mon, 09 Jun 2025 02:58:38 +0300, Laurent Pinchart wrote:
> The CSI-2 receiver can be instantiated with up to four output channels.
> This is an integration-specific property, specify the number of
> instantiated channels through a new fsl,num-channels property. The
> property is optional, and defaults to 1 as only one channel is currently
> supported by drivers.
> 
> The only known SoC to have more than one channel is the i.MX8MP. As the
> binding examples do not cover that SoC, don't update them.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>