Hi, On Fri, Jan 30, 2015 at 07:43:24AM +0100, Lothar Waßmann wrote: > Hi, > > Markus Pargmann wrote: > > This is the core driver for imx25 touchscreen/adc driver. The module > > has one shared ADC and two different conversion queues which use the > > ADC. The two queues are identical. Both can be used for general purpose > > ADC but one is meant to be used for touchscreens. > > > > This driver is the core which manages the central components and > > registers of the TSC/ADC unit. It manages the IRQs and forwards them to > > the correct components. > > > [...] > > diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c > > new file mode 100644 > > index 000000000000..8e4013d57500 > > --- /dev/null > > +++ b/drivers/mfd/fsl-imx25-tsadc.c > > @@ -0,0 +1,167 @@ > [...] > > +#define MX25_TGCR_POWERMODE_MASK (3 << 8) > > +#define MX25_TGCR_POWERMODE_SAVE BIT(8) > > +#define MX25_TGCR_POWERMODE_ON (2 << 8) > > > This looks a bit weird and conceals the fact, that > MX25_TGCR_POWERMODE_SAVE is in fact one of the possible settings > of a two bit bitfield. For consistency I would write: > #define MX25_TGCR_POWERMODE_MASK (3 << 8) > #define MX25_TGCR_POWERMODE_SAVE (1 << 8) > #define MX25_TGCR_POWERMODE_ON (2 << 8) > > [...] > > +#define MX25_ADCQ_CFG_YPLL_HIGH 0 > > +#define MX25_ADCQ_CFG_YPLL_OFF BIT(12) > > +#define MX25_ADCQ_CFG_YPLL_LOW (3 << 12) > > > dto. > > > +#define MX25_ADCQ_CFG_XNUR_HIGH 0 > > +#define MX25_ADCQ_CFG_XNUR_OFF BIT(10) > > +#define MX25_ADCQ_CFG_XNUR_LOW (3 << 10) > > > dto. > > > +#define MX25_ADCQ_CFG_XPUL_OFF BIT(9) > > +#define MX25_ADCQ_CFG_XPUL_HIGH 0 > > > |#define MX25_ADCQ_CFG_XPUL_OFF (1 << 9) > |#define MX25_ADCQ_CFG_XPUL_HIGH (0 << 9) > would make it more clear, that these refer to the two states of the same > bit. > > > +#define MX25_ADCQ_CFG_REFP(sel) (sel << 7) > > > missing () around macro argument > > > +#define MX25_ADCQ_CFG_REFP_YP 0 > > +#define MX25_ADCQ_CFG_REFP_XP (1 << 7) > > +#define MX25_ADCQ_CFG_REFP_EXT (2 << 7) > > +#define MX25_ADCQ_CFG_REFP_INT (3 << 7) > > +#define MX25_ADCQ_CFG_REFP_MASK (3 << 7) > > > see my previous comment. > > > +#define MX25_ADCQ_CFG_IN(sel) (sel << 4) > > > missing () around macro argument > > > +#define MX25_ADCQ_CFG_IN_XP 0 > > +#define MX25_ADCQ_CFG_IN_YP (1 << 4) > > +#define MX25_ADCQ_CFG_IN_XN (2 << 4) > > +#define MX25_ADCQ_CFG_IN_YN (3 << 4) > > > see my previous comment. > > > +#define MX25_ADCQ_CFG_IN_WIPER (4 << 4) > > +#define MX25_ADCQ_CFG_IN_AUX0 (5 << 4) > > +#define MX25_ADCQ_CFG_IN_AUX1 (6 << 4) > > +#define MX25_ADCQ_CFG_IN_AUX2 (7 << 4) > > +#define MX25_ADCQ_CFG_REFN(sel) (sel << 2) > > > missing () around macro argument > > > +#define MX25_ADCQ_CFG_REFN_XN 0 > > +#define MX25_ADCQ_CFG_REFN_YN (1 << 2) > > +#define MX25_ADCQ_CFG_REFN_NGND (2 << 2) > > +#define MX25_ADCQ_CFG_REFN_NGND2 (3 << 2) > > +#define MX25_ADCQ_CFG_REFN_MASK (3 << 2) > > > see my previous comment.
Thanks, I fixed all of the comments. Best regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
signature.asc
Description: Digital signature
