[PATCH v2 05/12] media: imx-mipi-csis: Rename register macros to match reference manual

[Laurent Pinchart]

Hi Frank,

On Thu, Aug 21, 2025 at 11:25:55AM -0400, Frank Li wrote:
> On Thu, Aug 21, 2025 at 03:09:37AM +0300, Laurent Pinchart wrote:
> > The CSIS driver uses register macro names that do not match the
> > reference manual of the i.MX7[DS] and i.MX8M[MNP] SoCs in which the CSIS
> > is integrated. Rename them to match the documentation, making the code
> > easier to read alongside the reference manuals.
> >
> > One of the misnamed register fields is MIPI_CSIS_INT_SRC_ERR_UNKNOWN,
> > which led to the corresponding event being logged as "Unknown Error".
> > The correct register field name is MIPI_CSIS_INT_SRC_ERR_ID, documented
> > as "Unknown ID error". Update the event description accordingly.
> >
> > While at it, also replace a few *_OFFSET macros with parametric macros
> > for consistency, and add the missing MIPI_CSIS_ISP_RESOL_VRESOL and
> > MIPI_CSIS_ISP_RESOL_HRESOL register field macros.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 69 +++++++++++-----------
> >  1 file changed, 36 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 894d12fef519..1ca327e6be00 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -55,13 +55,13 @@
> >  /* CSIS common control */
> >  #define MIPI_CSIS_CMN_CTRL			0x04
> >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
> > -#define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
> > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE	(0 << 10)
> > +#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT	(1 << 10)
> 
> BIT(10)?

INTERLAVE_MODE is a 2-bit field. I'm working on a series that add
support for the VC interleave mode, which has value 2. I'll however drop
this change and keep BIT(10) for now, as the commit message doesn't
explain why this has been modified.

> > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n)	((n) << 8)
> > +#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK	(3 << 8)
> 
> GEN_MASK() is better here, And below other registers.

It is, but I wanted this patch to focus on the renames. I actually have
a patch in my branch to switch to GENMASK for all masks, I will add it
to the next version of this series.

> >  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
> > -#define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
> > -#define MIPI_CSIS_CMN_CTRL_ENABLE		BIT(0)
> > -
> > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET	8
> > -#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK		(3 << 8)
> > +#define MIPI_CSIS_CMN_CTRL_SW_RESET		BIT(1)
> > +#define MIPI_CSIS_CMN_CTRL_CSI_EN		BIT(0)
> >
> >  /* CSIS clock control */
> ...

-- 
Regards,

Laurent Pinchart