[PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt

Isaac Scott posted 3 patches 2 weeks, 3 days ago
There is a newer version of this series
[PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Isaac Scott 2 weeks, 3 days ago
The number of lanes actively used by a MIPI CSI transmitter may differ
from that which is defined in device tree. To allow us to be able to set
the number of configured lanes without changing the maximum lane count,
store the number of lanes configured in device tree, and adjust the
debug print to reflect the device tree value.

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..6afbedfe131e 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -313,6 +313,8 @@ struct mipi_csis_device {
 	u32 hs_settle;
 	u32 clk_settle;
 
+	unsigned int num_data_lanes;
+
 	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 	struct dentry *debugfs_root;
@@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
 	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
 	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
 	if (on) {
-		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
+		mask = (1 << (csis->num_data_lanes + 1)) - 1;
 		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
 	}
 	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
@@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
 
 	/* Calculate the line rate from the pixel rate. */
 	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
-				       csis->bus.num_data_lanes * 2);
+				       csis->num_data_lanes * 2);
 	if (link_freq < 0) {
 		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
 			(int)link_freq);
@@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 				 const struct v4l2_mbus_framefmt *format,
 				 const struct csis_pix_format *csis_fmt)
 {
-	int lanes = csis->bus.num_data_lanes;
+	int lanes = csis->num_data_lanes;
 	u32 val;
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
@@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
+	csis->num_data_lanes = csis->bus.num_data_lanes;
 
-	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
+	dev_dbg(csis->dev, "data lanes: %d\n", csis->num_data_lanes);
 	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
 
 	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
@@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	}
 
 	dev_info(dev, "lanes: %d, freq: %u\n",
-		 csis->bus.num_data_lanes, csis->clk_frequency);
+		 csis->num_data_lanes, csis->clk_frequency);
 
 	return 0;
 

-- 
2.43.0
Re: [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Sakari Ailus 1 week, 3 days ago
Hi Isaac,

On Mon, Sep 15, 2025 at 02:18:34PM +0100, Isaac Scott wrote:
> The number of lanes actively used by a MIPI CSI transmitter may differ
> from that which is defined in device tree. To allow us to be able to set
> the number of configured lanes without changing the maximum lane count,
> store the number of lanes configured in device tree, and adjust the
> debug print to reflect the device tree value.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 2beb5f43c2c0..6afbedfe131e 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -313,6 +313,8 @@ struct mipi_csis_device {
>  	u32 hs_settle;
>  	u32 clk_settle;
>  
> +	unsigned int num_data_lanes;
> +
>  	spinlock_t slock;	/* Protect events */
>  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>  	struct dentry *debugfs_root;
> @@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
>  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
>  	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
>  	if (on) {
> -		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
> +		mask = (1 << (csis->num_data_lanes + 1)) - 1;

Please use 1U or BIT() for bit-shifted values.

>  		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
>  	}
>  	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
> @@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
>  
>  	/* Calculate the line rate from the pixel rate. */
>  	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
> -				       csis->bus.num_data_lanes * 2);
> +				       csis->num_data_lanes * 2);
>  	if (link_freq < 0) {
>  		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
>  			(int)link_freq);
> @@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  				 const struct v4l2_mbus_framefmt *format,
>  				 const struct csis_pix_format *csis_fmt)
>  {
> -	int lanes = csis->bus.num_data_lanes;
> +	int lanes = csis->num_data_lanes;
>  	u32 val;
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> @@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>  
>  	csis->bus = vep.bus.mipi_csi2;
> +	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> +	csis->num_data_lanes = csis->bus.num_data_lanes;
>  
> -	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
> +	dev_dbg(csis->dev, "data lanes: %d\n", csis->num_data_lanes);
>  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
>  
>  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> @@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
>  	}
>  
>  	dev_info(dev, "lanes: %d, freq: %u\n",
> -		 csis->bus.num_data_lanes, csis->clk_frequency);
> +		 csis->num_data_lanes, csis->clk_frequency);
>  
>  	return 0;
>  
> 

-- 
Regards,

Sakari Ailus
Re: [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Laurent Pinchart 1 week, 3 days ago
On Mon, Sep 22, 2025 at 09:49:27AM +0300, Sakari Ailus wrote:
> On Mon, Sep 15, 2025 at 02:18:34PM +0100, Isaac Scott wrote:
> > The number of lanes actively used by a MIPI CSI transmitter may differ
> > from that which is defined in device tree. To allow us to be able to set
> > the number of configured lanes without changing the maximum lane count,
> > store the number of lanes configured in device tree, and adjust the
> > debug print to reflect the device tree value.
> > 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 2beb5f43c2c0..6afbedfe131e 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -313,6 +313,8 @@ struct mipi_csis_device {
> >  	u32 hs_settle;
> >  	u32 clk_settle;
> >  
> > +	unsigned int num_data_lanes;
> > +
> >  	spinlock_t slock;	/* Protect events */
> >  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> >  	struct dentry *debugfs_root;
> > @@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> >  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
> >  	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
> >  	if (on) {
> > -		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
> > +		mask = (1 << (csis->num_data_lanes + 1)) - 1;
> 
> Please use 1U or BIT() for bit-shifted values.

BIT() isn't appropriate here. GENMASK(csis->num_data_lanes, 0) could be
fine, but this patch just changes the variable, I wouldn't insist in
fixing separate issues.

> >  		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
> >  	}
> >  	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
> > @@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> >  
> >  	/* Calculate the line rate from the pixel rate. */
> >  	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
> > -				       csis->bus.num_data_lanes * 2);
> > +				       csis->num_data_lanes * 2);
> >  	if (link_freq < 0) {
> >  		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> >  			(int)link_freq);
> > @@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >  				 const struct v4l2_mbus_framefmt *format,
> >  				 const struct csis_pix_format *csis_fmt)
> >  {
> > -	int lanes = csis->bus.num_data_lanes;
> > +	int lanes = csis->num_data_lanes;
> >  	u32 val;
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > @@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> >  	}
> >  
> >  	csis->bus = vep.bus.mipi_csi2;
> > +	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;

That doesn't seem to be needed.

> > +	csis->num_data_lanes = csis->bus.num_data_lanes;
> >  
> > -	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
> > +	dev_dbg(csis->dev, "data lanes: %d\n", csis->num_data_lanes);

Neither is this change. What you print here is the number of connected
data lanes, not the number of effectively used lanes. You could change
it to 

	dev_dbg(csis->dev, "max data lanes: %u\n", csis->bus.num_data_lanes);

You'll want to also update the commit message, which I think needs some
improvement regardless.

> >  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> >  
> >  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> > @@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	dev_info(dev, "lanes: %d, freq: %u\n",
> > -		 csis->bus.num_data_lanes, csis->clk_frequency);
> > +		 csis->num_data_lanes, csis->clk_frequency);

Drop this change too. This message is actually redundant, the number of
lanes is printed in mipi_csis_async_register(). You could submit a
separate patch to remove this, possibly replacing it with a dev_dbg() in
mipi_csis_parse_dt() to print clk_frequency.

> >  
> >  	return 0;
> >  

-- 
Regards,

Laurent Pinchart