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

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

[Jani Nikula]

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

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
---
 arch/arm/plat-omap/gpio-switch.c              |   39 +++++++++++++++++++++++-
 arch/arm/plat-omap/include/mach/gpio-switch.h |    7 ++++-
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/gpio-switch.c b/arch/arm/plat-omap/gpio-switch.c
index 2b5665d..955cd29 100644
--- a/arch/arm/plat-omap/gpio-switch.c
+++ b/arch/arm/plat-omap/gpio-switch.c
@@ -40,6 +40,7 @@ struct gpio_switch {
 	void (* notify)(void *data, int state);
 	void *notify_data;
 
+	spinlock_t		lock;
 	struct work_struct	work;
 	struct timer_list	timer;
 	struct platform_device	pdev;
@@ -188,6 +189,7 @@ static irqreturn_t gpio_sw_irq_handler(int irq, void *arg)
 	struct gpio_switch *sw = arg;
 	unsigned long timeout;
 	int state;
+	unsigned long flags;
 
 	if (!sw->both_edges) {
 		if (gpio_get_value(sw->gpio))
@@ -200,10 +202,13 @@ static irqreturn_t gpio_sw_irq_handler(int irq, void *arg)
 	if (sw->state == state)
 		return IRQ_HANDLED;
 
+	spin_lock_irqsave(&sw->lock, flags);
 	if (state)
 		timeout = sw->debounce_rising;
 	else
 		timeout = sw->debounce_falling;
+	spin_unlock_irqrestore(&sw->lock, flags);
+
 	if (!timeout)
 		schedule_work(&sw->work);
 	else
@@ -223,14 +228,24 @@ static void gpio_sw_handler(struct work_struct *work)
 {
 	struct gpio_switch *sw = container_of(work, struct gpio_switch, work);
 	int state;
+	unsigned long flags;
+	void (*notify)(void *data, int state);
+	void *notify_data;
 
 	state = gpio_sw_get_state(sw);
 	if (sw->state == state)
 		return;
 
 	sw->state = state;
-	if (sw->notify != NULL)
-		sw->notify(sw->notify_data, state);
+
+	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);
+
 	sysfs_notify(&sw->pdev.dev.kobj, NULL, "state");
 	print_sw_state(sw, state);
 }
@@ -323,6 +338,7 @@ static int __init new_switch(struct gpio_switch *sw)
 		return r;
 	}
 
+	spin_lock_init(&sw->lock);
 	INIT_WORK(&sw->work, gpio_sw_handler);
 	init_timer(&sw->timer);
 
@@ -388,6 +404,25 @@ no_check:
 	return NULL;
 }
 
