Re: [PATCH v2 1/3] Extcon (external connector): import Android's switch class and modify.

[Greg KH <gregkh@suse.de>]

On Thu, Dec 15, 2011 at 02:41:38PM +0900, MyungJoo Ham wrote:
> On Thu, Dec 15, 2011 at 10:01 AM, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Dec 14, 2011 at 07:28:26PM +0900, MyungJoo Ham wrote:
> >> External connector class (extcon) is based on and an extension of Android
> >> kernel's switch class located at linux/drivers/switch/.
> >> This patch provides the before-extension switch class moved to the
> >> location where the extcon will be located (linux/drivers/extcon/).
> >>
> >> The before-extension class, switch class of Android kernel, commits
> >> imported are:
> >>
> >> switch: switch class and GPIO drivers.
> >> Author: Mike Lockwood <lockwood@android.com>
> >>
> >> switch: gpio: Don't call request_irq with interrupts disabled
> >> Author: Arve Hjønnevåg <arve@android.com>
> >>
> >> switch: Use device_create instead of device_create_drvdata.
> >> Author: Arve Hjønnevåg <arve@android.com>
> >>
> >> switch_gpio: Add missing #include <linux/interrupt.h>
> >> Author: Mike Lockwood <lockwood@android.com>
> >>
> >> In this patch, upon the commits of Android kernel, we have added:
> >> - Relocated and renamed for extcon.
> >> - Comments, module name, and author information are updated
> >> - Code clean for successing patches
> >> - Bugfix: enabling write access without write functions
> >
> > Nice, but if we accept this, will someone also make the needed changes
> > to the Android userspace code to handle the user api changes that this
> > causes?
> 
> I have no idea about how Android will react to this as I have no
> developmental experiences with Android.
> However, from the perspective of general userspace, this modification
> incurs path changes (/sys/class/switch/.... to /sys/class/extcon/...)
> only.

Well, without such changes, any Android platform will still have to
include the switch code in their system, making this work a bit
pointless, right?

Please look into changing this in userspace, if for no other reason than
to test that this kernel code works properly with the Android userspace
needs as well.

> >> +                     kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
> >
> > I really dislike using uevents, what is listening for them?  Are you
> > hooked into udev's event chain in userspace to properly handle this?  If
> > not, what is the point of sending them?
> 
> It is to let userspace processes get notified for the events in extcon.
> Do you think sysfs_notify() would be better for this purpose?

No, I don't think it does what you think it does :)

What are you trying to accomplish here?  And how would sysfs_notify()
accomplish that?

thanks,

greg k-h