linux-kernel - Re: [PATCH 01/12] Blackfin arch: add peripheral resource allocation support

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]

Message-Id: <200708171112.04697.david-b@pacbell.net>
Date:	Fri, 17 Aug 2007 11:12:04 -0700
From:	David Brownell <david-b@...bell.net>
To:	Bryan Wu <bryan.wu@...log.com>
Cc:	torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org,
	Michael Hennerich <michael.hennerich@...log.com>
Subject: Re: [PATCH 01/12] Blackfin arch: add peripheral resource allocation support

On Tuesday 07 August 2007, Bryan Wu wrote:
> From: Michael Hennerich <michael.hennerich@...log.com>

The patch description here is IMO misleading, and is clearly
weak-to-nonexistent ...  what this patch does is

 * Start tracking the label strings provided by gpio_request()
 * Provide a new portmux mechanisms
 * Start using those in the serial support code

When I read "resource allocation" I think of "struct resource"
from <linux/ioport.h>, allocate_resource(), and so on.  So while
it's true there are other kinds of driver resource, it's rather
unnatural for me to think about pin mux and gpio issues in any
terms other than chip and board setup.

> +static int cmp_label(unsigned short ident, const char *label)
> +{
> +	if (label && str_ident)
> +		return strncmp(str_ident + ident * RESOURCE_LABEL_SIZE,
> +				 label, strlen(label));
> +	else
> +		return -EINVAL;
> +}

GRPIO labels are purely for diagnostics.  There's no reason to
compare one to another.  You seem to be using these for purposes
in addition to GPIOs though ... probably worth commenting on that
unusual scheme.

> +int peripheral_request(unsigned short per, const char *label)
> +{
> +	...
> +
> +	if (unlikely(reserved_peri_map[gpio_bank(ident)] & gpio_bit(ident))) {
> +
> +	/*
> +	 * Pin functions like AMC address strobes my
> +	 * be requested and used by several drivers
> +	 */
> +
> +	if (!(per & P_MAYSHARE)) {

Goofy indentation.  And as a rule, drivers have been kept out of
the business of configuring pin usage.  It's simpler that way;
they don't need to try coping with configuration errors like two
drivers wanting conflicting usage ... or as you say above, needing
some explicit sharing mechanism ...

> +
> +	/*
> +	 * Allow that the identical pin function can
> +	 * be requested from the same driver twice
> +	 */

... or as you say here, needing to structure themselves so they
don't configure the same usage more than once ...

That said, how you handle pinmux on Blackfin is your business.

But you should know that this approach seems idiosyncratic and
more complex than needed:  when pin config is done early and as
part of board setup, drivers don't need to care about it or to
handle any pinmux errors.  And heck, products can sometimes be
shipped with the bootloader having done all pinmux setup, so
Linux won't need to worry about it at all.  That can help ship
multiple board revisions using the same kernel.

- Dave

-
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/