[PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements
@ 2025-08-22  0:27 Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 01/13] media: v4l2-common: Constify media_pad argument to v4l2_get_link_freq() Laurent Pinchart
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Hello,

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

Patch 01/13 starts by making the v4l2_get_link_freq() function accept a
const media_pad argument. This conflicts with [1], I don't mind rebasing
on top of it. Sakari, if you plan to get that patch merged soon, feel
free to take 01/13 too.

The next three patches, 02/13 to 04/13, are small cleanups. Patch 05/13
then aligns the code with the reference manual for register field names,
increasing readability of the driver when read alongside the hardware
documentation. Patch 06/13 (new in this version) also improves the
register macros by standardizing usage of GENMASK. Patch 07/13 fixes a
small alignment issue in register dumps, and patch 08/13 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 09/13 improves
handling of the clock frequency in the driver, and patch 10/13
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 11/13 adds a
corresponding DT property, and patch 12/13 adds initial support for that
property in the driver, and uses it to dump per-channel registers and
event counters. Finally, patch 13/13 sets the property in the i.MX8MP
DT.

[1] https://lore.kernel.org/linux-media/20250819094533.2335-3-sakari.ailus@linux.intel.com

Laurent Pinchart (13):
  media: v4l2-common: Constify media_pad argument to
    v4l2_get_link_freq()
  media: imx-mipi-csis: Simplify access to source pad
  media: imx-mipi-csis: Standardize const keyword placement
  media: imx-mipi-csis: Shorten name of subdev state variables
  media: imx-mipi-csis: Rename register macros to match reference manual
  media: imx-mipi-csis: Use GENMASK for all register field masks
  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
  media: imx-mipi-csis: Initial support for multiple output channels
  arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers

 .../bindings/media/nxp,imx-mipi-csi2.yaml     |  18 +-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |   2 +
 drivers/media/platform/nxp/imx-mipi-csis.c    | 353 +++++++++++-------
 drivers/media/v4l2-core/v4l2-common.c         |   2 +-
 include/media/v4l2-common.h                   |   3 +-
 5 files changed, 232 insertions(+), 146 deletions(-)

base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
-- 
Regards,

Laurent Pinchart

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

* [PATCH v3 01/13] media: v4l2-common: Constify media_pad argument to v4l2_get_link_freq()
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 02/13] media: imx-mipi-csis: Simplify access to source pad Laurent Pinchart
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

The v4l2_get_link_freq() macro doesn't modify the pad argument. Make it
possible to call it with a const media_pad pointer.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 2 +-
 include/media/v4l2-common.h           | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index a5334aa35992..4d686338e345 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -551,7 +551,7 @@ s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler,
 EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl);
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
+s64 __v4l2_get_link_freq_pad(const struct media_pad *pad, unsigned int mul,
 			     unsigned int div)
 {
 	struct v4l2_mbus_config mbus_config = {};
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 39dd0c78d70f..ede24e60dbec 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -580,10 +580,11 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
 #ifdef CONFIG_MEDIA_CONTROLLER
 #define v4l2_get_link_freq(pad, mul, div)				\
 	_Generic(pad,							\
+		 const struct media_pad *: __v4l2_get_link_freq_pad,	\
 		 struct media_pad *: __v4l2_get_link_freq_pad,		\
 		 struct v4l2_ctrl_handler *: __v4l2_get_link_freq_ctrl)	\
 	(pad, mul, div)
-s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
+s64 __v4l2_get_link_freq_pad(const struct media_pad *pad, unsigned int mul,
 			     unsigned int div);
 #else
 #define v4l2_get_link_freq(handler, mul, div)		\
-- 
Regards,

Laurent Pinchart



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

* [PATCH v3 02/13] media: imx-mipi-csis: Simplify access to source pad
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 01/13] media: v4l2-common: Constify media_pad argument to v4l2_get_link_freq() Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 03/13] media: imx-mipi-csis: Standardize const keyword placement Laurent Pinchart
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

The mipi_csis_calculate_params() function needs to access the pad of the
connected source. The pad is already available in csis->source.pad, but
the function takes a convoluted path by getting the pad index and
indexing the source subdev's pads array. Simplify it.