+int omap_update_gpio_switch(const struct omap_gpio_switch *cfg)
+{
+	unsigned long flags;
+	struct gpio_switch *sw = find_switch(cfg->gpio, cfg->name);
+
+	if (!sw)
+		return -EINVAL;
+
+	spin_lock_irqsave(&sw->lock, flags);
+	sw->debounce_rising = cfg->debounce_rising;
+	sw->debounce_falling = cfg->debounce_falling;
+	sw->notify = cfg->notify;
+	sw->notify_data = cfg->notify_data;
+	spin_unlock_irqrestore(&sw->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL(omap_update_gpio_switch);
+
 static int __init add_board_switches(void)
 {
 	int i;
diff --git a/arch/arm/plat-omap/include/mach/gpio-switch.h b/arch/arm/plat-omap/include/mach/gpio-switch.h
index a143253..53c1fd5 100644
--- a/arch/arm/plat-omap/include/mach/gpio-switch.h
+++ b/arch/arm/plat-omap/include/mach/gpio-switch.h
@@ -47,12 +47,17 @@ struct omap_gpio_switch {
 	void *notify_data;
 };
 
-/* Call at init time only */
 #ifdef CONFIG_OMAP_GPIO_SWITCH
+/* Call at init time only */
 extern void omap_register_gpio_switches(const struct omap_gpio_switch *tbl,
 					int count);
+extern int omap_update_gpio_switch(const struct omap_gpio_switch *cfg);
 #else
 #define omap_register_gpio_switches(tbl, count)	do { } while (0)
+static inline int omap_update_gpio_switch(const struct omap_gpio_switch *cfg)
+{
+	return 0;
+}
 #endif
 
 #endif
-- 
1.6.0.4

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

[Trilok Soni]

Hi Tony

On Mon, Dec 15, 2008 at 5:38 PM, Jani Nikula
<ext-jani.1.nikula@nokia.com> wrote:
> Add new function omap_update_gpio_switch() to support dynamically
> changing the GPIO switch notify callback functions and debounce
> timeouts.
>
> Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>

Not related to this patch, but why not we convert this omap specific
gpio switch code into generic switch framework and attach gpios to
that switch framework, other arm-linux platforms seems to also use
such home-brew frameworks for this functionality it seems.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

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

[Felipe Balbi]

On Mon, Dec 15, 2008 at 05:44:26PM +0530, ext Trilok Soni wrote:
> Hi Tony
> 
> On Mon, Dec 15, 2008 at 5:38 PM, Jani Nikula
> <ext-jani.1.nikula@nokia.com> wrote:
> > Add new function omap_update_gpio_switch() to support dynamically
> > changing the GPIO switch notify callback functions and debounce
> > timeouts.
> >
> > Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
> 
> Not related to this patch, but why not we convert this omap specific
> gpio switch code into generic switch framework and attach gpios to
> that switch framework, other arm-linux platforms seems to also use
> such home-brew frameworks for this functionality it seems.

Yes, would be cool. Move it to gpiolib, I'm sure there are other
platforms that might get interested in this :-)

Jani, do you plan to do that ??

-- 
balbi

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

[Felipe Balbi]

Hi,

one comment below

On Mon, Dec 15, 2008 at 02:08:16PM +0200, ext Jani Nikula wrote:
> +int omap_update_gpio_switch(const struct omap_gpio_switch *cfg)
> +{
> +	unsigned long flags;
> +	struct gpio_switch *sw = find_switch(cfg->gpio, cfg->name);
> +
> +	if (!sw)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&sw->lock, flags);
> +	sw->debounce_rising = cfg->debounce_rising;
> +	sw->debounce_falling = cfg->debounce_falling;
> +	sw->notify = cfg->notify;
> +	sw->notify_data = cfg->notify_data;
> +	spin_unlock_irqrestore(&sw->lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(omap_update_gpio_switch);

how about you only change what's not null ?? then you could only change
the notify callback and keep the same debounce_rising/falling ??

-- 
balbi

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

[Jani Nikula]

On Mon, 2008-12-15 at 15:22 +0200, Felipe Balbi wrote:
> Hi,
> 
> one comment below
> 
> On Mon, Dec 15, 2008 at 02:08:16PM +0200, ext Jani Nikula wrote:
> > +int omap_update_gpio_switch(const struct omap_gpio_switch *cfg)
> > +{
> > +	unsigned long flags;
> > +	struct gpio_switch *sw = find_switch(cfg->gpio, cfg->name);
> > +
> > +	if (!sw)
> > +		return -EINVAL;
> > +
> > +	spin_lock_irqsave(&sw->lock, flags);
> > +	sw->debounce_rising = cfg->debounce_rising;
> > +	sw->debounce_falling = cfg->debounce_falling;
> > +	sw->notify = cfg->notify;
> > +	sw->notify_data = cfg->notify_data;
> > +	spin_unlock_irqrestore(&sw->lock, flags);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(omap_update_gpio_switch);
> 
> how about you only change what's not null ?? then you could only change
> the notify callback and keep the same debounce_rising/falling ??

That's a nice idea, but gpio_sw_irq_handler() actually supports having 0
debounce timeouts, i.e. no settling time. Of course, I could use -1 for
"don't update". However, the semantics above is exactly the same as in
add_board_switches() for the update case. I'm not sure if it would be a
good idea to deviate from this - what do you think? 

Jani.

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

[Felipe Balbi]

On Mon, Dec 15, 2008 at 03:44:05PM +0200, Jani Nikula wrote:
> That's a nice idea, but gpio_sw_irq_handler() actually supports having 0
> debounce timeouts, i.e. no settling time. Of course, I could use -1 for
> "don't update". However, the semantics above is exactly the same as in
> add_board_switches() for the update case. I'm not sure if it would be a
> good idea to deviate from this - what do you think? 

makes sense to me. But then again, if the uses does something like:

static struct omap_gpio_switch *cfg;

static int __init blabla_probe(struct platform_device *dev)
{
	cfg = kmalloc(sizeof(cfg), GFP_KERNEL));

	cfg->notify = my_notify;

	omap_update_gpio_swtich(cfg);

	return 0;
};

that means that if cfg->debounce_rising was different than 0, it'll get
overwritten to 0, right ? So, at least, you should put a big note for
users to initialize all necessary fields. Or, again, you only change if
cfg-><whatever> is different than 0 (or NULL), but that could a problem
when you really wanna change cfg->debounce_rising from, say, 100 to 0.
That wouldn't happen :-p

-- 
balbi

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

[Juha Yrjölä]

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

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

[Jani Nikula]

On Mon, 2008-12-15 at 15:40 +0200, ext Juha Yrjölä wrote:
> 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.

The switches themselves don't change (nor does this patch support
changing them), but different kinds of external devices may be connected
to the GPIO swiches. It would be useful to be able to change the
notification callbacks and debounce timeouts according to the device.

> > +	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.

Umm, I suppose I was more worried that the write might not be an atomic
operation, messing up the read as well. But I'll take your word for it
if you say the write is okay. :)

> > +	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.

I was, of course, making sure a NULL pointer is not called and there's
no mismatch between old/new notify/notify_data. Your scenario might
theoretically actually occur on a single processor system as well, don't
you think? 

But is it actually a problem or not? And if yes, do you have any
suggestions as to handling the case? If there's no need to lock for
reading the debounce timeouts in gpio_sw_irq_handler() as you say above,
do you think I could switch to a mutex and call notify callback holding
that?


BR,
Jani.


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Juha Yrjölä]

On Mon, Dec 15, 2008 at 04:52:32PM +0200, Jani Nikula wrote:

> > Why do they need to be changed? GPIO switches related to the board schematics,
> > which do not hopefully change dynamically.
> 
> The switches themselves don't change (nor does this patch support
> changing them), but different kinds of external devices may be connected
> to the GPIO swiches. It would be useful to be able to change the
> notification callbacks and debounce timeouts according to the device.

Could you give an example use-case? If the gpio-switch framework starts
getting extended and used more, it might be sensible to convert it to
the general GPIO API, as Trilok suggested.

> Umm, I suppose I was more worried that the write might not be an atomic
> operation, messing up the read as well. But I'll take your word for it
> if you say the write is okay. :)

Integer reads/writes are atomic, unless the system has some serious cache
coherency issues. And in that case, spinlocks won't save you either -- only
the HW engineers can. =)

> > 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.
>
> I was, of course, making sure a NULL pointer is not called and there's
> no mismatch between old/new notify/notify_data. Your scenario might
> theoretically actually occur on a single processor system as well, don't
> you think? 

Ah, yes. I was under the impression that gpio_sw_handler() was called from
IRQ context, but since it's a work handler, the function certainly can be
preempted.

> But is it actually a problem or not? And if yes, do you have any
> suggestions as to handling the case? If there's no need to lock for
> reading the debounce timeouts in gpio_sw_irq_handler() as you say above,
> do you think I could switch to a mutex and call notify callback holding
> that?

Especially in the case of frameworks, it's good to make things as correct
as possible. A mutex is safe.

Cheers,
Juha

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

[Jani Nikula]

On Mon, 2008-12-15 at 17:29 +0200, ext Juha Yrjölä wrote:
> On Mon, Dec 15, 2008 at 04:52:32PM +0200, Jani Nikula wrote:
> 
> > The switches themselves don't change (nor does this patch support
> > changing them), but different kinds of external devices may be connected
> > to the GPIO swiches. It would be useful to be able to change the
> > notification callbacks and debounce timeouts according to the device.
> 
> Could you give an example use-case? 

For example a jack that supports headsets with a button. The button
could function by simply grounding one of the lines (connected to a
GPIO, of course) or by generating some sort of an interrupt pulse. The
jack might support other kinds of accessories as well. And the required
actions and settling times might differ vastly.

> If the gpio-switch framework starts
> getting extended and used more, it might be sensible to convert it to
> the general GPIO API, as Trilok suggested.

I agree with this, but I'm afraid I don't have the time to extend the
scope of my work that much right now.


BR,
Jani.


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Trilok Soni]

Hi Jani,

On Mon, Dec 15, 2008 at 9:28 PM, Jani Nikula
<ext-jani.1.nikula@nokia.com> wrote:
> On Mon, 2008-12-15 at 17:29 +0200, ext Juha Yrjölä wrote:
>> On Mon, Dec 15, 2008 at 04:52:32PM +0200, Jani Nikula wrote:
>>
>> > The switches themselves don't change (nor does this patch support
>> > changing them), but different kinds of external devices may be connected
>> > to the GPIO swiches. It would be useful to be able to change the
>> > notification callbacks and debounce timeouts according to the device.
>>
>> Could you give an example use-case?
>
> For example a jack that supports headsets with a button. The button
> could function by simply grounding one of the lines (connected to a
> GPIO, of course) or by generating some sort of an interrupt pulse. The
> jack might support other kinds of accessories as well. And the required
> actions and settling times might differ vastly.
>
>> If the gpio-switch framework starts
>> getting extended and used more, it might be sensible to convert it to
>> the general GPIO API, as Trilok suggested.
>
> I agree with this, but I'm afraid I don't have the time to extend the
> scope of my work that much right now.
>

OK, I found other guys (android ??) using such home brew frameworks.
Time to write a switch framework.

android-msm git tree: drivers/switch

Link:
http://android.git.kernel.org/?p=kernel/msm.git;a=tree;f=drivers/switch;h=b064ed6374257163439f861f105a651126f11f08;hb=HEAD

As you have explained above headset and headset switch/button
detection usecase, similar one is in case of android using their
switch framework:

Link:
http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=arch/arm/mach-msm/board-trout-h2w.c;h=c6dfd726adb6decf5b5e1a9d18606b2154b8f229;hb=refs/heads/android-msm-htc-2.6.25

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[Jani Nikula]

On Tue, 2008-12-16 at 11:35 +0530, ext Trilok Soni wrote:

> OK, I found other guys (android ??) using such home brew frameworks.
> Time to write a switch framework.

To start a discussion on what such a GPIO switch framework should be
like if someone were to write it, here's a list of the kind of things
I'd like to see in it (mostly from gpio-switch.c):

* Based on or integrated in the gpiolib.

* Dynamically changeable notification callbacks in kernel.

* Sysfs notifications to userspace.

* Debouncing for the notifications to filter spurious events.

* Dynamically adjustable debounce timeouts.

* Arbitrary names for the GPIOs in sysfs instead of (or in addition to)
the gpiolib style "gpioN".

Anything else? I didn't see anything in the Android source that couldn't
be covered with this.


Jani.

Re: GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[Trilok Soni]

Hi Jani,

On Thu, Dec 18, 2008 at 5:12 PM, Jani Nikula
<ext-jani.1.nikula@nokia.com> wrote:
> On Tue, 2008-12-16 at 11:35 +0530, ext Trilok Soni wrote:
>
>> OK, I found other guys (android ??) using such home brew frameworks.
>> Time to write a switch framework.
>
> To start a discussion on what such a GPIO switch framework should be
> like if someone were to write it, here's a list of the kind of things
> I'd like to see in it (mostly from gpio-switch.c):

Why limit to GPIO based switches only? GPIOs should be client of
switch framework I think, and how h/w implements that switch should be
hidden by client specific driver.

>
> * Based on or integrated in the gpiolib.

gpio-switch-client would do that. Main framework just provides the
generic operations hooks.

>
> * Dynamically changeable notification callbacks in kernel.

>
> * Sysfs notifications to userspace.
>

Yes, possible to through uevents, android-switch framework does that.

> * Debouncing for the notifications to filter spurious events.
>
> * Dynamically adjustable debounce timeouts.
>
> * Arbitrary names for the GPIOs in sysfs instead of (or in addition to)
> the gpiolib style "gpioN".

It is left to gpio-switch-client to provide good names to switch
framework which can be exposed by main framework through sysfs. So,
users of this files in userspace doesn't care if it is gpio or
something else.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[Jani Nikula]

On Thu, 2008-12-18 at 18:30 +0530, ext Trilok Soni wrote:

> Why limit to GPIO based switches only? GPIOs should be client of
> switch framework I think, and how h/w implements that switch should be
> hidden by client specific driver.

You're quite right, it's better not to make such unnecessary
limitations. All the more reason to discuss before anyone submits a
truckload of patches for review. :)

Jani.

Re: GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[Trilok Soni]

Hi Jani,

On Thu, Dec 18, 2008 at 7:10 PM, Jani Nikula
<ext-jani.1.nikula@nokia.com> wrote:
> On Thu, 2008-12-18 at 18:30 +0530, ext Trilok Soni wrote:
>
>> Why limit to GPIO based switches only? GPIOs should be client of
>> switch framework I think, and how h/w implements that switch should be
>> hidden by client specific driver.
>
> You're quite right, it's better not to make such unnecessary
> limitations. All the more reason to discuss before anyone submits a
> truckload of patches for review. :)

