linux-kernel - Re: [PATCH 02/12] Blackfin arch: Add label to call new GPIO API

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: <200708171124.54839.david-b@pacbell.net>
Date:	Fri, 17 Aug 2007 11:24:54 -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 02/12] Blackfin arch: Add label to call new GPIO API

Again, the patch descriptions need work.  This changes the
IRQ code (to add those labels).  $SUBJECT doesn't mention IRQs,
neither does the description ...


On Tuesday 07 August 2007, Bryan Wu wrote:
> --- a/arch/blackfin/mach-common/ints-priority-dc.c
> +++ b/arch/blackfin/mach-common/ints-priority-dc.c
> @@ -221,7 +221,7 @@ static unsigned int bf561_gpio_irq_startup(unsigned int irq)
>  
>         if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
>  
> -               ret = gpio_request(gpionr, NULL);
> +               ret = gpio_request(gpionr, "IRQ");
>                 if (ret)
>                         return ret;
>  
> @@ -261,7 +261,7 @@ static int bf561_gpio_irq_type(unsigned int irq, unsigned int type)
>  
>                 if (!(gpio_enabled[gpio_bank(gpionr)] & gpio_bit(gpionr))) {
>  
> -                       ret = gpio_request(gpionr, NULL);
> +                       ret = gpio_request(gpionr, "IRQ");
>                         if (ret)
>                                 return ret;
>  

Just for the record, this is an unusual way to use these calls.

Other platforms completely decouple these issues from the
IRQ infrastructure ... doing the pinmux and gpio claiming
separately from the request_irq()/free_irq() paths, mostly
as part of board setup.  Doing all of that "early":

 - keeps those error returns from causing hard-to-track-down
   runtime bugs;

 - works always, even on platforms where a given IRQ may
   appear on any of several pins/balls;

 - makes it easier to cross-check against board schematics,
   by keeping most board-specific setup in one source file;

 - shrinks the kernel's runtime footprint;

 - allows the label to be more descriptive ... describeing
   exactly *which* IRQ, so that using the labels for better
   diagnostics actually gives better diagnostics.

Again, not "wrong"; but probably sub-optimal.  You might
want to move towards earlier binding now, while Linux is
still young on Blackfin and you don't have legacy code to
worry about.

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