No functional change is intended.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..46f93cd677e3 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -579,13 +579,11 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
 static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
 				      const struct csis_pix_format *csis_fmt)
 {
-	struct media_pad *src_pad =
-		&csis->source.sd->entity.pads[csis->source.pad->index];
 	s64 link_freq;
 	u32 lane_rate;
 
 	/* Calculate the line rate from the pixel rate. */
-	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
+	link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
 				       csis->bus.num_data_lanes * 2);
 	if (link_freq < 0) {
 		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
-- 
Regards,

Laurent Pinchart



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

* [PATCH v3 03/13] media: imx-mipi-csis: Standardize const keyword placement
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 01/13] media: v4l2-common: Constify media_pad argument to v4l2_get_link_freq() Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 02/13] media: imx-mipi-csis: Simplify access to source pad Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 04/13] media: imx-mipi-csis: Shorten name of subdev state variables Laurent Pinchart
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

The const keyword for pointer variables is placed before the type
everywhere except in one location. Change it to improve consistency.

No functional change is intended.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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 46f93cd677e3..0f0863011230 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -1029,7 +1029,7 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_format *sdformat)
 {
-	struct csis_pix_format const *csis_fmt;
+	const struct csis_pix_format *csis_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 	unsigned int align;
 
-- 
Regards,

Laurent Pinchart



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

* [PATCH v3 04/13] media: imx-mipi-csis: Shorten name of subdev state variables
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (2 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 03/13] media: imx-mipi-csis: Standardize const keyword placement Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual Laurent Pinchart
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Rename sd_state to state to standardize the naming of the subdev state
variables and help shortening lines.

No functional change is intended.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 0f0863011230..894d12fef519 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -996,7 +996,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 }
 
 static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
-				    struct v4l2_subdev_state *sd_state,
+				    struct v4l2_subdev_state *state,
 				    struct v4l2_subdev_mbus_code_enum *code)
 {
 	/*
@@ -1009,7 +1009,7 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
 		if (code->index > 0)
 			return -EINVAL;
 
-		fmt = v4l2_subdev_state_get_format(sd_state, code->pad);
+		fmt = v4l2_subdev_state_get_format(state, code->pad);
 		code->code = fmt->code;
 		return 0;
 	}
@@ -1026,7 +1026,7 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
 }
 
 static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
-			     struct v4l2_subdev_state *sd_state,
+			     struct v4l2_subdev_state *state,
 			     struct v4l2_subdev_format *sdformat)
 {
 	const struct csis_pix_format *csis_fmt;
@@ -1038,7 +1038,7 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 	 * modified.
 	 */
 	if (sdformat->pad == CSIS_PAD_SOURCE)
-		return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
+		return v4l2_subdev_get_fmt(sd, state, sdformat);
 
 	if (sdformat->pad != CSIS_PAD_SINK)
 		return -EINVAL;
@@ -1076,7 +1076,7 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 			      &sdformat->format.height, 1,
 			      CSIS_MAX_PIX_HEIGHT, 0, 0);
 
-	fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
+	fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
 
 	fmt->code = csis_fmt->code;
 	fmt->width = sdformat->format.width;
@@ -1090,7 +1090,7 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 	sdformat->format = *fmt;
 
 	/* Propagate the format from sink to source. */
-	fmt = v4l2_subdev_state_get_format(sd_state, CSIS_PAD_SOURCE);
+	fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
 	*fmt = sdformat->format;
 
 	/* The format on the source pad might change due to unpacking. */
@@ -1130,7 +1130,7 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 }
 
 static int mipi_csis_init_state(struct v4l2_subdev *sd,
-				struct v4l2_subdev_state *sd_state)
+				struct v4l2_subdev_state *state)
 {
 	struct v4l2_subdev_format fmt = {
 		.pad = CSIS_PAD_SINK,
@@ -1147,7 +1147,7 @@ static int mipi_csis_init_state(struct v4l2_subdev *sd,
 		V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
 					      fmt.format.ycbcr_enc);
 
-	return mipi_csis_set_fmt(sd, sd_state, &fmt);
+	return mipi_csis_set_fmt(sd, state, &fmt);
 }
 
 static int mipi_csis_log_status(struct v4l2_subdev *sd)
-- 
Regards,

Laurent Pinchart



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

* [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (3 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 04/13] media: imx-mipi-csis: Shorten name of subdev state variables Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22 14:10   ` Frank Li
  2025-08-22  0:27 ` [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks Laurent Pinchart
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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.

No functional change intended.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v2:

- Drop MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE
- Restore usage of BIT() for MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 68 +++++++++++-----------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 894d12fef519..ce889c436cb1 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -55,13 +55,12 @@
 /* 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_DT	BIT(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 +86,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 +106,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 +122,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 +178,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 +247,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 +518,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 +528,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 +550,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 +569,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,
@@ -633,10 +635,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);
@@ -645,10 +647,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



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

* [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (4 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22 14:13   ` Frank Li
  2025-08-22  0:27 ` [PATCH v3 07/13] media: imx-mipi-csis: Fix field alignment in register dump Laurent Pinchart
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Multiple register field mask macros use GENMASK, while other define the
mask value manually. Standardize on GENMASK everywhere, as well as on
the _MASK suffix to name the macros. This improves consistency and helps
with readability.

No functional change is intended.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index ce889c436cb1..50f6f4468f7b 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -57,7 +57,7 @@
 #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
 #define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	BIT(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_LANE_NUMBER_MASK	GENMASK(9, 8)
 #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
 #define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
 #define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
@@ -68,7 +68,7 @@
 #define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)	((x) << 24)
 #define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)	((x) << 20)
 #define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)	((x) << 16)
-#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK	(0xf << 4)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK	GENMASK(7, 4)
 #define MIPI_CSIS_CLK_CTRL_WCLK_SRC		BIT(0)
 
 /* CSIS Interrupt mask */
@@ -173,15 +173,15 @@
 
 /* ISP Configuration register */
 #define MIPI_CSIS_ISP_CONFIG_CH(n)		(0x40 + (n) * 0x10)
-#define MIPI_CSIS_ISPCFG_MEM_FULL_GAP_MSK	(0xff << 24)
+#define MIPI_CSIS_ISPCFG_MEM_FULL_GAP_MASK	GENMASK(31, 24)
 #define MIPI_CSIS_ISPCFG_MEM_FULL_GAP(x)	((x) << 24)
 #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_MODE_MASK	(3 << 12)
+#define MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK	GENMASK(13, 12)
 #define MIPI_CSIS_ISPCFG_PARALLEL		BIT(11)
 #define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt)	((fmt) << 2)
-#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	(0x3f << 2)
+#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK	GENMASK(7, 2)
 
 /* ISP Image Resolution register */
 #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
@@ -655,7 +655,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
 	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
 	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
-	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK;
+	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
 	mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
 
 	mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
-- 
Regards,

Laurent Pinchart



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

* [PATCH v3 07/13] media: imx-mipi-csis: Fix field alignment in register dump
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (5 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 08/13] media: imx-mipi-csis: Log per-lane start of transmission errors Laurent Pinchart
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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 50f6f4468f7b..346599b7a517 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -892,7 +892,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



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

* [PATCH v3 08/13] media: imx-mipi-csis: Log per-lane start of transmission errors
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (6 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 07/13] media: imx-mipi-csis: Fix field alignment in register dump Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 09/13] media: imx-mipi-csis: Only set clock rate when specified in DT Laurent Pinchart
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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>
Reviewed-by: Frank Li <Frank.Li@nxp.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 346599b7a517..77f1bf05b520 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -99,7 +99,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)
@@ -240,7 +240,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



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

* [PATCH v3 09/13] media: imx-mipi-csis: Only set clock rate when specified in DT
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (7 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 08/13] media: imx-mipi-csis: Log per-lane start of transmission errors Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 10/13] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated Laurent Pinchart
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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>
Reviewed-by: Frank Li <Frank.Li@nxp.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 77f1bf05b520..e1588990beda 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -229,8 +229,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;
@@ -707,12 +705,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;
 }
@@ -1416,9 +1419,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



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

* [PATCH v3 10/13] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (8 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 09/13] media: imx-mipi-csis: Only set clock rate when specified in DT Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 11/13] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property Laurent Pinchart
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../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



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

