From: Mike Frysinger Date: Tue, 24 Jan 2012 16:35:00 -0500 Subject: [U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs In-Reply-To: References: <1326242752-9981-1-git-send-email-sjg@chromium.org> <201201201359.21521.vapier@gentoo.org> Message-ID: <201201241635.01261.vapier@gentoo.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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: