Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1400666 > unrolled thread
| Started by | Roger Quadros <rogerq@ti.com> |
|---|---|
| First post | 2016-05-13 12:10 +0200 |
| Last post | 2016-05-30 16:10 +0200 |
| Articles | 12 on this page of 52 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 09:20 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 10:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 11:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 12:00 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 09:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-17 10:10 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 10:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:50 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 16:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-19 09:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 04:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-23 05:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:20 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-23 12:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-24 05:00 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 15:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-18 05:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 03:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-20 09:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 05:00 +0200
[PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-13 20:20 +0200
Re: [PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-16 10:20 +0200
[PATCH v9 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-20 11:40 +0200
Re: [PATCH v9 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-23 23:10 +0200
[PATCH v8 11/14] usb: otg: use dev_dbg() instead of VDBG() Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 12/14] usb: hcd: Adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 14/14] usb: host: xhci-plat: Add otg device to platform data Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 03/14] usb: hcd.h: Add OTG to HCD interface Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-16 11:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 10:40 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 11:20 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 12:00 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-24 11:50 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-25 04:50 +0200
RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 06:00 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 16:50 +0200
[PATCH v8 01/14] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 00/14] USB OTG/dual-role framework Peter Chen <hzpeterchen@gmail.com> - 2016-05-30 11:40 +0200
Re: [PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-30 16:10 +0200
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-23 12:10 +0200 |
| Subject | Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rBUY9-4uz-7@gated-at.bofh.it> |
| In reply to | #1404296 |
On 20/05/16 12:53, Peter Chen wrote:
> On Fri, May 20, 2016 at 12:19:07PM +0300, Roger Quadros wrote:
>> On 20/05/16 11:31, Roger Quadros wrote:
>>> On 18/05/16 15:59, Roger Quadros wrote:
>>>> Hi Peter,
>>>>
>>>> On 18/05/16 10:45, Peter Chen wrote:
>>>>>
>>>>>
>>>>> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros <rogerq@ti.com <mailto:rogerq@ti.com>> wrote:
>>>>>
>>>>> On 13/05/16 13:03, Roger Quadros wrote:
>>>>> > It provides APIs for the following tasks
>>>>> >
>>>>> > - Registering an OTG/dual-role capable controller
>>>>> > - Registering Host and Gadget controllers to OTG core
>>>>> > - Providing inputs to and kicking the OTG state machine
>>>>> >
>>>>> > Provide a dual-role device (DRD) state machine.
>>>>> > DRD mode is a reduced functionality OTG mode. In this mode
>>>>> > we don't support SRP, HNP and dynamic role-swap.
>>>>> >
>>>>> > In DRD operation, the controller mode (Host or Peripheral)
>>>>> > is decided based on the ID pin status. Once a cable plug (Type-A
>>>>> > or Type-B) is attached the controller selects the state
>>>>> > and doesn't change till the cable in unplugged and a different
>>>>> > cable type is inserted.
>>>>> >
>>>>> > As we don't need most of the complex OTG states and OTG timers
>>>>> > we implement a lean DRD state machine in usb-otg.c.
>>>>> > The DRD state machine is only interested in 2 hardware inputs
>>>>> > 'id' and 'b_sess_vld'.
>>>>> >
>>>>> > Signed-off-by: Roger Quadros <rogerq@ti.com <mailto:rogerq@ti.com>>
>>>>> > ---
>>>>> > drivers/usb/common/Makefile | 2 +-
>>>>> > drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
>>>>> > drivers/usb/core/Kconfig | 4 +-
>>>>> > include/linux/usb/gadget.h | 2 +
>>>>> > include/linux/usb/hcd.h | 1 +
>>>>> > include/linux/usb/otg-fsm.h | 7 +
>>>>> > include/linux/usb/otg.h | 154 ++++++-
>>>>> > 7 files changed, 1206 insertions(+), 6 deletions(-)
>>>>> > create mode 100644 drivers/usb/common/usb-otg.c
>>>>>
>>>>>
>>>>> This patch causes the following build issues when CONFIG_USB_GADGET=m, CONFIG_USB=m,
>>>>> CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
>>>>>
>>>>> ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>>>>> ERROR: "usb_otg_unregister_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>>>>> ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
>>>>> ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] undefined!
>>>>> ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
>>>>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>>>>> make[1]: *** [__modpost] Error 1
>>>>> Makefile:1141: recipe for target 'modules' failed
>>>>> make: *** [modules] Error 2
>>>>> make: *** Waiting for unfinished jobs....
>>>>>
>>>>> drivers/built-in.o: In function `drd_set_state':
>>>>> usb-otg.c:(.text+0x2b4242): undefined reference to `usb_otg_state_string'
>>>>> drivers/built-in.o: In function `drd_statemachine':
>>>>> (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
>>>>> Makefile:937: recipe for target 'vmlinux' failed
>>>>>
>>>>> I'll fix it up with the following diff.
>>>>>
>>>>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>>>>> index dca7856..16a5b55 100644
>>>>> --- a/drivers/usb/Makefile
>>>>> +++ b/drivers/usb/Makefile
>>>>> @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/
>>>>> obj-$(CONFIG_USB_GADGET) += gadget/
>>>>>
>>>>> obj-$(CONFIG_USB_COMMON) += common/
>>>>> +obj-$(CONFIG_USB_OTG) += common/
>>>>>
>>>>> obj-$(CONFIG_USBIP_CORE) += usbip/
>>>>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>>>>> index 77048aa..17e449e 100644
>>>>> --- a/drivers/usb/common/usb-otg.c
>>>>> +++ b/drivers/usb/common/usb-otg.c
>>>>> @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
>>>>> return hcd == hcd->primary_hcd;
>>>>> }
>>>>>
>>>>> +static const char *otg_state_string(enum usb_otg_state state)
>>>>> +{
>>>>> + static const char *const names[] = {
>>>>> + [OTG_STATE_A_IDLE] = "a_idle",
>>>>> + [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
>>>>> + [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
>>>>> + [OTG_STATE_A_HOST] = "a_host",
>>>>> + [OTG_STATE_A_SUSPEND] = "a_suspend",
>>>>> + [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
>>>>> + [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
>>>>> + [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
>>>>> + [OTG_STATE_B_IDLE] = "b_idle",
>>>>> + [OTG_STATE_B_SRP_INIT] = "b_srp_init",
>>>>> + [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
>>>>> + [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
>>>>> + [OTG_STATE_B_HOST] = "b_host",
>>>>> + };
>>>>> +
>>>>> + if (state < 0 || state >= ARRAY_SIZE(names))
>>>>> + return "UNDEFINED";
>>>>> +
>>>>> + return names[state];
>>>>> +}
>>>>> +
>>>>>
>>>>>
>>>>>
>>>>> From my point, make another copy for otg stuff is not a good way,
>>>>> could we make folder under usb/ named otg for dedicated otg stuffs,
>>>>> in that case, build otg stuffs can not depend on USB_COMMON.
>>>>
>>>> OK. I can try that. I'll delete otg_state_string from usb-common.c and
>>>> move it into usb/otg/usb-otg.c
>>>>
>>>> I'll also move usb-otg-fsm.c to usb/otg/.
>>>
>>> But we can't delete usb_otg_state_string() from usb-common.c. That is used at a
>>> number of places whether OTG is enabled or not.
>>>
>>> One option is to make usb-common built in when otg is enabled. What do you say?
>>>
>>
>> This should also get solved if we make USB_OTG tristate so that it is same as
>> USB_COMMON.
>>
>> However I haven't had success in making Kconfig behave like this.
>>
>> USB_OTG = y if USB == y && GADGET = m
>> USB_OTG = y if USB == m && GADGET = y
>>
>> Is there any Kconfig trickery to get this behaviour?
>>
>
> Unless let the USB_OTG works like USB_COMMON which is selected by GADGET
> or HCD. In fact, HCD and Gadget code uses USB OTG symbol directly in
> this framework, it seems like HCD and Gadget depends on OTG, but not
> otherwise.
Not necessarily for build. We have dummy otg functions if USB_OTG is not defined,
and we have checks in place if OTG drivers use OTG related functions while
USB_OTG is not enabled.
>
> If we want OTG to depend on HCD && GADGET, we need not to use OTG symbol
> at HCD and GADGET, and the OTG can use HCD and GADGET symbol directly.
>
We can't use select as we don't want USB_OTG enabled when only one of HCD or GADGET
is enabled. We also want users to be able to disable USB_OTG when both HCD and
GADGET are enabled.
cheers,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-24 11:50 +0200 |
| Subject | Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCh8l-1xw-3@gated-at.bofh.it> |
| In reply to | #1400674 |
Hi Peter,
I have one question here. Please see below.
On 13/05/16 13:03, Roger Quadros wrote:
> It provides APIs for the following tasks
>
> - Registering an OTG/dual-role capable controller
> - Registering Host and Gadget controllers to OTG core
> - Providing inputs to and kicking the OTG state machine
>
> Provide a dual-role device (DRD) state machine.
> DRD mode is a reduced functionality OTG mode. In this mode
> we don't support SRP, HNP and dynamic role-swap.
>
> In DRD operation, the controller mode (Host or Peripheral)
> is decided based on the ID pin status. Once a cable plug (Type-A
> or Type-B) is attached the controller selects the state
> and doesn't change till the cable in unplugged and a different
> cable type is inserted.
>
> As we don't need most of the complex OTG states and OTG timers
> we implement a lean DRD state machine in usb-otg.c.
> The DRD state machine is only interested in 2 hardware inputs
> 'id' and 'b_sess_vld'.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/common/Makefile | 2 +-
> drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
> drivers/usb/core/Kconfig | 4 +-
> include/linux/usb/gadget.h | 2 +
> include/linux/usb/hcd.h | 1 +
> include/linux/usb/otg-fsm.h | 7 +
> include/linux/usb/otg.h | 154 ++++++-
> 7 files changed, 1206 insertions(+), 6 deletions(-)
> create mode 100644 drivers/usb/common/usb-otg.c
>
<snip>
> +
> +/**
> + * usb_otg_register() - Register the OTG/dual-role device to OTG core
> + * @dev: OTG/dual-role controller device.
> + * @config: OTG configuration.
> + *
> + * Registers the OTG/dual-role controller device with the USB OTG core.
> + *
> + * Return: struct usb_otg * if success, ERR_PTR() if error.
> + */
> +struct usb_otg *usb_otg_register(struct device *dev,
> + struct usb_otg_config *config)
> +{
> + struct usb_otg *otg;
> + struct otg_wait_data *wait;
> + int ret = 0;
> +
> + if (!dev || !config || !config->fsm_ops)
> + return ERR_PTR(-EINVAL);
> +
> + /* already in list? */
> + mutex_lock(&otg_list_mutex);
> + if (usb_otg_get_data(dev)) {
> + dev_err(dev, "otg: %s: device already in otg list\n",
> + __func__);
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + /* allocate and add to list */
> + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
> + if (!otg) {
> + ret = -ENOMEM;
> + goto unlock;
> + }
> +
> + otg->dev = dev;
> + otg->caps = config->otg_caps;
Here, we should be checking if user needs to disable any OTG features. So,
if (dev->of_node)
of_usb_update_otg_caps(dev->of_node, &otg->caps);
Do you agree?
This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
to 'struct usb_otg_caps caps;' so that we can modify the local copy instead
of the one passed by the OTG controller.
We can also move of_usb_update_otg_caps() to otg.h.
We will also need to modify the udc-core code so that it sets gadget->otg_caps
to the modified otg_caps from the OTG core. This will ensure that the right
OTG descriptors are sent.
So we will have to introduce an API.
struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
And in udc-core.c,
static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver)
{
..
ret = driver->bind(udc->gadget, driver);
if (ret)
goto err1;
/* If OTG, the otg core starts the UDC when needed */
if (udc->gadget->otg_dev) {
+ udc->gadget->is_otg = true;
+ udc->gadget->otg_caps = usb_otg_get_otg_caps(udc->gadget->otg_dev);
mutex_unlock(&udc_lock);
usb_otg_register_gadget(udc->gadget, &otg_gadget_intf);
mutex_lock(&udc_lock);
} else {
ret = usb_gadget_udc_start(udc);
if (ret) {
driver->unbind(udc->gadget);
goto err1;
}
usb_udc_connect_control(udc);
}
..
}
What do you say?
regards,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-25 04:50 +0200 |
| Subject | Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCx3r-3y6-1@gated-at.bofh.it> |
| In reply to | #1406010 |
On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
> Hi Peter,
>
> I have one question here. Please see below.
>
> On 13/05/16 13:03, Roger Quadros wrote:
> > It provides APIs for the following tasks
> >
> > - Registering an OTG/dual-role capable controller
> > - Registering Host and Gadget controllers to OTG core
> > - Providing inputs to and kicking the OTG state machine
> >
> > Provide a dual-role device (DRD) state machine.
> > DRD mode is a reduced functionality OTG mode. In this mode
> > we don't support SRP, HNP and dynamic role-swap.
> >
> > In DRD operation, the controller mode (Host or Peripheral)
> > is decided based on the ID pin status. Once a cable plug (Type-A
> > or Type-B) is attached the controller selects the state
> > and doesn't change till the cable in unplugged and a different
> > cable type is inserted.
> >
> > As we don't need most of the complex OTG states and OTG timers
> > we implement a lean DRD state machine in usb-otg.c.
> > The DRD state machine is only interested in 2 hardware inputs
> > 'id' and 'b_sess_vld'.
> >
> > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > ---
> > drivers/usb/common/Makefile | 2 +-
> > drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
> > drivers/usb/core/Kconfig | 4 +-
> > include/linux/usb/gadget.h | 2 +
> > include/linux/usb/hcd.h | 1 +
> > include/linux/usb/otg-fsm.h | 7 +
> > include/linux/usb/otg.h | 154 ++++++-
> > 7 files changed, 1206 insertions(+), 6 deletions(-)
> > create mode 100644 drivers/usb/common/usb-otg.c
> >
>
> <snip>
>
> > +
> > +/**
> > + * usb_otg_register() - Register the OTG/dual-role device to OTG core
> > + * @dev: OTG/dual-role controller device.
> > + * @config: OTG configuration.
> > + *
> > + * Registers the OTG/dual-role controller device with the USB OTG core.
> > + *
> > + * Return: struct usb_otg * if success, ERR_PTR() if error.
> > + */
> > +struct usb_otg *usb_otg_register(struct device *dev,
> > + struct usb_otg_config *config)
> > +{
> > + struct usb_otg *otg;
> > + struct otg_wait_data *wait;
> > + int ret = 0;
> > +
> > + if (!dev || !config || !config->fsm_ops)
> > + return ERR_PTR(-EINVAL);
> > +
> > + /* already in list? */
> > + mutex_lock(&otg_list_mutex);
> > + if (usb_otg_get_data(dev)) {
> > + dev_err(dev, "otg: %s: device already in otg list\n",
> > + __func__);
> > + ret = -EINVAL;
> > + goto unlock;
> > + }
> > +
> > + /* allocate and add to list */
> > + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
> > + if (!otg) {
> > + ret = -ENOMEM;
> > + goto unlock;
> > + }
> > +
> > + otg->dev = dev;
> > + otg->caps = config->otg_caps;
>
> Here, we should be checking if user needs to disable any OTG features. So,
>
> if (dev->of_node)
> of_usb_update_otg_caps(dev->of_node, &otg->caps);
>
> Do you agree?
> This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
> to 'struct usb_otg_caps caps;' so that we can modify the local copy instead
> of the one passed by the OTG controller.
Why can't modify the one from OTG controller directly?
>
> We can also move of_usb_update_otg_caps() to otg.h.
>
> We will also need to modify the udc-core code so that it sets gadget->otg_caps
> to the modified otg_caps from the OTG core. This will ensure that the right
> OTG descriptors are sent.
>
> So we will have to introduce an API.
>
> struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
>
> And in udc-core.c,
>
> static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver)
> {
> ..
> ret = driver->bind(udc->gadget, driver);
> if (ret)
> goto err1;
>
> /* If OTG, the otg core starts the UDC when needed */
> if (udc->gadget->otg_dev) {
> + udc->gadget->is_otg = true;
gadget->is_otg is only set to true if fully OTG is supported and it
needs to send OTG descriptors at this case. DRD devices should not send OTG
descriptors.
> + udc->gadget->otg_caps = usb_otg_get_otg_caps(udc->gadget->otg_dev);
Getting otg capabilities should be prior to driver->bind since
usb_otg_descriptor_init is called at that. Besides, Gadget driver
may be probed before otg driver is registered
I am wonder if we can implement defer probe for gadget/udc/host driver
if otg driver is not probed, in that case, some designs can be simpler
like wait list in otg driver.
> mutex_unlock(&udc_lock);
> usb_otg_register_gadget(udc->gadget, &otg_gadget_intf);
> mutex_lock(&udc_lock);
> } else {
> ret = usb_gadget_udc_start(udc);
> if (ret) {
> driver->unbind(udc->gadget);
> goto err1;
> }
> usb_udc_connect_control(udc);
> }
> ..
> }
>
> What do you say?
>
> regards,
> -roger
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-25 06:00 +0200 |
| Subject | RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCy9c-4pG-7@gated-at.bofh.it> |
| In reply to | #1406569 |
> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen@gmail.com]
> Sent: Wednesday, May 25, 2016 10:44 AM
> To: Roger Quadros <rogerq@ti.com>
> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
>
> On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
> > Hi Peter,
> >
> > I have one question here. Please see below.
> >
> > On 13/05/16 13:03, Roger Quadros wrote:
> > > It provides APIs for the following tasks
> > >
> > > - Registering an OTG/dual-role capable controller
> > > - Registering Host and Gadget controllers to OTG core
> > > - Providing inputs to and kicking the OTG state machine
> > >
> > > Provide a dual-role device (DRD) state machine.
> > > DRD mode is a reduced functionality OTG mode. In this mode we don't
> > > support SRP, HNP and dynamic role-swap.
> > >
> > > In DRD operation, the controller mode (Host or Peripheral) is
> > > decided based on the ID pin status. Once a cable plug (Type-A or
> > > Type-B) is attached the controller selects the state and doesn't
> > > change till the cable in unplugged and a different cable type is
> > > inserted.
> > >
> > > As we don't need most of the complex OTG states and OTG timers we
> > > implement a lean DRD state machine in usb-otg.c.
> > > The DRD state machine is only interested in 2 hardware inputs 'id'
> > > and 'b_sess_vld'.
> > >
> > > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > > ---
> > > drivers/usb/common/Makefile | 2 +-
> > > drivers/usb/common/usb-otg.c | 1042
> ++++++++++++++++++++++++++++++++++++++++++
> > > drivers/usb/core/Kconfig | 4 +-
> > > include/linux/usb/gadget.h | 2 +
> > > include/linux/usb/hcd.h | 1 +
> > > include/linux/usb/otg-fsm.h | 7 +
> > > include/linux/usb/otg.h | 154 ++++++-
> > > 7 files changed, 1206 insertions(+), 6 deletions(-) create mode
> > > 100644 drivers/usb/common/usb-otg.c
> > >
> >
> > <snip>
> >
> > > +
> > > +/**
> > > + * usb_otg_register() - Register the OTG/dual-role device to OTG
> > > +core
> > > + * @dev: OTG/dual-role controller device.
> > > + * @config: OTG configuration.
> > > + *
> > > + * Registers the OTG/dual-role controller device with the USB OTG
> core.
> > > + *
> > > + * Return: struct usb_otg * if success, ERR_PTR() if error.
> > > + */
> > > +struct usb_otg *usb_otg_register(struct device *dev,
> > > + struct usb_otg_config *config) {
> > > + struct usb_otg *otg;
> > > + struct otg_wait_data *wait;
> > > + int ret = 0;
> > > +
> > > + if (!dev || !config || !config->fsm_ops)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + /* already in list? */
> > > + mutex_lock(&otg_list_mutex);
> > > + if (usb_otg_get_data(dev)) {
> > > + dev_err(dev, "otg: %s: device already in otg list\n",
> > > + __func__);
> > > + ret = -EINVAL;
> > > + goto unlock;
> > > + }
> > > +
> > > + /* allocate and add to list */
> > > + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
> > > + if (!otg) {
> > > + ret = -ENOMEM;
> > > + goto unlock;
> > > + }
> > > +
> > > + otg->dev = dev;
> > > + otg->caps = config->otg_caps;
> >
> > Here, we should be checking if user needs to disable any OTG features.
> > So,
> >
> > if (dev->of_node)
> > of_usb_update_otg_caps(dev->of_node, &otg->caps);
> >
> > Do you agree?
> > This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
> > to 'struct usb_otg_caps caps;' so that we can modify the local copy
> > instead of the one passed by the OTG controller.
>
> Why can't modify the one from OTG controller directly?
Yes, if user wants to disable any OTG features, this should have been
done in 'config->otg_caps', if not, 'config->otg_caps' from controller
driver is invalid and making no sense.
>
> >
> > We can also move of_usb_update_otg_caps() to otg.h.
> >
> > We will also need to modify the udc-core code so that it sets
> > gadget->otg_caps to the modified otg_caps from the OTG core. This will
> > ensure that the right OTG descriptors are sent.
> >
> > So we will have to introduce an API.
> >
> > struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
> >
> > And in udc-core.c,
> >
> > static int udc_bind_to_driver(struct usb_udc *udc, struct
> > usb_gadget_driver *driver) { ..
> > ret = driver->bind(udc->gadget, driver);
> > if (ret)
> > goto err1;
> >
> > /* If OTG, the otg core starts the UDC when needed */
> > if (udc->gadget->otg_dev) {
> > + udc->gadget->is_otg = true;
>
> gadget->is_otg is only set to true if fully OTG is supported and it
> needs to send OTG descriptors at this case. DRD devices should not send
> OTG descriptors.
>
> > + udc->gadget->otg_caps = usb_otg_get_otg_caps(udc->gadget-
> >otg_dev);
>
> Getting otg capabilities should be prior to driver->bind since
> usb_otg_descriptor_init is called at that. Besides, Gadget driver may be
> probed before otg driver is registered
>
> I am wonder if we can implement defer probe for gadget/udc/host driver if
> otg driver is not probed, in that case, some designs can be simpler like
> wait list in otg driver.
I even don't see much benefit of this kind of random order of OTG/HCD/GADGET
registration, anyway OTG register can be done firstly.
HCD and GADGET are both separated drivers, but OTG is newly added
and what we need is just a registration in controller driver.
>
> > mutex_unlock(&udc_lock);
> > usb_otg_register_gadget(udc->gadget, &otg_gadget_intf);
> > mutex_lock(&udc_lock);
> > } else {
> > ret = usb_gadget_udc_start(udc);
> > if (ret) {
> > driver->unbind(udc->gadget);
> > goto err1;
> > }
> > usb_udc_connect_control(udc);
> > }
> > ..
> > }
> >
> > What do you say?
> >
> > regards,
> > -roger
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at http://vger.kernel.org/majordomo-info.html
>
> --
>
> Best Regards,
> Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-25 14:30 +0200 |
| Subject | Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCG6K-Vn-11@gated-at.bofh.it> |
| In reply to | #1406600 |
On 25/05/16 06:19, Jun Li wrote:
>
>
>> -----Original Message-----
>> From: Peter Chen [mailto:hzpeterchen@gmail.com]
>> Sent: Wednesday, May 25, 2016 10:44 AM
>> To: Roger Quadros <rogerq@ti.com>
>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
>> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
>>
>> On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
>>> Hi Peter,
>>>
>>> I have one question here. Please see below.
>>>
>>> On 13/05/16 13:03, Roger Quadros wrote:
>>>> It provides APIs for the following tasks
>>>>
>>>> - Registering an OTG/dual-role capable controller
>>>> - Registering Host and Gadget controllers to OTG core
>>>> - Providing inputs to and kicking the OTG state machine
>>>>
>>>> Provide a dual-role device (DRD) state machine.
>>>> DRD mode is a reduced functionality OTG mode. In this mode we don't
>>>> support SRP, HNP and dynamic role-swap.
>>>>
>>>> In DRD operation, the controller mode (Host or Peripheral) is
>>>> decided based on the ID pin status. Once a cable plug (Type-A or
>>>> Type-B) is attached the controller selects the state and doesn't
>>>> change till the cable in unplugged and a different cable type is
>>>> inserted.
>>>>
>>>> As we don't need most of the complex OTG states and OTG timers we
>>>> implement a lean DRD state machine in usb-otg.c.
>>>> The DRD state machine is only interested in 2 hardware inputs 'id'
>>>> and 'b_sess_vld'.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>> drivers/usb/common/Makefile | 2 +-
>>>> drivers/usb/common/usb-otg.c | 1042
>> ++++++++++++++++++++++++++++++++++++++++++
>>>> drivers/usb/core/Kconfig | 4 +-
>>>> include/linux/usb/gadget.h | 2 +
>>>> include/linux/usb/hcd.h | 1 +
>>>> include/linux/usb/otg-fsm.h | 7 +
>>>> include/linux/usb/otg.h | 154 ++++++-
>>>> 7 files changed, 1206 insertions(+), 6 deletions(-) create mode
>>>> 100644 drivers/usb/common/usb-otg.c
>>>>
>>>
>>> <snip>
>>>
>>>> +
>>>> +/**
>>>> + * usb_otg_register() - Register the OTG/dual-role device to OTG
>>>> +core
>>>> + * @dev: OTG/dual-role controller device.
>>>> + * @config: OTG configuration.
>>>> + *
>>>> + * Registers the OTG/dual-role controller device with the USB OTG
>> core.
>>>> + *
>>>> + * Return: struct usb_otg * if success, ERR_PTR() if error.
>>>> + */
>>>> +struct usb_otg *usb_otg_register(struct device *dev,
>>>> + struct usb_otg_config *config) {
>>>> + struct usb_otg *otg;
>>>> + struct otg_wait_data *wait;
>>>> + int ret = 0;
>>>> +
>>>> + if (!dev || !config || !config->fsm_ops)
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + /* already in list? */
>>>> + mutex_lock(&otg_list_mutex);
>>>> + if (usb_otg_get_data(dev)) {
>>>> + dev_err(dev, "otg: %s: device already in otg list\n",
>>>> + __func__);
>>>> + ret = -EINVAL;
>>>> + goto unlock;
>>>> + }
>>>> +
>>>> + /* allocate and add to list */
>>>> + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
>>>> + if (!otg) {
>>>> + ret = -ENOMEM;
>>>> + goto unlock;
>>>> + }
>>>> +
>>>> + otg->dev = dev;
>>>> + otg->caps = config->otg_caps;
>>>
>>> Here, we should be checking if user needs to disable any OTG features.
>>> So,
>>>
>>> if (dev->of_node)
>>> of_usb_update_otg_caps(dev->of_node, &otg->caps);
>>>
>>> Do you agree?
>>> This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
>>> to 'struct usb_otg_caps caps;' so that we can modify the local copy
>>> instead of the one passed by the OTG controller.
>>
>> Why can't modify the one from OTG controller directly?
>
> Yes, if user wants to disable any OTG features, this should have been
> done in 'config->otg_caps', if not, 'config->otg_caps' from controller
> driver is invalid and making no sense.
I've tried to why to Peter's mail.
>
>>
>>>
>>> We can also move of_usb_update_otg_caps() to otg.h.
>>>
>>> We will also need to modify the udc-core code so that it sets
>>> gadget->otg_caps to the modified otg_caps from the OTG core. This will
>>> ensure that the right OTG descriptors are sent.
>>>
>>> So we will have to introduce an API.
>>>
>>> struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
>>>
>>> And in udc-core.c,
>>>
>>> static int udc_bind_to_driver(struct usb_udc *udc, struct
>>> usb_gadget_driver *driver) { ..
>>> ret = driver->bind(udc->gadget, driver);
>>> if (ret)
>>> goto err1;
>>>
>>> /* If OTG, the otg core starts the UDC when needed */
>>> if (udc->gadget->otg_dev) {
>>> + udc->gadget->is_otg = true;
>>
>> gadget->is_otg is only set to true if fully OTG is supported and it
>> needs to send OTG descriptors at this case. DRD devices should not send
>> OTG descriptors.
>>
>>> + udc->gadget->otg_caps = usb_otg_get_otg_caps(udc->gadget-
>>> otg_dev);
>>
>> Getting otg capabilities should be prior to driver->bind since
>> usb_otg_descriptor_init is called at that. Besides, Gadget driver may be
>> probed before otg driver is registered
>>
>> I am wonder if we can implement defer probe for gadget/udc/host driver if
>> otg driver is not probed, in that case, some designs can be simpler like
>> wait list in otg driver.
>
> I even don't see much benefit of this kind of random order of OTG/HCD/GADGET
> registration, anyway OTG register can be done firstly.
You cannot control the order in systems where all 3 are implemented by separate
hardware modules. We have to consider that they can be registered
in any order and make sure it works.
returning -EPROBE_DEFER in usb_otg_add_gadget_udc()/usb_otg_add_hcd() if OTG
controller driver is not yet probed makes perfect sense and is the way to solve
this dependency issue.
> HCD and GADGET are both separated drivers, but OTG is newly added
> and what we need is just a registration in controller driver.
How do you solve the problem when host registers before OTG? I tried to solve it
using the wait_list in usb-otg.c. The other alternative is -EPROBE_DEFER during
usb_otg_add_hcd/udc().
--
cheers,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-25 14:30 +0200 |
| Subject | Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCG6K-Vn-23@gated-at.bofh.it> |
| In reply to | #1406569 |
On 25/05/16 05:44, Peter Chen wrote:
> On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
>> Hi Peter,
>>
>> I have one question here. Please see below.
>>
>> On 13/05/16 13:03, Roger Quadros wrote:
>>> It provides APIs for the following tasks
>>>
>>> - Registering an OTG/dual-role capable controller
>>> - Registering Host and Gadget controllers to OTG core
>>> - Providing inputs to and kicking the OTG state machine
>>>
>>> Provide a dual-role device (DRD) state machine.
>>> DRD mode is a reduced functionality OTG mode. In this mode
>>> we don't support SRP, HNP and dynamic role-swap.
>>>
>>> In DRD operation, the controller mode (Host or Peripheral)
>>> is decided based on the ID pin status. Once a cable plug (Type-A
>>> or Type-B) is attached the controller selects the state
>>> and doesn't change till the cable in unplugged and a different
>>> cable type is inserted.
>>>
>>> As we don't need most of the complex OTG states and OTG timers
>>> we implement a lean DRD state machine in usb-otg.c.
>>> The DRD state machine is only interested in 2 hardware inputs
>>> 'id' and 'b_sess_vld'.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> drivers/usb/common/Makefile | 2 +-
>>> drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
>>> drivers/usb/core/Kconfig | 4 +-
>>> include/linux/usb/gadget.h | 2 +
>>> include/linux/usb/hcd.h | 1 +
>>> include/linux/usb/otg-fsm.h | 7 +
>>> include/linux/usb/otg.h | 154 ++++++-
>>> 7 files changed, 1206 insertions(+), 6 deletions(-)
>>> create mode 100644 drivers/usb/common/usb-otg.c
>>>
>>
>> <snip>
>>
>>> +
>>> +/**
>>> + * usb_otg_register() - Register the OTG/dual-role device to OTG core
>>> + * @dev: OTG/dual-role controller device.
>>> + * @config: OTG configuration.
>>> + *
>>> + * Registers the OTG/dual-role controller device with the USB OTG core.
>>> + *
>>> + * Return: struct usb_otg * if success, ERR_PTR() if error.
>>> + */
>>> +struct usb_otg *usb_otg_register(struct device *dev,
>>> + struct usb_otg_config *config)
>>> +{
>>> + struct usb_otg *otg;
>>> + struct otg_wait_data *wait;
>>> + int ret = 0;
>>> +
>>> + if (!dev || !config || !config->fsm_ops)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + /* already in list? */
>>> + mutex_lock(&otg_list_mutex);
>>> + if (usb_otg_get_data(dev)) {
>>> + dev_err(dev, "otg: %s: device already in otg list\n",
>>> + __func__);
>>> + ret = -EINVAL;
>>> + goto unlock;
>>> + }
>>> +
>>> + /* allocate and add to list */
>>> + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
>>> + if (!otg) {
>>> + ret = -ENOMEM;
>>> + goto unlock;
>>> + }
>>> +
>>> + otg->dev = dev;
>>> + otg->caps = config->otg_caps;
>>
>> Here, we should be checking if user needs to disable any OTG features. So,
>>
>> if (dev->of_node)
>> of_usb_update_otg_caps(dev->of_node, &otg->caps);
>>
>> Do you agree?
>> This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
>> to 'struct usb_otg_caps caps;' so that we can modify the local copy instead
>> of the one passed by the OTG controller.
>
> Why can't modify the one from OTG controller directly?
>
There are 2 things.
1) OTG features supported by hardware. This is the controller's config->otg_caps
2) OTG features needed by system designer. This can be a subset of (1).
This needs to be maintained in the OTG core, based on config->otg_caps and
device tree disable flags.
So we can't use OTG controller config->otg_caps directly.
>>
>> We can also move of_usb_update_otg_caps() to otg.h.
>>
>> We will also need to modify the udc-core code so that it sets gadget->otg_caps
>> to the modified otg_caps from the OTG core. This will ensure that the right
>> OTG descriptors are sent.
>>
>> So we will have to introduce an API.
>>
>> struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
>>
>> And in udc-core.c,
>>
>> static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver)
>> {
>> ..
>> ret = driver->bind(udc->gadget, driver);
>> if (ret)
>> goto err1;
>>
>> /* If OTG, the otg core starts the UDC when needed */
>> if (udc->gadget->otg_dev) {
>> + udc->gadget->is_otg = true;
>
> gadget->is_otg is only set to true if fully OTG is supported and it
> needs to send OTG descriptors at this case. DRD devices should not send OTG
> descriptors.
Agreed.
>
>> + udc->gadget->otg_caps = usb_otg_get_otg_caps(udc->gadget->otg_dev);
>
> Getting otg capabilities should be prior to driver->bind since
> usb_otg_descriptor_init is called at that. Besides, Gadget driver
Correct.
> may be probed before otg driver is registered
udc_bind_to_driver gets called only when both, udc driver and function driver
are available. Defer probing will have to be done in usb_add_gadget_udc_release().
>
> I am wonder if we can implement defer probe for gadget/udc/host driver
> if otg driver is not probed, in that case, some designs can be simpler
> like wait list in otg driver.
Agree with you on that. Should I work on implementing -EPROBE_DEFER for
hcd.c and udc-core.c if hcd/udc is meant for otg and OTG controller is not
yet probed?
--
cheers,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-25 16:50 +0200 |
| Subject | RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core |
| Message-ID | <rCIie-28x-11@gated-at.bofh.it> |
| In reply to | #1406866 |
Hi Roger > >> > >> Here, we should be checking if user needs to disable any OTG > >> features. So, > >> > >> if (dev->of_node) > >> of_usb_update_otg_caps(dev->of_node, &otg->caps); > >> > >> Do you agree? > >> This means we need to change otg->caps from 'struct usb_otg_caps > *caps;' > >> to 'struct usb_otg_caps caps;' so that we can modify the local copy > >> instead of the one passed by the OTG controller. > > > > Why can't modify the one from OTG controller directly? > > > > There are 2 things. > 1) OTG features supported by hardware. This is the controller's config- > >otg_caps > 2) OTG features needed by system designer. This can be a subset of (1). Let's make things simple, we only need this subnet, which can be set by controller driver in config->otg_caps before pass (its address) to OTG core. So controller driver should get the capability of HW(+SW) and user config by whatever approach, then set its config->otg_caps. Li Jun
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-13 12:10 +0200 |
| Subject | [PATCH v8 01/14] usb: hcd: Initialize hcd->flags to 0 |
| Message-ID | <ryicH-1fj-37@gated-at.bofh.it> |
| In reply to | #1400666 |
When using the OTG/drd library we can call hcd_add/remove consecutively without calling usb_put_hcd/usb_create_hcd in between so hcd->flags can be stale. If the HC dies due to whatever reason then without this patch we get the below error on next hcd_add. [ 91.494257] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up [ 91.502068] hub 3-0:1.0: state 0 ports 1 chg 0000 evt 0000 [ 91.510240] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller [ 91.516940] xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 4 [ 91.529745] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 91.540637] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003 [ 91.757865] irq 254: nobody cared (try booting with the "irqpoll" option) [ 91.757880] CPU: 0 PID: 68 Comm: kworker/u2:2 Not tainted 4.1.4-00828-g1f0ed8c-dirty #44 [ 91.757885] Hardware name: Generic AM43 (Flattened Device Tree) [ 91.757914] Workqueue: usb_otg usb_otg_work [ 91.757921] Backtrace: [ 91.757954] [<c0012af0>] (dump_backtrace) from [<c0012c8c>] (show_stack+0x18/0x1c) [ 91.757972] r6:c089d4a4 r5:ffffffff r4:00000000 r3:ee440000 [ 91.757991] [<c0012c74>] (show_stack) from [<c05f7c14>] (dump_stack+0x84/0xd0) [ 91.758008] [<c05f7b90>] (dump_stack) from [<c0084b30>] (__report_bad_irq+0x28/0xc8) [ 91.758024] r7:00000000 r6:000000fe r5:00000000 r4:ee514c40 [ 91.758037] [<c0084b08>] (__report_bad_irq) from [<c00850b0>] (note_interrupt+0x24c/0x2ac) [ 91.758052] r6:000000fe r5:00000000 r4:ee514c40 r3:00000000 [ 91.758065] [<c0084e64>] (note_interrupt) from [<c00828fc>] (handle_irq_event_percpu+0xb0/0x158) [ 91.758085] r10:ee514c40 r9:c08ce49a r8:000000fe r7:00000000 r6:00000000 r5:00000000 [ 91.758094] r4:00000000 r3:00000000 [ 91.758105] [<c008284c>] (handle_irq_event_percpu) from [<c00829e8>] (handle_irq_event+0x44/0x64) [ 91.758126] r10:00000001 r9:ee441ab0 r8:ee441bb8 r7:c0858b4c r6:ed174280 r5:ee514ca0 [ 91.758132] r4:ee514c40 [ 91.758144] [<c00829a4>] (handle_irq_event) from [<c0085970>] (handle_fasteoi_irq+0x100/0x1bc) [ 91.758159] r6:c085dba0 r5:ee514ca0 r4:ee514c40 r3:00000000 [ 91.758171] [<c0085870>] (handle_fasteoi_irq) from [<c0082058>] (generic_handle_irq+0x28/0x38) [ 91.758186] r7:c0853d40 r6:c0858b4c r5:000000fe r4:000000fe [ 91.758197] [<c0082030>] (generic_handle_irq) from [<c00821c0>] (__handle_domain_irq+0x98/0x12c) [ 91.758207] r4:c0853d40 r3:00000100 [ 91.758219] [<c0082128>] (__handle_domain_irq) from [<c00094e0>] (gic_handle_irq+0x28/0x68) [ 91.758239] r10:00000001 r9:ee441bb8 r8:fa240100 r7:c0858d70 r6:ee441ab0 r5:000000b8 [ 91.758245] r4:fa24010c [ 91.758264] [<c00094b8>] (gic_handle_irq) from [<c05fd540>] (__irq_svc+0x40/0x74) [ 91.758271] Exception stack(0xee441ab0 to 0xee441af8) [ 91.758280] 1aa0: 00000000 c08d2980 ee441ac0 00000000 [ 91.758292] 1ac0: 00000008 00000089 c0858b4c c0858080 00000000 ee441bb8 00000001 ee441b3c [ 91.758301] 1ae0: 00000101 ee441af8 c02fc418 c0046a1c 20000113 ffffffff [ 91.758321] r8:00000000 r7:ee441ae4 r6:ffffffff r5:20000113 r4:c0046a1c r3:c02fc418 [ 91.758347] [<c00469a0>] (__do_softirq) from [<c0046eac>] (irq_exit+0xb8/0x104) [ 91.758367] r10:00000001 r9:ee441bb8 r8:00000000 r7:c0853d40 r6:c0858b4c r5:00000089 [ 91.758373] r4:00000000 [ 91.758386] [<c0046df4>] (irq_exit) from [<c00821c8>] (__handle_domain_irq+0xa0/0x12c) [ 91.758395] r4:00000000 r3:00000100 [ 91.758406] [<c0082128>] (__handle_domain_irq) from [<c00094e0>] (gic_handle_irq+0x28/0x68) [ 91.758426] r10:c08e3510 r9:20000013 r8:fa240100 r7:c0858d70 r6:ee441bb8 r5:00000039 [ 91.758433] r4:fa24010c [ 91.758445] [<c00094b8>] (gic_handle_irq) from [<c05fd540>] (__irq_svc+0x40/0x74) [ 91.758450] Exception stack(0xee441bb8 to 0xee441c00) [ 91.758457] 1ba0: 00000000 00000001 [ 91.758468] 1bc0: 00000000 ee440000 c08e2524 0000004d 00000274 00000000 00000000 20000013 [ 91.758479] 1be0: c08e3510 ee441c4c ee441b60 ee441c00 c03acfec c0080d4c 60000013 ffffffff [ 91.758499] r8:00000000 r7:ee441bec r6:ffffffff r5:60000013 r4:c0080d4c r3:c03acfec [ 91.758524] [<c0080950>] (console_unlock) from [<c0081670>] (vprintk_emit+0x20c/0x500) [ 91.758544] r10:ee441cc0 r9:c08d3550 r8:c08e3ea0 r7:00000000 r6:00000001 r5:0000003d [ 91.758551] r4:c08d3550 [ 91.758573] [<c0081464>] (vprintk_emit) from [<c03f6f70>] (dev_vprintk_emit+0x104/0x1ac) [ 91.758593] r10:ee441d8c r9:0000000e r8:c07951e0 r7:00000006 r6:ee441cc0 r5:0000000d [ 91.758599] r4:ee731068 [ 91.758612] [<c03f6e6c>] (dev_vprintk_emit) from [<c03f7040>] (dev_printk_emit+0x28/0x30) [ 91.758632] r10:00000001 r9:ee5f8410 r8:ee731000 r7:ed429000 r6:00000006 r5:ee441dc0 [ 91.758638] r4:ee731068 [ 91.758651] [<c03f701c>] (dev_printk_emit) from [<c03f7098>] (__dev_printk+0x50/0x70) [ 91.758660] r3:bf2268cc r2:c07951e0 [ 91.758673] [<c03f7048>] (__dev_printk) from [<c03f70f4>] (_dev_info+0x3c/0x48) [ 91.758686] r6:00000000 r5:ee731068 r4:ee731000 [ 91.758790] [<c03f70bc>] (_dev_info) from [<bf20ec3c>] (usb_new_device+0x11c/0x518 [usbcore]) [ 91.758804] r3:00000003 r2:00001d6b r1:bf225bc4 [ 91.758881] [<bf20eb20>] (usb_new_device [usbcore]) from [<bf213560>] (usb_otg_add_hcd+0x514/0x7f8 [usbcore]) [ 91.758903] r10:00000001 r9:ee5f8410 r8:ee731000 r7:000000fe r6:ed4290c8 r5:00000000 [ 91.758909] r4:ed429000 [ 91.758957] [<bf21304c>] (usb_otg_add_hcd [usbcore]) from [<c047a238>] (usb_otg_start_host+0xb8/0xf8) [ 91.758978] r10:00000000 r9:00000002 r8:00000000 r7:ee02b000 r6:ee452808 r5:ee452808 [ 91.758985] r4:ee452808 [ 91.758997] [<c047a180>] (usb_otg_start_host) from [<c047a020>] (drd_set_protocol+0xac/0xd8) [ 91.759007] r4:00000001 r3:c047a180 [ 91.759018] [<c0479f74>] (drd_set_protocol) from [<c047a2ec>] (drd_set_state+0x74/0x98) [ 91.759027] r5:ee452808 r4:00000009 [ 91.759039] [<c047a278>] (drd_set_state) from [<c047a3dc>] (usb_otg_work+0xcc/0x154) [ 91.759054] r6:ee452808 r5:ee4528b8 r4:ee452968 r3:00000000 [ 91.759072] [<c047a310>] (usb_otg_work) from [<c005754c>] (process_one_work+0x128/0x340) [ 91.759087] r6:ee02ac00 r5:ee452968 r4:ee42b900 r3:c047a310 [ 91.759100] [<c0057424>] (process_one_work) from [<c00578f8>] (worker_thread+0x158/0x49c) [ 91.759120] r10:ee42b900 r9:00000002 r8:ee02ac00 r7:00000088 r6:ee42b918 r5:ee02ac00 [ 91.759127] r4:ee02ac14 [ 91.759145] [<c00577a0>] (worker_thread) from [<c005cc40>] (kthread+0xdc/0xf8) [ 91.759165] r10:00000000 r9:00000000 r8:00000000 r7:c00577a0 r6:ee42b900 r5:ee429940 [ 91.759174] r4:00000000 r3:00000000 [ 91.759190] [<c005cb64>] (kthread) from [<c000fc08>] (ret_from_fork+0x14/0x2c) [ 91.759206] r7:00000000 r6:00000000 r5:c005cb64 r4:ee429940 [ 91.759209] handlers: [ 91.759255] [<bf211b5c>] usb_hcd_irq [usbcore] [ 91.759260] Disabling IRQ #254 Signed-off-by: Roger Quadros <rogerq@ti.com> Reviewed-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/core/hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2ca2cef..9484539 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -3009,6 +3009,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) } usb_put_invalidate_rhdev(hcd); + hcd->flags = 0; } EXPORT_SYMBOL_GPL(usb_remove_hcd); -- 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-13 12:10 +0200 |
| Subject | [PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG |
| Message-ID | <ryicH-1fj-41@gated-at.bofh.it> |
| In reply to | #1400666 |
Let's use CONFIG_USB_OTG as a single config option to enable
USB OTG and the OTG FSM. This makes things a lot less confusing.
Update all users of CONFIG_USB_OTG_FSM to CONFIG_USB_OTG.
Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Peter Chen <peter.chen@nxp.com>
---
Documentation/usb/chipidea.txt | 2 +-
drivers/usb/chipidea/Makefile | 2 +-
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/otg_fsm.h | 2 +-
drivers/usb/common/Makefile | 3 ++-
drivers/usb/core/Kconfig | 8 --------
drivers/usb/phy/Kconfig | 2 +-
7 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index 678741b..3b1f263 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -5,7 +5,7 @@ with 2 Freescale i.MX6Q sabre SD boards.
1.1 How to enable OTG FSM in menuconfig
---------------------------------------
-Select CONFIG_USB_OTG_FSM, rebuild kernel Image and modules.
+Select CONFIG_USB_OTG, rebuild kernel Image and modules.
If you want to check some internal variables for otg fsm,
mount debugfs, there are 2 files which can show otg fsm
variables and some controller registers value:
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 518e445..45aa24d 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc.o
ci_hdrc-y := core.o otg.o debug.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST) += host.o
-ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o
+ci_hdrc-$(CONFIG_USB_OTG) += otg_fsm.o
# Glue/Bridge layers go here
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index c523975..1a32b8c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -406,7 +406,7 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci, enum ci_hw_regs reg,
*/
static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
{
-#ifdef CONFIG_USB_OTG_FSM
+#ifdef CONFIG_USB_OTG
struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 6366fe3..2d451bb 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -64,7 +64,7 @@
#define TB_AIDL_BDIS (20) /* 4ms ~ 150ms, section 5.2.1 */
-#if IS_ENABLED(CONFIG_USB_OTG_FSM)
+#if IS_ENABLED(CONFIG_USB_OTG)
int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
int ci_otg_fsm_work(struct ci_hdrc *ci);
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..f8f2c88 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
usb-common-$(CONFIG_USB_LED_TRIG) += led.o
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+usbotg-y := usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG) += usbotg.o
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index dd28010..ae228d0 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -75,14 +75,6 @@ config USB_OTG_BLACKLIST_HUB
and software costs by not supporting external hubs. So
are "Embedded Hosts" that don't offer OTG support.
-config USB_OTG_FSM
- tristate "USB 2.0 OTG FSM implementation"
- depends on USB && USB_OTG
- select USB_PHY
- help
- Implements OTG Finite State Machine as specified in On-The-Go
- and Embedded Host Supplement to the USB Revision 2.0 Specification.
-
config USB_ULPI_BUS
tristate "USB ULPI PHY interface support"
depends on USB_SUPPORT
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index c690474..06794e2 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -20,7 +20,7 @@ config AB8500_USB
config FSL_USB2_OTG
bool "Freescale USB OTG Transceiver Driver"
- depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
+ depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG && PM
select USB_PHY
help
Enable this to support Freescale USB OTG transceiver.
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-13 12:10 +0200 |
| Subject | [PATCH v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops |
| Message-ID | <ryicH-1fj-43@gated-at.bofh.it> |
| In reply to | #1400666 |
This is to prevent missing symbol build error if OTG is
enabled (built-in) and HCD core (CONFIG_USB) is module.
Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Peter Chen <peter.chen@nxp.com>
---
drivers/usb/chipidea/otg_fsm.c | 7 +++++++
drivers/usb/common/usb-otg-fsm.c | 15 +++++++++++----
drivers/usb/phy/phy-fsl-usb.c | 7 +++++++
include/linux/usb/otg.h | 2 ++
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 1c0c750..2d8d659 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -582,6 +582,12 @@ static struct otg_fsm_ops ci_otg_ops = {
.start_gadget = ci_otg_start_gadget,
};
+static struct otg_hcd_ops ci_hcd_ops = {
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
int ci_otg_fsm_work(struct ci_hdrc *ci)
{
/*
@@ -804,6 +810,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
ci->otg.fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
ci->otg.state = OTG_STATE_UNDEFINED;
ci->otg.fsm.ops = &ci_otg_ops;
+ ci->otg.hcd_ops = &ci_hcd_ops;
ci->gadget.hnp_polling_support = 1;
ci->otg.fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
if (!ci->otg.fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 4bfc6a5..2986b66 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -141,11 +141,16 @@ static void otg_hnp_polling_work(struct work_struct *work)
enum usb_otg_state state = otg->state;
u8 flag;
int retval;
+ struct otg_hcd_ops *hcd_ops = otg->hcd_ops;
if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
return;
- udev = usb_hub_find_child(otg->host->root_hub, 1);
+ if (!hcd_ops || !hcd_ops->usb_control_msg ||
+ !hcd_ops->usb_hub_find_child)
+ return;
+
+ udev = hcd_ops->usb_hub_find_child(otg->host->root_hub, 1);
if (!udev) {
dev_err(otg->host->controller,
"no usb dev connected, can't start HNP polling\n");
@@ -154,7 +159,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
*fsm->host_req_flag = 0;
/* Get host request flag from connected USB device */
- retval = usb_control_msg(udev,
+ retval = hcd_ops->usb_control_msg(udev,
usb_rcvctrlpipe(udev, 0),
USB_REQ_GET_STATUS,
USB_DIR_IN | USB_RECIP_DEVICE,
@@ -183,7 +188,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
if (state == OTG_STATE_A_HOST) {
/* Set b_hnp_enable */
if (!otg->host->b_hnp_enable) {
- retval = usb_control_msg(udev,
+ retval = hcd_ops->usb_control_msg(udev,
usb_sndctrlpipe(udev, 0),
USB_REQ_SET_FEATURE, 0,
USB_DEVICE_B_HNP_ENABLE,
@@ -262,7 +267,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
otg_loc_conn(otg, 0);
otg_loc_sof(otg, 1);
otg_set_protocol(fsm, PROTO_HOST);
- usb_bus_start_enum(otg->host, otg->host->otg_port);
+ if (otg->hcd_ops && otg->hcd_ops->usb_bus_start_enum)
+ otg->hcd_ops->usb_bus_start_enum(otg->host,
+ otg->host->otg_port);
otg_start_hnp_polling(fsm);
break;
case OTG_STATE_A_IDLE:
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 587a187..9dbd9f0 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -792,6 +792,12 @@ static struct otg_fsm_ops fsl_otg_ops = {
.start_gadget = fsl_otg_start_gadget,
};
+static struct otg_hcd_ops fsl_hcd_ops = {
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
/* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
static int fsl_otg_conf(struct platform_device *pdev)
{
@@ -820,6 +826,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
/* Set OTG state machine operations */
fsl_otg_tc->otg.fsm.ops = &fsl_otg_ops;
+ fsl_otg_tc->otg.hcd_ops = &fsl_hcd_ops;
/* initialize the otg structure */
fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index e8a14dc..85b8fb5 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -12,6 +12,7 @@
#include <linux/phy/phy.h>
#include <linux/usb/phy.h>
#include <linux/usb/otg-fsm.h>
+#include <linux/usb/hcd.h>
struct usb_otg {
u8 default_a;
@@ -24,6 +25,7 @@ struct usb_otg {
enum usb_otg_state state;
struct otg_fsm fsm;
+ struct otg_hcd_ops *hcd_ops;
/* bind/unbind the host controller */
int (*set_host)(struct usb_otg *otg, struct usb_bus *host);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-30 11:40 +0200 |
| Message-ID | <rErQ2-2lt-5@gated-at.bofh.it> |
| In reply to | #1400666 |
On Fri, May 13, 2016 at 01:03:14PM +0300, Roger Quadros wrote:
> Hi,
>
> This series centralizes OTG/Dual-role functionality in the kernel.
> As of now I've got Dual-role functionality working pretty reliably on
> dra7-evm and am437x-gp-evm.
> NOTE: my am437x-gp-evm broke so I couldn't test v8 on it.
> But the changes since v7 are trivial and shouldn't impact am437x-gp-evm.
>
> DWC3 controller and platform related patches will be sent separately.
>
> Series is based on v4.6-rc1 and depends on first 2 patches of [1]
> [1] - OTG fsm cleanup - https://lkml.org/lkml/2016/3/30/186
>
> Why?:
> ----
>
> Currently there is no central location where OTG/dual-role functionality is
> implemented in the Linux USB stack and every USB controller driver is
> doing their own thing for OTG/dual-role. We can benefit from code-reuse
> and simplicity by adding the OTG/dual-role core driver.
>
> Newer OTG cores support standard host interface (e.g. xHCI) so
> host and gadget functionality are no longer closely knit like older
> cores. There needs to be a way to co-ordinate the operation of the
> host and gadget controllers in dual-role mode. i.e. to stop and start them
> from a central location. This central location should be the
> USB OTG/dual-role core.
>
> Host and gadget controllers might be sharing resources and can't
> be always running. One has to be stopped for the other to run.
> This couldn't be done till now but can be done from the OTG core.
>
> What?:
> -----
>
> The OTG/dual-role core consists of a set of APIs that allow
> registration of OTG controller device and OTG capable host and gadget
> controllers.
>
> - The OTG controller driver can provide the OTG capabilities and the
> Finite State Machine work function via 'struct usb_otg_config'
> at the time of registration i.e. usb_otg_register();
>
> struct usb_otg *usb_otg_register(struct device *dev,
> struct usb_otg_config *config);
> int usb_otg_unregister(struct device *dev);
> /**
> * struct usb_otg_config - otg controller configuration
> * @caps: otg capabilities of the controller
> * @ops: otg fsm operations
> * @otg_work: optional custom otg state machine work function
> */
> struct usb_otg_config {
> struct usb_otg_caps *otg_caps;
> struct otg_fsm_ops *fsm_ops;
> void (*otg_work)(struct work_struct *work);
> };
>
> The dual-role state machine is built-into the OTG core so nothing
> special needs to be provided if only dual-role functionality is desired.
> The low level OTG controller driver ops are povided via
> 'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.
>
> After registration, the OTG core waits for host, gadget controller
> and the gadget function driver to be registered. Once all resources are
> available it instantiates the Finite State Machine (FSM).
> The host/gadget controllers are started/stopped according to the FSM.
>
> - Host and gadget controllers that are a part of OTG/dual-role port must
> use the OTG core provided APIs to add/remove the host/gadget.
> i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
> gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
> This ensures that the host and gadget controllers are not started till
> the state machine is ready and the right bus conditions are met.
> It also allows the host and gadget controllers to provide the OTG
> controller device to link them together. For Device tree boots
> the related OTG controller is automatically picked up via the
> 'otg-controller' property in the Host/Gadget controller nodes.
>
> int usb_otg_add_hcd(struct usb_hcd *hcd,
> unsigned int irqnum, unsigned long irqflags,
> struct device *otg_dev);
> void usb_otg_remove_hcd(struct usb_hcd *hcd);
>
> int usb_otg_add_gadget_udc(struct device *parent,
> struct usb_gadget *gadget,
> struct device *otg_dev);
> usb_del_gadget_udc() must be used for removal.
>
>
> - During the lifetime of the FSM, the OTG controller driver can provide
> inputs event changes using usb_otg_sync_inputs(). The OTG core will
> then schedule the FSM work function (or internal dual-role state machine)
> to update the FSM state. The FSM then calls the OTG controller
> operations (fsm_ops) as necessary.
> void usb_otg_sync_inputs(struct usb_otg *otg);
>
> - The following 2 functions are provided as helpers for use by the
> OTG controller driver to start/stop the host/gadget controllers.
> int usb_otg_start_host(struct usb_otg *otg, int on);
> int usb_otg_start_gadget(struct usb_otg *otg, int on);
>
> - The following function is provided for use by the USB host stack
> to sync OTG related events to the OTG state machine.
> e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
> int usb_otg_kick_fsm(struct device *otg_device);
>
> Changelog:
> ---------
> v8:
> - split out start/stop gadget and connect/disconnect operations.
> - make CONFIG_OTG dpend on CONFIG_USB_GADGET as well apart from CONFIG_USB
> - use create_freezable_workqueue() for OTG work as per Peter's suggestion.
> - remove usb-otg.h as we're not initializing any OTG timers.
> - don't include unnecessary headers in usb-otg.c (i.e. hrtimer.h & ktime.h)
>
Since you have agreed to move CONFIG_USB_OTG out of HCD, I suggest you
can have a folder to put current OTG and OTG_FSM stuffs under the root
of usb folder, it can benefit the user who wants to add their dual-role
switch driver or OTG driver. What do you think?
Peter
> v7:
> - added dual-role support for host controllers requiring a companion
> controller. e.g. EHCI + OHCI.
> - added of_usb_get_otg() to get the OTG controller device
> from the USB controller's device node.
> - addressed review comments.
>
> v6:
> - added otg specific APIs for host/gadget registration. behaviour of
> original host/gadget API remains unchanged. Platform devices can now
> pass the otg device explicitly while registering host/gadget.
> - moved hcd specific operations from struct otg_fsm to struct hcd_ops.
> - made struct usb_otg mandatory for all otg related APIs.
> - allow otg controller to provide it's own otg_work function so that
> it can implement it's own state machine.
> - removed otg fsm and timers from usb-otg.c. Only dual-role state machine
> is implemented.
> - vbus is controlled in the dual-role state machine.
> - PM runtime is used around drd_statemachine().
> - added otg_dev to xhci platform data to allow platform code to specify
> the otg controller tied to the xhci host controller.
>
> v5: Internal version. Not sent to mailing list
>
> v4:
> - Added DT support for tying otg-controller to host and gadget
> controllers. For DT we no longer have the constraint that
> OTG controller needs to be parent of host and gadget. They can be
> tied together using the "otg-controller" property.
> - Relax the requirement for DT case that otg controller must register
> before host/gadget. We maintain a wait list of host/gadget devices
> waiting on the otg controller.
> - Use a single struct usb_otg for otg data.
> - Don't override host/gadget start/stop APIs. Let the controller
> drivers do what they want as they know best. Helper API is provided
> for controller start/stop that controller driver can use.
> - Introduce struct usb_otg_config to pass the otg capabilities,
> otg ops and otg timer timeouts during otg controller registration.
> - rebased on Greg's usb.git/usb-next
>
> v3:
> - all otg related definations now in otg.h
> - single kernel config USB_OTG to enable OTG core and FSM.
> - resolved symbol dependency issues.
> - use dev_vdbg instead of VDBG() in usb-otg-fsm.c
> - rebased on v4.2-rc1
>
> v2:
> - Use add/remove_hcd() instead of start/stop_hcd() to enable/disable
> the host controller
> - added dual-role-device (DRD) state machine which is a much simpler
> mode of operation when compared to OTG. Here we don't support fancy
> OTG features like HNP, SRP, on the fly role-swap. The mode of operation
> is determined based on ID pin (cable type) and the role doesn't change
> till the cable type changes.
>
> --
> cheers,
> -roger
>
> Roger Quadros (13):
> usb: hcd: Initialize hcd->flags to 0
> usb: otg-fsm: Prevent build warning "VDBG" redefined
> usb: hcd.h: Add OTG to HCD interface
> usb: otg-fsm: use usb_otg wherever possible
> usb: otg-fsm: move host controller operations into usb_otg->hcd_ops
> usb: gadget.h: Add OTG to gadget interface
> usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG
> usb: otg: add OTG/dual-role core
> usb: of: add an API to get OTG device from USB controller node
> usb: otg: use dev_dbg() instead of VDBG()
> usb: hcd: Adapt to OTG core
> usb: gadget: udc: adapt to OTG core
> usb: host: xhci-plat: Add otg device to platform data
>
> Yoshihiro Shimoda (1):
> usb: otg: add hcd companion support
>
> Documentation/devicetree/bindings/usb/generic.txt | 6 +
> Documentation/usb/chipidea.txt | 2 +-
> drivers/usb/chipidea/Makefile | 2 +-
> drivers/usb/chipidea/ci.h | 3 +-
> drivers/usb/chipidea/core.c | 14 +-
> drivers/usb/chipidea/debug.c | 2 +-
> drivers/usb/chipidea/otg_fsm.c | 176 ++--
> drivers/usb/chipidea/otg_fsm.h | 2 +-
> drivers/usb/chipidea/udc.c | 17 +-
> drivers/usb/common/Makefile | 3 +-
> drivers/usb/common/common.c | 27 +
> drivers/usb/common/usb-otg-fsm.c | 203 ++--
> drivers/usb/common/usb-otg.c | 1056 +++++++++++++++++++++
> drivers/usb/core/Kconfig | 12 +-
> drivers/usb/core/hcd.c | 56 ++
> drivers/usb/gadget/udc/udc-core.c | 194 +++-
> drivers/usb/host/xhci-plat.c | 35 +-
> drivers/usb/phy/Kconfig | 2 +-
> drivers/usb/phy/phy-fsl-usb.c | 155 +--
> drivers/usb/phy/phy-fsl-usb.h | 3 +-
> include/linux/usb/gadget.h | 22 +
> include/linux/usb/hcd.h | 29 +
> include/linux/usb/of.h | 9 +
> include/linux/usb/otg-fsm.h | 154 +--
> include/linux/usb/otg.h | 264 +++++-
> include/linux/usb/xhci_pdriver.h | 3 +
> 26 files changed, 2013 insertions(+), 438 deletions(-)
> create mode 100644 drivers/usb/common/usb-otg.c
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-30 16:10 +0200 |
| Message-ID | <rEw3g-5dn-11@gated-at.bofh.it> |
| In reply to | #1408941 |
On 30/05/16 12:29, Peter Chen wrote:
> On Fri, May 13, 2016 at 01:03:14PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> This series centralizes OTG/Dual-role functionality in the kernel.
>> As of now I've got Dual-role functionality working pretty reliably on
>> dra7-evm and am437x-gp-evm.
>> NOTE: my am437x-gp-evm broke so I couldn't test v8 on it.
>> But the changes since v7 are trivial and shouldn't impact am437x-gp-evm.
>>
>> DWC3 controller and platform related patches will be sent separately.
>>
>> Series is based on v4.6-rc1 and depends on first 2 patches of [1]
>> [1] - OTG fsm cleanup - https://lkml.org/lkml/2016/3/30/186
>>
>> Why?:
>> ----
>>
>> Currently there is no central location where OTG/dual-role functionality is
>> implemented in the Linux USB stack and every USB controller driver is
>> doing their own thing for OTG/dual-role. We can benefit from code-reuse
>> and simplicity by adding the OTG/dual-role core driver.
>>
>> Newer OTG cores support standard host interface (e.g. xHCI) so
>> host and gadget functionality are no longer closely knit like older
>> cores. There needs to be a way to co-ordinate the operation of the
>> host and gadget controllers in dual-role mode. i.e. to stop and start them
>> from a central location. This central location should be the
>> USB OTG/dual-role core.
>>
>> Host and gadget controllers might be sharing resources and can't
>> be always running. One has to be stopped for the other to run.
>> This couldn't be done till now but can be done from the OTG core.
>>
>> What?:
>> -----
>>
>> The OTG/dual-role core consists of a set of APIs that allow
>> registration of OTG controller device and OTG capable host and gadget
>> controllers.
>>
>> - The OTG controller driver can provide the OTG capabilities and the
>> Finite State Machine work function via 'struct usb_otg_config'
>> at the time of registration i.e. usb_otg_register();
>>
>> struct usb_otg *usb_otg_register(struct device *dev,
>> struct usb_otg_config *config);
>> int usb_otg_unregister(struct device *dev);
>> /**
>> * struct usb_otg_config - otg controller configuration
>> * @caps: otg capabilities of the controller
>> * @ops: otg fsm operations
>> * @otg_work: optional custom otg state machine work function
>> */
>> struct usb_otg_config {
>> struct usb_otg_caps *otg_caps;
>> struct otg_fsm_ops *fsm_ops;
>> void (*otg_work)(struct work_struct *work);
>> };
>>
>> The dual-role state machine is built-into the OTG core so nothing
>> special needs to be provided if only dual-role functionality is desired.
>> The low level OTG controller driver ops are povided via
>> 'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.
>>
>> After registration, the OTG core waits for host, gadget controller
>> and the gadget function driver to be registered. Once all resources are
>> available it instantiates the Finite State Machine (FSM).
>> The host/gadget controllers are started/stopped according to the FSM.
>>
>> - Host and gadget controllers that are a part of OTG/dual-role port must
>> use the OTG core provided APIs to add/remove the host/gadget.
>> i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
>> gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
>> This ensures that the host and gadget controllers are not started till
>> the state machine is ready and the right bus conditions are met.
>> It also allows the host and gadget controllers to provide the OTG
>> controller device to link them together. For Device tree boots
>> the related OTG controller is automatically picked up via the
>> 'otg-controller' property in the Host/Gadget controller nodes.
>>
>> int usb_otg_add_hcd(struct usb_hcd *hcd,
>> unsigned int irqnum, unsigned long irqflags,
>> struct device *otg_dev);
>> void usb_otg_remove_hcd(struct usb_hcd *hcd);
>>
>> int usb_otg_add_gadget_udc(struct device *parent,
>> struct usb_gadget *gadget,
>> struct device *otg_dev);
>> usb_del_gadget_udc() must be used for removal.
>>
>>
>> - During the lifetime of the FSM, the OTG controller driver can provide
>> inputs event changes using usb_otg_sync_inputs(). The OTG core will
>> then schedule the FSM work function (or internal dual-role state machine)
>> to update the FSM state. The FSM then calls the OTG controller
>> operations (fsm_ops) as necessary.
>> void usb_otg_sync_inputs(struct usb_otg *otg);
>>
>> - The following 2 functions are provided as helpers for use by the
>> OTG controller driver to start/stop the host/gadget controllers.
>> int usb_otg_start_host(struct usb_otg *otg, int on);
>> int usb_otg_start_gadget(struct usb_otg *otg, int on);
>>
>> - The following function is provided for use by the USB host stack
>> to sync OTG related events to the OTG state machine.
>> e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
>> int usb_otg_kick_fsm(struct device *otg_device);
>>
>> Changelog:
>> ---------
>> v8:
>> - split out start/stop gadget and connect/disconnect operations.
>> - make CONFIG_OTG dpend on CONFIG_USB_GADGET as well apart from CONFIG_USB
>> - use create_freezable_workqueue() for OTG work as per Peter's suggestion.
>> - remove usb-otg.h as we're not initializing any OTG timers.
>> - don't include unnecessary headers in usb-otg.c (i.e. hrtimer.h & ktime.h)
>>
>
> Since you have agreed to move CONFIG_USB_OTG out of HCD, I suggest you
> can have a folder to put current OTG and OTG_FSM stuffs under the root
> of usb folder, it can benefit the user who wants to add their dual-role
> switch driver or OTG driver. What do you think?
Yes, I will give it a try and post if any problems.
--
cheers,
-roger
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web