Hi Marek,

On Wednesday 04 September 2013 16:06:34 Marek Vasut wrote:
> [...]
> > +# define LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE      (1 << 23)
> > +# define LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE        (1 << 22)
> > +# define LRADC_CTRL0_MX28_YNNSW    /* YM */        (1 << 21)
> > +# define LRADC_CTRL0_MX28_YPNSW    /* YP */        (1 << 20)
> > +# define LRADC_CTRL0_MX28_YPPSW    /* YP */        (1 << 19)
> > +# define LRADC_CTRL0_MX28_XNNSW    /* XM */        (1 << 18)
> > +# define LRADC_CTRL0_MX28_XNPSW    /* XM */        (1 << 17)
> > +# define LRADC_CTRL0_MX28_XPPSW    /* XP */        (1 << 16)
>
> Why do you put this space between # and define?

Matter of taste: to visible show a difference between registers itself and
their bits inside. Its my default state to help readers to read my code.

> > [...]
> >  #define    LRADC_CTRL1                             0x10
> >  #define    LRADC_CTRL1_TOUCH_DETECT_IRQ_EN         (1 << 24)
> >  #define    LRADC_CTRL1_LRADC_IRQ_EN(n)             (1 << ((n) + 16))
> > -#define    LRADC_CTRL1_LRADC_IRQ_EN_MASK           (0x1fff << 16)
> > +#define    LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK      (0x1fff << 16)
> >  #define    LRADC_CTRL1_LRADC_IRQ_EN_OFFSET         16
> >  #define    LRADC_CTRL1_TOUCH_DETECT_IRQ            (1 << 8)
> >  #define    LRADC_CTRL1_LRADC_IRQ(n)                (1 << (n))
> > -#define    LRADC_CTRL1_LRADC_IRQ_MASK              0x1fff
> > +#define    LRADC_CTRL1_MX28_LRADC_IRQ_MASK         0x1fff
>
> MIght just be easier to define this as
>
> LRADC....IRQ_MASK(id) (((id) == MX23) ? 0x1ff : 0x1fff)
>
> just like the MXS SSP driver does it. Then there won't be so much churn.

Nice. Will change it.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to