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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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