linux-kernel - Re: [PATCH v4 3/4] media: imx-mipi-csis: Store the number of data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <175887638283.180868.8800087471301856681@isaac-ThinkPad-T16-Gen-2>
Date: Fri, 26 Sep 2025 09:46:22 +0100
From: Isaac Scott <isaac.scott@...asonboard.com>
To: Frank Li <Frank.li@....com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Rui Miguel Silva <rmfrfs@...il.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, Martin Kepplinger <martink@...teo.de>, Purism Kernel Team <kernel@...i.sm>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 3/4] media: imx-mipi-csis: Store the number of data_lanes configured in dt

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@...asonboard.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
> >