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.pinchart@xxxxxxxxxxxxxxxx> > > --- > > 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