Hi Marek,

On Wed, Sep 28, 2022 at 02:10:26AM +0200, Marek Vasut wrote:
> On 9/28/22 01:38, Laurent Pinchart wrote:
> > The BIT() macro is meant to represent a single bit. Don't use it for
> > values of register fields that span multiple bits.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > ---
> >   drivers/gpu/drm/mxsfb/lcdif_regs.h | 28 ++++++++++++++--------------
> >   1 file changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h 
> > b/drivers/gpu/drm/mxsfb/lcdif_regs.h
> > index 013f2cace2a0..bc4d020aaa7c 100644
> > --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h
> > +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h
> > @@ -138,9 +138,9 @@
> >   
> >   #define DISP_PARA_DISP_ON         BIT(31)
> >   #define DISP_PARA_SWAP_EN         BIT(30)
> > -#define DISP_PARA_LINE_PATTERN_UYVY_H      (GENMASK(29, 28) | BIT(26))
> > -#define DISP_PARA_LINE_PATTERN_RGB565      GENMASK(28, 26)
> > -#define DISP_PARA_LINE_PATTERN_RGB888      0
> > +#define DISP_PARA_LINE_PATTERN_UYVY_H      (13 << 26)
> > +#define DISP_PARA_LINE_PATTERN_RGB565      (7 << 26)
> > +#define DISP_PARA_LINE_PATTERN_RGB888      (0 << 26)
> 
> Can we use hex here for the left size of the shift operation, so it's 
> subjectively easier to read ? DTTO for the other values ?
> 
> That is:
> -#define DISP_PARA_LINE_PATTERN_UYVY_H        (13 << 26)
> +#define DISP_PARA_LINE_PATTERN_UYVY_H        (0xd << 26)
> 
> [...]

Sure, I'll fix that.

-- 
Regards,

Laurent Pinchart

Reply via email to