[PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

Hello Arnd,


Presuming that the requests and comments upto v5 patchsets were
addressed in the last patchset (v6), here goes a pull request of
Extcon.



The following changes since commit b01543dfe67bb1d191998e90d20534dc354de059:

  Linux 3.3-rc4 (2012-02-18 15:53:33 -0800)

are available in the git repository at:
  git://git.infradead.org/users/kmpark/linux-samsung extcon-for-next

Donggeun Kim (1):
      Extcon: support notification based on the state changes.

MyungJoo Ham (4):
      Extcon (external connector): import Android's switch class and modify.
      Extcon: support generic GPIO extcon driver
      Extcon: support multiple states at a device.
      Extcon: support mutually exclusive relation between cables.

 Documentation/ABI/testing/sysfs-class-extcon |   97 +++
 drivers/Kconfig                              |    2 +
 drivers/Makefile                             |    1 +
 drivers/extcon/Kconfig                       |   35 ++
 drivers/extcon/Makefile                      |    6 +
 drivers/extcon/extcon_class.c                |  812 ++++++++++++++++++++++++++
 drivers/extcon/extcon_gpio.c                 |  175 ++++++
 include/linux/extcon.h                       |  322 ++++++++++
 include/linux/extcon/extcon_gpio.h           |   52 ++
 9 files changed, 1502 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-extcon
 create mode 100644 drivers/extcon/Kconfig
 create mode 100644 drivers/extcon/Makefile
 create mode 100644 drivers/extcon/extcon_class.c
 create mode 100644 drivers/extcon/extcon_gpio.c
 create mode 100644 include/linux/extcon.h
 create mode 100644 include/linux/extcon/extcon_gpio.h



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Greg KH]

On Tue, Feb 28, 2012 at 05:58:02PM +0900, MyungJoo Ham wrote:
> Hello Arnd,
> 
> 
> Presuming that the requests and comments upto v5 patchsets were
> addressed in the last patchset (v6), here goes a pull request of
> Extcon.

Wait, please, before this code can be accepted, I would like to get
verification that you have also made the needed changes to the Android
userspace side of this interface, and that the code works there, and it
is on its way to be included in the main Android repo.

Is this true?  If not, what is the plans for this to happen?  Without
that happening, I don't think this code can be accepted, do you?

thanks,

greg k-h

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Tue, Feb 28, 2012 at 08:44:13AM -0800, Greg KH wrote:

> Wait, please, before this code can be accepted, I would like to get
> verification that you have also made the needed changes to the Android
> userspace side of this interface, and that the code works there, and it
> is on its way to be included in the main Android repo.

> Is this true?  If not, what is the plans for this to happen?  Without
> that happening, I don't think this code can be accepted, do you?

There's a few more kernel changes needed there before full acceptance
would be possible - for example, should have an integration into the
audio stack as one of the uses of the switch API on Android is for
headset detection.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Greg KH]

On Tue, Feb 28, 2012 at 04:57:28PM +0000, Mark Brown wrote:
> On Tue, Feb 28, 2012 at 08:44:13AM -0800, Greg KH wrote:
> 
> > Wait, please, before this code can be accepted, I would like to get
> > verification that you have also made the needed changes to the Android
> > userspace side of this interface, and that the code works there, and it
> > is on its way to be included in the main Android repo.
> 
> > Is this true?  If not, what is the plans for this to happen?  Without
> > that happening, I don't think this code can be accepted, do you?
> 
> There's a few more kernel changes needed there before full acceptance
> would be possible - for example, should have an integration into the
> audio stack as one of the uses of the switch API on Android is for
> headset detection.

Is this integration planned?

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Tue, Feb 28, 2012 at 09:02:46AM -0800, Greg KH wrote:
> On Tue, Feb 28, 2012 at 04:57:28PM +0000, Mark Brown wrote:

> > There's a few more kernel changes needed there before full acceptance
> > would be possible - for example, should have an integration into the
> > audio stack as one of the uses of the switch API on Android is for
> > headset detection.

> Is this integration planned?

Don't know if anyone has a specific plan at the moment, though there's
enough people care that I'd imagine it'll happen if Android decides to
use this stuff.

That said I don't see this as a blocker myself - it's more like an
afternoon or whatever of work than a serious development effort to do
the updates, but a similar thing applies to most of the Android
userspace updates.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

On Wed, Feb 29, 2012 at 2:45 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 28, 2012 at 09:02:46AM -0800, Greg KH wrote:
>> On Tue, Feb 28, 2012 at 04:57:28PM +0000, Mark Brown wrote:
>
>> > There's a few more kernel changes needed there before full acceptance
>> > would be possible - for example, should have an integration into the
>> > audio stack as one of the uses of the switch API on Android is for
>> > headset detection.
>
>> Is this integration planned?
>
> Don't know if anyone has a specific plan at the moment, though there's
> enough people care that I'd imagine it'll happen if Android decides to
> use this stuff.
>
> That said I don't see this as a blocker myself - it's more like an
> afternoon or whatever of work than a serious development effort to do
> the updates, but a similar thing applies to most of the Android
> userspace updates.