* [PATCH v3 11/13] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (9 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 10/13] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels Laurent Pinchart
  2025-08-22  0:27 ` [PATCH v3 13/13] arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers Laurent Pinchart
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
Changes since v1:

- Expand commit message
---
 .../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



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

* [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (10 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 11/13] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  2025-08-22 14:12   ` Stefan Klug
  2025-08-22 14:18   ` Frank Li
  2025-08-22  0:27 ` [PATCH v3 13/13] arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers Laurent Pinchart
  12 siblings, 2 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Some CSIS instances feature more than one output channel. Update
register macros accordingly, parse the number of channels from the
device tree, and update register dumps and event counters to log
per-channel data.

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>
---
Changes since v1:

- Update more per-channel registers
- Update commit message
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
 1 file changed, 152 insertions(+), 87 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index e1588990beda..ee8e541dcb1d 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -54,7 +54,7 @@
 
 /* CSIS common control */
 #define MIPI_CSIS_CMN_CTRL			0x04
-#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
+#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n)	BIT((n) + 16)
 #define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	BIT(10)
 #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
 #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	GENMASK(9, 8)
@@ -64,12 +64,9 @@
 
 /* CSIS clock control */
 #define MIPI_CSIS_CLK_CTRL			0x08
-#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)	((x) << 28)
-#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)	((x) << 24)
-#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)	((x) << 20)
-#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)	((x) << 16)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(n, x)	((x) << ((n) * 4 + 16))
 #define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK	GENMASK(7, 4)
-#define MIPI_CSIS_CLK_CTRL_WCLK_SRC		BIT(0)
+#define MIPI_CSIS_CLK_CTRL_WCLK_SRC(n)		BIT(n)
 
 /* CSIS Interrupt mask */
 #define MIPI_CSIS_INT_MSK			0x10
@@ -97,12 +94,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)
@@ -204,23 +201,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)
 
@@ -229,8 +226,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;
@@ -238,36 +238,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,
@@ -299,7 +333,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];
@@ -654,8 +690,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 			MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
-	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
-	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
+	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
+	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
 	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
 	mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
 
@@ -672,7 +708,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 	/* Update the shadow register. */
 	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
 	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
-			val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW |
+			val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
 			MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
 }
 
@@ -763,16 +799,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);
@@ -849,7 +888,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;
@@ -860,45 +899,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);
@@ -1421,6 +1482,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;
 }
 
@@ -1444,10 +1511,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



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