Let me CC Brian from Android here.

Brian, we are discussing here of making generic switch framework and
android-kernel also have near to generic switch fwk implementation, so
that we can combine the linux-omap gpio switch fwk ideas and android
one to create generic switch framework.

For complete thread information, please start from this link:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg07845.html

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[Brian Swetland]

Definitely worth discussing -- I'm going to be out of the office for two
weeks over the holidays, and most of the rest of our team is on vacation
as well.  I'll pass this on, but I'm not sure if people will have much
time to take a look before January.

[Trilok Soni <soni.trilok@gmail.com>]
> Hi Jani,
> 
> On Thu, Dec 18, 2008 at 7:10 PM, Jani Nikula
> <ext-jani.1.nikula@nokia.com> wrote:
> > On Thu, 2008-12-18 at 18:30 +0530, ext Trilok Soni wrote:
> >
> >> Why limit to GPIO based switches only? GPIOs should be client of
> >> switch framework I think, and how h/w implements that switch should be
> >> hidden by client specific driver.
> >
> > You're quite right, it's better not to make such unnecessary
> > limitations. All the more reason to discuss before anyone submits a
> > truckload of patches for review. :)
> 
> Let me CC Brian from Android here.
> 
> Brian, we are discussing here of making generic switch framework and
> android-kernel also have near to generic switch fwk implementation, so
> that we can combine the linux-omap gpio switch fwk ideas and android
> one to create generic switch framework.
> 
> For complete thread information, please start from this link:
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg07845.html
> 
> -- 
> ---Trilok Soni
> http://triloksoni.wordpress.com
> http://www.linkedin.com/in/triloksoni

Re: GPIO switch framework (was: Re: [PATCH] ARM: OMAP: Add support for dynamic GPIO switch update)

[David Brownell]

On Thursday 18 December 2008, Jani Nikula wrote:
> On Tue, 2008-12-16 at 11:35 +0530, ext Trilok Soni wrote:
> 
> > OK, I found other guys (android ??) using such home brew frameworks.
> > Time to write a switch framework.
> 
> To start a discussion on what such a GPIO switch framework should be
> like if someone were to write it, here's a list of the kind of things
> I'd like to see in it (mostly from gpio-switch.c):
> 
> * Based on or integrated in the gpiolib.

As noted earlier:  such a thing should be a "switch" framework,
with "gpio" just on implementation.  Not every switch will be
implemented as a GPIO.


> * Dynamically changeable notification callbacks in kernel.

Not clear what this means.  Reconfigure the switch type
on the fly?  I'm not a big fan of the event namespace
which <linux/notifier.h> defines, I hope that's not what
is meant here ... though maybe its callback model is OK,
adding and removing callbacks is easy enough.

Current OMAP gpio-switch has a single callback, and
that might be part of the problem.  Linux has a single
callback for things like timers and request completions;
but these switches have different lifecycle model.

Decoupling caller/switch and callee(s)/callbacks() may
provide a better model.


> * Sysfs notifications to userspace.

Why sysfs in particular, rather than some /dev/input/*
event channel?  Note that input devices already decouple
the event reporters and event receivers fairly well.


> * Debouncing for the notifications to filter spurious events.
> 
> * Dynamically adjustable debounce timeouts.

Debouncing might not be a framework issue; at first
thought, it seems like something the GPIO glue into
that framework might need to handle.


> * Arbitrary names for the GPIOs in sysfs instead of (or in addition to)
> the gpiolib style "gpioN".

This mostly sounds to me like an input device...

Except maybe for the notion that in-kernel software
needs to be responsive to the events too.  I seem to
recall "push those policies to userspace" arguments
cropping up in similar cases.  (My two cents:  kernel
is full of policy, sometimes reconfigurable; easy to
believe system integrity can require a few more.)


> Anything else? I didn't see anything in the Android source that couldn't
> be covered with this.

The Android stuff seems already split into a class
and GPIO glue.  (Hence my CC to Mike Lockwood, the
author of that drivers/switch code.)

Any reason you couldn't use that as-is, maybe after
updating it a bit?  Add some GPIO debouncing, use
input framework not miscdev in the core, different
headset issues, etc.

- Dave


> 
> 
> Jani.
> 
> 
>