I'm not sure which Android kernel devide drivers are going to be upstreamed.

However, if the swtich device driver writers decide to keep the
functionality limited to the original Android switch class by not
providing cable names additionally, the changes will probably be
limited to ":s/switch/extcon/g" for API calls and struct name.


Cheers!
MyungJoo.

-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Wed, Feb 29, 2012 at 11:05:39AM +0900, MyungJoo Ham wrote:
> On Wed, Feb 29, 2012 at 2:45 AM, Mark Brown

> > Don't know if anyone has a specific plan at the moment, though there's
> > enough people care that I'd imagine it'll happen if Android decides to
> > use this stuff.

> > That said I don't see this as a blocker myself - it's more like an
> > afternoon or whatever of work than a serious development effort to do
> > the updates, but a similar thing applies to most of the Android
> > userspace updates.

> I'm not sure which Android kernel devide drivers are going to be upstreamed.

There's a lot of things that are just generic functionality and aren't
at all Android specific, a lot of which is already upstream but just not
exporting the switch API but instead using other existing kernel APIs.

> However, if the swtich device driver writers decide to keep the
> functionality limited to the original Android switch class by not
> providing cable names additionally, the changes will probably be
> limited to ":s/switch/extcon/g" for API calls and struct name.

For things like headphone detection the actual switch they're providing
has issues too even just within the switch API - it's called "h2w" with
magic values.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

On Wed, Feb 29, 2012 at 9:35 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Feb 29, 2012 at 11:05:39AM +0900, MyungJoo Ham wrote:
>> On Wed, Feb 29, 2012 at 2:45 AM, Mark Brown
>
>> > Don't know if anyone has a specific plan at the moment, though there's
>> > enough people care that I'd imagine it'll happen if Android decides to
>> > use this stuff.
>
>> > That said I don't see this as a blocker myself - it's more like an
>> > afternoon or whatever of work than a serious development effort to do
>> > the updates, but a similar thing applies to most of the Android
>> > userspace updates.
>
>> I'm not sure which Android kernel devide drivers are going to be upstreamed.
>
> There's a lot of things that are just generic functionality and aren't
> at all Android specific, a lot of which is already upstream but just not
> exporting the switch API but instead using other existing kernel APIs.
>
>> However, if the swtich device driver writers decide to keep the
>> functionality limited to the original Android switch class by not
>> providing cable names additionally, the changes will probably be
>> limited to ":s/switch/extcon/g" for API calls and struct name.
>
> For things like headphone detection the actual switch they're providing
> has issues too even just within the switch API - it's called "h2w" with
> magic values.

