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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Nov 2007 20:36:13 -0800
From:	David Brownell <david-b@...bell.net>
To:	"eric miao" <eric.y.miao@...il.com>
Cc:	"Linux Kernel list" <linux-kernel@...r.kernel.org>,
	"Felipe Balbi" <felipebalbi@...rs.sourceforge.net>,
	"Bill Gatliff" <bgat@...lgatliff.com>,
	"Haavard Skinnemoen" <hskinnemoen@...el.com>,
	"Andrew Victor" <andrew@...people.com>,
	"Tony Lindgren" <tony@...mide.com>,
	"Jean Delvare" <khali@...ux-fr.org>,
	"Kevin Hilman" <khilman@...sta.com>,
	"Paul Mundt" <lethal@...ux-sh.org>,
	"Ben Dooks" <ben@...nity.fluff.org>
Subject: Re: [patch/rfc 1/4] GPIO implementation framework

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.  
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/