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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..fc89325f2f94 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 max_data_lanes;
+
 	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 	struct dentry *debugfs_root;
@@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	csis->max_data_lanes = vep.bus.mipi_csi2.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->max_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

On Wed, Sep 03, 2025 at 11:22:41AM +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 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 2beb5f43c2c0..fc89325f2f94 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 max_data_lanes;
> +

is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes

Frank

>  	spinlock_t slock;	/* Protect events */
>  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>  	struct dentry *debugfs_root;
> @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>
>  	csis->bus = vep.bus.mipi_csi2;
> +	csis->max_data_lanes = vep.bus.mipi_csi2.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->max_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
>

On Wed, Sep 03, 2025 at 11:33:15AM -0400, Frank Li wrote:
> On Wed, Sep 03, 2025 at 11:22:41AM +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 | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 2beb5f43c2c0..fc89325f2f94 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 max_data_lanes;
> > +
> 
> is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes

That's a good point, but I think I prefer max_data_lanes here as it
conveys better the fact that the field stores the maximum number of data
lanes that can be used, not the number of data lanes being used at a
given point of time.

This being said, why do we need this ? The maximum number of data lanes
can be accessed through csis->bus.num_data_lanes. I've looked at patch
3/3 to answer this question, it there csis->bus.num_data_lanes is
modified to store the number of data lanes used at runtime. I think it
would be better to consider csis->bus as immutable after probe, and
store the number of used data lanes in csis->num_data_lanes.

Isaac, could you replace this patch by another one that adds
csis->num_data_lanes, sets it to csis->bus.num_data_lanes in
mipi_csis_async_register(), and replace usage of
csis->bus.num_data_lanes with csis->num_data_lanes through the driver ?
Patch 3/3 should then modify csis->num_data_lanes, not
csis->bus.num_data_lanes.

> >  	spinlock_t slock;	/* Protect events */
> >  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> >  	struct dentry *debugfs_root;
> > @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> >  	}
> >
> >  	csis->bus = vep.bus.mipi_csi2;
> > +	csis->max_data_lanes = vep.bus.mipi_csi2.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->max_data_lanes);
> >  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> >
> >  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,

-- 
Regards,

Laurent Pinchart

Hi all,

Thank you very much for the reviews!

Quoting Laurent Pinchart (2025-09-04 16:01:53)
> On Wed, Sep 03, 2025 at 11:33:15AM -0400, Frank Li wrote:
> > On Wed, Sep 03, 2025 at 11:22:41AM +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 | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index 2beb5f43c2c0..fc89325f2f94 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 max_data_lanes;
> > > +
> > 
> > is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes
> 
> That's a good point, but I think I prefer max_data_lanes here as it
> conveys better the fact that the field stores the maximum number of data
> lanes that can be used, not the number of data lanes being used at a
> given point of time.
> 
> This being said, why do we need this ? The maximum number of data lanes
> can be accessed through csis->bus.num_data_lanes. I've looked at patch
> 3/3 to answer this question, it there csis->bus.num_data_lanes is
> modified to store the number of data lanes used at runtime. I think it
> would be better to consider csis->bus as immutable after probe, and
> store the number of used data lanes in csis->num_data_lanes.

Yes, this makes a lot of sense. I only used max_data_lanes because I
wanted to avoid potential confusion between csis->bus.num_data_lanes and
csis->num_data_lanes; if that's not acutally an issue, I'll change it to
num_data_lanes.

> 
> Isaac, could you replace this patch by another one that adds
> csis->num_data_lanes, sets it to csis->bus.num_data_lanes in
> mipi_csis_async_register(), and replace usage of
> csis->bus.num_data_lanes with csis->num_data_lanes through the driver ?
> Patch 3/3 should then modify csis->num_data_lanes, not
> csis->bus.num_data_lanes.
> 

Good idea! Thanks, I'll take a look at submitting a v3.

Best wishes,

Isaac

> > >     spinlock_t slock;       /* Protect events */
> > >     struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> > >     struct dentry *debugfs_root;
> > > @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> > >     }
> > >
> > >     csis->bus = vep.bus.mipi_csi2;
> > > +   csis->max_data_lanes = vep.bus.mipi_csi2.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->max_data_lanes);
> > >     dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> > >
> > >     asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> 
> -- 
> Regards,
> 
> Laurent Pinchart