Messages in this thread | | | Date | Fri, 16 Dec 2011 10:18:55 -0800 | From | Greg KH <> | Subject | Re: [PATCH v2 1/3] Extcon (external connector): import Android's switch class and modify. |
| |
On Fri, Dec 16, 2011 at 02:38:38PM +0900, MyungJoo Ham wrote: > On Thu, Dec 15, 2011 at 4:18 PM, Greg KH <gregkh@suse.de> wrote: > > 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: > >> > > >> > 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. > > 1. Kernel source with Android patch has "switch" class drivers at > /drivers/switch, which are not compatible with "extcon" class. > 2. Android userspace access /sys/class/switch/ nodes, which are > compatible with extcon nodes. > > Not to interfere with Android kernel patches, not to be confused with > incompatible Android switch class and its drivers, and to be > compatible with Android userspace at the same time, we may put the > device drivers located at Linux/drivers/extcon/ and let the class uses > /sys/class/switch/*.
No, that's really not going to help as you changed the way the sysfs files worked, right?
> I remember there were big no-es on using the name "switch" for these > external connectors; however, would this userspace class name be fine? > Or would it be fine to make the class name customizable? (let the > device driver choose whether to be named "switch" or "extcon" or let > the class define its location based on kernel config). > > In order to get some comments from Android kernel builders, I've added > android-kernel@googlegroups.com.
Why can't you submit patches for the userspace Android code to use this new api instead of their sys/class/switch/ code? If functionally it's the same, it should be ok to do this, right?
> >> >> + 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? > > Android has been "observing" kobject uevents. It has been observing > the switch class kobject (&edev->dev->kobj) and that has not been > changed from Android kernel. > In Android, with "startObserving" method, one can let a function to be > called with incoming UEvent. > I haven't look into how "startObserving" is implemented. However, they > do observe KOBJ_CHANGE at userspace. And this is not limted to > Android.
It would be good to figure out how "observing" actually works here.
> Here, my intention is the same. It is to invoke a function in a user > process that has registered to be invoked for a UEvent. If we are > going to use other methods for this, we will also force userspace > processes to change their methods to get events (at least for switch > class and chargers) not from UEVENT. > > Maybe the userspace is abusing UEVENT/KOBJ_CHANGE, though... that's > what userspace observes to get events from kernel often.
I don't mind using the kobject change call, I just want to be very careful about it as there aren't many users of it in the kernel at the moment, and very little userspace code that I know of that takes advantage of it.
If the Android framework is using it, that's fine, it's one reason to use it, but actually determining this would be a good thing for you to do, right?
thanks,
greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |