Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update

["Juha Yrjölä" <juha.yrjola@solidboot.com>]

On Mon, Dec 15, 2008 at 02:08:16PM +0200, Jani Nikula wrote:

> Add new function omap_update_gpio_switch() to support dynamically
> changing the GPIO switch notify callback functions and debounce
> timeouts.

Why do they need to be changed? GPIO switches related to the board schematics,
which do not hopefully change dynamically.

> +	spin_lock_irqsave(&sw->lock, flags);
>  	if (state)
>  		timeout = sw->debounce_rising;
>  	else
>  		timeout = sw->debounce_falling;
> +	spin_unlock_irqrestore(&sw->lock, flags);

What's the point of this? The above read can be consider an atomic operation.

> +	spin_lock_irqsave(&sw->lock, flags);
> +	notify = sw->notify;
> +	notify_data = sw->notify_data;
> +	spin_unlock_irqrestore(&sw->lock, flags);
> +
> +	if (notify != NULL)
> +		notify(notify_data, state);

What if omap_update_gpio_switch() is called just before the check for
notify != NULL from another (hypothetical =)) CPU? Then you end up with the
function completing, but still having the old notify callback called with
old notify_data.

Cheers,
Juha