Reading an H2W switch device driver
(http://git.ziggy471.com/frankenstine.git/plain/arch/arm/mach-msm/htc_headset_mgr.c),
I don't see why it won't be compatible after replacing the name from
switch to extcon except for one more parameter at
switch_dev_register() to extcon_dev_register(). This needs to add the
struct device* dev of the caller probe function:
switch_dev_register(&hi->sdev); should become
extcon_dev_register(&hi->sdev, &pdev->dev);

Because the probe function of this H2W driver keeps
hi->sdev->supported_cable NULL, which was not available in switch
class, any magic values with switch_set_state() and switch_get_state()
should not be affected. Besides, adding callbacks (print_name,
print_state) overrides the behavior of extcon as well as with switch.
I don't see elsewhere to keep any magic values within switch device
drivers.


Cheers!
MyungJoo.

-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Mon, Mar 05, 2012 at 04:59:42PM +0900, MyungJoo Ham wrote:
> On Wed, Feb 29, 2012 at 9:35 PM, Mark Brown

> > For things like headphone detection the actual switch they're providing
> > has issues too even just within the switch API - it's called "h2w" with
> > magic values.

> Reading an H2W switch device driver
> (http://git.ziggy471.com/frankenstine.git/plain/arch/arm/mach-msm/htc_headset_mgr.c),
> I don't see why it won't be compatible after replacing the name from

That's not the only such driver, that's one example of a machine
specific accessory detection driver.  There's others out there such as
the sec_jack driver which you converted into the ADC driver in your
patches (if you look at how that gets deployed the overwhelming majority
of users are headsets).

The issue is the combination of the naming (h2w isn't very meaningful
and only works with a single jack) and the magic numbers used for
reporting state, plus the registration collisions.  Existing code can be
ported over fairly easily but it's got issues as an ABI which we'd want
to fix.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

On Mon, Mar 5, 2012 at 9:07 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Mar 05, 2012 at 04:59:42PM +0900, MyungJoo Ham wrote:
>> On Wed, Feb 29, 2012 at 9:35 PM, Mark Brown
>
>> > For things like headphone detection the actual switch they're providing
>> > has issues too even just within the switch API - it's called "h2w" with
>> > magic values.
>
>> Reading an H2W switch device driver
>> (http://git.ziggy471.com/frankenstine.git/plain/arch/arm/mach-msm/htc_headset_mgr.c),
>> I don't see why it won't be compatible after replacing the name from
>
> That's not the only such driver, that's one example of a machine
> specific accessory detection driver.  There's others out there such as
> the sec_jack driver which you converted into the ADC driver in your
> patches (if you look at how that gets deployed the overwhelming majority
> of users are headsets).
>
> The issue is the combination of the naming (h2w isn't very meaningful
> and only works with a single jack) and the magic numbers used for
> reporting state, plus the registration collisions.  Existing code can be
> ported over fairly easily but it's got issues as an ABI which we'd want
> to fix.

The ABI of simply ported (only with API name changes) ex-switch extcon
device won't be changed:

sprintf(buf, "%u\n", edev->state);

is kept used if edev->max_supported == 0, which means that it is not
using extcon-native multi-state features, which is true for any simply
ported ex-switch extcon device drivers because they won't provide
"supported_cable" array with extcon_dev struct.


Or, do you mean that extcon class should support not ex-swtich extcon
device drivers such as adc-jack.c (not ported from oldies) to support
"legacy" userspace application that is aware of only switch interface?

Then, we may simply let state's read ABI to use sprintf(buf, "%u\n",
edev->state); only regardless of device drivers. How do you think
about this?
(simply removing line 119, 121-130 of extcon_class.c)


Cheers!
MyungJoo.
-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Fri, Mar 09, 2012 at 05:26:26PM +0900, MyungJoo Ham wrote:
> On Mon, Mar 5, 2012 at 9:07 PM, Mark Brown

> > The issue is the combination of the naming (h2w isn't very meaningful
> > and only works with a single jack) and the magic numbers used for
> > reporting state, plus the registration collisions.  Existing code can be
> > ported over fairly easily but it's got issues as an ABI which we'd want
> > to fix.

> The ABI of simply ported (only with API name changes) ex-switch extcon
> device won't be changed:

> Or, do you mean that extcon class should support not ex-swtich extcon
> device drivers such as adc-jack.c (not ported from oldies) to support
> "legacy" userspace application that is aware of only switch interface?

Neither of these, I mean that we'll probably want to do some updates to
the drivers as they go into mainline which will change their interface
beyond the simple s/switch/extcon/ change.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

On Wed, Feb 29, 2012 at 1:44 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Feb 28, 2012 at 05:58:02PM +0900, MyungJoo Ham wrote:
>> Hello Arnd,
>>
>>
>> Presuming that the requests and comments upto v5 patchsets were
>> addressed in the last patchset (v6), here goes a pull request of
>> Extcon.
>
> Wait, please, before this code can be accepted, I would like to get
> verification that you have also made the needed changes to the Android
> userspace side of this interface, and that the code works there, and it
> is on its way to be included in the main Android repo.
>
> Is this true?  If not, what is the plans for this to happen?  Without
> that happening, I don't think this code can be accepted, do you?
>
> thanks,
>
> greg k-h

When an extcon device driver is not using the extended features
(multistate & mutually-exclusiveness) and CONFIG_ANDROID is on, the
userspace interface is same as the Android kernel's switch class (name
and state ABI, which were read-only in Android kernel).

If an extcon device drivers uses the extended features (multistate &
mutually-exclusiveness) by adding "cable names", the state ABI will
provided information in different format. Besides, Android kernel's
switch device driver cannot provide the extended feature anyway.


Thanks.


Cheers!
MyungJoo.
-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

Re: [PULL-REQUEST] EXTCON Patches (Re: [PATCH v6 0/5] Introduce External Connector Class (extcon))

[MyungJoo Ham]

On Wed, Feb 29, 2012 at 10:57 AM, MyungJoo Ham <myungjoo.ham@gmail.com> wrote:
> On Wed, Feb 29, 2012 at 1:44 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Tue, Feb 28, 2012 at 05:58:02PM +0900, MyungJoo Ham wrote:
>>> Hello Arnd,
>>>
>>>
>>> Presuming that the requests and comments upto v5 patchsets were
>>> addressed in the last patchset (v6), here goes a pull request of
>>> Extcon.
>>
>> Wait, please, before this code can be accepted, I would like to get
>> verification that you have also made the needed changes to the Android
>> userspace side of this interface, and that the code works there, and it
>> is on its way to be included in the main Android repo.
>>
>> Is this true?  If not, what is the plans for this to happen?  Without
>> that happening, I don't think this code can be accepted, do you?
>>
>> thanks,
>>
>> greg k-h
>
> When an extcon device driver is not using the extended features
> (multistate & mutually-exclusiveness) and CONFIG_ANDROID is on, the
> userspace interface is same as the Android kernel's switch class (name
> and state ABI, which were read-only in Android kernel).
>
> If an extcon device drivers uses the extended features (multistate &
> mutually-exclusiveness) by adding "cable names", the state ABI will
> provided information in different format. Besides, Android kernel's
> switch device driver cannot provide the extended feature anyway.


ps. If CONFIG_ANDROID is on and the extended features (that were not
in Android kernel) are used by an extcon device driver, then, the
format of state ABI will be different. However, the path should be the
same although there will be additional sysfs entries.


>
>
> Thanks.
>
>
> Cheers!
> MyungJoo.
> --
> MyungJoo Ham, Ph.D.
> System S/W Lab, S/W Center, Samsung Electronics



-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics