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

Isaac Scott posted 4 patches 6 days, 9 hours ago
[PATCH v4 3/4] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Isaac Scott 6 days, 9 hours ago
The number of active data lanes in use on a MIPI CSI2 bus is not
necessarily always the maximum. To allow us to configure the number of
data lanes actively in use, store the maximum to ensure we can configure
a number of data lanes that is supported.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 7c2a679dca2e..838a1ad123b5 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -351,6 +351,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;
@@ -573,7 +575,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);
@@ -623,7 +625,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(csis->source.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);
@@ -668,7 +670,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);
@@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	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, "max data lanes: %d\n", csis->bus.num_data_lanes);
 	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
 
 	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,

-- 
2.43.0
Re: [PATCH v4 3/4] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Frank Li 6 days, 3 hours ago
On Thu, Sep 25, 2025 at 04:54:28PM +0100, Isaac Scott wrote:
> The number of active data lanes in use on a MIPI CSI2 bus is not
> necessarily always the maximum. To allow us to configure the number of
> data lanes actively in use, store the maximum to ensure we can configure
> a number of data lanes that is supported.
>

This patch just add num_data_lanes, and use csis->num_data_lanes instead
of bus.num_data_lanes.

So commit message not reflect what you did

"
media: imx-mipi-csis: Add num_data_lanes in mipi_csis_device

Add num_data_lanes field in mipi_csis_device, set equal to
csis->bus.num_data_lanes. Prepare to support cases where the number of
active data lanes differs from the maximum supported lanes.

No functional changes.
"

Frank

> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 7c2a679dca2e..838a1ad123b5 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -351,6 +351,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;
> @@ -573,7 +575,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);
> @@ -623,7 +625,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(csis->source.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);
> @@ -668,7 +670,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);
> @@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>
>  	csis->bus = vep.bus.mipi_csi2;
> +	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, "max data lanes: %d\n", csis->bus.num_data_lanes);
>  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
>
>  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
>
> --
> 2.43.0
>
Re: [PATCH v4 3/4] media: imx-mipi-csis: Store the number of data_lanes configured in dt
Posted by Isaac Scott 5 days, 16 hours ago
Hi Frank,

Thank you for your review!

Quoting Frank Li (2025-09-25 22:49:00)
> On Thu, Sep 25, 2025 at 04:54:28PM +0100, Isaac Scott wrote:
> > The number of active data lanes in use on a MIPI CSI2 bus is not
> > necessarily always the maximum. To allow us to configure the number of
> > data lanes actively in use, store the maximum to ensure we can configure
> > a number of data lanes that is supported.
> >
> 
> This patch just add num_data_lanes, and use csis->num_data_lanes instead
> of bus.num_data_lanes.
> 
> So commit message not reflect what you did
> 
> "
> media: imx-mipi-csis: Add num_data_lanes in mipi_csis_device
> 
> Add num_data_lanes field in mipi_csis_device, set equal to
> csis->bus.num_data_lanes. Prepare to support cases where the number of
> active data lanes differs from the maximum supported lanes.
> 
> No functional changes.
> "

Yes, that is much better, thank you for your suggestions (on this and
the next patch), I'll wait to see if there are any other comments and
improve my commit messages in the next version.

Best wishes,
Isaac

> 
> Frank
> 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 7c2a679dca2e..838a1ad123b5 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -351,6 +351,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;
> > @@ -573,7 +575,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);
> > @@ -623,7 +625,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(csis->source.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);
> > @@ -668,7 +670,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);
> > @@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> >       }
> >
> >       csis->bus = vep.bus.mipi_csi2;
> > +     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, "max data lanes: %d\n", csis->bus.num_data_lanes);
> >       dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> >
> >       asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> >
> > --
> > 2.43.0
> >