On Wednesday, June 08, 2011 18:08:12 Tom Warren wrote: > --- a/common/cmd_gpio.c > +++ b/common/cmd_gpio.c > > +#ifdef CONFIG_TEGRA2_GPIO > +#include <asm/arch/gpio.h> > +#else > #include <asm/gpio.h> > +#endif
sorry, but no. create an asm/gpio.h in the arch/arm/ subtree that includes
asm/arch/gpio.h. you could probably even move some of the common stuff (like
the gpio func prototypes) into that header.
if we get enough people picking this up in u-boot, we can probably eventually
unify further in asm-generic/gpio.h. but that day isnt today :P.
> --- /dev/null
> +++ b/drivers/gpio/tegra2_gpio.c
>
> +int gpio_num = 0;
> +char gpio_label[20] = "";
this doesnt make much sense. the gpio code is not 1-pin at a time, it allows
for any number of pins to be requested at a time. i'd suggest you just drop
these two variables and their users.
> +/* Config pin 'gp' as GPIO or SFPIO, based on 'type' */
> +void __set_config(int gp, int type)
probably want all these internal funcs to be marked static
> +int gpio_request(int gp, const char *label)
> +{
> + /* Configure as a GPIO */
> + __set_config(gp, 1);
you probably want to do error checking on gp. if someone does
gpio_request(10000), i'm pretty sure your current code is going to crash and
burn.
> +int gpio_direction_input(int gp)
> +{
> ...
> + /* Configure as a GPIO */
> + __set_config(gp, 1);
> ...
> +int gpio_direction_output(int gp, int value)
> +{
> ...
> + /* Configure as a GPIO */
> + __set_config(gp, 1);
this should be unnecessary. code that calls any gpio_* funcs on a pin that
first did not call gpio_request (and succeed) is broken. dont work around
that in the core code.
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
