[PATCH v1 0/3] staging: media: imx7-mipi-csis: Small improvements

[PATCH v1 0/3] staging: media: imx7-mipi-csis: Small improvements

[Laurent Pinchart]

Hello,

This small patch series contains small debugging improvements for the
imx7-mipi-csis driver, as well as a fix to make entity names unique for
platforms that have multiple CSI receiver instances (namely the
i.MX8MP).

There's not much more to tell here, please see individual patches for
details.

Laurent Pinchart (3):
  staging: media: imx: imx7-mipi-csis: Dump MIPI_CSIS_FRAME_COUNTER_CH0
    register
  staging: media: imx: imx7_mipi_csis: Add timings override through
    debugfs
  staging: media: imx: imx7-mipi-csis: Make subdev name unique

 drivers/staging/media/imx/imx7-mipi-csis.c | 44 +++++++++++++++++-----
 1 file changed, 34 insertions(+), 10 deletions(-)


base-commit: 68b9bcc8a534cd11fe55f8bc82f948aae7d81b3c
-- 
Regards,

Laurent Pinchart

[PATCH v1 1/3] staging: media: imx: imx7-mipi-csis: Dump MIPI_CSIS_FRAME_COUNTER_CH0 register

[Laurent Pinchart]

The frame counter is useful debugging information, add it to the
register dump printed by mipi_csis_dump_regs().

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

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 2b73fa55c938..c9c0089ad816 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -211,6 +211,8 @@
 #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
 #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
 
+#define MIPI_CSIS_FRAME_COUNTER_CH(n)		(0x0100 + (n) * 4)
+
 /* Non-image packet data buffers */
 #define MIPI_CSIS_PKTDATA_ODD			0x2000
 #define MIPI_CSIS_PKTDATA_EVEN			0x3000
@@ -773,6 +775,7 @@ static int mipi_csis_dump_regs(struct csi_state *state)
 		{ MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" },
 		{ MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" },
 		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
+		{ MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" },
 	};
 
 	unsigned int i;
-- 
Regards,

Laurent Pinchart

[PATCH v1 2/3] staging: media: imx: imx7_mipi_csis: Add timings override through debugfs

[Laurent Pinchart]

Add two debugfs files, ths_settle and tclk_settle, to allow overriding
the corresponding timing parameters for test purpose.

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

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index c9c0089ad816..d7bcfb1a0c52 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -333,7 +333,11 @@ struct csi_state {
 	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 	struct dentry *debugfs_root;
-	bool debug;
+	struct {
+		bool debug;
+		u32 hs_settle;
+		u32 clk_settle;
+	} debug;
 };
 
 /* -----------------------------------------------------------------------------
@@ -543,6 +547,18 @@ static int mipi_csis_calculate_params(struct csi_state *state)
 	dev_dbg(state->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
 		lane_rate, state->clk_settle, state->hs_settle);
 
+	if (state->debug.hs_settle < 0xff) {
+		dev_dbg(state->dev, "overriding Ths_settle with %u\n",
+			state->debug.hs_settle);
+		state->hs_settle = state->debug.hs_settle;
+	}
+
+	if (state->debug.clk_settle < 4) {
+		dev_dbg(state->dev, "overriding Tclk_settle with %u\n",
+			state->debug.clk_settle);
+		state->clk_settle = state->debug.clk_settle;
+	}
+
 	return 0;
 }
 
@@ -659,7 +675,7 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
 	spin_lock_irqsave(&state->slock, flags);
 
 	/* Update the event/error counters */
-	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
+	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug.debug) {
 		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
 			struct mipi_csis_event *event = &state->events[i];
 
@@ -749,7 +765,7 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
 	spin_lock_irqsave(&state->slock, flags);
 
 	for (i = 0; i < num_events; ++i) {
-		if (state->events[i].counter > 0 || state->debug)
+		if (state->events[i].counter > 0 || state->debug.debug)
 			dev_info(state->dev, "%s events: %d\n",
 				 state->events[i].name,
 				 state->events[i].counter);
@@ -801,12 +817,19 @@ DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
 
 static void mipi_csis_debugfs_init(struct csi_state *state)
 {
+	state->debug.hs_settle = UINT_MAX;
+	state->debug.clk_settle = UINT_MAX;
+
 	state->debugfs_root = debugfs_create_dir(dev_name(state->dev), NULL);
 
 	debugfs_create_bool("debug_enable", 0600, state->debugfs_root,
-			    &state->debug);
+			    &state->debug.debug);
 	debugfs_create_file("dump_regs", 0600, state->debugfs_root, state,
 			    &mipi_csis_dump_regs_fops);
+	debugfs_create_u32("tclk_settle", 0600, state->debugfs_root,
+			   &state->debug.clk_settle);
+	debugfs_create_u32("ths_settle", 0600, state->debugfs_root,
+			   &state->debug.hs_settle);
 }
 
 static void mipi_csis_debugfs_exit(struct csi_state *state)
@@ -867,7 +890,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 			ret = 0;
 		mipi_csis_stop_stream(state);
 		state->state &= ~ST_STREAMING;
-		if (state->debug)
+		if (state->debug.debug)
 			mipi_csis_log_counters(state, true);
 	}
 
@@ -1064,7 +1087,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->state & ST_POWERED))
+	if (state->debug.debug && (state->state & ST_POWERED))
 		mipi_csis_dump_regs(state);
 	mutex_unlock(&state->lock);
 
-- 
Regards,

Laurent Pinchart

[PATCH v1 3/3] staging: media: imx: imx7-mipi-csis: Make subdev name unique

[Laurent Pinchart]

When multiple CSIS instances are present in a single graph, they are
currently all named "imx7-mipi-csis.0", which breaks the entity name
uniqueness requirement. Fix it by using the device name to create the
subdev name.

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

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index d7bcfb1a0c52..6443cca817fe 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -32,7 +32,6 @@
 #include <media/v4l2-subdev.h>
 
 #define CSIS_DRIVER_NAME			"imx7-mipi-csis"
-#define CSIS_SUBDEV_NAME			CSIS_DRIVER_NAME
 
 #define CSIS_PAD_SINK				0
 #define CSIS_PAD_SOURCE				1
@@ -313,7 +312,6 @@ struct csi_state {
 	struct reset_control *mrst;
 	struct regulator *mipi_phy_regulator;
 	const struct mipi_csis_info *info;
-	u8 index;
 
 	struct v4l2_subdev sd;
 	struct media_pad pads[CSIS_PADS_NUM];
@@ -1329,8 +1327,8 @@ static int mipi_csis_subdev_init(struct csi_state *state)
 
 	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);
+	snprintf(sd->name, sizeof(sd->name), "csis-%s",
+		 dev_name(state->dev));
 
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	sd->ctrl_handler = NULL;
-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 1/3] staging: media: imx: imx7-mipi-csis: Dump MIPI_CSIS_FRAME_COUNTER_CH0 register

[Rui Miguel Silva]

Hi Laurent,
On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote:

> The frame counter is useful debugging information, add it to the
> register dump printed by mipi_csis_dump_regs().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
LGTM

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

Cheers,
  Rui

> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 2b73fa55c938..c9c0089ad816 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -211,6 +211,8 @@
>  #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL	BIT(4)
>  #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE	BIT(0)
>  
> +#define MIPI_CSIS_FRAME_COUNTER_CH(n)		(0x0100 + (n) * 4)
> +
>  /* Non-image packet data buffers */
>  #define MIPI_CSIS_PKTDATA_ODD			0x2000
>  #define MIPI_CSIS_PKTDATA_EVEN			0x3000
> @@ -773,6 +775,7 @@ static int mipi_csis_dump_regs(struct csi_state *state)
>  		{ MIPI_CSIS_SDW_CONFIG_CH(0), "SDW_CONFIG_CH0" },
>  		{ MIPI_CSIS_SDW_RESOL_CH(0), "SDW_RESOL_CH0" },
>  		{ MIPI_CSIS_DBG_CTRL, "DBG_CTRL" },
> +		{ MIPI_CSIS_FRAME_COUNTER_CH(0), "FRAME_COUNTER_CH0" },
>  	};
>  
>  	unsigned int i;
> -- 
> Regards,
>
> Laurent Pinchart

Re: [PATCH v1 2/3] staging: media: imx: imx7_mipi_csis: Add timings override through debugfs

[Rui Miguel Silva]

Hi Laurent,
thanks for the patch.

On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote:

> Add two debugfs files, ths_settle and tclk_settle, to allow overriding
> the corresponding timing parameters for test purpose.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 35 ++++++++++++++++++----
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index c9c0089ad816..d7bcfb1a0c52 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -333,7 +333,11 @@ struct csi_state {
>  	spinlock_t slock;	/* Protect events */
>  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>  	struct dentry *debugfs_root;
> -	bool debug;
> +	struct {
> +		bool debug;

I think here would make more sense to call it "enable", I think
state->debug.enable is more readable than state->debug.debug.

Other than this LGTM

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

Cheers,
   Rui

> +		u32 hs_settle;
> +		u32 clk_settle;
> +	} debug;
>  };
>  
>  /* -----------------------------------------------------------------------------
> @@ -543,6 +547,18 @@ static int mipi_csis_calculate_params(struct csi_state *state)
>  	dev_dbg(state->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
>  		lane_rate, state->clk_settle, state->hs_settle);
>  
> +	if (state->debug.hs_settle < 0xff) {
> +		dev_dbg(state->dev, "overriding Ths_settle with %u\n",
> +			state->debug.hs_settle);
> +		state->hs_settle = state->debug.hs_settle;
> +	}
> +
> +	if (state->debug.clk_settle < 4) {
> +		dev_dbg(state->dev, "overriding Tclk_settle with %u\n",
> +			state->debug.clk_settle);
> +		state->clk_settle = state->debug.clk_settle;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -659,7 +675,7 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
>  	spin_lock_irqsave(&state->slock, flags);
>  
>  	/* Update the event/error counters */
> -	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
> +	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug.debug) {
>  		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
>  			struct mipi_csis_event *event = &state->events[i];
>  
> @@ -749,7 +765,7 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
>  	spin_lock_irqsave(&state->slock, flags);
>  
>  	for (i = 0; i < num_events; ++i) {
> -		if (state->events[i].counter > 0 || state->debug)
> +		if (state->events[i].counter > 0 || state->debug.debug)
>  			dev_info(state->dev, "%s events: %d\n",
>  				 state->events[i].name,
>  				 state->events[i].counter);
> @@ -801,12 +817,19 @@ DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
>  
>  static void mipi_csis_debugfs_init(struct csi_state *state)
>  {
> +	state->debug.hs_settle = UINT_MAX;
> +	state->debug.clk_settle = UINT_MAX;
> +
>  	state->debugfs_root = debugfs_create_dir(dev_name(state->dev), NULL);
>  
>  	debugfs_create_bool("debug_enable", 0600, state->debugfs_root,
> -			    &state->debug);
> +			    &state->debug.debug);
>  	debugfs_create_file("dump_regs", 0600, state->debugfs_root, state,
>  			    &mipi_csis_dump_regs_fops);
> +	debugfs_create_u32("tclk_settle", 0600, state->debugfs_root,
> +			   &state->debug.clk_settle);
> +	debugfs_create_u32("ths_settle", 0600, state->debugfs_root,
> +			   &state->debug.hs_settle);
>  }
>  
>  static void mipi_csis_debugfs_exit(struct csi_state *state)
> @@ -867,7 +890,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
>  			ret = 0;
>  		mipi_csis_stop_stream(state);
>  		state->state &= ~ST_STREAMING;
> -		if (state->debug)
> +		if (state->debug.debug)
>  			mipi_csis_log_counters(state, true);
>  	}
>  
> @@ -1064,7 +1087,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->state & ST_POWERED))
> +	if (state->debug.debug && (state->state & ST_POWERED))
>  		mipi_csis_dump_regs(state);
>  	mutex_unlock(&state->lock);
>  
> -- 
> Regards,
>
> Laurent Pinchart

Re: [PATCH v1 3/3] staging: media: imx: imx7-mipi-csis: Make subdev name unique

[Rui Miguel Silva]

Hi Laurent,
Thanks for the patch.

On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote:

> When multiple CSIS instances are present in a single graph, they are
> currently all named "imx7-mipi-csis.0", which breaks the entity name
> uniqueness requirement. Fix it by using the device name to create the
> subdev name.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Maybe a Fix tag here to make sure it is backported, since this look to
me a legit bug fix for multiple instances.

Other than that LGTM.

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

Cheers,
  Rui
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index d7bcfb1a0c52..6443cca817fe 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -32,7 +32,6 @@
>  #include <media/v4l2-subdev.h>
>  
>  #define CSIS_DRIVER_NAME			"imx7-mipi-csis"
> -#define CSIS_SUBDEV_NAME			CSIS_DRIVER_NAME
>  
>  #define CSIS_PAD_SINK				0
>  #define CSIS_PAD_SOURCE				1
> @@ -313,7 +312,6 @@ struct csi_state {
>  	struct reset_control *mrst;
>  	struct regulator *mipi_phy_regulator;
>  	const struct mipi_csis_info *info;
> -	u8 index;
>  
>  	struct v4l2_subdev sd;
>  	struct media_pad pads[CSIS_PADS_NUM];
> @@ -1329,8 +1327,8 @@ static int mipi_csis_subdev_init(struct csi_state *state)
>  
>  	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);
> +	snprintf(sd->name, sizeof(sd->name), "csis-%s",
> +		 dev_name(state->dev));
>  
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  	sd->ctrl_handler = NULL;
> -- 
> Regards,
>
> Laurent Pinchart

Re: [PATCH v1 2/3] staging: media: imx: imx7_mipi_csis: Add timings override through debugfs

[Laurent Pinchart]

Hi Rui,

