Re: [PATCH 3/4] gpio: add ARM Primcell pl061 support

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Oct 05, 2012 at 01:18:42PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> +
> +	ret = gpiochip_add(&chip->gc);
> +	if (ret) {
> +		dev_err(&dev->dev, "couldn't add gpiochip\n");

Print error value?

> +		goto free_mem;
> +	}
> +
> +	writeb(0, chip->base + GPIOIE); /* disable irqs */
> +
> +	for (i = 0; i < PL061_GPIO_NR; i++) {
> +		if (pdata) {
> +			if (pdata->directions & (1 << i))
> +				pl061_direction_output(&chip->gc, i,
> +						pdata->values & (1 << i));
> +			else
> +				pl061_direction_input(&chip->gc, i);

I don't think a gpio driver should alter the directions without being
asked. Just leave the directions untouched without platform_data.

> +
> +MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>");
> +MODULE_DESCRIPTION("PL061 GPIO driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/amba/pl061.h b/include/linux/amba/pl061.h
> new file mode 100644
> index 0000000..fb83c04
> --- /dev/null
> +++ b/include/linux/amba/pl061.h
> @@ -0,0 +1,16 @@
> +#include <linux/types.h>
> +
> +/* platform data for the PL061 GPIO driver */
> +
> +struct pl061_platform_data {
> +	/* number of the first GPIO */
> +	unsigned	gpio_base;
> +
> +	/* number of the first IRQ.
> +	 * If the IRQ functionality in not desired this must be set to 0.
> +	 */
> +	unsigned	irq_base;
> +
> +	u8		directions;	/* startup directions, 1: out, 0: in */
> +	u8		values;		/* startup values */
> +};

include file protection?

Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox