Re: [PATCH v4 03/13] media: nxp: imx8-isi: Remove unused offset in mxc_isi_reg and use BIT() macro for mask

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

On Thu, May 01, 2025 at 08:53:17PM -0400, Frank Li wrote:
> On Tue, Apr 22, 2025 at 12:06:55AM +0300, Laurent Pinchart wrote:
> > Hi Frank,
> >
> > Thank you for the patch.
> >
> > On Tue, Apr 08, 2025 at 05:53:01PM -0400, Frank Li wrote:
> > > Preserve clarity by removing the unused 'offset' field in struct mxc_isi_reg,
> > > as it duplicates information already indicated by the mask and remains unused.
> >
> > The commit message line length limit is normally 72 characters. I can
> > reflow when applying if no other change to the series is needed.
> 
> I remember it is 75 chars. Any document show it is 72. I need correct for
> the other patches also.

You're absolutely right, I don't know where I got the 72 columns limit
from. Please ignore my comment.

> > >
> > > Improve readability by replacing hex value masks with the BIT() macro.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > > ---
> > >  .../media/platform/nxp/imx8-isi/imx8-isi-core.c    | 25 +++++++++++-----------
> > >  .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  1 -
> > >  2 files changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > index 1e79b1211b603..ecfc95882f903 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > @@ -3,6 +3,7 @@
> > >   * Copyright 2019-2020 NXP
> > >   */
> > >
> > > +#include <linux/bits.h>
> > >  #include <linux/clk.h>
> > >  #include <linux/device.h>
> > >  #include <linux/errno.h>
> > > @@ -247,24 +248,24 @@ static void mxc_isi_v4l2_cleanup(struct mxc_isi_dev *isi)
> > >
> > >  /* For i.MX8QXP C0 and i.MX8MN ISI IER version */
> > >  static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v1 = {
> > > -	.oflw_y_buf_en = { .offset = 19, .mask = 0x80000  },
> > > -	.oflw_u_buf_en = { .offset = 21, .mask = 0x200000 },
> > > -	.oflw_v_buf_en = { .offset = 23, .mask = 0x800000 },
> > > +	.oflw_y_buf_en = { .mask = BIT(19) },
> > > +	.oflw_u_buf_en = { .mask = BIT(21) },
> > > +	.oflw_v_buf_en = { .mask = BIT(23) },
> > >
> > > -	.panic_y_buf_en = {.offset = 20, .mask = 0x100000  },
> > > -	.panic_u_buf_en = {.offset = 22, .mask = 0x400000  },
> > > -	.panic_v_buf_en = {.offset = 24, .mask = 0x1000000 },
> > > +	.panic_y_buf_en = { .mask = BIT(20) },
> > > +	.panic_u_buf_en = { .mask = BIT(22) },
> > > +	.panic_v_buf_en = { .mask = BIT(24) },
> > >  };
> > >
> > >  /* For i.MX8MP ISI IER version */
> > >  static const struct mxc_isi_ier_reg mxc_imx8_isi_ier_v2 = {
> > > -	.oflw_y_buf_en = { .offset = 18, .mask = 0x40000  },
> > > -	.oflw_u_buf_en = { .offset = 20, .mask = 0x100000 },
> > > -	.oflw_v_buf_en = { .offset = 22, .mask = 0x400000 },
> > > +	.oflw_y_buf_en = { .mask = BIT(18) },
> > > +	.oflw_u_buf_en = { .mask = BIT(20) },
> > > +	.oflw_v_buf_en = { .mask = BIT(22) },
> > >
> > > -	.panic_y_buf_en = {.offset = 19, .mask = 0x80000  },
> > > -	.panic_u_buf_en = {.offset = 21, .mask = 0x200000 },
> > > -	.panic_v_buf_en = {.offset = 23, .mask = 0x800000 },
> > > +	.panic_y_buf_en = { .mask = BIT(19) },
> > > +	.panic_u_buf_en = { .mask = BIT(21) },
> > > +	.panic_v_buf_en = { .mask = BIT(23) },
> > >  };
> > >
> > >  /* Panic will assert when the buffers are 50% full */
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > index 9c7fe9e5f941f..e7534a80af7b4 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > @@ -114,7 +114,6 @@ struct mxc_isi_buffer {
> > >  };
> > >
> > >  struct mxc_isi_reg {
> > > -	u32 offset;
> > >  	u32 mask;
> > >  };
> > >

-- 
Regards,

Laurent Pinchart