Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB

[Chanwoo Choi <cw00.choi@samsung.com>]

Hi Peter,

On 05/29/2015 10:22 AM, Peter Chen wrote:
> On Thu, May 28, 2015 at 05:23:57PM +0300, Roger Quadros wrote:
>> +Peter & Li,
>>
>> Ivan,
>>
>> On 28/05/15 11:45, Ivan T. Ivanov wrote:
>>>
>>> Hi Chanwoo,
>>>
>>> On Wed, 2015-05-27 at 21:15 +0900, Chanwoo Choi wrote:
>>>> Previously, I discussed how to inform the changed state of both ID
>>>> and VBUS pin for USB connector on patch-set[1].
>>>> [1] https://lkml.org/lkml/2015/4/2/310
>>>>
>>>> So, this patch adds the extcon_set_cable_line_state() function to inform
>>>> the additional state of external connectors without additional register/
>>>> unregister functions. This function uses the existing notifier chain
>>>> which is registered by extcon_register_notifier() / extcon_register_interest().
>>>>
>>>> The extcon_set_cable_line_state() can inform the new state of both
>>>> ID and VBUS pin state through extcon_set_cable_line_state().
>>>>
>>>> For exmaple:
>>>> - On extcon-usb-gpio.c as extcon provider driver as following:
>>>>         static void usb_extcon_detect_cable(struct work_struct *work)
>>>>         {
>>>>                 ...
>>>>                 /* check ID and update cable state */
>>>>                 id = gpiod_get_value_cansleep(info->id_gpiod);
>>>>                 if (id) {
>>>>                         extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, false);
>>>>                         extcon_set_cable_state_(info->edev, EXTCON_USB, true);
>>>>
>>>>                         extcon_set_cable_line_state(info->edev, EXTCON_USB,
>>>>                                                         EXTCON_USB_ID_HIGH);
>>>
>>> I am getting more and more confused :-). Why EXTCON_USB is now used for ID notifications?
>>> It should be EXTCON_USB_HOST, no? Why we need another function, framework already have
>>> required information from the function one line above, do I miss something?
>>
>> This is because the existing EXTCON_USB_HOST and EXTCON_USB do not capture all
>> the 4 states of ID and VBUS pins that we need for a real USB driver to work.
>>
>> It looks like it was designed from user space users perspective where they are
>> only interested in USB role. i.e. host or peripheral.
>>
>> Right now we are mixing both ID/VBUS and HOST/Peripheral states.
>> This will break when we consider OTG role switching.
>> With role switching, the USB device might start as a peripheral but switch role to host
>> on the fly and the existing setup (including these patches) can't cater to that
>> if user space is relying on EXTCON_USB_HOST/EXTCON_USB events.
>> Because they are hard-wired to the ID pin state which doesn't change during
>> role switch without cable switch.
>>
>> The USB driver doesn't care about EXTCON_USB_HOST/peripheral states.
>> It just needs ID/VBUS states and should decide the Host/Peripheral state from
>> that and other inputs (like HNP/user request/etc).
>>
>> The flow could be like this
>>
>> (extcon-usb-driver) -> [ID/VBUS states] -> (USB driver) -> [HOST/Peripheral states]
> 
> Agree. Chanwoo, USB driver knows better than extcon driver about USB
> role (host/peripheral), so the app should use USB interface to know it,
> in fact, I don't be aware any use case needs to know USB role?
> Are there any users for EXTCON_USB and EXTCON_USB_HOST currently?

You're right. But, extcon can just distinguish the type of external connectors
and inform the type to the user-space and extcon consumer driver in kernel-space.

When USB mouse or keyboard is attached, user-space can check the state of externel connector which is attaced to the H/W target as following:
- /sys/class/extcon/extconX/cable.0/name -> USB
- /sys/class/extcon/extconX/cable.0/state -> 0
- /sys/class/extcon/extconX/cable.1/name -> USB-HOST
- /sys/class/extcon/extconX/cable.1/state -> 1

Thanks,
Chanwoo Choi