Re: [patch/rfc 1/4] GPIO implementation framework

[David Brownell <david-b@pacbell.net>]

On Tuesday 13 November 2007, eric miao wrote:
> Subject: [PATCH 5/5] move per GPIO "requested" to "struct gpio_desc"
> 

>  struct gpio_desc {
>  	struct gpio_chip *chip;
>  	unsigned is_out:1;
> +	unsigned requested:1;
> +#ifdef CONFIG_DEBUG_FS
> +	const char *requested_str;
> +#endif

A better name for this would be "label", matching what's
passed from gpio_request().  Ndls abrviatns r bd.


Note that this means (on typical 32-bit embedded hardware)
twelve bytes per GPIO, which if you assume 256 GPIOs means
an extra 3 KB static memory compared to the patch I sent.


> @@ -43,20 +43,19 @@ static inline int gpio_is_onchip(unsigned gpio,
> struct gpio_chip *chip)
>  /* Warn when drivers omit gpio_request() calls -- legal but
>   * ill-advised when setting direction, and otherwise illegal.
>   */
> -static void gpio_ensure_requested(struct gpio_chip *chip, unsigned offset)
> +static void gpio_ensure_requested(unsigned gpio)

Simpler to pass a gpio_desc pointer ...


>  	if (!requested)
> -		printk(KERN_DEBUG "GPIO-%d autorequested\n",
> -				chip->base + offset);
> +		pr_debug("GPIO-%d autorequested\n", gpio);

Leave the printk in ... this is the sort of thing we want
to see fixed, which becomes unlikely once you hide such
diagnostics.  And for that matter, what would be enabling
the "-DDEBUG" that would trigger a pr_debug() message?



... overall the main downside of these patches seems to
be that it consumes more static memory.