[U-Boot] [PATCH 3/8] drivers/misc: add stmpe2401 port extender and keypad controller

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <ed1819daf7e13ba4ae675c5ba4c96bb97350ea94.1255086085.git.rubini@ unipv.it> you wrote:
> From: Alessandro Rubini <rubini at unipv.it>
> 
> Signed-off-by: Alessandro Rubini <rubini at unipv.it>
> Acked-by: Andrea Gallo <andrea.gallo at stericsson.com>
...
> +int pe_getreg(int addr, int reg)
> +{
> +	unsigned char val8 = reg;
> +	int ret;
> +
> +	ret = i2c_read(addr, reg, 1 /* len */, &val8, 1);
> +	if (ret < 0) return ret;

Please split:

	if (ret < 0)
		return ret;

Please fix globally in whole patch set.

...
> +int pe_gpio_dir(int addr, int pin, int dir)
> +{
> +	int regval;
> +
> +	/* 0 == input, 1 == output */
> +	regval = pe_getreg(addr, PE_GPIO_GPDR(pin));
> +	if (regval < 0) return regval;
> +	regval &= ~PE_GPIO_MASK(pin);
> +	if (dir) regval |= PE_GPIO_MASK(pin);

ditto in cases like here.

Please check if you want to run the whole code through lindent or
similar.

> +int pe_gpio_set(int addr, int pin, int val)
> +{
> +	int reg;
> +
> +	if (val) reg = PE_GPIO_GPSR(pin);
> +	else reg = PE_GPIO_GPCR(pin);

Another of the countless cases of inacceptable indentation.

...
> diff --git a/include/stmpe2401.h b/include/stmpe2401.h
> new file mode 100644
> index 0000000..fe7691e
> --- /dev/null
> +++ b/include/stmpe2401.h
...
> +#define PE_GPIO_GPMR(gpio)	(0xa4 + PE_GPIO_OFFSET(gpio)) /* monitor */
> +#define PE_GPIO_GPCR(gpio)	(0x88 + PE_GPIO_OFFSET(gpio)) /* clear */
> +#define PE_GPIO_GPSR(gpio)	(0x85 + PE_GPIO_OFFSET(gpio)) /* set */
> +#define PE_GPIO_GPDR(gpio)	(0x8b + PE_GPIO_OFFSET(gpio)) /* direction */
> +#define PE_GPIO_GPPUR(gpio)	(0x97 + PE_GPIO_OFFSET(gpio)) /* pull-up */
> +#define PE_GPIO_GPPDR(gpio)	(0x9a + PE_GPIO_OFFSET(gpio)) /* pull-down */

Looks as if you want to turn this into a C struct?

> +/* keypad controller registers */
> +#define PE_KPC_COL		0x60
> +#define PE_KPC_ROW_MSB		0x61
> +#define PE_KPC_ROW_LSB		0x62
> +#define PE_KPC_CTRL_MSB		0x63
> +#define PE_KPC_CTRL_LSB		0x64
> +#define PE_KPC_DATA0		0x68
> +#define PE_KPC_DATA1		0x69
> +#define PE_KPC_DATA2		0x6a

Please make this a C struct.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What is wanted is not the will to believe,  but the will to find out,
which is the exact opposite.
		        -- Bertrand Russell, "Skeptical Essays", 1928