2009/3/19 Greg KH <[email protected]>:
> On Wed, Mar 18, 2009 at 10:52 PM, Misael Lopez <[email protected]> wrote:
>>>> Below are the my changes in kernel side. As mentioned in previous
>>>> mail, I'm using Switch GPIO driver (which uses Switch Class driver) to
>>>> generate the headset uevent. In the patch, it's also included the
>>>> board file (for OMAP 3430SDP board) although it's not part of standard
>>>> android kernel it may be useful for reference.
>>>
>>> Do you have a pointer to those drivers?
>>
>> Those drivers are provided along with the android kernel. You can find them 
>> at:
>>
>> http://android.git.kernel.org/?p=kernel/common.git;a=tree;f=drivers/switch;h=c02cb39fc78a60aa4b046481596d7b184b39fa8a;hb=453dc2b690b52248ac6dcb068965f1a37d675cef
>
> Ah, thanks, missed that.

> Also, why a special class?  Why not use the input layer for stuff like
> this?

In the particular case of headset detection, I originally considered
using ALSA Jack mechanism (and Soc Jack wrapper), which uses input
layer. You probably know about that already.

- Generic ALSA Jack mechanism
http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=blob;f=sound/core/jack.c;h=dd4a12dc09aa44f4c50c5b60f2cf5584c5635589;hb=632087748c3795a54d5631e640df65592774e045

- Wrapper for ALSA SoC drivers
http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=blob;f=sound/soc/soc-jack.c;h=28346fb2e70c08033158920f4ae7a35032239762;hb=632087748c3795a54d5631e640df65592774e045

> Also, putting 280 bytes on the stack isn't that nice in the
> switch_set_state() function, is it really needed?  Why can't that be
> dynamic?

Please take a look of below change:

diff --git a/drivers/switch/switch_class.c b/drivers/switch/switch_class.c
index 7702882..eee5188 100644
--- a/drivers/switch/switch_class.c
+++ b/drivers/switch/switch_class.c
@@ -22,10 +22,14 @@
 #include <linux/fs.h>
 #include <linux/err.h>
 #include <linux/switch.h>
+#include <linux/string.h>

 struct class *switch_class;
 static atomic_t device_count;

+static const char *switch_name_text = "SWITCH_NAME=";
+static const char *switch_state_text = "SWITCH_STATE=";
+
 static ssize_t state_show(struct device *dev, struct device_attribute *attr,
                char *buf)
 {
@@ -59,8 +63,8 @@ static DEVICE_ATTR(name, S_IRUGO | S_IWUSR, name_show, NULL);

 void switch_set_state(struct switch_dev *sdev, int state)
 {
-       char name_buf[120];
-       char state_buf[120];
+       char *name_buf;
+       char *state_buf;
        char *prop_buf;
        char *envp[3];
        int env_offset = 0;
@@ -75,21 +79,27 @@ void switch_set_state(struct switch_dev *sdev, int state)
                        if (length > 0) {
                                if (prop_buf[length - 1] == '\n')
                                        prop_buf[length - 1] = 0;
-                               snprintf(name_buf, sizeof(name_buf),
-                                       "SWITCH_NAME=%s", prop_buf);
+                               length += strlen(switch_name_text);
+                               name_buf = kzalloc(length, GFP_KERNEL);
+                               snprintf(name_buf, length,
+                                       "%s%s", switch_name_text, prop_buf);
                                envp[env_offset++] = name_buf;
                        }
                        length = state_show(sdev->dev, NULL, prop_buf);
                        if (length > 0) {
                                if (prop_buf[length - 1] == '\n')
                                        prop_buf[length - 1] = 0;
-                               snprintf(state_buf, sizeof(state_buf),
-                                       "SWITCH_STATE=%s", prop_buf);
+                               length += strlen(switch_state_text);
+                               state_buf = kzalloc(length, GFP_KERNEL);
+                               snprintf(state_buf, length,
+                                       "%s%s", switch_state_text, prop_buf);
                                envp[env_offset++] = state_buf;
                        }
                        envp[env_offset] = NULL;
                        kobject_uevent_env(&sdev->dev->kobj, KOBJ_CHANGE, envp);
                        free_page((unsigned long)prop_buf);
+                       kfree(name_buf);
+                       kfree(state_buf);
                } else {
                        printk(KERN_ERR "out of memory in switch_set_state\n");
                        kobject_uevent(&sdev->dev->kobj, KOBJ_CHANGE);

> And I don't think the "name" has to be duplicated here as it is already
> transmitted in the uevent.

The uevent generated by switch_class contains:

change@/devices/virtual/switch/h2w
ACTION=change
DEVPATH=/devices/virtual/switch/h2w
SUBSYSTEM=switch
SWITCH_NAME=h2w
SWITCH_STATE=0
SEQNUM=846

Where to get the "name" if SWITCH_NAME were not included? To extract
from the path?

How can switch's value be checked? Is it right using SWITCH_STATE?
Does the uevent sends that information in some way already?
I guess ACTION could be a choice, although switch_class driver sets it
always to KOBJ_CHANGE.

When I _cat_ the uevent entry (of Jack in ALSA (input subsystem based)
I see PHYSDEVPATH, PRODUCT, NAME, etc; but when I _cat_ the uevent
entry generated by Switch Class it's empty. Any reason behind that?

Thanks,
-Misa

--~--~---------~--~----~------------~-------~--~----~
unsubscribe: [email protected]
website: http://groups.google.com/group/android-kernel
-~----------~----~----~----~------~----~------~--~---

Reply via email to