* [PATCH v3 13/13] arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers
  2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
                   ` (11 preceding siblings ...)
  2025-08-22  0:27 ` [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels Laurent Pinchart
@ 2025-08-22  0:27 ` Laurent Pinchart
  12 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22  0:27 UTC (permalink / raw)
  To: linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

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.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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 bb24dba7338e..1e52840078df 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1764,6 +1764,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 {
@@ -1799,6 +1800,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



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

* Re: [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual
  2025-08-22  0:27 ` [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual Laurent Pinchart
@ 2025-08-22 14:10   ` Frank Li
  0 siblings, 0 replies; 23+ messages in thread
From: Frank Li @ 2025-08-22 14:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

On Fri, Aug 22, 2025 at 03:27:25AM +0300, Laurent Pinchart wrote:
> 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.
>
> No functional change intended.

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

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v2:
>
> - Drop MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE
> - Restore usage of BIT() for MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 68 +++++++++++-----------
>  1 file changed, 35 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 894d12fef519..ce889c436cb1 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -55,13 +55,12 @@
>  /* 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_DT	BIT(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 +86,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 +106,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 +122,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 +178,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 +247,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 +518,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 +528,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 +550,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 +569,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,
> @@ -633,10 +635,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);
> @@ -645,10 +647,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
>


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

* Re: [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22  0:27 ` [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels Laurent Pinchart
@ 2025-08-22 14:12   ` Stefan Klug
  2025-08-22 20:08     ` Laurent Pinchart
  2025-08-22 14:18   ` Frank Li
  1 sibling, 1 reply; 23+ messages in thread
From: Stefan Klug @ 2025-08-22 14:12 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media
  Cc: Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Hi Laurent,

Thank you for the patch.

Quoting Laurent Pinchart (2025-08-22 02:27:32)
> Some CSIS instances feature more than one output channel. Update
> register macros accordingly, parse the number of channels from the
> device tree, and update register dumps and event counters to log
> per-channel data.
> 
> 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>
> ---
> Changes since v1:
> 
> - Update more per-channel registers
> - Update commit message
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
>  1 file changed, 152 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index e1588990beda..ee8e541dcb1d 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -54,7 +54,7 @@
>  
>  /* CSIS common control */
>  #define MIPI_CSIS_CMN_CTRL                     0x04
> -#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW       BIT(16)
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n)    BIT((n) + 16)
>  #define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT  BIT(10)
>  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)      ((n) << 8)
>  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK    GENMASK(9, 8)
> @@ -64,12 +64,9 @@
>  
>  /* CSIS clock control */
>  #define MIPI_CSIS_CLK_CTRL                     0x08
> -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)        ((x) << 28)
> -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)        ((x) << 24)
> -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)        ((x) << 20)
> -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)        ((x) << 16)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(n, x) ((x) << ((n) * 4 + 16))
>  #define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK     GENMASK(7, 4)
> -#define MIPI_CSIS_CLK_CTRL_WCLK_SRC            BIT(0)
> +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC(n)         BIT(n)
>  
>  /* CSIS Interrupt mask */
>  #define MIPI_CSIS_INT_MSK                      0x10
> @@ -97,12 +94,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)
> @@ -204,23 +201,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)
>  
> @@ -229,8 +226,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;
> @@ -238,36 +238,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)

Why is that (MIPI_CSIS_NUM_EVENTS - 20)? Should that just be 20?

The rest looks fine to me.

Best regards,
Stefan

>  
>  enum mipi_csis_clk {
>         MIPI_CSIS_CLK_PCLK,
> @@ -299,7 +333,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];
> @@ -654,8 +690,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>                         MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
>  
>         val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> -       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> -       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> +       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> +       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
>         val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
>         mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
>  
> @@ -672,7 +708,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>         /* Update the shadow register. */
>         val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
>         mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> -                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW |
> +                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
>                         MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
>  }
>  
> @@ -763,16 +799,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);
> @@ -849,7 +888,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;
> @@ -860,45 +899,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);
> @@ -1421,6 +1482,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;
>  }
>  
> @@ -1444,10 +1511,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
> 
>


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

* Re: [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks
  2025-08-22  0:27 ` [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks Laurent Pinchart
@ 2025-08-22 14:13   ` Frank Li
  2025-08-22 14:40     ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2025-08-22 14:13 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

On Fri, Aug 22, 2025 at 03:27:26AM +0300, Laurent Pinchart wrote:
> Multiple register field mask macros use GENMASK, while other define the
> mask value manually. Standardize on GENMASK everywhere, as well as on
> the _MASK suffix to name the macros. This improves consistency and helps
> with readability.
>
> No functional change is intended.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index ce889c436cb1..50f6f4468f7b 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -57,7 +57,7 @@

...
>
>  /* ISP Image Resolution register */
>  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> @@ -655,7 +655,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
>  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
>  	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> -	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK;
> +	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;

nit: if need create new verison, I suggest add "change MSK to MASK"
informaiton at commit message.

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

>  	mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
>
>  	mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
> --
> Regards,
>
> Laurent Pinchart
>


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

* Re: [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22  0:27 ` [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels Laurent Pinchart
  2025-08-22 14:12   ` Stefan Klug
@ 2025-08-22 14:18   ` Frank Li
  2025-08-22 17:39     ` Laurent Pinchart
  1 sibling, 1 reply; 23+ messages in thread
From: Frank Li @ 2025-08-22 14:18 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

On Fri, Aug 22, 2025 at 03:27:32AM +0300, Laurent Pinchart wrote:
> Some CSIS instances feature more than one output channel. Update
> register macros accordingly, parse the number of channels from the
> device tree, and update register dumps and event counters to log
> per-channel data.
>
> 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>
> ---
> Changes since v1:
>
> - Update more per-channel registers
> - Update commit message
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
>  1 file changed, 152 insertions(+), 87 deletions(-)
...
> +		return dev_err_probe(csis->dev, -EINVAL,
> +				     "Invalid fsl,num-channels value\n");
> +
>  	return 0;
>  }
>
> @@ -1444,10 +1511,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;
> -	}

I think this change is not belong to this patch.

Frank
>
>  	/* Acquire resources. */
>  	csis->regs = devm_platform_ioremap_resource(pdev, 0);
> --
> Regards,
>
> Laurent Pinchart
>


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

* Re: [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks
  2025-08-22 14:13   ` Frank Li
@ 2025-08-22 14:40     ` Laurent Pinchart
  2025-08-22 14:47       ` Frank Li
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22 14:40 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Hi Frank,

On Fri, Aug 22, 2025 at 10:13:34AM -0400, Frank Li wrote:
> On Fri, Aug 22, 2025 at 03:27:26AM +0300, Laurent Pinchart wrote:
> > Multiple register field mask macros use GENMASK, while other define the
> > mask value manually. Standardize on GENMASK everywhere, as well as on
> > the _MASK suffix to name the macros. This improves consistency and helps
> > with readability.
> >
> > No functional change is intended.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index ce889c436cb1..50f6f4468f7b 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -57,7 +57,7 @@
> 
> ...
> >
> >  /* ISP Image Resolution register */
> >  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> > @@ -655,7 +655,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> >  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> >  	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> > -	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK;
> > +	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> 
> nit: if need create new verison, I suggest add "change MSK to MASK"
> informaiton at commit message.

The information is there already, the commit message already states
"Standardize [...] on the _MASK suffix to name the macros".

> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 
> >  	mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> >
> >  	mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks
  2025-08-22 14:40     ` Laurent Pinchart
@ 2025-08-22 14:47       ` Frank Li
  0 siblings, 0 replies; 23+ messages in thread
From: Frank Li @ 2025-08-22 14:47 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

On Fri, Aug 22, 2025 at 05:40:00PM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> On Fri, Aug 22, 2025 at 10:13:34AM -0400, Frank Li wrote:
> > On Fri, Aug 22, 2025 at 03:27:26AM +0300, Laurent Pinchart wrote:
> > > Multiple register field mask macros use GENMASK, while other define the
> > > mask value manually. Standardize on GENMASK everywhere, as well as on
> > > the _MASK suffix to name the macros. This improves consistency and helps
> > > with readability.
> > >
> > > No functional change is intended.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/nxp/imx-mipi-csis.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index ce889c436cb1..50f6f4468f7b 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -57,7 +57,7 @@
> >
> > ...
> > >
> > >  /* ISP Image Resolution register */
> > >  #define MIPI_CSIS_ISP_RESOL_CH(n)		(0x44 + (n) * 0x10)
> > > @@ -655,7 +655,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > >  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> > >  	val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> > > -	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK;
> > > +	val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> >
> > nit: if need create new verison, I suggest add "change MSK to MASK"
> > informaiton at commit message.
>
> The information is there already, the commit message already states
> "Standardize [...] on the _MASK suffix to name the macros".

Sorry, I missed it.

Frank
>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> >
> > >  	mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> > >
> > >  	mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
>
> --
> Regards,
>
> Laurent Pinchart


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

* Re: [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22 14:18   ` Frank Li
@ 2025-08-22 17:39     ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22 17:39 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Hi Frank,

On Fri, Aug 22, 2025 at 10:18:28AM -0400, Frank Li wrote:
> On Fri, Aug 22, 2025 at 03:27:32AM +0300, Laurent Pinchart wrote:
> > Some CSIS instances feature more than one output channel. Update
> > register macros accordingly, parse the number of channels from the
> > device tree, and update register dumps and event counters to log
> > per-channel data.
> >
> > 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>
> > ---
> > Changes since v1:
> >
> > - Update more per-channel registers
> > - Update commit message
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
> >  1 file changed, 152 insertions(+), 87 deletions(-)
> ...
> > +		return dev_err_probe(csis->dev, -EINVAL,
> > +				     "Invalid fsl,num-channels value\n");
> > +
> >  	return 0;
> >  }
> >
> > @@ -1444,10 +1511,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;
> > -	}
> 
> I think this change is not belong to this patch.

The reason why this message is removed is because the
mipi_csis_parse_dt() function now prints error messages internally, so
this would be a duplicate.

> >
> >  	/* Acquire resources. */
> >  	csis->regs = devm_platform_ioremap_resource(pdev, 0);

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22 14:12   ` Stefan Klug
@ 2025-08-22 20:08     ` Laurent Pinchart
  2025-08-22 21:15       ` Stefan Klug
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2025-08-22 20:08 UTC (permalink / raw)
  To: Stefan Klug
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

On Fri, Aug 22, 2025 at 04:12:29PM +0200, Stefan Klug wrote:
> Quoting Laurent Pinchart (2025-08-22 02:27:32)
> > Some CSIS instances feature more than one output channel. Update
> > register macros accordingly, parse the number of channels from the
> > device tree, and update register dumps and event counters to log
> > per-channel data.
> > 
> > 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>
> > ---
> > Changes since v1:
> > 
> > - Update more per-channel registers
> > - Update commit message
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
> >  1 file changed, 152 insertions(+), 87 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index e1588990beda..ee8e541dcb1d 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -54,7 +54,7 @@
> >  
> >  /* CSIS common control */
> >  #define MIPI_CSIS_CMN_CTRL                     0x04
> > -#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW       BIT(16)
> > +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n)    BIT((n) + 16)
> >  #define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT  BIT(10)
> >  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)      ((n) << 8)
> >  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK    GENMASK(9, 8)
> > @@ -64,12 +64,9 @@
> >  
> >  /* CSIS clock control */
> >  #define MIPI_CSIS_CLK_CTRL                     0x08
> > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)        ((x) << 28)
> > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)        ((x) << 24)
> > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)        ((x) << 20)
> > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)        ((x) << 16)
> > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(n, x) ((x) << ((n) * 4 + 16))
> >  #define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK     GENMASK(7, 4)
> > -#define MIPI_CSIS_CLK_CTRL_WCLK_SRC            BIT(0)
> > +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC(n)         BIT(n)
> >  
> >  /* CSIS Interrupt mask */
> >  #define MIPI_CSIS_INT_MSK                      0x10
> > @@ -97,12 +94,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)
> > @@ -204,23 +201,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)
> >  
> > @@ -229,8 +226,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;
> > @@ -238,36 +238,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)
> 
> Why is that (MIPI_CSIS_NUM_EVENTS - 20)? Should that just be 20?

