[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Mike Frysinger]

On Monday 23 January 2012 01:20:16 Simon Glass wrote:
> On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger wrote:
> > On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
> >> --- /dev/null
> >> +++ b/arch/sandbox/include/asm/gpio.h
> >> 
> >> +int sandbox_gpio_get_value(int gp);
> > 
> > why bother with parallel sandbox gpio API ?  why can't we just implement
> > the gpio API directly and throw away sandbox_gpio_xxx ?  then we can
> > also stub out sandbox/include/asm/gpio.h ...
> 
> Because the current state of the GPIOs needs to be stored somewhere.
> Test code which wants a GPIO to appear to be high to U-Boot can call
> sandbox_gpio_set_value() and that value will be recorded and provided
> to future gpio_get_value() calls.
> 
> Without this virtualisation, the driver would have no purpose.

i'm not seeing it.  why does external code need to reach into the guts at all 
when we have a gpio API for them to use ?

> > also, missing gpio_status() define to gpio_info()
> 
> I see gpio_info() there at the bottom - gpio_status() is optional I
> think. Do we need to bring this in?

gpio_info is pointless without gpio_status.  honestly, i don't even know why 
that symbol exists at all ... the public API is gpio_status().  might as well 
just call it that.

> >> +/* read GPIO IN value of port 'gp' */
> >> +int gpio_get_value(int gp)
> >> ...
> >> +     if (get_gpio_flag(gp, GPIOF_OUTPUT))
> >> ...
> >> +int gpio_set_value(int gp, int value)
> >> ...
> >> +     if (get_gpio_flag(gp, GPIOF_OUTPUT)) {
> > 
> > drop valid gpio checking in these funcs ... only the request func should
> > do this
> 
> What if someone passes in garbage value? Don't we want to catch this?

normally, no.  in the sandbox world, i guess we do, but there should be 
verbose dump messages ... perhaps an assert() ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120124/a52b82a3/attachment.pgp>