Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Chanwoo,
On 29/05/15 15:15, Chanwoo Choi wrote:
Dear all,
On 05/29/2015 07:53 PM, Chanwoo Choi wrote:
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
On last month, Robert Baldyga sent the patch[1] for extcon-usb-gpio driver.
[1] https://lkml.org/lkml/2015/4/2/311
- [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection
This patch[1] included a following table which show the type of external
connectors
according to both ID and VBUS pin state.
State |ID | VBUS
[1] USB|H|H
[2] none |H|L
[3] USB & USB-HOST |L|H
[4] USB-HOST |L|L
So, I think that extcon-usb-gpio.c could set below two status with set:
- the state of whether USB/USB-HOST are attached or detached -> send the state
to both kernel-space and user-space.
: extcon_set_cable_state_()
- the h/w line state of both ID and VBUS pin state -> send the state to only
kernel space.
: extcon_set_cable_line_state()
[1]
State |ID | VBUS
[1] USB|H|H
extcon_set_cable_state_(edev, EXTCON_USB, true);
extcon_set_cable_state_(edev, EXTCON_USB_HOST, false);
extcon_set_cable_line_state(edev, EXTCON_USB, EXTCON_USB_ID_HIGH);
extcon_set_cable_line_state(edev, EXTCON_USB, EXTCON_USB_VBUS_HIGH);
[2]
State |ID | VBUS
---
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Hi Ivan,
On Sat, May 30, 2015 at 2:15 AM, Chanwoo Choi wrote:
> On Fri, May 29, 2015 at 11:32 PM, Ivan T. Ivanov wrote:
>>
>> On Fri, 2015-05-29 at 19:44 +0900, Chanwoo Choi wrote:
>>> Hi Ivan,
>>>
>>> On 05/28/2015 05:45 PM, 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?
>>>
>>> The EXTCON fwk has the follwoing different functions:
>>> - extcon_set_cable_state()
>>> : Send whether external connectors is attached or detached to the extcon
>>> consumer driver in kernel space and to the user-space by using the uevent.
>>> - extcon_set_cable_line_state()
>>> : Send the specific line state of both ID and VBUS pin state of USB
>>> connector to only the extcon consumer driver in kernel space. This function
>>> don't send the uevent to the user-space because user-
>>> space process don't consider the h/w pin state.
>>
>> My understanding, from discussion several letters back, is that clients
>> will receive single event per change (EXTCON_USB_ID_HIGH,
>> EXTCON_USB_ID_LOW...),
>
> There are following constraints between events for EXTCON_USB:
> - EXTCON_USB_ID_HIGH and EXTCON_USB_ID_LOW are mutually exclusive.
> - EXTCON_USB_VBUS_HIGH and EXTCON_USB_VBUS_LOW are mutually exclusive.
>
> If extcon provider (e.g., extcon-usb-gpio) don't violate the constraints,
> extcon provider can send the multiple events as following. Namely,
> extcon consumer/clients will receive the the multiple events.
>
> For example:
> extcon_set_cable_line_state(edev, EXTCON_USB, EXTCON_USB_ID_HIGH |
> EXTCON_USB_VBUS_LOW);
>
>>
>> My point was, why we need another function (extcon_set_cable_line_state) if
>> extcon_set_cable_state_ already have required information? Or the plan is to
>> move extcon_set_cable_state_ to USB drivers and use only
>> extcon_set_cable_line_state
>> by extcon drivers?
>
> You pointed out appropriately. I worried about adding new
> extcon_set_cable_line_state().
>
> I'll use extcon_set_cable_state_() without adding
> extcon_set_cable_line_state() as following but it need the
> modification of function prototype.
>
> - extcon_set_cable_state(struct extcon_dev *edev, enum extcon id, bool state)
> -> extcon_set_cable_state(struct extcon_dev *edev, enum extcon id, u64 state)
>
> And extcon use the bit mask for both attached state and detached state
> of external connectors instead of boolean.
>
> #define EXTCON_DETACHED BIT(0)
> #define EXTCON_ATTACHED BIT(1)
>
> #define EXTCON_USB_ID_LOW BIT(2)
> #define EXTCON_USB_ID_HIGH BIT(3)
> #define EXTCON_USB_VBUS_LOWBIT(4)
> #define EXTCON_USB_VBUS_HIGH BIT(5)
>
> After it, extcon_set_cable_state() would send multiple events at time
> as following:
>
> e.g., extcon_set_cable_state(edev, EXTCON_USB, EXTCON_ATTACHED |
> EXTCON_USB_ID_HIGH | EXTCON_USB_VBUS_HIGH);
This design has the problem.
If extcon uses only extcon_set_cable_state_(), the prototype of
extcon_{set|get}_cable_state_() functions should be changed.
extcon_set_cable_state_(struct extcon_dev *edev, enum extcon id, bool state)
-> extcon_set_cable_state_(struct extcon_dev *edev, enum extcon id, u64 state)
bool extcon_get_cable_state_(struct extcon_dev *edev, enum extcon id)
-
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
On Fri, May 29, 2015 at 11:32 PM, Ivan T. Ivanov wrote:
>
> On Fri, 2015-05-29 at 19:44 +0900, Chanwoo Choi wrote:
>> Hi Ivan,
>>
>> On 05/28/2015 05:45 PM, 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?
>>
>> The EXTCON fwk has the follwoing different functions:
>> - extcon_set_cable_state()
>> : Send whether external connectors is attached or detached to the extcon
>> consumer driver in kernel space and to the user-space by using the uevent.
>> - extcon_set_cable_line_state()
>> : Send the specific line state of both ID and VBUS pin state of USB
>> connector to only the extcon consumer driver in kernel space. This function
>> don't send the uevent to the user-space because user-
>> space process don't consider the h/w pin state.
>
> My understanding, from discussion several letters back, is that clients
> will receive single event per change (EXTCON_USB_ID_HIGH,
> EXTCON_USB_ID_LOW...),
There are following constraints between events for EXTCON_USB:
- EXTCON_USB_ID_HIGH and EXTCON_USB_ID_LOW are mutually exclusive.
- EXTCON_USB_VBUS_HIGH and EXTCON_USB_VBUS_LOW are mutually exclusive.
If extcon provider (e.g., extcon-usb-gpio) don't violate the constraints,
extcon provider can send the multiple events as following. Namely,
extcon consumer/clients will receive the the multiple events.
For example:
extcon_set_cable_line_state(edev, EXTCON_USB, EXTCON_USB_ID_HIGH |
EXTCON_USB_VBUS_LOW);
>
> My point was, why we need another function (extcon_set_cable_line_state) if
> extcon_set_cable_state_ already have required information? Or the plan is to
> move extcon_set_cable_state_ to USB drivers and use only
> extcon_set_cable_line_state
> by extcon drivers?
You pointed out appropriately. I worried about adding new
extcon_set_cable_line_state().
I'll use extcon_set_cable_state_() without adding
extcon_set_cable_line_state() as following but it need the
modification of function prototype.
- extcon_set_cable_state(struct extcon_dev *edev, enum extcon id, bool state)
-> extcon_set_cable_state(struct extcon_dev *edev, enum extcon id, u64 state)
And extcon use the bit mask for both attached state and detached state
of external connectors instead of boolean.
#define EXTCON_DETACHED BIT(0)
#define EXTCON_ATTACHED BIT(1)
#define EXTCON_USB_ID_LOW BIT(2)
#define EXTCON_USB_ID_HIGH BIT(3)
#define EXTCON_USB_VBUS_LOWBIT(4)
#define EXTCON_USB_VBUS_HIGH BIT(5)
After it, extcon_set_cable_state() would send multiple events at time
as following:
e.g., extcon_set_cable_state(edev, EXTCON_USB, EXTCON_ATTACHED |
EXTCON_USB_ID_HIGH | EXTCON_USB_VBUS_HIGH);
Thanks,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
On Fri, 2015-05-29 at 19:44 +0900, Chanwoo Choi wrote:
> Hi Ivan,
>
> On 05/28/2015 05:45 PM, 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?
>
> The EXTCON fwk has the follwoing different functions:
> - extcon_set_cable_state()
> : Send whether external connectors is attached or detached to the extcon
> consumer driver in kernel space and to the user-space by using the uevent.
> - extcon_set_cable_line_state()
> : Send the specific line state of both ID and VBUS pin state of USB connector
> to only the extcon consumer driver in kernel space. This function don't send
> the uevent to the user-space because user-
> space process don't consider the h/w pin state.
My understanding, from discussion several letters back, is that clients
will receive single event per change (EXTCON_USB_ID_HIGH, EXTCON_USB_ID_LOW...)
and not some combined bit mask, right?
My point was, why we need another function (extcon_set_cable_line_state) if
extcon_set_cable_state_ already have required information? Or the plan is to
move extcon_set_cable_state_ to USB drivers and use only
extcon_set_cable_line_state
by extcon drivers?
Regards,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Dear all,
On 05/29/2015 07:53 PM, Chanwoo Choi wrote:
> 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
>
On last month, Robert Baldyga sent the patch[1] for extcon-usb-gpio driver.
[1] https://lkml.org/lkml/2015/4/2/311
- [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection
This patch[1] included a following table which show the type of external
connectors
according to both ID and VBUS pin state.
State |ID | VBUS
[1] USB|H|H
[2] none |H|L
[3] USB & USB-HOST |L|H
[4] USB-HOST |L|L
So, I think that extcon-usb-gpio.c could set below two status with set:
- the state of whether USB/USB-HOST are attached or detached -> send the state
to both kernel-space and user-space.
: extcon_set_cable_state_()
- the h/w line state of both ID and VBUS pin state -> send the state to only
kernel space.
: extcon_set_cable_line_state()
[1]
State |ID | VBUS
[1] USB
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Hi Ivan,
On 05/28/2015 05:45 PM, 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?
The EXTCON fwk has the follwoing different functions:
- extcon_set_cable_state()
: Send whether external connectors is attached or detached to the extcon
consumer driver in kernel space and to the user-space by using the uevent.
- extcon_set_cable_line_state()
: Send the specific line state of both ID and VBUS pin state of USB connector
to only the extcon consumer driver in kernel space. This function don't send
the uevent to the user-space because user-space process don't consider the h/w
pin state.
>
>> } else {
>> extcon_set_cable_state_(info->edev, EXTCON_USB,
>> false);
>> extcon_set_cable_state_(info->edev, EXTCON_USB_HOST,
>> true);
>>
>> extcon_set_cable_line_state(info->edev, EXTCON_USB,
>> EXTCON_USB_ID_LOW);
>> }
>> }
>>
>> - On specific extcon consumder driver as following:
>> static int xxx_probe(struct platform_device *pdev)
>> {
>> struct notifier_chain nh;
>>
>> nb.notifier_call = extcon_usb_notifier;
>> ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
>
> This is bit misleading. 'nb' could not be in stack.
Right. It is my mistake. I'll fix it for example.
Thanks,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Hi Roger,
On 05/28/2015 11:23 PM, 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]
>
> If that is not going to happen then we will have to switch to this
>
> (usb phy driver) -> [ID/VBUS states] -> (USB driver) -> (extcon f/w) ->
> [Host/Peripheral states]
>
>
> Felipe, Peter, Li,
>
> what do you guys suggest?
The EXTCON framework have to definitely distinguish the correct type of
external connectors which is connected to H/W target or other machines. And
then the EXTCON framework inform the state and type of attached external
connector to the extcon consumer and user-space. It is absolutely necessary
role of EXTCON framework.
The EXTCON_USB_HOST connector is different from EXTCON_USB connector. When USB
mouse or keyboard is attached to H/W target, EXTCON fwk will inform both
correct type(USB_HOST) and state(1) of attached external connector. If EXTCON
fwk don't support the EXTCON_USB_HOST, when USB mouse/keyboard is attached,
EXTCON cannot provide the information of attached external connectors.
The EXTCON don't have the permission of USB operation but have the role to
inform the correct type of external connectors to the extcon consumer.
As you said, USB driver and framework can receive the notifier event of only
ID/VBUS pin state. If USB driver don't need the state of EXTCON_USB_HOST, USB
driver don't have to recevie the event of EXTCON_USB_HOST. Just USB driver
might receive the ID/VBUS event by extcon_register_notifier().
>
> cheers,
> -roger
>
>>
>>> } else {
>>> extcon_set_cable_state_(info->edev, EXTCON_USB,
>>> false);
>>> extcon_set_cable_state_(info->edev,
>>> EXTCON_USB_HOST, true);
>>>
>>> extcon_set_cable_line_state(info->edev, EXTCON_USB,
>>> EXTCON_USB_ID_LOW);
>>> }
>>> }
>>>
>>> - On specific extcon consumder driver as following:
>>> static int xxx_probe(struct platform_device *pdev)
>>> {
>>> struct notifier_chain nh;
>>>
>>> nb.notifier
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Hi,
On Thu, 2015-05-28 at 17:23 +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.
Are they any producers or consumers of "USB-HOST" and "USB" which are using
these
for anything different than ID and VBUS state tracking, except the user space?
If not, could we just rename in kernel definitions, keeping user space
notification
strings and be done?
Regadrs,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Hi,
On Thu, 2015-05-28 at 17:23 +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.
Are they any producers or consumers of "USB-HOST" and "USB" which are using
these
for anything different than ID and VBUS state tracking, except the user space?
If not, could we just rename in kernel definitions, keeping user space
notification
strings and be done?
Regadrs,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
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?
>
> If that is not going to happen then we will have to switch to this
>
> (usb phy driver) -> [ID/VBUS states] -> (USB driver) -> (extcon f/w) ->
> [Host/Peripheral states]
>
>
> Felipe, Peter, Li,
>
> what do you guys suggest?
>
> cheers,
> -roger
>
> >
> >> } else {
> >> extcon_set_cable_state_(info->edev, EXTCON_USB,
> >> false);
> >> extcon_set_cable_state_(info->edev,
> >> EXTCON_USB_HOST, true);
> >>
> >> extcon_set_cable_line_state(info->edev, EXTCON_USB,
> >> EXTCON_USB_ID_LOW);
> >> }
> >> }
> >>
> >>- On specific extcon consumder driver as following:
> >> static int xxx_probe(struct platform_device *pdev)
> >> {
> >> struct notifier_chain nh;
> >>
> >> nb.notifier_call = extcon_usb_notifier;
> >> ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
> >
> >This is bit misleading. 'nb' could not be in stack.
> >
> >Regards,
> >Ivan
> >
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
+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]
If that is not going to happen then we will have to switch to this
(usb phy driver) -> [ID/VBUS states] -> (USB driver) -> (extcon f/w) ->
[Host/Peripheral states]
Felipe, Peter, Li,
what do you guys suggest?
cheers,
-roger
} else {
extcon_set_cable_state_(info->edev, EXTCON_USB, false);
extcon_set_cable_state_(info->edev, EXTCON_USB_HOST,
true);
extcon_set_cable_line_state(info->edev, EXTCON_USB,
EXTCON_USB_ID_LOW);
}
}
- On specific extcon consumder driver as following:
static int xxx_probe(struct platform_device *pdev)
{
struct notifier_chain nh;
nb.notifier_call = extcon_usb_notifier;
ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
This is bit misleading. 'nb' could not be in stack.
Regards,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
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?
> } else {
> extcon_set_cable_state_(info->edev, EXTCON_USB,
> false);
> extcon_set_cable_state_(info->edev, EXTCON_USB_HOST,
> true);
>
> extcon_set_cable_line_state(info->edev, EXTCON_USB,
> EXTCON_USB_ID_LOW);
> }
> }
>
> - On specific extcon consumder driver as following:
> static int xxx_probe(struct platform_device *pdev)
> {
> struct notifier_chain nh;
>
> nb.notifier_call = extcon_usb_notifier;
> ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
This is bit misleading. 'nb' could not be in stack.
Regards,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
On Wed, May 27, 2015 at 11:09 PM, Roger Quadros wrote:
> Chanwoo,
>
> On 27/05/15 15:15, 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);
>
>
> Now that all USB line states can be captured by a single cable type
> can we get rid of EXTCON_USB_HOST?
EXTCON_USB_HOST is necessary to inform the event to the user-space by uevent.
For example, when the USB mouse device is attached, extcon have to inform
the staet to user-space. The role of extcon should inform the state of
all external connectors.
>
> That way the extcon driver doesn't need to make any decisions as to
> what mode we're in (host/cable) and this is best left to the USB driver.
The extcon just infrom the new event to the user-space.
If the notification of USB_HOST is not necessary in kernel space,
both device driver and framework in kernel space cannot register the notifier.
Thanks,
Chanwoo Choi
>
> cheers,
> -roger
>
>
>> extcon_set_cable_state_(info->edev, EXTCON_USB,
>> true);
>>
>> extcon_set_cable_line_state(info->edev,
>> EXTCON_USB,
>>
>> EXTCON_USB_ID_HIGH);
>> } else {
>> extcon_set_cable_state_(info->edev, EXTCON_USB,
>> false);
>> extcon_set_cable_state_(info->edev,
>> EXTCON_USB_HOST, true);
>>
>> extcon_set_cable_line_state(info->edev,
>> EXTCON_USB,
>>
>> EXTCON_USB_ID_LOW);
>> }
>> }
>>
>> - On specific extcon consumder driver as following:
>> static int xxx_probe(struct platform_device *pdev)
>> {
>> struct notifier_chain nh;
>>
>> nb.notifier_call = extcon_usb_notifier;
>> ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
>> ...
>> }
>>
>> static int extcon_usb_notifier(struct notifier_block *self,
>> unsigned long event, void *ptr)
>> {
>> switch (event) {
>> case EXTCON_DETACHED:
>> printk("USB is detached\n");
>> break;
>> case EXTCON_ATTACHED:
>> printk("USB is attached\n");
>> break;
>>
>> case EXTCON_USB_ID_LOW:
>> printk("USB's ID pin is low state\n");
>> break;
>> case EXTCON_USB_ID_HIGH:
>> printk("USB's ID pin is high state\n");
>> break;
>> case EXTCON_USB_VBUS_LOW:
>> printk("USB's VBUS pin is high state\n");
>> break;
>> case EXTCON_USB_VBUS_HIGH:
>> printk("USB's VBUS pin is high state\n");
>> break;
>> default:
>> return -EINVAL;
>> };
>> }
>>
>> Changes from v1:
>> - Use dev_warn() instead of dev_info() if set the same extcon_line_state
>> value.
>>
>> Chanwoo Choi (2):
>>extcon: Add extcon_set_cable_line_state() to inform the additional
>> state of external connectors
>>extcon: usb-gpio: Update the ID pin state of USB when cable state is
>> changed
>>
>> drivers/extcon/extcon-usb-gpio.c | 6
>> drivers/extcon/extcon.c | 74
>> +++-
>> include/linux/extcon.h | 24 +
>> 3 files changed, 103 insertions(+), 1 deletion(-)
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
Chanwoo,
On 27/05/15 15:15, 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);
Now that all USB line states can be captured by a single cable type
can we get rid of EXTCON_USB_HOST?
That way the extcon driver doesn't need to make any decisions as to
what mode we're in (host/cable) and this is best left to the USB driver.
cheers,
-roger
extcon_set_cable_state_(info->edev, EXTCON_USB, true);
extcon_set_cable_line_state(info->edev, EXTCON_USB,
EXTCON_USB_ID_HIGH);
} else {
extcon_set_cable_state_(info->edev, EXTCON_USB, false);
extcon_set_cable_state_(info->edev, EXTCON_USB_HOST,
true);
extcon_set_cable_line_state(info->edev, EXTCON_USB,
EXTCON_USB_ID_LOW);
}
}
- On specific extcon consumder driver as following:
static int xxx_probe(struct platform_device *pdev)
{
struct notifier_chain nh;
nb.notifier_call = extcon_usb_notifier;
ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
...
}
static int extcon_usb_notifier(struct notifier_block *self,
unsigned long event, void *ptr)
{
switch (event) {
case EXTCON_DETACHED:
printk("USB is detached\n");
break;
case EXTCON_ATTACHED:
printk("USB is attached\n");
break;
case EXTCON_USB_ID_LOW:
printk("USB's ID pin is low state\n");
break;
case EXTCON_USB_ID_HIGH:
printk("USB's ID pin is high state\n");
break;
case EXTCON_USB_VBUS_LOW:
printk("USB's VBUS pin is high state\n");
break;
case EXTCON_USB_VBUS_HIGH:
printk("USB's VBUS pin is high state\n");
break;
default:
return -EINVAL;
};
}
Changes from v1:
- Use dev_warn() instead of dev_info() if set the same extcon_line_state value.
Chanwoo Choi (2):
extcon: Add extcon_set_cable_line_state() to inform the additional state of
external connectors
extcon: usb-gpio: Update the ID pin state of USB when cable state is changed
drivers/extcon/extcon-usb-gpio.c | 6
drivers/extcon/extcon.c | 74 +++-
include/linux/extcon.h | 24 +
3 files changed, 103 insertions(+), 1 deletion(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/2] extcon: Inform the state of both ID and VBUS pin for USB
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);
} else {
extcon_set_cable_state_(info->edev, EXTCON_USB, false);
extcon_set_cable_state_(info->edev, EXTCON_USB_HOST,
true);
extcon_set_cable_line_state(info->edev, EXTCON_USB,
EXTCON_USB_ID_LOW);
}
}
- On specific extcon consumder driver as following:
static int xxx_probe(struct platform_device *pdev)
{
struct notifier_chain nh;
nb.notifier_call = extcon_usb_notifier;
ret = extcon_register_notifier(edev, EXTCON_USB, &nb);
...
}
static int extcon_usb_notifier(struct notifier_block *self,
unsigned long event, void *ptr)
{
switch (event) {
case EXTCON_DETACHED:
printk("USB is detached\n");
break;
case EXTCON_ATTACHED:
printk("USB is attached\n");
break;
case EXTCON_USB_ID_LOW:
printk("USB's ID pin is low state\n");
break;
case EXTCON_USB_ID_HIGH:
printk("USB's ID pin is high state\n");
break;
case EXTCON_USB_VBUS_LOW:
printk("USB's VBUS pin is high state\n");
break;
case EXTCON_USB_VBUS_HIGH:
printk("USB's VBUS pin is high state\n");
break;
default:
return -EINVAL;
};
}
Changes from v1:
- Use dev_warn() instead of dev_info() if set the same extcon_line_state value.
Chanwoo Choi (2):
extcon: Add extcon_set_cable_line_state() to inform the additional state of
external connectors
extcon: usb-gpio: Update the ID pin state of USB when cable state is changed
drivers/extcon/extcon-usb-gpio.c | 6
drivers/extcon/extcon.c | 74 +++-
include/linux/extcon.h | 24 +
3 files changed, 103 insertions(+), 1 deletion(-)
--
2.2.0.GIT
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