On Fri, Jan 07, 2022 at 10:27:31AM +0000, Rui Miguel Silva wrote:
> Hi Laurent,
> thanks for the patch.
> 
> On Thu Jan 6, 2022 at 5:24 PM WET, Laurent Pinchart wrote:
> 
> > Add two debugfs files, ths_settle and tclk_settle, to allow overriding
> > the corresponding timing parameters for test purpose.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/staging/media/imx/imx7-mipi-csis.c | 35 ++++++++++++++++++----
> >  1 file changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> > index c9c0089ad816..d7bcfb1a0c52 100644
> > --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> > @@ -333,7 +333,11 @@ struct csi_state {
> >  	spinlock_t slock;	/* Protect events */
> >  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> >  	struct dentry *debugfs_root;
> > -	bool debug;
> > +	struct {
> > +		bool debug;
> 
> I think here would make more sense to call it "enable", I think
> state->debug.enable is more readable than state->debug.debug.

Good idea, I'll do that.

> Other than this LGTM
> 
> Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>
> 
> > +		u32 hs_settle;
> > +		u32 clk_settle;
> > +	} debug;
> >  };
> >  
> >  /* -----------------------------------------------------------------------------
> > @@ -543,6 +547,18 @@ static int mipi_csis_calculate_params(struct csi_state *state)
> >  	dev_dbg(state->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
> >  		lane_rate, state->clk_settle, state->hs_settle);
> >  
> > +	if (state->debug.hs_settle < 0xff) {
> > +		dev_dbg(state->dev, "overriding Ths_settle with %u\n",
> > +			state->debug.hs_settle);
> > +		state->hs_settle = state->debug.hs_settle;
> > +	}
> > +
> > +	if (state->debug.clk_settle < 4) {
> > +		dev_dbg(state->dev, "overriding Tclk_settle with %u\n",
> > +			state->debug.clk_settle);
> > +		state->clk_settle = state->debug.clk_settle;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -659,7 +675,7 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
> >  	spin_lock_irqsave(&state->slock, flags);
> >  
> >  	/* Update the event/error counters */
> > -	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug) {
> > +	if ((status & MIPI_CSIS_INT_SRC_ERRORS) || state->debug.debug) {
> >  		for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
> >  			struct mipi_csis_event *event = &state->events[i];
> >  
> > @@ -749,7 +765,7 @@ static void mipi_csis_log_counters(struct csi_state *state, bool non_errors)
> >  	spin_lock_irqsave(&state->slock, flags);
> >  
> >  	for (i = 0; i < num_events; ++i) {
> > -		if (state->events[i].counter > 0 || state->debug)
> > +		if (state->events[i].counter > 0 || state->debug.debug)
> >  			dev_info(state->dev, "%s events: %d\n",
> >  				 state->events[i].name,
> >  				 state->events[i].counter);
> > @@ -801,12 +817,19 @@ DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
> >  
> >  static void mipi_csis_debugfs_init(struct csi_state *state)
> >  {
> > +	state->debug.hs_settle = UINT_MAX;
> > +	state->debug.clk_settle = UINT_MAX;
> > +
> >  	state->debugfs_root = debugfs_create_dir(dev_name(state->dev), NULL);
> >  
> >  	debugfs_create_bool("debug_enable", 0600, state->debugfs_root,
> > -			    &state->debug);
> > +			    &state->debug.debug);
> >  	debugfs_create_file("dump_regs", 0600, state->debugfs_root, state,
> >  			    &mipi_csis_dump_regs_fops);
> > +	debugfs_create_u32("tclk_settle", 0600, state->debugfs_root,
> > +			   &state->debug.clk_settle);
> > +	debugfs_create_u32("ths_settle", 0600, state->debugfs_root,
> > +			   &state->debug.hs_settle);
> >  }
> >  
> >  static void mipi_csis_debugfs_exit(struct csi_state *state)
> > @@ -867,7 +890,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
> >  			ret = 0;
> >  		mipi_csis_stop_stream(state);
> >  		state->state &= ~ST_STREAMING;
> > -		if (state->debug)
> > +		if (state->debug.debug)
> >  			mipi_csis_log_counters(state, true);
> >  	}
> >  
> > @@ -1064,7 +1087,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->state & ST_POWERED))
> > +	if (state->debug.debug && (state->state & ST_POWERED))
> >  		mipi_csis_dump_regs(state);
> >  	mutex_unlock(&state->lock);
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 3/3] staging: media: imx: imx7-mipi-csis: Make subdev name unique

[Sakari Ailus]

On Thu, Jan 06, 2022 at 07:24:41PM +0200, Laurent Pinchart wrote:
> When multiple CSIS instances are present in a single graph, they are
> currently all named "imx7-mipi-csis.0", which breaks the entity name
> uniqueness requirement. Fix it by using the device name to create the
> subdev name.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index d7bcfb1a0c52..6443cca817fe 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -32,7 +32,6 @@
>  #include <media/v4l2-subdev.h>
>  
>  #define CSIS_DRIVER_NAME			"imx7-mipi-csis"
> -#define CSIS_SUBDEV_NAME			CSIS_DRIVER_NAME
>  
>  #define CSIS_PAD_SINK				0
>  #define CSIS_PAD_SOURCE				1
> @@ -313,7 +312,6 @@ struct csi_state {
>  	struct reset_control *mrst;
>  	struct regulator *mipi_phy_regulator;
>  	const struct mipi_csis_info *info;
> -	u8 index;
>  
>  	struct v4l2_subdev sd;
>  	struct media_pad pads[CSIS_PADS_NUM];
> @@ -1329,8 +1327,8 @@ static int mipi_csis_subdev_init(struct csi_state *state)
>  
>  	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);
> +	snprintf(sd->name, sizeof(sd->name), "csis-%s",
> +		 dev_name(state->dev));
>  
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  	sd->ctrl_handler = NULL;

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus