[PATCH v2 00/25] media: imx: imx7-mipi-csis: Add i.MX8MM support

[PATCH v2 00/25] media: imx: imx7-mipi-csis: Add i.MX8MM support

[Laurent Pinchart]

Hello,

This patch series adds support for the CSIS found in the NXP i.MX8MM SoC
to the imx7-mipi-csis driver.

The CSIS is an IP core from Samsung, integrated in different NXP SoCs.
The driver currently supports v3.3 of the CSIS, found in SoCs from the
i.MX6 and i.MX7 families. This series extends the driver to support
v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8
family.

The first 22 patches are miscellaneous cleanups and improvements. Please
see individual patches for details.

Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support.
Support for other members of the i.MX8 family will come later, and for
SoCs including an ISI IP core (such as the i.MX8MP) this will require
more work to handle additional glue logic.

Patch 24/25 then extends the imx7-mipi-csis driver accordingly.

Finally, patch 25/25 updates MAINTAINERS per popular request from people
who believe I have too much free time :-)

The series has been tested on an i.MX6ULL (for the CSIS v3.3) and
i.MX8MM (for the CSIS v3.6.3).

The changes in the integration of the CSIS between i.MX7 and i.MX8, as
described in the DT bindings, have been found through reading of
reference manuals and BSP source code, with different sources of
information contradicting each other. A confirmation from NXP would be
nice (in particular regarding the clocks).

Laurent Pinchart (25):
  media: imx: imx7_mipi_csis: Fix logging of only error event counters
  media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts
  media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel
    mode
  media: imx: imx7_mipi_csis: Move static data to top of
    mipi_csis_dump_regs()
  media: imx: imx7_mipi_csis: Minimize locking in get/set format
  media: imx: imx7_mipi_csis: Don't set subdev data
  media: imx: imx7_mipi_csis: Reorganize code in sections
  media: imx: imx7_mipi_csis: Set the CLKSETTLE register field
  media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure
  media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure
  media: imx: imx7_mipi_csis: Drop csi_state phy field
  media: imx: imx7_mipi_csis: Rename mipi_sd to sd
  media: imx: imx7_mipi_csis: Rename csi_state flag field to state
  media: imx: imx7_mipi_csis: Turn csi_state irq field into local
    variable
  media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt()
  media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init()
  media: imx: imx7_mipi_csis: Drop csi_state pdev field
  media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned
  media: imx: imx7_mipi_csis: Reorganize csi_state structure
  media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe()
  media: imx: imx7_mipi_csis: Reject invalid data-lanes settings
  media: imx: imx7_mipi_csis: Move PHY control to dedicated functions
  dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support
  media: imx: imx7_mipi_csis: Add i.MX8MM support
  media: imx: imx7_mipi_csis: Update MAINTAINERS

 .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 109 +-
 MAINTAINERS                                   |   1 +
 drivers/staging/media/imx/imx7-mipi-csis.c    | 994 ++++++++++--------
 3 files changed, 658 insertions(+), 446 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 01/25] media: imx: imx7_mipi_csis: Fix logging of only error event counters

[Laurent Pinchart]

The mipi_csis_events array ends with 6 non-error events, not 4. Update
mipi_csis_log_counters() accordingly. While at it, log event counters in
forward order, as there's no reason to log them backward.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 025fdc488bd6..25d0f89b2e53 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -666,13 +666,15 @@ static void mipi_csis_clear_counters(struct csi_state *state)
 
 static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 {
-	int i = non_errors ? MIPI_CSIS_NUM_EVENTS : MIPI_CSIS_NUM_EVENTS - 4;
+	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
+				: MIPI_CSIS_NUM_EVENTS - 6;
 	struct device *dev = &state->pdev->dev;
 	unsigned long flags;
+	unsigned int i;
 
 	spin_lock_irqsave(&state->slock, flags);
 
-	for (i--; i >= 0; i--) {
+	for (i = 0; i < num_events; ++i) {
 		if (state->events[i].counter > 0 || state->debug)
 			dev_info(dev, "%s events: %d\n", state->events[i].name,
 				 state->events[i].counter);
-- 
Regards,

Laurent Pinchart

[PATCH v2 02/25] media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts

[Laurent Pinchart]

In addition to the main interrupts that flag errors and other events,
the CSI-2 receiver has debug interrupt sources that flag various events
useful for debugging. Add those sources to the event counter mechanism
and print them when debugging is enabled.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 69 ++++++++++++++++------
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 25d0f89b2e53..67911eb8761f 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -195,6 +195,24 @@
 
 /* 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_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)
 
 /* Non-image packet data buffers */
 #define MIPI_CSIS_PKTDATA_ODD			0x2000
@@ -210,6 +228,7 @@ enum {
 };
 
 struct mipi_csis_event {
+	bool debug;
 	u32 mask;
 	const char * const name;
 	unsigned int counter;
@@ -217,22 +236,30 @@ struct mipi_csis_event {
 
 static const struct mipi_csis_event mipi_csis_events[] = {
 	/* Errors */
-	{ MIPI_CSIS_INT_SRC_ERR_SOT_HS,		"SOT Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_LOST_FS,	"Lost Frame Start Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_LOST_FE,	"Lost Frame End Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_OVER,		"FIFO Overflow Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_WRONG_CFG,	"Wrong Configuration Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_ECC,		"ECC Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_CRC,		"CRC Error" },
-	{ MIPI_CSIS_INT_SRC_ERR_UNKNOWN,	"Unknown Error" },
+	{ false, MIPI_CSIS_INT_SRC_ERR_SOT_HS,		"SOT 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_UNKNOWN,		"Unknown 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" },
 	/* Non-image data receive events */
-	{ MIPI_CSIS_INT_SRC_EVEN_BEFORE,	"Non-image data before even frame" },
-	{ MIPI_CSIS_INT_SRC_EVEN_AFTER,		"Non-image data after even frame" },
-	{ MIPI_CSIS_INT_SRC_ODD_BEFORE,		"Non-image data before odd frame" },
-	{ MIPI_CSIS_INT_SRC_ODD_AFTER,		"Non-image data after odd frame" },
+	{ 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" },
 	/* Frame start/end */
-	{ MIPI_CSIS_INT_SRC_FRAME_START,	"Frame Start" },
-	{ MIPI_CSIS_INT_SRC_FRAME_END,		"Frame 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" },
 };
 
 #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events)
@@ -455,6 +482,7 @@ static const struct csis_pix_format *find_csis_format(u32 code)
 static void mipi_csis_enable_interrupts(struct csi_state *state, bool on)
 {
 	mipi_csis_write(state, MIPI_CSIS_INT_MSK, on ? 0xffffffff : 0);
+	mipi_csis_write(state, MIPI_CSIS_DBG_INTR_MSK, on ? 0xffffffff : 0);
 }
 
 static void mipi_csis_sw_reset(struct csi_state *state)
@@ -667,7 +695,7 @@ static void mipi_csis_clear_counters(struct csi_state *state)
 static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 {
 	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
-				: MIPI_CSIS_NUM_EVENTS - 6;
+				: MIPI_CSIS_NUM_EVENTS - 8;
 	struct device *dev = &state->pdev->dev;
 	unsigned long flags;
 	unsigned int i;
@@ -963,22 +991,27 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
 	unsigned long flags;
 	unsigned int i;
 	u32 status;
+	u32 dbg_status;
 
 	status = mipi_csis_read(state, MIPI_CSIS_INT_SRC);
+	dbg_status = mipi_csis_read(state, MIPI_CSIS_DBG_INTR_SRC);
 
 	spin_lock_irqsave(&state->slock, flags);
 
 	/* Update the event/error counters */
 	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
 		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
-			if (!(status & state->events[i].mask))
-				continue;
-			state->events[i].counter++;
+			struct mipi_csis_event *event = &state->events[i];
+
+			if ((!event->debug && (status & event->mask)) ||
+			    (event->debug && (dbg_status & event->mask)))
+				event->counter++;
 		}
 	}
 	spin_unlock_irqrestore(&state->slock, flags);
 
 	mipi_csis_write(state, MIPI_CSIS_INT_SRC, status);
+	mipi_csis_write(state, MIPI_CSIS_DBG_INTR_SRC, dbg_status);
 
 	return IRQ_HANDLED;
 }
-- 
Regards,

Laurent Pinchart

[PATCH v2 03/25] media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel mode

[Laurent Pinchart]

The i.MX8MM expands the DOUBLE_CMPNT bit in the ISP_CONFIG register into
a two bits field that support quad pixel mode in addition to the single
and double modes. Update the ISP_CONFIG register macros to support this.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 67911eb8761f..f7c8b6d67e1c 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -166,7 +166,9 @@
 #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(x)	((x) << 24)
-#define MIPI_CSIS_ISPCFG_DOUBLE_CMPNT		BIT(12)
+#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_ALIGN_32BIT		BIT(11)
 #define MIPI_CSIS_ISPCFG_FMT_YCBCR422_8BIT	(0x1e << 2)
 #define MIPI_CSIS_ISPCFG_FMT_RAW8		(0x2a << 2)
-- 
Regards,

Laurent Pinchart

[PATCH v2 04/25] media: imx: imx7_mipi_csis: Move static data to top of mipi_csis_dump_regs()

[Laurent Pinchart]

It's customary to declare static variables at the top of the function,
with a blank line separating them from the non-static variables.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index f7c8b6d67e1c..25125e067aa7 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -429,9 +429,6 @@ static inline u32 mipi_csis_read(struct csi_state *state, u32 reg)
 
 static int mipi_csis_dump_regs(struct csi_state *state)
 {
-	struct device *dev = &state->pdev->dev;
-	unsigned int i;
-	u32 cfg;
 	static const struct {
 		u32 offset;
 		const char * const name;
@@ -450,6 +447,10 @@ static int mipi_csis_dump_regs(struct csi_state *state)
 		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
 	};
 
+	struct device *dev = &state->pdev->dev;
+	unsigned int i;
+	u32 cfg;
+
 	dev_info(dev, "--- REGISTERS ---\n");
 
 	for (i = 0; i < ARRAY_SIZE(registers); i++) {
-- 
Regards,

Laurent Pinchart

[PATCH v2 05/25] media: imx: imx7_mipi_csis: Minimize locking in get/set format

[Laurent Pinchart]

Reduce the code sections that are run with the lock held in the get/set
format handlers:

- mipi_csis_get_format() retrieves a pointer to the format, and thus
  doesn't need locking as long as the arguments passed to the function
  don't require locking either.

- sdformat is a structure passed by the caller, not an internal state,
  and thus doesn't require locking.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 36 ++++++++++++----------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 25125e067aa7..bbcd39672f92 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -860,8 +860,9 @@ static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
 	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
 	struct v4l2_mbus_framefmt *fmt;
 
-	mutex_lock(&state->lock);
 	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
+
+	mutex_lock(&state->lock);
 	sdformat->format = *fmt;
 	mutex_unlock(&state->lock);
 
@@ -919,24 +920,17 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
 	if (sdformat->pad != CSIS_PAD_SINK)
 		return -EINVAL;
 
-	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
-
-	mutex_lock(&state->lock);
-
-	/* Validate the media bus code and clamp the size. */
-	csis_fmt = find_csis_format(sdformat->format.code);
-	if (!csis_fmt)
-		csis_fmt = &mipi_csis_formats[0];
-
-	fmt->code = csis_fmt->code;
-	fmt->width = sdformat->format.width;
-	fmt->height = sdformat->format.height;
-
 	/*
+	 * Validate the media bus code and clamp and align the size.
+	 *
 	 * The total number of bits per line must be a multiple of 8. We thus
 	 * need to align the width for formats that are not multiples of 8
 	 * bits.
 	 */
+	csis_fmt = find_csis_format(sdformat->format.code);
+	if (!csis_fmt)
+		csis_fmt = &mipi_csis_formats[0];
+
 	switch (csis_fmt->width % 8) {
 	case 0:
 		align = 0;
@@ -956,8 +950,18 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
 		break;
 	}
 
-	v4l_bound_align_image(&fmt->width, 1, CSIS_MAX_PIX_WIDTH, align,
-			      &fmt->height, 1, CSIS_MAX_PIX_HEIGHT, 0, 0);
+	v4l_bound_align_image(&sdformat->format.width, 1,
+			      CSIS_MAX_PIX_WIDTH, align,
+			      &sdformat->format.height, 1,
+			      CSIS_MAX_PIX_HEIGHT, 0, 0);
+
+	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
+
+	mutex_lock(&state->lock);
+
+	fmt->code = csis_fmt->code;
+	fmt->width = sdformat->format.width;
+	fmt->height = sdformat->format.height;
 
 	sdformat->format = *fmt;
 
-- 
Regards,

Laurent Pinchart

[PATCH v2 06/25] media: imx: imx7_mipi_csis: Don't set subdev data

[Laurent Pinchart]

The driver doesn't need to store subdev data, as the subdev is embedded
in csi_state and is thus accessed using container_of.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index bbcd39672f92..030ef0925cd5 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1105,8 +1105,6 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
 	state->csis_fmt = &mipi_csis_formats[0];
 	mipi_csis_init_cfg(mipi_sd, NULL);
 
-	v4l2_set_subdevdata(mipi_sd, &pdev->dev);
-
 	state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK
 					 | MEDIA_PAD_FL_MUST_CONNECT;
 	state->pads[CSIS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
-- 
Regards,

Laurent Pinchart

[PATCH v2 07/25] media: imx: imx7_mipi_csis: Reorganize code in sections

[Laurent Pinchart]

Improve readability by reorganizing the code in sections. No functional
change intended.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 535 +++++++++++----------
 1 file changed, 281 insertions(+), 254 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 030ef0925cd5..eea865ba486d 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -324,6 +324,10 @@ struct csi_state {
 	struct regulator *mipi_phy_regulator;
 };
 
+/* -----------------------------------------------------------------------------
+ * Format helpers
+ */
+
 struct csis_pix_format {
 	u32 code;
 	u32 fmt_reg;
@@ -417,61 +421,6 @@ static const struct csis_pix_format mipi_csis_formats[] = {
 	}
 };
 
-static inline void mipi_csis_write(struct csi_state *state, u32 reg, u32 val)
-{
-	writel(val, state->regs + reg);
-}
-
-static inline u32 mipi_csis_read(struct csi_state *state, u32 reg)
-{
-	return readl(state->regs + reg);
-}
-
-static int mipi_csis_dump_regs(struct csi_state *state)
-{
-	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" },
-	};
-
-	struct device *dev = &state->pdev->dev;
-	unsigned int i;
-	u32 cfg;
-
-	dev_info(dev, "--- REGISTERS ---\n");
-
-	for (i = 0; i < ARRAY_SIZE(registers); i++) {
-		cfg = mipi_csis_read(state, registers[i].offset);
-		dev_info(dev, "%14s: 0x%08x\n", registers[i].name, cfg);
-	}
-
-	return 0;
-}
-
-static struct csi_state *
-mipi_notifier_to_csis_state(struct v4l2_async_notifier *n)
-{
-	return container_of(n, struct csi_state, notifier);
-}
-
-static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)
-{
-	return container_of(sdev, struct csi_state, mipi_sd);
-}
-
 static const struct csis_pix_format *find_csis_format(u32 code)
 {
 	unsigned int i;
@@ -482,6 +431,20 @@ static const struct csis_pix_format *find_csis_format(u32 code)
 	return NULL;
 }
 
+/* -----------------------------------------------------------------------------
+ * Hardware configuration
+ */
+
+static inline u32 mipi_csis_read(struct csi_state *state, u32 reg)
+{
+	return readl(state->regs + reg);
+}
+
+static inline void mipi_csis_write(struct csi_state *state, u32 reg, u32 val)
+{
+	writel(val, state->regs + reg);
+}
+
 static void mipi_csis_enable_interrupts(struct csi_state *state, bool on)
 {
 	mipi_csis_write(state, MIPI_CSIS_INT_MSK, on ? 0xffffffff : 0);
@@ -684,6 +647,41 @@ static void mipi_csis_stop_stream(struct csi_state *state)
 	mipi_csis_system_enable(state, false);
 }
 
+static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
+{
+	struct csi_state *state = dev_id;
+	unsigned long flags;
+	unsigned int i;
+	u32 status;
+	u32 dbg_status;
+
+	status = mipi_csis_read(state, MIPI_CSIS_INT_SRC);
+	dbg_status = mipi_csis_read(state, MIPI_CSIS_DBG_INTR_SRC);
+
+	spin_lock_irqsave(&state->slock, flags);
+
+	/* Update the event/error counters */
+	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
+		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
+			struct mipi_csis_event *event = &state->events[i];
+
+			if ((!event->debug && (status & event->mask)) ||
+			    (event->debug && (dbg_status & event->mask)))
+				event->counter++;
+		}
+	}
+	spin_unlock_irqrestore(&state->slock, flags);
+
+	mipi_csis_write(state, MIPI_CSIS_INT_SRC, status);
+	mipi_csis_write(state, MIPI_CSIS_DBG_INTR_SRC, dbg_status);
+
+	return IRQ_HANDLED;
+}
+
+/* -----------------------------------------------------------------------------
+ * Debug
+ */
+
 static void mipi_csis_clear_counters(struct csi_state *state)
 {
 	unsigned long flags;
@@ -713,9 +711,72 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 	spin_unlock_irqrestore(&state->slock, flags);
 }
 
-/*
+static int mipi_csis_dump_regs(struct csi_state *state)
+{
+	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" },
+	};
+
+	struct device *dev = &state->pdev->dev;
+	unsigned int i;
+	u32 cfg;
+
+	dev_info(dev, "--- REGISTERS ---\n");
+
+	for (i = 0; i < ARRAY_SIZE(registers); i++) {
+		cfg = mipi_csis_read(state, registers[i].offset);
+		dev_info(dev, "%14s: 0x%08x\n", registers[i].name, cfg);
+	}
+
+	return 0;
+}
+
+static int mipi_csis_dump_regs_show(struct seq_file *m, void *private)
+{
+	struct csi_state *state = m->private;
+
+	return mipi_csis_dump_regs(state);
+}
+DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
+
+static void mipi_csis_debugfs_init(struct csi_state *state)
+{
+	state->debugfs_root = debugfs_create_dir(dev_name(state->dev), NULL);
+
+	debugfs_create_bool("debug_enable", 0600, state->debugfs_root,
+			    &state->debug);
+	debugfs_create_file("dump_regs", 0600, state->debugfs_root, state,
+			    &mipi_csis_dump_regs_fops);
+}
+
+static void mipi_csis_debugfs_exit(struct csi_state *state)
+{
+	debugfs_remove_recursive(state->debugfs_root);
+}
+
+/* -----------------------------------------------------------------------------
  * V4L2 subdev operations
  */
+
+static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)
+{
+	return container_of(sdev, struct csi_state, mipi_sd);
+}
+
 static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, int enable)
 {
 	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
@@ -775,35 +836,6 @@ static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, int enable)
 	return ret;
 }
 
-static int mipi_csis_link_setup(struct media_entity *entity,
-				const struct media_pad *local_pad,
-				const struct media_pad *remote_pad, u32 flags)
-{
-	struct v4l2_subdev *mipi_sd = media_entity_to_v4l2_subdev(entity);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-	struct v4l2_subdev *remote_sd;
-
-	dev_dbg(state->dev, "link setup %s -> %s", remote_pad->entity->name,
-		local_pad->entity->name);
-
-	/* We only care about the link to the source. */
-	if (!(local_pad->flags & MEDIA_PAD_FL_SINK))
-		return 0;
-
-	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
-
-	if (flags & MEDIA_LNK_FL_ENABLED) {
-		if (state->src_sd)
-			return -EBUSY;
-
-		state->src_sd = remote_sd;
-	} else {
-		state->src_sd = NULL;
-	}
-
-	return 0;
-}
-
 static struct v4l2_mbus_framefmt *
 mipi_csis_get_format(struct csi_state *state,
 		     struct v4l2_subdev_pad_config *cfg,
@@ -992,47 +1024,10 @@ static int mipi_csis_log_status(struct v4l2_subdev *mipi_sd)
 	return 0;
 }
 
-static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
-{
-	struct csi_state *state = dev_id;
-	unsigned long flags;
-	unsigned int i;
-	u32 status;
-	u32 dbg_status;
-
-	status = mipi_csis_read(state, MIPI_CSIS_INT_SRC);
-	dbg_status = mipi_csis_read(state, MIPI_CSIS_DBG_INTR_SRC);
-
-	spin_lock_irqsave(&state->slock, flags);
-
-	/* Update the event/error counters */
-	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
-		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
-			struct mipi_csis_event *event = &state->events[i];
-
-			if ((!event->debug && (status & event->mask)) ||
-			    (event->debug && (dbg_status & event->mask)))
-				event->counter++;
-		}
-	}
-	spin_unlock_irqrestore(&state->slock, flags);
-
-	mipi_csis_write(state, MIPI_CSIS_INT_SRC, status);
-	mipi_csis_write(state, MIPI_CSIS_DBG_INTR_SRC, dbg_status);
-
-	return IRQ_HANDLED;
-}
-
 static const struct v4l2_subdev_core_ops mipi_csis_core_ops = {
 	.log_status	= mipi_csis_log_status,
 };
 
-static const struct media_entity_operations mipi_csis_entity_ops = {
-	.link_setup	= mipi_csis_link_setup,
-	.link_validate	= v4l2_subdev_link_validate,
-	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
-};
-
 static const struct v4l2_subdev_video_ops mipi_csis_video_ops = {
 	.s_stream	= mipi_csis_s_stream,
 };
@@ -1050,24 +1045,54 @@ static const struct v4l2_subdev_ops mipi_csis_subdev_ops = {
 	.pad	= &mipi_csis_pad_ops,
 };
 
-static int mipi_csis_parse_dt(struct platform_device *pdev,
-			      struct csi_state *state)
+/* -----------------------------------------------------------------------------
+ * Media entity operations
+ */
+
+static int mipi_csis_link_setup(struct media_entity *entity,
+				const struct media_pad *local_pad,
+				const struct media_pad *remote_pad, u32 flags)
 {
-	struct device_node *node = pdev->dev.of_node;
+	struct v4l2_subdev *mipi_sd = media_entity_to_v4l2_subdev(entity);
+	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_subdev *remote_sd;
 
-	if (of_property_read_u32(node, "clock-frequency",
-				 &state->clk_frequency))
-		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
+	dev_dbg(state->dev, "link setup %s -> %s", remote_pad->entity->name,
+		local_pad->entity->name);
 
-	/* Get MIPI PHY resets */
-	state->mrst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (IS_ERR(state->mrst))
-		return PTR_ERR(state->mrst);
+	/* We only care about the link to the source. */
+	if (!(local_pad->flags & MEDIA_PAD_FL_SINK))
+		return 0;
+
+	remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+	if (flags & MEDIA_LNK_FL_ENABLED) {
+		if (state->src_sd)
+			return -EBUSY;
+
+		state->src_sd = remote_sd;
+	} else {
+		state->src_sd = NULL;
+	}
 
 	return 0;
 }
 
-static int mipi_csis_pm_resume(struct device *dev, bool runtime);
+static const struct media_entity_operations mipi_csis_entity_ops = {
+	.link_setup	= mipi_csis_link_setup,
+	.link_validate	= v4l2_subdev_link_validate,
+	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
+};
+
+/* -----------------------------------------------------------------------------
+ * Async subdev notifier
+ */
+
+static struct csi_state *
+mipi_notifier_to_csis_state(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct csi_state, notifier);
+}
 
 static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
 				  struct v4l2_subdev *sd,
@@ -1083,36 +1108,6 @@ static const struct v4l2_async_notifier_operations mipi_csis_notify_ops = {
 	.bound = mipi_csis_notify_bound,
 };
 
-static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
-				 struct platform_device *pdev,
-				 const struct v4l2_subdev_ops *ops)
-{
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-
-	v4l2_subdev_init(mipi_sd, ops);
-	mipi_sd->owner = THIS_MODULE;
-	snprintf(mipi_sd->name, sizeof(mipi_sd->name), "%s.%d",
-		 CSIS_SUBDEV_NAME, state->index);
-
-	mipi_sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-	mipi_sd->ctrl_handler = NULL;
-
-	mipi_sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
-	mipi_sd->entity.ops = &mipi_csis_entity_ops;
-
-	mipi_sd->dev = &pdev->dev;
-
-	state->csis_fmt = &mipi_csis_formats[0];
-	mipi_csis_init_cfg(mipi_sd, NULL);
-
-	state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK
-					 | MEDIA_PAD_FL_MUST_CONNECT;
-	state->pads[CSIS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
-					   | MEDIA_PAD_FL_MUST_CONNECT;
-	return media_entity_pads_init(&mipi_sd->entity, CSIS_PADS_NUM,
-				      state->pads);
-}
-
 static int mipi_csis_async_register(struct csi_state *state)
 {
 	struct v4l2_fwnode_endpoint vep = {
@@ -1162,27 +1157,138 @@ static int mipi_csis_async_register(struct csi_state *state)
 	return ret;
 }
 
-static int mipi_csis_dump_regs_show(struct seq_file *m, void *private)
+/* -----------------------------------------------------------------------------
+ * Suspend/resume
+ */
+
+static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
+{
+	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	int ret = 0;
+
+	mutex_lock(&state->lock);
+	if (state->flags & ST_POWERED) {
+		mipi_csis_stop_stream(state);
+		ret = regulator_disable(state->mipi_phy_regulator);
+		if (ret)
+			goto unlock;
+		mipi_csis_clk_disable(state);
+		state->flags &= ~ST_POWERED;
+		if (!runtime)
+			state->flags |= ST_SUSPENDED;
+	}
+
+unlock:
+	mutex_unlock(&state->lock);
+
+	return ret ? -EAGAIN : 0;
+}
+
+static int mipi_csis_pm_resume(struct device *dev, bool runtime)
+{
+	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	int ret = 0;
+
+	mutex_lock(&state->lock);
+	if (!runtime && !(state->flags & ST_SUSPENDED))
+		goto unlock;
+
+	if (!(state->flags & ST_POWERED)) {
+		ret = regulator_enable(state->mipi_phy_regulator);
+		if (ret)
+			goto unlock;
+
+		state->flags |= ST_POWERED;
+		mipi_csis_clk_enable(state);
+	}
+	if (state->flags & ST_STREAMING)
+		mipi_csis_start_stream(state);
+
+	state->flags &= ~ST_SUSPENDED;
+
+unlock:
+	mutex_unlock(&state->lock);
+
+	return ret ? -EAGAIN : 0;
+}
+
+static int __maybe_unused mipi_csis_suspend(struct device *dev)
+{
+	return mipi_csis_pm_suspend(dev, false);
+}
+
+static int __maybe_unused mipi_csis_resume(struct device *dev)
 {
-	struct csi_state *state = m->private;
+	return mipi_csis_pm_resume(dev, false);
+}
 
-	return mipi_csis_dump_regs(state);
+static int __maybe_unused mipi_csis_runtime_suspend(struct device *dev)
+{
+	return mipi_csis_pm_suspend(dev, true);
 }
-DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
 
-static void mipi_csis_debugfs_init(struct csi_state *state)
+static int __maybe_unused mipi_csis_runtime_resume(struct device *dev)
+{
+	return mipi_csis_pm_resume(dev, true);
+}
+
+static const struct dev_pm_ops mipi_csis_pm_ops = {
+	SET_RUNTIME_PM_OPS(mipi_csis_runtime_suspend, mipi_csis_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(mipi_csis_suspend, mipi_csis_resume)
+};
+
+/* -----------------------------------------------------------------------------
+ * Probe/remove & platform driver
+ */
+
+static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
+				 struct platform_device *pdev,
+				 const struct v4l2_subdev_ops *ops)
 {
-	state->debugfs_root = debugfs_create_dir(dev_name(state->dev), NULL);
+	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
 
-	debugfs_create_bool("debug_enable", 0600, state->debugfs_root,
-			    &state->debug);
-	debugfs_create_file("dump_regs", 0600, state->debugfs_root, state,
-			    &mipi_csis_dump_regs_fops);
+	v4l2_subdev_init(mipi_sd, ops);
+	mipi_sd->owner = THIS_MODULE;
+	snprintf(mipi_sd->name, sizeof(mipi_sd->name), "%s.%d",
+		 CSIS_SUBDEV_NAME, state->index);
+
+	mipi_sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	mipi_sd->ctrl_handler = NULL;
+
+	mipi_sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	mipi_sd->entity.ops = &mipi_csis_entity_ops;
+
+	mipi_sd->dev = &pdev->dev;
+
+	state->csis_fmt = &mipi_csis_formats[0];
+	mipi_csis_init_cfg(mipi_sd, NULL);
+
+	state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+					 | MEDIA_PAD_FL_MUST_CONNECT;
+	state->pads[CSIS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
+					   | MEDIA_PAD_FL_MUST_CONNECT;
+	return media_entity_pads_init(&mipi_sd->entity, CSIS_PADS_NUM,
+				      state->pads);
 }
 
-static void mipi_csis_debugfs_exit(struct csi_state *state)
+static int mipi_csis_parse_dt(struct platform_device *pdev,
+			      struct csi_state *state)
 {
-	debugfs_remove_recursive(state->debugfs_root);
+	struct device_node *node = pdev->dev.of_node;
+
+	if (of_property_read_u32(node, "clock-frequency",
+				 &state->clk_frequency))
+		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
+
+	/* Get MIPI PHY resets */
+	state->mrst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(state->mrst))
+		return PTR_ERR(state->mrst);
+
+	return 0;
 }
 
 static int mipi_csis_probe(struct platform_device *pdev)
@@ -1280,79 +1386,6 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
-{
-	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-	int ret = 0;
-
-	mutex_lock(&state->lock);
-	if (state->flags & ST_POWERED) {
-		mipi_csis_stop_stream(state);
-		ret = regulator_disable(state->mipi_phy_regulator);
-		if (ret)
-			goto unlock;
-		mipi_csis_clk_disable(state);
-		state->flags &= ~ST_POWERED;
-		if (!runtime)
-			state->flags |= ST_SUSPENDED;
-	}
-
-unlock:
-	mutex_unlock(&state->lock);
-
-	return ret ? -EAGAIN : 0;
-}
-
-static int mipi_csis_pm_resume(struct device *dev, bool runtime)
-{
-	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-	int ret = 0;
-
-	mutex_lock(&state->lock);
-	if (!runtime && !(state->flags & ST_SUSPENDED))
-		goto unlock;
-
-	if (!(state->flags & ST_POWERED)) {
-		ret = regulator_enable(state->mipi_phy_regulator);
-		if (ret)
-			goto unlock;
-
-		state->flags |= ST_POWERED;
-		mipi_csis_clk_enable(state);
-	}
-	if (state->flags & ST_STREAMING)
-		mipi_csis_start_stream(state);
-
-	state->flags &= ~ST_SUSPENDED;
-
-unlock:
-	mutex_unlock(&state->lock);
-
-	return ret ? -EAGAIN : 0;
-}
-
-static int __maybe_unused mipi_csis_suspend(struct device *dev)
-{
-	return mipi_csis_pm_suspend(dev, false);
-}
-
-static int __maybe_unused mipi_csis_resume(struct device *dev)
-{
-	return mipi_csis_pm_resume(dev, false);
-}
-
-static int __maybe_unused mipi_csis_runtime_suspend(struct device *dev)
-{
-	return mipi_csis_pm_suspend(dev, true);
-}
-
-static int __maybe_unused mipi_csis_runtime_resume(struct device *dev)
-{
-	return mipi_csis_pm_resume(dev, true);
-}
-
 static int mipi_csis_remove(struct platform_device *pdev)
 {
 	struct v4l2_subdev *mipi_sd = platform_get_drvdata(pdev);
@@ -1373,12 +1406,6 @@ static int mipi_csis_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct dev_pm_ops mipi_csis_pm_ops = {
-	SET_RUNTIME_PM_OPS(mipi_csis_runtime_suspend, mipi_csis_runtime_resume,
-			   NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(mipi_csis_suspend, mipi_csis_resume)
-};
-
 static const struct of_device_id mipi_csis_of_match[] = {
 	{ .compatible = "fsl,imx7-mipi-csi2", },
 	{ /* sentinel */ },
-- 
Regards,

Laurent Pinchart

[PATCH v2 08/25] media: imx: imx7_mipi_csis: Set the CLKSETTLE register field

[Laurent Pinchart]

Set the CLKSETTLE field explicitly, with a value hardcoded to 0. This
brings no functional change, but prepares for calculation of the
CLKSETTLE value.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index eea865ba486d..224d3ac9c9bf 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -310,6 +310,7 @@ struct csi_state {
 
 	u32 clk_frequency;
 	u32 hs_settle;
+	u32 clk_settle;
 
 	struct reset_control *mrst;
 
@@ -540,11 +541,15 @@ static int mipi_csis_calculate_params(struct csi_state *state)
 
 	/*
 	 * The HSSETTLE counter value is document in a table, but can also
-	 * easily be calculated.
+	 * easily be calculated. Hardcode the CLKSETTLE value to 0 for now
+	 * (which is documented as corresponding to CSI-2 v0.87 to v1.00) until
+	 * we figure out how to compute it correctly.
 	 */
 	state->hs_settle = (lane_rate - 5000000) / 45000000;
-	dev_dbg(state->dev, "lane rate %u, Ths_settle %u\n",
-		lane_rate, state->hs_settle);
+	state->clk_settle = 0;
+
+	dev_dbg(state->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
+		lane_rate, state->clk_settle, state->hs_settle);
 
 	return 0;
 }
@@ -563,7 +568,8 @@ static void mipi_csis_set_params(struct csi_state *state)
 	__mipi_csis_set_format(state);
 
 	mipi_csis_write(state, MIPI_CSIS_DPHY_CMN_CTRL,
-			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(state->hs_settle));
+			MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(state->hs_settle) |
+			MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(state->clk_settle));
 
 	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
 	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
-- 
Regards,

Laurent Pinchart

[PATCH v2 09/25] media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure

[Laurent Pinchart]

The csis_hw_reset structure is instantiated as a member of csi_state,
but that member is never used. Drop it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 224d3ac9c9bf..695cb8aebcab 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -278,12 +278,6 @@ static const char * const mipi_csis_clk_id[] = {
 	"phy",
 };
 
-struct csis_hw_reset {
-	struct regmap *src;
-	u8 req_src;
-	u8 rst_bit;
-};
-
 struct csi_state {
 	/* lock elements below */
 	struct mutex lock;
@@ -321,7 +315,6 @@ struct csi_state {
 
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 
-	struct csis_hw_reset hw_reset;
 	struct regulator *mipi_phy_regulator;
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH v2 10/25] media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure

[Laurent Pinchart]

Replace the register value stored in the csis_pix_format structure with
the CSI-2 data type. The register value is simply computed from the data
type using a shift. This prepares for i.MX8MP support that needs the
same data type in a different hardware register.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 71 +++++++++++++---------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 695cb8aebcab..94afb103f951 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -170,13 +170,7 @@
 #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_ALIGN_32BIT		BIT(11)
-#define MIPI_CSIS_ISPCFG_FMT_YCBCR422_8BIT	(0x1e << 2)
-#define MIPI_CSIS_ISPCFG_FMT_RAW8		(0x2a << 2)
-#define MIPI_CSIS_ISPCFG_FMT_RAW10		(0x2b << 2)
-#define MIPI_CSIS_ISPCFG_FMT_RAW12		(0x2c << 2)
-#define MIPI_CSIS_ISPCFG_FMT_RAW14		(0x2d << 2)
-/* User defined formats, x = 1...4 */
-#define MIPI_CSIS_ISPCFG_FMT_USER(x)		((0x30 + (x) - 1) << 2)
+#define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
 #define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
 
 /* ISP Image Resolution register */
@@ -223,6 +217,25 @@
 
 #define DEFAULT_SCLK_CSIS_FREQ			166000000UL
 
+/* MIPI CSI-2 Data Types */
+#define MIPI_CSI2_DATA_TYPE_YUV420_8		0x18
+#define MIPI_CSI2_DATA_TYPE_YUV420_10		0x19
+#define MIPI_CSI2_DATA_TYPE_LE_YUV420_8		0x1a
+#define MIPI_CSI2_DATA_TYPE_CS_YUV420_8		0x1c
+#define MIPI_CSI2_DATA_TYPE_CS_YUV420_10	0x1d
+#define MIPI_CSI2_DATA_TYPE_YUV422_8		0x1e
+#define MIPI_CSI2_DATA_TYPE_YUV422_10		0x1f
+#define MIPI_CSI2_DATA_TYPE_RGB565		0x22
+#define MIPI_CSI2_DATA_TYPE_RGB666		0x23
+#define MIPI_CSI2_DATA_TYPE_RGB888		0x24
+#define MIPI_CSI2_DATA_TYPE_RAW6		0x28
+#define MIPI_CSI2_DATA_TYPE_RAW7		0x29
+#define MIPI_CSI2_DATA_TYPE_RAW8		0x2a
+#define MIPI_CSI2_DATA_TYPE_RAW10		0x2b
+#define MIPI_CSI2_DATA_TYPE_RAW12		0x2c
+#define MIPI_CSI2_DATA_TYPE_RAW14		0x2d
+#define MIPI_CSI2_DATA_TYPE_USER(x)		(0x30 + (x))
+
 enum {
 	ST_POWERED	= 1,
 	ST_STREAMING	= 2,
@@ -324,7 +337,7 @@ struct csi_state {
 
 struct csis_pix_format {
 	u32 code;
-	u32 fmt_reg;
+	u32 data_type;
 	u8 width;
 };
 
@@ -332,85 +345,85 @@ static const struct csis_pix_format mipi_csis_formats[] = {
 	/* YUV formats. */
 	{
 		.code = MEDIA_BUS_FMT_UYVY8_1X16,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_YCBCR422_8BIT,
+		.data_type = MIPI_CSI2_DATA_TYPE_YUV422_8,
 		.width = 16,
 	},
 	/* RAW (Bayer and greyscale) formats. */
 	{
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW8,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW8,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW8,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW8,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
 	}, {
 		.code = MEDIA_BUS_FMT_Y8_1X8,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW8,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW10,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW10,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW10,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW10,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
 	}, {
 		.code = MEDIA_BUS_FMT_Y10_1X10,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW10,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW12,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW12,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW12,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW12,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
 	}, {
 		.code = MEDIA_BUS_FMT_Y12_1X12,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW12,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW14,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW14,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW14,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
-		.fmt_reg = MIPI_CSIS_ISPCFG_FMT_RAW14,
+		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
 	}
 };
@@ -502,7 +515,7 @@ static void __mipi_csis_set_format(struct csi_state *state)
 	/* Color format */
 	val = mipi_csis_read(state, MIPI_CSIS_ISP_CONFIG_CH(0));
 	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK);
-	val |= state->csis_fmt->fmt_reg;
+	val |= MIPI_CSIS_ISPCFG_FMT(state->csis_fmt->data_type);
 	mipi_csis_write(state, MIPI_CSIS_ISP_CONFIG_CH(0), val);
 
 	/* Pixel resolution */
-- 
Regards,

Laurent Pinchart

[PATCH v2 11/25] media: imx: imx7_mipi_csis: Drop csi_state phy field

[Laurent Pinchart]

The phy field of the csi_state structure is unused. Drop it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 94afb103f951..78014ae02d34 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -304,7 +304,6 @@ struct csi_state {
 
 	u8 index;
 	struct platform_device *pdev;
-	struct phy *phy;
 	void __iomem *regs;
 	int irq;
 	u32 flags;
-- 
Regards,

Laurent Pinchart

[PATCH v2 12/25] media: imx: imx7_mipi_csis: Rename mipi_sd to sd

[Laurent Pinchart]

The CSIS is modelled as a single subdev, there's thus no ambiguity
regarding which subdev the code refers to. Rename mipi_sd to sd.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 91 +++++++++++-----------
 1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 78014ae02d34..6b9c05b1cdf3 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -298,7 +298,7 @@ struct csi_state {
 	spinlock_t slock;
 	struct device *dev;
 	struct media_pad pads[CSIS_PADS_NUM];
-	struct v4l2_subdev mipi_sd;
+	struct v4l2_subdev sd;
 	struct v4l2_async_notifier notifier;
 	struct v4l2_subdev *src_sd;
 
@@ -785,12 +785,12 @@ static void mipi_csis_debugfs_exit(struct csi_state *state)
 
 static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)
 {
-	return container_of(sdev, struct csi_state, mipi_sd);
+	return container_of(sdev, struct csi_state, sd);
 }
 
-static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, int enable)
+static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	int ret;
 
 	if (enable) {
@@ -854,15 +854,15 @@ mipi_csis_get_format(struct csi_state *state,
 		     unsigned int pad)
 {
 	if (which == V4L2_SUBDEV_FORMAT_TRY)
-		return v4l2_subdev_get_try_format(&state->mipi_sd, cfg, pad);
+		return v4l2_subdev_get_try_format(&state->sd, cfg, pad);
 
 	return &state->format_mbus;
 }
 
-static int mipi_csis_init_cfg(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_init_cfg(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	struct v4l2_mbus_framefmt *fmt_sink;
 	struct v4l2_mbus_framefmt *fmt_source;
 	enum v4l2_subdev_format_whence which;
@@ -896,11 +896,11 @@ static int mipi_csis_init_cfg(struct v4l2_subdev *mipi_sd,
 	return 0;
 }
 
-static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_get_fmt(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_format *sdformat)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	struct v4l2_mbus_framefmt *fmt;
 
 	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
@@ -912,11 +912,11 @@ static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
 	return 0;
 }
 
-static int mipi_csis_enum_mbus_code(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
 				    struct v4l2_subdev_pad_config *cfg,
 				    struct v4l2_subdev_mbus_code_enum *code)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 
 	/*
 	 * The CSIS can't transcode in any way, the source format is identical
@@ -944,11 +944,11 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *mipi_sd,
 	return 0;
 }
 
-static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_format *sdformat)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	struct csis_pix_format const *csis_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 	unsigned int align;
@@ -958,7 +958,7 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
 	 * modified.
 	 */
 	if (sdformat->pad == CSIS_PAD_SOURCE)
-		return mipi_csis_get_fmt(mipi_sd, cfg, sdformat);
+		return mipi_csis_get_fmt(sd, cfg, sdformat);
 
 	if (sdformat->pad != CSIS_PAD_SINK)
 		return -EINVAL;
@@ -1022,9 +1022,9 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
 	return 0;
 }
 
-static int mipi_csis_log_status(struct v4l2_subdev *mipi_sd)
+static int mipi_csis_log_status(struct v4l2_subdev *sd)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 
 	mutex_lock(&state->lock);
 	mipi_csis_log_counters(state, true);
@@ -1064,8 +1064,8 @@ static int mipi_csis_link_setup(struct media_entity *entity,
 				const struct media_pad *local_pad,
 				const struct media_pad *remote_pad, u32 flags)
 {
-	struct v4l2_subdev *mipi_sd = media_entity_to_v4l2_subdev(entity);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	struct v4l2_subdev *remote_sd;
 
 	dev_dbg(state->dev, "link setup %s -> %s", remote_pad->entity->name,
@@ -1110,7 +1110,7 @@ static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
 				  struct v4l2_async_subdev *asd)
 {
 	struct csi_state *state = mipi_notifier_to_csis_state(notifier);
-	struct media_pad *sink = &state->mipi_sd.entity.pads[CSIS_PAD_SINK];
+	struct media_pad *sink = &state->sd.entity.pads[CSIS_PAD_SINK];
 
 	return v4l2_create_fwnode_links_to_pad(sd, sink, 0);
 }
@@ -1155,12 +1155,11 @@ static int mipi_csis_async_register(struct csi_state *state)
 
 	state->notifier.ops = &mipi_csis_notify_ops;
 
-	ret = v4l2_async_subdev_notifier_register(&state->mipi_sd,
-						  &state->notifier);
+	ret = v4l2_async_subdev_notifier_register(&state->sd, &state->notifier);
 	if (ret)
 		return ret;
 
-	return v4l2_async_register_subdev(&state->mipi_sd);
+	return v4l2_async_register_subdev(&state->sd);
 
 err_parse:
 	fwnode_handle_put(ep);
@@ -1174,8 +1173,8 @@ static int mipi_csis_async_register(struct csi_state *state)
 
 static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
 {
-	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	int ret = 0;
 
 	mutex_lock(&state->lock);
@@ -1198,8 +1197,8 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
 
 static int mipi_csis_pm_resume(struct device *dev, bool runtime)
 {
-	struct v4l2_subdev *mipi_sd = dev_get_drvdata(dev);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 	int ret = 0;
 
 	mutex_lock(&state->lock);
@@ -1255,33 +1254,33 @@ static const struct dev_pm_ops mipi_csis_pm_ops = {
  * Probe/remove & platform driver
  */
 
-static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_subdev_init(struct v4l2_subdev *sd,
 				 struct platform_device *pdev,
 				 const struct v4l2_subdev_ops *ops)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 
-	v4l2_subdev_init(mipi_sd, ops);
-	mipi_sd->owner = THIS_MODULE;
-	snprintf(mipi_sd->name, sizeof(mipi_sd->name), "%s.%d",
+	v4l2_subdev_init(sd, ops);
+	sd->owner = THIS_MODULE;
+	snprintf(sd->name, sizeof(sd->name), "%s.%d",
 		 CSIS_SUBDEV_NAME, state->index);
 
-	mipi_sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-	mipi_sd->ctrl_handler = NULL;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->ctrl_handler = NULL;
 
-	mipi_sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
-	mipi_sd->entity.ops = &mipi_csis_entity_ops;
+	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	sd->entity.ops = &mipi_csis_entity_ops;
 
-	mipi_sd->dev = &pdev->dev;
+	sd->dev = &pdev->dev;
 
 	state->csis_fmt = &mipi_csis_formats[0];
-	mipi_csis_init_cfg(mipi_sd, NULL);
+	mipi_csis_init_cfg(sd, NULL);
 
 	state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK
 					 | MEDIA_PAD_FL_MUST_CONNECT;
 	state->pads[CSIS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
 					   | MEDIA_PAD_FL_MUST_CONNECT;
-	return media_entity_pads_init(&mipi_sd->entity, CSIS_PADS_NUM,
+	return media_entity_pads_init(&sd->entity, CSIS_PADS_NUM,
 				      state->pads);
 }
 
@@ -1354,10 +1353,10 @@ static int mipi_csis_probe(struct platform_device *pdev)
 		goto disable_clock;
 	}
 
-	platform_set_drvdata(pdev, &state->mipi_sd);
+	platform_set_drvdata(pdev, &state->sd);
 
 	mutex_init(&state->lock);
-	ret = mipi_csis_subdev_init(&state->mipi_sd, pdev,
+	ret = mipi_csis_subdev_init(&state->sd, pdev,
 				    &mipi_csis_subdev_ops);
 	if (ret < 0)
 		goto disable_clock;
@@ -1386,10 +1385,10 @@ static int mipi_csis_probe(struct platform_device *pdev)
 unregister_all:
 	mipi_csis_debugfs_exit(state);
 cleanup:
-	media_entity_cleanup(&state->mipi_sd.entity);
+	media_entity_cleanup(&state->sd.entity);
 	v4l2_async_notifier_unregister(&state->notifier);
 	v4l2_async_notifier_cleanup(&state->notifier);
-	v4l2_async_unregister_subdev(&state->mipi_sd);
+	v4l2_async_unregister_subdev(&state->sd);
 disable_clock:
 	mipi_csis_clk_disable(state);
 	mutex_destroy(&state->lock);
@@ -1399,18 +1398,18 @@ static int mipi_csis_probe(struct platform_device *pdev)
 
 static int mipi_csis_remove(struct platform_device *pdev)
 {
-	struct v4l2_subdev *mipi_sd = platform_get_drvdata(pdev);
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
+	struct csi_state *state = mipi_sd_to_csis_state(sd);
 
 	mipi_csis_debugfs_exit(state);
 	v4l2_async_notifier_unregister(&state->notifier);
 	v4l2_async_notifier_cleanup(&state->notifier);
-	v4l2_async_unregister_subdev(&state->mipi_sd);
+	v4l2_async_unregister_subdev(&state->sd);
 
 	pm_runtime_disable(&pdev->dev);
 	mipi_csis_pm_suspend(&pdev->dev, true);
 	mipi_csis_clk_disable(state);
-	media_entity_cleanup(&state->mipi_sd.entity);
+	media_entity_cleanup(&state->sd.entity);
 	mutex_destroy(&state->lock);
 	pm_runtime_set_suspended(&pdev->dev);
 
-- 
Regards,

Laurent Pinchart

[PATCH v2 13/25] media: imx: imx7_mipi_csis: Rename csi_state flag field to state

[Laurent Pinchart]

The flag field of the csi_state structure contains the device state.
Rename it accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 26 +++++++++++-----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 6b9c05b1cdf3..49b8afc0f2f8 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -306,7 +306,7 @@ struct csi_state {
 	struct platform_device *pdev;
 	void __iomem *regs;
 	int irq;
-	u32 flags;
+	u32 state;
 
 	struct dentry *debugfs_root;
 	bool debug;
@@ -813,7 +813,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 	mutex_lock(&state->lock);
 
 	if (enable) {
-		if (state->flags & ST_SUSPENDED) {
+		if (state->state & ST_SUSPENDED) {
 			ret = -EBUSY;
 			goto unlock;
 		}
@@ -825,14 +825,14 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 
 		mipi_csis_log_counters(state, true);
 
-		state->flags |= ST_STREAMING;
+		state->state |= ST_STREAMING;
 	} else {
 		v4l2_subdev_call(state->src_sd, video, s_stream, 0);
 		ret = v4l2_subdev_call(state->src_sd, core, s_power, 0);
 		if (ret == -ENOIOCTLCMD)
 			ret = 0;
 		mipi_csis_stop_stream(state);
-		state->flags &= ~ST_STREAMING;
+		state->state &= ~ST_STREAMING;
 		if (state->debug)
 			mipi_csis_log_counters(state, true);
 	}
@@ -1028,7 +1028,7 @@ static int mipi_csis_log_status(struct v4l2_subdev *sd)
 
 	mutex_lock(&state->lock);
 	mipi_csis_log_counters(state, true);
-	if (state->debug && (state->flags & ST_POWERED))
+	if (state->debug && (state->state & ST_POWERED))
 		mipi_csis_dump_regs(state);
 	mutex_unlock(&state->lock);
 
@@ -1178,15 +1178,15 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
 	int ret = 0;
 
 	mutex_lock(&state->lock);
-	if (state->flags & ST_POWERED) {
+	if (state->state & ST_POWERED) {
 		mipi_csis_stop_stream(state);
 		ret = regulator_disable(state->mipi_phy_regulator);
 		if (ret)
 			goto unlock;
 		mipi_csis_clk_disable(state);
-		state->flags &= ~ST_POWERED;
+		state->state &= ~ST_POWERED;
 		if (!runtime)
-			state->flags |= ST_SUSPENDED;
+			state->state |= ST_SUSPENDED;
 	}
 
 unlock:
@@ -1202,21 +1202,21 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime)
 	int ret = 0;
 
 	mutex_lock(&state->lock);
-	if (!runtime && !(state->flags & ST_SUSPENDED))
+	if (!runtime && !(state->state & ST_SUSPENDED))
 		goto unlock;
 
-	if (!(state->flags & ST_POWERED)) {
+	if (!(state->state & ST_POWERED)) {
 		ret = regulator_enable(state->mipi_phy_regulator);
 		if (ret)
 			goto unlock;
 
-		state->flags |= ST_POWERED;
+		state->state |= ST_POWERED;
 		mipi_csis_clk_enable(state);
 	}
-	if (state->flags & ST_STREAMING)
+	if (state->state & ST_STREAMING)
 		mipi_csis_start_stream(state);
 
-	state->flags &= ~ST_SUSPENDED;
+	state->state &= ~ST_SUSPENDED;
 
 unlock:
 	mutex_unlock(&state->lock);
-- 
Regards,

Laurent Pinchart

[PATCH v2 14/25] media: imx: imx7_mipi_csis: Turn csi_state irq field into local variable

[Laurent Pinchart]

The irq field of the csi_state structure is only used in
mipi_csis_probe(). Turn it into a local variable.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 49b8afc0f2f8..0b164436117c 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -305,7 +305,6 @@ struct csi_state {
 	u8 index;
 	struct platform_device *pdev;
 	void __iomem *regs;
-	int irq;
 	u32 state;
 
 	struct dentry *debugfs_root;
@@ -1305,6 +1304,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct csi_state *state;
+	int irq;
 	int ret;
 
 	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
@@ -1332,9 +1332,9 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	if (IS_ERR(state->regs))
 		return PTR_ERR(state->regs);
 
-	state->irq = platform_get_irq(pdev, 0);
-	if (state->irq < 0)
-		return state->irq;
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
 
 	ret = mipi_csis_clk_get(state);
 	if (ret < 0)
@@ -1346,8 +1346,8 @@ static int mipi_csis_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_request_irq(dev, state->irq, mipi_csis_irq_handler,
-			       0, dev_name(dev), state);
+	ret = devm_request_irq(dev, irq, mipi_csis_irq_handler, 0,
+			       dev_name(dev), state);
 	if (ret) {
 		dev_err(dev, "Interrupt request failed\n");
 		goto disable_clock;
-- 
Regards,

Laurent Pinchart

[PATCH v2 15/25] media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt()

[Laurent Pinchart]

The mipi_csis_parse_dt() function is called with a pointer to the
csi_state, which contains all the information necessary. Don't pass the
platform device pointer as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 0b164436117c..19fa4891737d 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1283,17 +1283,16 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *sd,
 				      state->pads);
 }
 
-static int mipi_csis_parse_dt(struct platform_device *pdev,
-			      struct csi_state *state)
+static int mipi_csis_parse_dt(struct csi_state *state)
 {
-	struct device_node *node = pdev->dev.of_node;
+	struct device_node *node = state->dev->of_node;
 
 	if (of_property_read_u32(node, "clock-frequency",
 				 &state->clk_frequency))
 		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
 
 	/* Get MIPI PHY resets */
-	state->mrst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
 	if (IS_ERR(state->mrst))
 		return PTR_ERR(state->mrst);
 
@@ -1316,7 +1315,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	state->pdev = pdev;
 	state->dev = dev;
 
-	ret = mipi_csis_parse_dt(pdev, state);
+	ret = mipi_csis_parse_dt(state);
 	if (ret < 0) {
 		dev_err(dev, "Failed to parse device tree: %d\n", ret);
 		return ret;
-- 
Regards,

Laurent Pinchart

[PATCH v2 16/25] media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init()

[Laurent Pinchart]

Pass the csi_state pointer to the mipi_csis_subdev_init() function,
instead of miscellaneous information scattered in different arguments.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 19fa4891737d..2548f6442619 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1253,13 +1253,11 @@ static const struct dev_pm_ops mipi_csis_pm_ops = {
  * Probe/remove & platform driver
  */
 
-static int mipi_csis_subdev_init(struct v4l2_subdev *sd,
-				 struct platform_device *pdev,
-				 const struct v4l2_subdev_ops *ops)
+static int mipi_csis_subdev_init(struct csi_state *state)
 {
-	struct csi_state *state = mipi_sd_to_csis_state(sd);
+	struct v4l2_subdev *sd = &state->sd;
 
-	v4l2_subdev_init(sd, ops);
+	v4l2_subdev_init(sd, &mipi_csis_subdev_ops);
 	sd->owner = THIS_MODULE;
 	snprintf(sd->name, sizeof(sd->name), "%s.%d",
 		 CSIS_SUBDEV_NAME, state->index);
@@ -1270,7 +1268,7 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *sd,
 	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
 	sd->entity.ops = &mipi_csis_entity_ops;
 
-	sd->dev = &pdev->dev;
+	sd->dev = state->dev;
 
 	state->csis_fmt = &mipi_csis_formats[0];
 	mipi_csis_init_cfg(sd, NULL);
@@ -1355,8 +1353,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, &state->sd);
 
 	mutex_init(&state->lock);
-	ret = mipi_csis_subdev_init(&state->sd, pdev,
-				    &mipi_csis_subdev_ops);
+	ret = mipi_csis_subdev_init(state);
 	if (ret < 0)
 		goto disable_clock;
 
-- 
Regards,

Laurent Pinchart

[PATCH v2 17/25] media: imx: imx7_mipi_csis: Drop csi_state pdev field

[Laurent Pinchart]

The pdev field of the csi_state structure is only used to access the
device pointer, which is stored in a separate field. Drop the pdev
field, as well as a few local dev variables.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 32 ++++++++++------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 2548f6442619..1cb8eeb2fdac 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -303,7 +303,6 @@ struct csi_state {
 	struct v4l2_subdev *src_sd;
 
 	u8 index;
-	struct platform_device *pdev;
 	void __iomem *regs;
 	u32 state;
 
@@ -615,13 +614,12 @@ static void mipi_csis_clk_disable(struct csi_state *state)
 
 static int mipi_csis_clk_get(struct csi_state *state)
 {
-	struct device *dev = &state->pdev->dev;
 	unsigned int i;
 	int ret;
 
 	state->num_clks = ARRAY_SIZE(mipi_csis_clk_id);
-	state->clks = devm_kcalloc(dev, state->num_clks, sizeof(*state->clks),
-				   GFP_KERNEL);
+	state->clks = devm_kcalloc(state->dev, state->num_clks,
+				   sizeof(*state->clks), GFP_KERNEL);
 
 	if (!state->clks)
 		return -ENOMEM;
@@ -629,7 +627,7 @@ static int mipi_csis_clk_get(struct csi_state *state)
 	for (i = 0; i < state->num_clks; i++)
 		state->clks[i].id = mipi_csis_clk_id[i];
 
-	ret = devm_clk_bulk_get(dev, state->num_clks, state->clks);
+	ret = devm_clk_bulk_get(state->dev, state->num_clks, state->clks);
 	if (ret < 0)
 		return ret;
 
@@ -637,8 +635,8 @@ static int mipi_csis_clk_get(struct csi_state *state)
 	ret = clk_set_rate(state->clks[MIPI_CSIS_CLK_WRAP].clk,
 			   state->clk_frequency);
 	if (ret < 0)
-		dev_err(dev, "set rate=%d failed: %d\n", state->clk_frequency,
-			ret);
+		dev_err(state->dev, "set rate=%d failed: %d\n",
+			state->clk_frequency, ret);
 
 	return ret;
 }
@@ -707,7 +705,6 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 {
 	unsigned int num_events = non_errors ? MIPI_CSIS_NUM_EVENTS
 				: MIPI_CSIS_NUM_EVENTS - 8;
-	struct device *dev = &state->pdev->dev;
 	unsigned long flags;
 	unsigned int i;
 
@@ -715,7 +712,8 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 
 	for (i = 0; i < num_events; ++i) {
 		if (state->events[i].counter > 0 || state->debug)
-			dev_info(dev, "%s events: %d\n", state->events[i].name,
+			dev_info(state->dev, "%s events: %d\n",
+				 state->events[i].name,
 				 state->events[i].counter);
 	}
 	spin_unlock_irqrestore(&state->slock, flags);
@@ -741,15 +739,14 @@ static int mipi_csis_dump_regs(struct csi_state *state)
 		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
 	};
 
-	struct device *dev = &state->pdev->dev;
 	unsigned int i;
 	u32 cfg;
 
-	dev_info(dev, "--- REGISTERS ---\n");
+	dev_info(state->dev, "--- REGISTERS ---\n");
 
 	for (i = 0; i < ARRAY_SIZE(registers); i++) {
 		cfg = mipi_csis_read(state, registers[i].offset);
-		dev_info(dev, "%14s: 0x%08x\n", registers[i].name, cfg);
+		dev_info(state->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
 	}
 
 	return 0;
@@ -799,9 +796,9 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 
 		mipi_csis_clear_counters(state);
 
-		ret = pm_runtime_get_sync(&state->pdev->dev);
+		ret = pm_runtime_get_sync(state->dev);
 		if (ret < 0) {
-			pm_runtime_put_noidle(&state->pdev->dev);
+			pm_runtime_put_noidle(state->dev);
 			return ret;
 		}
 		ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
@@ -841,7 +838,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 
 done:
 	if (!enable || ret < 0)
-		pm_runtime_put(&state->pdev->dev);
+		pm_runtime_put(state->dev);
 
 	return ret;
 }
@@ -1310,7 +1307,6 @@ static int mipi_csis_probe(struct platform_device *pdev)
 
 	spin_lock_init(&state->slock);
 
-	state->pdev = pdev;
 	state->dev = dev;
 
 	ret = mipi_csis_parse_dt(state);
@@ -1359,7 +1355,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 
 	ret = mipi_csis_async_register(state);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "async register failed: %d\n", ret);
+		dev_err(dev, "async register failed: %d\n", ret);
 		goto cleanup;
 	}
 
@@ -1373,7 +1369,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 			goto unregister_all;
 	}
 
-	dev_info(&pdev->dev, "lanes: %d, freq: %u\n",
+	dev_info(dev, "lanes: %d, freq: %u\n",
 		 state->bus.num_data_lanes, state->clk_frequency);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH v2 18/25] media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned

[Laurent Pinchart]

The num_clocks field of the csi_state only stores positive values, make
it unsigned.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 1cb8eeb2fdac..0ec6870f98a8 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -309,7 +309,7 @@ struct csi_state {
 	struct dentry *debugfs_root;
 	bool debug;
 
-	int num_clks;
+	unsigned int num_clks;
 	struct clk_bulk_data *clks;
 
 	u32 clk_frequency;
-- 
Regards,

Laurent Pinchart

[PATCH v2 19/25] media: imx: imx7_mipi_csis: Reorganize csi_state structure

[Laurent Pinchart]

Group the fiels of the csi_state structure logically to improve
readability.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 35 +++++++++-------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 0ec6870f98a8..61da4db292ef 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -292,40 +292,33 @@ static const char * const mipi_csis_clk_id[] = {
 };
 
 struct csi_state {
-	/* lock elements below */
-	struct mutex lock;
-	/* lock for event handler */
-	spinlock_t slock;
 	struct device *dev;
-	struct media_pad pads[CSIS_PADS_NUM];
-	struct v4l2_subdev sd;
-	struct v4l2_async_notifier notifier;
-	struct v4l2_subdev *src_sd;
-
-	u8 index;
 	void __iomem *regs;
-	u32 state;
-
-	struct dentry *debugfs_root;
-	bool debug;
-
 	unsigned int num_clks;
 	struct clk_bulk_data *clks;
+	struct reset_control *mrst;
+	struct regulator *mipi_phy_regulator;
+	u8 index;
 
+	struct v4l2_subdev sd;
+	struct media_pad pads[CSIS_PADS_NUM];
+	struct v4l2_async_notifier notifier;
+	struct v4l2_subdev *src_sd;
+
+	struct v4l2_fwnode_bus_mipi_csi2 bus;
 	u32 clk_frequency;
 	u32 hs_settle;
 	u32 clk_settle;
 
-	struct reset_control *mrst;
-
+	struct mutex lock;	/* Protect csis_fmt, format_mbus and state */
 	const struct csis_pix_format *csis_fmt;
 	struct v4l2_mbus_framefmt format_mbus;
+	u32 state;
 
-	struct v4l2_fwnode_bus_mipi_csi2 bus;
-
+	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
-
-	struct regulator *mipi_phy_regulator;
+	struct dentry *debugfs_root;
+	bool debug;
 };
 
 /* -----------------------------------------------------------------------------
-- 
Regards,

Laurent Pinchart

[PATCH v2 20/25] media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe()

[Laurent Pinchart]

Group the operations performed in mipi_csis_probe() logically to improve
readability.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++++++++++++++--------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 61da4db292ef..07b331667db7 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1298,22 +1298,21 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	if (!state)
 		return -ENOMEM;
 
+	mutex_init(&state->lock);
 	spin_lock_init(&state->slock);
 
 	state->dev = dev;
 
+	memcpy(state->events, mipi_csis_events, sizeof(state->events));
+
+	/* Parse DT properties. */
 	ret = mipi_csis_parse_dt(state);
 	if (ret < 0) {
 		dev_err(dev, "Failed to parse device tree: %d\n", ret);
 		return ret;
 	}
 
-	ret = mipi_csis_phy_init(state);
-	if (ret < 0)
-		return ret;
-
-	mipi_csis_phy_reset(state);
-
+	/* Acquire resources. */
 	state->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(state->regs))
 		return PTR_ERR(state->regs);
@@ -1322,16 +1321,24 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	ret = mipi_csis_phy_init(state);
+	if (ret < 0)
+		return ret;
+
 	ret = mipi_csis_clk_get(state);
 	if (ret < 0)
 		return ret;
 
+	/* Reset PHY and enable the clocks. */
+	mipi_csis_phy_reset(state);
+
 	ret = mipi_csis_clk_enable(state);
 	if (ret < 0) {
 		dev_err(state->dev, "failed to enable clocks: %d\n", ret);
 		return ret;
 	}
 
+	/* Now that the hardware is initialized, request the interrupt. */
 	ret = devm_request_irq(dev, irq, mipi_csis_irq_handler, 0,
 			       dev_name(dev), state);
 	if (ret) {
@@ -1339,22 +1346,23 @@ static int mipi_csis_probe(struct platform_device *pdev)
 		goto disable_clock;
 	}
 
-	platform_set_drvdata(pdev, &state->sd);
-
-	mutex_init(&state->lock);
+	/* Initialize and register the subdev. */
 	ret = mipi_csis_subdev_init(state);
 	if (ret < 0)
 		goto disable_clock;
 
+	platform_set_drvdata(pdev, &state->sd);
+
 	ret = mipi_csis_async_register(state);
 	if (ret < 0) {
 		dev_err(dev, "async register failed: %d\n", ret);
 		goto cleanup;
 	}
 
-	memcpy(state->events, mipi_csis_events, sizeof(state->events));
-
+	/* Initialize debugfs. */
 	mipi_csis_debugfs_init(state);
+
+	/* Enable runtime PM. */
 	pm_runtime_enable(dev);
 	if (!pm_runtime_enabled(dev)) {
 		ret = mipi_csis_pm_resume(dev, true);
-- 
Regards,

Laurent Pinchart

[PATCH v2 21/25] media: imx: imx7_mipi_csis: Reject invalid data-lanes settings

[Laurent Pinchart]

The CSIS doesn't support data lanes reordering. Reject invalid settings.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 07b331667db7..6e235c86e0aa 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1115,6 +1115,7 @@ static int mipi_csis_async_register(struct csi_state *state)
 	};
 	struct v4l2_async_subdev *asd;
 	struct fwnode_handle *ep;
+	unsigned int i;
 	int ret;
 
 	v4l2_async_notifier_init(&state->notifier);
@@ -1128,6 +1129,14 @@ static int mipi_csis_async_register(struct csi_state *state)
 	if (ret)
 		goto err_parse;
 
+	for (i = 0; i < vep.bus.mipi_csi2.num_data_lanes; ++i) {
+		if (vep.bus.mipi_csi2.data_lanes[i] != i + 1) {
+			dev_err(state->dev,
+				"data lanes reordering is not supported");
+			goto err_parse;
+		}
+	}
+
 	state->bus = vep.bus.mipi_csi2;
 
 	dev_dbg(state->dev, "data lanes: %d\n", state->bus.num_data_lanes);
-- 
Regards,

Laurent Pinchart

[PATCH v2 22/25] media: imx: imx7_mipi_csis: Move PHY control to dedicated functions

[Laurent Pinchart]

Move the PHY regulator and reset handling to dedicated functions. This
groups all related code together, and prepares for i.MX8 support that
doesn't require control of the PHY regulator and reset.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 64 +++++++++++++---------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 6e235c86e0aa..a8da8d6ddb7d 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -457,25 +457,6 @@ static void mipi_csis_sw_reset(struct csi_state *state)
 	usleep_range(10, 20);
 }
 
-static int mipi_csis_phy_init(struct csi_state *state)
-{
-	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
-	if (IS_ERR(state->mipi_phy_regulator))
-		return PTR_ERR(state->mipi_phy_regulator);
-
-	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
-				     1000000);
-}
-
-static void mipi_csis_phy_reset(struct csi_state *state)
-{
-	reset_control_assert(state->mrst);
-
-	msleep(20);
-
-	reset_control_deassert(state->mrst);
-}
-
 static void mipi_csis_system_enable(struct csi_state *state, int on)
 {
 	u32 val, mask;
@@ -679,6 +660,42 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/* -----------------------------------------------------------------------------
+ * PHY regulator and reset
+ */
+
+static int mipi_csis_phy_enable(struct csi_state *state)
+{
+	return regulator_enable(state->mipi_phy_regulator);
+}
+
+static int mipi_csis_phy_disable(struct csi_state *state)
+{
+	return regulator_disable(state->mipi_phy_regulator);
+}
+
+static void mipi_csis_phy_reset(struct csi_state *state)
+{
+	reset_control_assert(state->mrst);
+	msleep(20);
+	reset_control_deassert(state->mrst);
+}
+
+static int mipi_csis_phy_init(struct csi_state *state)
+{
+	/* Get MIPI PHY reset and regulator. */
+	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
+	if (IS_ERR(state->mrst))
+		return PTR_ERR(state->mrst);
+
+	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
+	if (IS_ERR(state->mipi_phy_regulator))
+		return PTR_ERR(state->mipi_phy_regulator);
+
+	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
+				     1000000);
+}
+
 /* -----------------------------------------------------------------------------
  * Debug
  */
@@ -1178,7 +1195,7 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
 	mutex_lock(&state->lock);
 	if (state->state & ST_POWERED) {
 		mipi_csis_stop_stream(state);
-		ret = regulator_disable(state->mipi_phy_regulator);
+		ret = mipi_csis_phy_disable(state);
 		if (ret)
 			goto unlock;
 		mipi_csis_clk_disable(state);
@@ -1204,7 +1221,7 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime)
 		goto unlock;
 
 	if (!(state->state & ST_POWERED)) {
-		ret = regulator_enable(state->mipi_phy_regulator);
+		ret = mipi_csis_phy_enable(state);
 		if (ret)
 			goto unlock;
 
@@ -1288,11 +1305,6 @@ static int mipi_csis_parse_dt(struct csi_state *state)
 				 &state->clk_frequency))
 		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
 
-	/* Get MIPI PHY resets */
-	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
-	if (IS_ERR(state->mrst))
-		return PTR_ERR(state->mrst);
-
 	return 0;
 }
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 22/25] media: imx: imx7_mipi_csis: Move PHY control to dedicated functions

[Laurent Pinchart]

On Sun, May 16, 2021 at 04:44:38AM +0300, Laurent Pinchart wrote:
> Move the PHY regulator and reset handling to dedicated functions. This
> groups all related code together, and prepares for i.MX8 support that
> doesn't require control of the PHY regulator and reset.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

My apologies, this is a new patch in v2, and Rui hasn't acked it. I'll
thus wait for acks and reviews before sending a pull request.

> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 64 +++++++++++++---------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 6e235c86e0aa..a8da8d6ddb7d 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -457,25 +457,6 @@ static void mipi_csis_sw_reset(struct csi_state *state)
>  	usleep_range(10, 20);
>  }
>  
> -static int mipi_csis_phy_init(struct csi_state *state)
> -{
> -	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> -	if (IS_ERR(state->mipi_phy_regulator))
> -		return PTR_ERR(state->mipi_phy_regulator);
> -
> -	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
> -				     1000000);
> -}
> -
> -static void mipi_csis_phy_reset(struct csi_state *state)
> -{
> -	reset_control_assert(state->mrst);
> -
> -	msleep(20);
> -
> -	reset_control_deassert(state->mrst);
> -}
> -
>  static void mipi_csis_system_enable(struct csi_state *state, int on)
>  {
>  	u32 val, mask;
> @@ -679,6 +660,42 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +/* -----------------------------------------------------------------------------
> + * PHY regulator and reset
> + */
> +
> +static int mipi_csis_phy_enable(struct csi_state *state)
> +{
> +	return regulator_enable(state->mipi_phy_regulator);
> +}
> +
> +static int mipi_csis_phy_disable(struct csi_state *state)
> +{
> +	return regulator_disable(state->mipi_phy_regulator);
> +}
> +
> +static void mipi_csis_phy_reset(struct csi_state *state)
> +{
> +	reset_control_assert(state->mrst);
> +	msleep(20);
> +	reset_control_deassert(state->mrst);
> +}
> +
> +static int mipi_csis_phy_init(struct csi_state *state)
> +{
> +	/* Get MIPI PHY reset and regulator. */
> +	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
> +	if (IS_ERR(state->mrst))
> +		return PTR_ERR(state->mrst);
> +
> +	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> +	if (IS_ERR(state->mipi_phy_regulator))
> +		return PTR_ERR(state->mipi_phy_regulator);
> +
> +	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
> +				     1000000);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Debug
>   */
> @@ -1178,7 +1195,7 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
>  	mutex_lock(&state->lock);
>  	if (state->state & ST_POWERED) {
>  		mipi_csis_stop_stream(state);
> -		ret = regulator_disable(state->mipi_phy_regulator);
> +		ret = mipi_csis_phy_disable(state);
>  		if (ret)
>  			goto unlock;
>  		mipi_csis_clk_disable(state);
> @@ -1204,7 +1221,7 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime)
>  		goto unlock;
>  
>  	if (!(state->state & ST_POWERED)) {
> -		ret = regulator_enable(state->mipi_phy_regulator);
> +		ret = mipi_csis_phy_enable(state);
>  		if (ret)
>  			goto unlock;
>  
> @@ -1288,11 +1305,6 @@ static int mipi_csis_parse_dt(struct csi_state *state)
>  				 &state->clk_frequency))
>  		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
>  
> -	/* Get MIPI PHY resets */
> -	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
> -	if (IS_ERR(state->mrst))
> -		return PTR_ERR(state->mrst);
> -
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 22/25] media: imx: imx7_mipi_csis: Move PHY control to dedicated functions

[Rui Miguel Silva]

Hi Laurent,
On Sun May 16, 2021 at 3:18 AM WEST, Laurent Pinchart wrote:

> On Sun, May 16, 2021 at 04:44:38AM +0300, Laurent Pinchart wrote:
> > Move the PHY regulator and reset handling to dedicated functions. This
> > groups all related code together, and prepares for i.MX8 support that
> > doesn't require control of the PHY regulator and reset.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
>
> My apologies, this is a new patch in v2, and Rui hasn't acked it. I'll
> thus wait for acks and reviews before sending a pull request.

Looks good. Thanks for the heads up.

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
Cheers,
     Rui
>
> > ---
> >  drivers/staging/media/imx/imx7-mipi-csis.c | 64 +++++++++++++---------
> >  1 file changed, 38 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> > index 6e235c86e0aa..a8da8d6ddb7d 100644
> > --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> > @@ -457,25 +457,6 @@ static void mipi_csis_sw_reset(struct csi_state *state)
> >  	usleep_range(10, 20);
> >  }
> >  
> > -static int mipi_csis_phy_init(struct csi_state *state)
> > -{
> > -	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> > -	if (IS_ERR(state->mipi_phy_regulator))
> > -		return PTR_ERR(state->mipi_phy_regulator);
> > -
> > -	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
> > -				     1000000);
> > -}
> > -
> > -static void mipi_csis_phy_reset(struct csi_state *state)
> > -{
> > -	reset_control_assert(state->mrst);
> > -
> > -	msleep(20);
> > -
> > -	reset_control_deassert(state->mrst);
> > -}
> > -
> >  static void mipi_csis_system_enable(struct csi_state *state, int on)
> >  {
> >  	u32 val, mask;
> > @@ -679,6 +660,42 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +/* -----------------------------------------------------------------------------
> > + * PHY regulator and reset
> > + */
> > +
> > +static int mipi_csis_phy_enable(struct csi_state *state)
> > +{
> > +	return regulator_enable(state->mipi_phy_regulator);
> > +}
> > +
> > +static int mipi_csis_phy_disable(struct csi_state *state)
> > +{
> > +	return regulator_disable(state->mipi_phy_regulator);
> > +}
> > +
> > +static void mipi_csis_phy_reset(struct csi_state *state)
> > +{
> > +	reset_control_assert(state->mrst);
> > +	msleep(20);
> > +	reset_control_deassert(state->mrst);
> > +}
> > +
> > +static int mipi_csis_phy_init(struct csi_state *state)
> > +{
> > +	/* Get MIPI PHY reset and regulator. */
> > +	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
> > +	if (IS_ERR(state->mrst))
> > +		return PTR_ERR(state->mrst);
> > +
> > +	state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> > +	if (IS_ERR(state->mipi_phy_regulator))
> > +		return PTR_ERR(state->mipi_phy_regulator);
> > +
> > +	return regulator_set_voltage(state->mipi_phy_regulator, 1000000,
> > +				     1000000);
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Debug
> >   */
> > @@ -1178,7 +1195,7 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime)
> >  	mutex_lock(&state->lock);
> >  	if (state->state & ST_POWERED) {
> >  		mipi_csis_stop_stream(state);
> > -		ret = regulator_disable(state->mipi_phy_regulator);
> > +		ret = mipi_csis_phy_disable(state);
> >  		if (ret)
> >  			goto unlock;
> >  		mipi_csis_clk_disable(state);
> > @@ -1204,7 +1221,7 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime)
> >  		goto unlock;
> >  
> >  	if (!(state->state & ST_POWERED)) {
> > -		ret = regulator_enable(state->mipi_phy_regulator);
> > +		ret = mipi_csis_phy_enable(state);
> >  		if (ret)
> >  			goto unlock;
> >  
> > @@ -1288,11 +1305,6 @@ static int mipi_csis_parse_dt(struct csi_state *state)
> >  				 &state->clk_frequency))
> >  		state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ;
> >  
> > -	/* Get MIPI PHY resets */
> > -	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
> > -	if (IS_ERR(state->mrst))
> > -		return PTR_ERR(state->mrst);
> > -
> >  	return 0;
> >  }
> >  
>
> -- 
> Regards,
>
> Laurent Pinchart

[PATCH v2 23/25] dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support

[Laurent Pinchart]

The i.MX8MM integrates a newer version of the CSIS CSI-2 receiver as the
i.MX7 family. Differences in integration are are:

- An additional clock is required
- Up to 4 data lanes are supported
- No reset or PHY supply is present

Support it in the DT binding.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 109 +++++++++++++++---
 1 file changed, 95 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
index d8ed480482b9..7c09eec78ce5 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
@@ -4,15 +4,17 @@
 $id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: NXP i.MX7 MIPI CSI-2 receiver
+title: NXP i.MX7 and i.MX8 MIPI CSI-2 receiver
 
 maintainers:
   - Rui Miguel Silva <rmfrfs@gmail.com>
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 
 description: |-
-  The NXP i.MX7 SoC family includes a MIPI CSI-2 receiver IP core, documented
-  as "CSIS V3.3". The IP core seems to originate from Samsung, and may be
-  compatible with some of the Exynos4 ad S5P SoCs.
+  The NXP i.MX7 and i.MX8 families contain SoCs that include a MIPI CSI-2
+  receiver IP core named CSIS. The IP core originates from Samsung, and may be
+  compatible with some of the Exynos4 and S5P SoCs. i.MX7 SoCs use CSIS version
+  3.3, and i.MX8 SoCs use CSIS version 3.6.3.
 
   While the CSI-2 receiver is separate from the MIPI D-PHY IP core, the PHY is
   completely wrapped by the CSIS and doesn't expose a control interface of its
@@ -20,7 +22,9 @@ description: |-
 
 properties:
   compatible:
-    const: fsl,imx7-mipi-csi2
+    enum:
+      - fsl,imx7-mipi-csi2
+      - fsl,imx8mm-mipi-csi2
 
   reg:
     maxItems: 1
@@ -29,16 +33,20 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 3
     items:
       - description: The peripheral clock (a.k.a. APB clock)
       - description: The external clock (optionally used as the pixel clock)
       - description: The MIPI D-PHY clock
+      - description: The AXI clock
 
   clock-names:
+    minItems: 3
     items:
       - const: pclk
       - const: wrap
       - const: phy
+      - const: axi
 
   power-domains:
     maxItems: 1
@@ -71,16 +79,30 @@ properties:
 
             properties:
               data-lanes:
-                oneOf:
-                  - items:
-                      - const: 1
-                  - items:
-                      - const: 1
-                      - const: 2
+                items:
+                  minItems: 1
+                  maxItems: 4
+                  items:
+                    - const: 1
+                    - const: 2
+                    - const: 3
+                    - const: 4
 
             required:
               - data-lanes
 
+            allOf:
+              - if:
+                  properties:
+                    compatible:
+                      contains:
+                        const: fsl,imx7-mipi-csi2
+                then:
+                  properties:
+                    data-lanes:
+                      items:
+                        maxItems: 2
+
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description:
@@ -93,12 +115,29 @@ required:
   - clocks
   - clock-names
   - power-domains
-  - phy-supply
-  - resets
   - ports
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx7-mipi-csi2
+    then:
+      required:
+        - phy-supply
+        - resets
+    else:
+      properties:
+        clocks:
+          minItems: 4
+        clock-names:
+          minItems: 4
+        phy-supply: false
+        resets: false
+
 examples:
   - |
     #include <dt-bindings/clock/imx7d-clock.h>
@@ -106,7 +145,7 @@ examples:
     #include <dt-bindings/interrupt-controller/irq.h>
     #include <dt-bindings/reset/imx7-reset.h>
 
-    mipi_csi: mipi-csi@30750000 {
+    mipi-csi@30750000 {
         compatible = "fsl,imx7-mipi-csi2";
         reg = <0x30750000 0x10000>;
         interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
@@ -144,4 +183,46 @@ examples:
         };
     };
 
+  - |
+    #include <dt-bindings/clock/imx8mm-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    mipi-csi@32e30000 {
+        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";
+        power-domains = <&mipi_pd>;
+
+        status = "disabled";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+
+                imx8mm_mipi_csi_in: endpoint {
+                    remote-endpoint = <&imx477_out>;
+                    data-lanes = <1 2 3 4>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+
+                imx8mm_mipi_csi_out: endpoint {
+                    remote-endpoint = <&csi_in>;
+                };
+            };
+        };
+    };
+
 ...
-- 
Regards,

Laurent Pinchart

[PATCH v2 24/25] media: imx: imx7_mipi_csis: Add i.MX8MM support

[Laurent Pinchart]

The CSI-2 receiver in the i.MX8MM is a newer version of the one found in
the i.MX7. Differences are minimal, support it in the imx7_mipi_csis
driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
Changes since v1:

- Moved .num_clks from csi_state to mipi_csis_info
- Rebased on top of "media: imx: imx7_mipi_csis: Move PHY control to
  dedicated functions"
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 59 ++++++++++++++++++----
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index a8da8d6ddb7d..136e21984bbd 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
@@ -283,21 +284,33 @@ enum mipi_csis_clk {
 	MIPI_CSIS_CLK_PCLK,
 	MIPI_CSIS_CLK_WRAP,
 	MIPI_CSIS_CLK_PHY,
+	MIPI_CSIS_CLK_AXI,
 };
 
 static const char * const mipi_csis_clk_id[] = {
 	"pclk",
 	"wrap",
 	"phy",
+	"axi",
+};
+
+enum mipi_csis_version {
+	MIPI_CSIS_V3_3,
+	MIPI_CSIS_V3_6_3,
+};
+
+struct mipi_csis_info {
+	enum mipi_csis_version version;
+	unsigned int num_clocks;
 };
 
 struct csi_state {
 	struct device *dev;
 	void __iomem *regs;
-	unsigned int num_clks;
 	struct clk_bulk_data *clks;
 	struct reset_control *mrst;
 	struct regulator *mipi_phy_regulator;
+	const struct mipi_csis_info *info;
 	u8 index;
 
 	struct v4l2_subdev sd;
@@ -539,7 +552,8 @@ static void mipi_csis_set_params(struct csi_state *state)
 	val = mipi_csis_read(state, 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_INTER_MODE;
+	if (state->info->version == MIPI_CSIS_V3_3)
+		val |= MIPI_CSIS_CMN_CTRL_INTER_MODE;
 	mipi_csis_write(state, MIPI_CSIS_CMN_CTRL, val);
 
 	__mipi_csis_set_format(state);
@@ -578,12 +592,12 @@ static void mipi_csis_set_params(struct csi_state *state)
 
 static int mipi_csis_clk_enable(struct csi_state *state)
 {
-	return clk_bulk_prepare_enable(state->num_clks, state->clks);
+	return clk_bulk_prepare_enable(state->info->num_clocks, state->clks);
 }
 
 static void mipi_csis_clk_disable(struct csi_state *state)
 {
-	clk_bulk_disable_unprepare(state->num_clks, state->clks);
+	clk_bulk_disable_unprepare(state->info->num_clocks, state->clks);
 }
 
 static int mipi_csis_clk_get(struct csi_state *state)
@@ -591,17 +605,17 @@ static int mipi_csis_clk_get(struct csi_state *state)
 	unsigned int i;
 	int ret;
 
-	state->num_clks = ARRAY_SIZE(mipi_csis_clk_id);
-	state->clks = devm_kcalloc(state->dev, state->num_clks,
+	state->clks = devm_kcalloc(state->dev, state->info->num_clocks,
 				   sizeof(*state->clks), GFP_KERNEL);
 
 	if (!state->clks)
 		return -ENOMEM;
 
-	for (i = 0; i < state->num_clks; i++)
+	for (i = 0; i < state->info->num_clocks; i++)
 		state->clks[i].id = mipi_csis_clk_id[i];
 
-	ret = devm_clk_bulk_get(state->dev, state->num_clks, state->clks);
+	ret = devm_clk_bulk_get(state->dev, state->info->num_clocks,
+				state->clks);
 	if (ret < 0)
 		return ret;
 
@@ -666,16 +680,25 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
 
 static int mipi_csis_phy_enable(struct csi_state *state)
 {
+	if (state->info->version != MIPI_CSIS_V3_3)
+		return 0;
+
 	return regulator_enable(state->mipi_phy_regulator);
 }
 
 static int mipi_csis_phy_disable(struct csi_state *state)
 {
+	if (state->info->version != MIPI_CSIS_V3_3)
+		return 0;
+
 	return regulator_disable(state->mipi_phy_regulator);
 }
 
 static void mipi_csis_phy_reset(struct csi_state *state)
 {
+	if (state->info->version != MIPI_CSIS_V3_3)
+		return;
+
 	reset_control_assert(state->mrst);
 	msleep(20);
 	reset_control_deassert(state->mrst);
@@ -683,6 +706,9 @@ static void mipi_csis_phy_reset(struct csi_state *state)
 
 static int mipi_csis_phy_init(struct csi_state *state)
 {
+	if (state->info->version != MIPI_CSIS_V3_3)
+		return 0;
+
 	/* Get MIPI PHY reset and regulator. */
 	state->mrst = devm_reset_control_get_exclusive(state->dev, NULL);
 	if (IS_ERR(state->mrst))
@@ -1323,6 +1349,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	spin_lock_init(&state->slock);
 
 	state->dev = dev;
+	state->info = of_device_get_match_data(dev);
 
 	memcpy(state->events, mipi_csis_events, sizeof(state->events));
 
@@ -1431,7 +1458,19 @@ static int mipi_csis_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id mipi_csis_of_match[] = {
-	{ .compatible = "fsl,imx7-mipi-csi2", },
+	{
+		.compatible = "fsl,imx7-mipi-csi2",
+		.data = &(const struct mipi_csis_info){
+			.version = MIPI_CSIS_V3_3,
+			.num_clocks = 3,
+		},
+	}, {
+		.compatible = "fsl,imx8mm-mipi-csi2",
+		.data = &(const struct mipi_csis_info){
+			.version = MIPI_CSIS_V3_6_3,
+			.num_clocks = 4,
+		},
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, mipi_csis_of_match);
@@ -1448,6 +1487,6 @@ static struct platform_driver mipi_csis_driver = {
 
 module_platform_driver(mipi_csis_driver);
 
-MODULE_DESCRIPTION("i.MX7 MIPI CSI-2 Receiver driver");
+MODULE_DESCRIPTION("i.MX7 & i.MX8 MIPI CSI-2 receiver driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:imx7-mipi-csi2");
-- 
Regards,

Laurent Pinchart

[PATCH v2 25/25] media: imx: imx7_mipi_csis: Update MAINTAINERS

[Laurent Pinchart]

Given my recent contributions to the imx7-mipi-csis driver, I can as
well be listed as a maintainer.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c120f..2d06dc43fe70 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11283,6 +11283,7 @@ F:	include/media/imx.h
 
 MEDIA DRIVERS FOR FREESCALE IMX7
 M:	Rui Miguel Silva <rmfrfs@gmail.com>
+M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 00/25] media: imx: imx7-mipi-csis: Add i.MX8MM support

[Laurent Pinchart]

On Sun, May 16, 2021 at 04:44:16AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series adds support for the CSIS found in the NXP i.MX8MM SoC
> to the imx7-mipi-csis driver.
> 
> The CSIS is an IP core from Samsung, integrated in different NXP SoCs.
> The driver currently supports v3.3 of the CSIS, found in SoCs from the
> i.MX6 and i.MX7 families. This series extends the driver to support
> v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8
> family.
> 
> The first 22 patches are miscellaneous cleanups and improvements. Please
> see individual patches for details.
> 
> Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support.
> Support for other members of the i.MX8 family will come later, and for
> SoCs including an ISI IP core (such as the i.MX8MP) this will require
> more work to handle additional glue logic.
> 
> Patch 24/25 then extends the imx7-mipi-csis driver accordingly.
> 
> Finally, patch 25/25 updates MAINTAINERS per popular request from people
> who believe I have too much free time :-)
> 
> The series has been tested on an i.MX6ULL (for the CSIS v3.3) and

I meant an i.MX7D, sorry.

> i.MX8MM (for the CSIS v3.6.3).
> 
> The changes in the integration of the CSIS between i.MX7 and i.MX8, as
> described in the DT bindings, have been found through reading of
> reference manuals and BSP source code, with different sources of
> information contradicting each other. A confirmation from NXP would be
> nice (in particular regarding the clocks).
> 
> Laurent Pinchart (25):
>   media: imx: imx7_mipi_csis: Fix logging of only error event counters
>   media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts
>   media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel
>     mode
>   media: imx: imx7_mipi_csis: Move static data to top of
>     mipi_csis_dump_regs()
>   media: imx: imx7_mipi_csis: Minimize locking in get/set format
>   media: imx: imx7_mipi_csis: Don't set subdev data
>   media: imx: imx7_mipi_csis: Reorganize code in sections
>   media: imx: imx7_mipi_csis: Set the CLKSETTLE register field
>   media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure
>   media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure
>   media: imx: imx7_mipi_csis: Drop csi_state phy field
>   media: imx: imx7_mipi_csis: Rename mipi_sd to sd
>   media: imx: imx7_mipi_csis: Rename csi_state flag field to state
>   media: imx: imx7_mipi_csis: Turn csi_state irq field into local
>     variable
>   media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt()
>   media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init()
>   media: imx: imx7_mipi_csis: Drop csi_state pdev field
>   media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned
>   media: imx: imx7_mipi_csis: Reorganize csi_state structure
>   media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe()
>   media: imx: imx7_mipi_csis: Reject invalid data-lanes settings
>   media: imx: imx7_mipi_csis: Move PHY control to dedicated functions
>   dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support
>   media: imx: imx7_mipi_csis: Add i.MX8MM support
>   media: imx: imx7_mipi_csis: Update MAINTAINERS
> 
>  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 109 +-
>  MAINTAINERS                                   |   1 +
>  drivers/staging/media/imx/imx7-mipi-csis.c    | 994 ++++++++++--------
>  3 files changed, 658 insertions(+), 446 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 00/25] media: imx: imx7-mipi-csis: Add i.MX8MM support

[Frieder Schrempf]

On 16.05.21 03:44, Laurent Pinchart wrote:
> Hello,
> 
> This patch series adds support for the CSIS found in the NXP i.MX8MM SoC
> to the imx7-mipi-csis driver.
> 
> The CSIS is an IP core from Samsung, integrated in different NXP SoCs.
> The driver currently supports v3.3 of the CSIS, found in SoCs from the
> i.MX6 and i.MX7 families. This series extends the driver to support
> v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8
> family.
> 
> The first 22 patches are miscellaneous cleanups and improvements. Please
> see individual patches for details.
> 
> Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support.
> Support for other members of the i.MX8 family will come later, and for
> SoCs including an ISI IP core (such as the i.MX8MP) this will require
> more work to handle additional glue logic.
> 
> Patch 24/25 then extends the imx7-mipi-csis driver accordingly.
> 
> Finally, patch 25/25 updates MAINTAINERS per popular request from people
> who believe I have too much free time :-)
> 
> The series has been tested on an i.MX6ULL (for the CSIS v3.3) and
> i.MX8MM (for the CSIS v3.6.3).
> 
> The changes in the integration of the CSIS between i.MX7 and i.MX8, as
> described in the DT bindings, have been found through reading of
> reference manuals and BSP source code, with different sources of
> information contradicting each other. A confirmation from NXP would be
> nice (in particular regarding the clocks).

I already tested your pre-v1 off-list patches with my setup with ADV7280-M and I now tried it again with your v2 patchset and the RFC set for the bridge driver. It works fine with some additional changes specific to the data format as already mentioned here: [1].

Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>

[1] https://patchwork.kernel.org/project/linux-media/cover/20210215042741.28850-1-laurent.pinchart@ideasonboard.com/#24139291

> 
> Laurent Pinchart (25):
>   media: imx: imx7_mipi_csis: Fix logging of only error event counters
>   media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts
>   media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel
>     mode
>   media: imx: imx7_mipi_csis: Move static data to top of
>     mipi_csis_dump_regs()
>   media: imx: imx7_mipi_csis: Minimize locking in get/set format
>   media: imx: imx7_mipi_csis: Don't set subdev data
>   media: imx: imx7_mipi_csis: Reorganize code in sections
>   media: imx: imx7_mipi_csis: Set the CLKSETTLE register field
>   media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure
>   media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure
>   media: imx: imx7_mipi_csis: Drop csi_state phy field
>   media: imx: imx7_mipi_csis: Rename mipi_sd to sd
>   media: imx: imx7_mipi_csis: Rename csi_state flag field to state
>   media: imx: imx7_mipi_csis: Turn csi_state irq field into local
>     variable
>   media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt()
>   media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init()
>   media: imx: imx7_mipi_csis: Drop csi_state pdev field
>   media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned
>   media: imx: imx7_mipi_csis: Reorganize csi_state structure
>   media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe()
>   media: imx: imx7_mipi_csis: Reject invalid data-lanes settings
>   media: imx: imx7_mipi_csis: Move PHY control to dedicated functions
>   dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support
>   media: imx: imx7_mipi_csis: Add i.MX8MM support
>   media: imx: imx7_mipi_csis: Update MAINTAINERS
> 
>  .../bindings/media/nxp,imx7-mipi-csi2.yaml    | 109 +-
>  MAINTAINERS                                   |   1 +
>  drivers/staging/media/imx/imx7-mipi-csis.c    | 994 ++++++++++--------
>  3 files changed, 658 insertions(+), 446 deletions(-)
>