That would then be 38. I don't have a preference.

> The rest looks fine to me.
> 
> >  
> >  enum mipi_csis_clk {
> >         MIPI_CSIS_CLK_PCLK,
> > @@ -299,7 +333,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];
> > @@ -654,8 +690,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >                         MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
> >  
> >         val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > -       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> > -       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> > +       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > +       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> >         val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> >         mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> >  
> > @@ -672,7 +708,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >         /* Update the shadow register. */
> >         val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> >         mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > -                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW |
> > +                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> >                         MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> >  }
> >  
> > @@ -763,16 +799,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);
> > @@ -849,7 +888,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;
> > @@ -860,45 +899,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);
> > @@ -1421,6 +1482,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;
> >  }
> >  
> > @@ -1444,10 +1511,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


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

* Re: [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels
  2025-08-22 20:08     ` Laurent Pinchart
@ 2025-08-22 21:15       ` Stefan Klug
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Klug @ 2025-08-22 21:15 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Conor Dooley, Fabio Estevam, Inbaraj E, Isaac Scott,
	Krzysztof Kozlowski, Martin Kepplinger, Mauro Carvalho Chehab,
	Rob Herring, Rui Miguel Silva, Sascha Hauer, Shawn Guo,
	Pengutronix Kernel Team, Purism Kernel Team, devicetree, imx,
	linux-arm-kernel

Hi Laurent,

Quoting Laurent Pinchart (2025-08-22 22:08:38)
> On Fri, Aug 22, 2025 at 04:12:29PM +0200, Stefan Klug wrote:
> > Quoting Laurent Pinchart (2025-08-22 02:27:32)
> > > Some CSIS instances feature more than one output channel. Update
> > > register macros accordingly, parse the number of channels from the
> > > device tree, and update register dumps and event counters to log
> > > per-channel data.
> > > 
> > > 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>
> > > ---
> > > Changes since v1:
> > > 
> > > - Update more per-channel registers
> > > - Update commit message
> > > ---
> > >  drivers/media/platform/nxp/imx-mipi-csis.c | 239 +++++++++++++--------
> > >  1 file changed, 152 insertions(+), 87 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index e1588990beda..ee8e541dcb1d 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -54,7 +54,7 @@
> > >  
> > >  /* CSIS common control */
> > >  #define MIPI_CSIS_CMN_CTRL                     0x04
> > > -#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW       BIT(16)
> > > +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n)    BIT((n) + 16)
> > >  #define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT  BIT(10)
> > >  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)      ((n) << 8)
> > >  #define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK    GENMASK(9, 8)
> > > @@ -64,12 +64,9 @@
> > >  
> > >  /* CSIS clock control */
> > >  #define MIPI_CSIS_CLK_CTRL                     0x08
> > > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)        ((x) << 28)
> > > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)        ((x) << 24)
> > > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)        ((x) << 20)
> > > -#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)        ((x) << 16)
> > > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(n, x) ((x) << ((n) * 4 + 16))
> > >  #define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK     GENMASK(7, 4)
> > > -#define MIPI_CSIS_CLK_CTRL_WCLK_SRC            BIT(0)
> > > +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC(n)         BIT(n)
> > >  
> > >  /* CSIS Interrupt mask */
> > >  #define MIPI_CSIS_INT_MSK                      0x10
> > > @@ -97,12 +94,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)
> > > @@ -204,23 +201,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)
> > >  
> > > @@ -229,8 +226,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;
> > > @@ -238,36 +238,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)
> > 
> > Why is that (MIPI_CSIS_NUM_EVENTS - 20)? Should that just be 20?
> 
> That would then be 38. I don't have a preference.

