linux-kernel - Re: [PATCH] gpiolib: add gpio_lookup_chip() to get chip information for gpio

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] [day] [month] [year] [list]

Date:	Wed, 20 Jan 2010 22:12:31 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Ben Dooks <ben-linux@...ff.org>
Cc:	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	David Brownell <david-b@...bell.net>
Subject: Re: [PATCH] gpiolib: add gpio_lookup_chip() to get chip information
 for gpio

On Fri,  8 Jan 2010 06:32:28 +0000 Ben Dooks <ben-linux@...ff.org> wrote:

> Add a call to get the 'struct gpio_chip' pointer for a given gpio, so
> that core implementations which want to use gpiolib gpio numbering for
> things like mux configuration can get back to the gpio_chip that they
> registered without having to have their own list of gpio.
> 
> This is especially useful for the Samsung S3C64XX series where the GPIO
> bank sizes can vary from 32 down to 3, making it difficult to store an
> array to convert a number to chip.
> 
> ...
>  
> +/**
> + * gpio_lookup_chip - return the chip for a given gpio
> + * @gpio: The GPIO number to lookup
> + *
> + * Returns NULL if the GPIO chip is not valid or there is no chip registered
> + * for that GPIO. This call is available for core code to turn a GPIO number
> + * into a chip so that further information can be looked up.
> + *
> + * This call makes no guarantees about the actuall gpio_chip's state, or
> + * whether the @gpio itself is requested.
> + */
> +struct gpio_chip *gpio_lookup_chip(unsigned gpio)
> +{
> +	struct gpio_chip *chip;
> +	unsigned long flags;
> +
> +	if (!gpio_is_valid(gpio))
> +		return NULL;
> +
> +	spin_lock_irqsave(&gpio_lock, flags);
> +	chip = gpio_to_chip(gpio);
> +	spin_unlock_irqrestore(&gpio_lock, flags);
> +
> +	return chip;
> +}
> +

The locking looks fishy.  What's the point in locking the array lookup and
then returning the thing which was looked up after dropping the lock?

Should this function have been exported to modules?

Please cc myself and David Brownell on gpio patches.
--
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/