Ugh, somehow my eyes locked in on *_INT_SRC_ERR_* which happen to be 20.
But there are 18 more errors after that. So 38 is correct. I think I'd
go for the plain 38, but either way is fine.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan

> 
> > The rest looks fine to me.
> > 
> > >  
> > >  enum mipi_csis_clk {
> > >         MIPI_CSIS_CLK_PCLK,
> > > @@ -299,7 +333,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];
> > > @@ -654,8 +690,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >                         MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
> > >  
> > >         val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > > -       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> > > -       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(15);
> > > +       val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > > +       val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> > >         val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> > >         mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> > >  
> > > @@ -672,7 +708,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >         /* Update the shadow register. */
> > >         val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > >         mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > > -                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW |
> > > +                       val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> > >                         MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> > >  }
> > >  
> > > @@ -763,16 +799,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);
> > > @@ -849,7 +888,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;
> > > @@ -860,45 +899,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);
> > > @@ -1421,6 +1482,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;
> > >  }
> > >  
> > > @@ -1444,10 +1511,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
>


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

end of thread, other threads:[~2025-08-23 10:49 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-22  0:27 [PATCH v3 00/13] media: imx-mipi-csis: Cleanups and debugging improvements Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 01/13] media: v4l2-common: Constify media_pad argument to v4l2_get_link_freq() Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 02/13] media: imx-mipi-csis: Simplify access to source pad Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 03/13] media: imx-mipi-csis: Standardize const keyword placement Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 04/13] media: imx-mipi-csis: Shorten name of subdev state variables Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 05/13] media: imx-mipi-csis: Rename register macros to match reference manual Laurent Pinchart
2025-08-22 14:10   ` Frank Li
2025-08-22  0:27 ` [PATCH v3 06/13] media: imx-mipi-csis: Use GENMASK for all register field masks Laurent Pinchart
2025-08-22 14:13   ` Frank Li
2025-08-22 14:40     ` Laurent Pinchart
2025-08-22 14:47       ` Frank Li
2025-08-22  0:27 ` [PATCH v3 07/13] media: imx-mipi-csis: Fix field alignment in register dump Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 08/13] media: imx-mipi-csis: Log per-lane start of transmission errors Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 09/13] media: imx-mipi-csis: Only set clock rate when specified in DT Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 10/13] dt-bindings: media: nxp,imx-mipi-csi2: Mark clock-frequency as deprecated Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 11/13] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 12/13] media: imx-mipi-csis: Initial support for multiple output channels Laurent Pinchart
2025-08-22 14:12   ` Stefan Klug
2025-08-22 20:08     ` Laurent Pinchart
2025-08-22 21:15       ` Stefan Klug
2025-08-22 14:18   ` Frank Li
2025-08-22 17:39     ` Laurent Pinchart
2025-08-22  0:27 ` [PATCH v3 13/13] arm64: dts: imx8mp: Specify the number of channels for CSI-2 receivers Laurent Pinchart

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