Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

[Peter Chen <hzpeterchen@gmail.com>]

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

On Wed, May 25, 2016 at 8:21 PM, Roger Quadros <rogerq@ti.com> wrote:

> 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?
> >
>
>
>
> 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().
>
>
Yes, you are right.


> >
> > 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?
>
>
No,  you only need to return  -EPROBE_DEFER if otg is not register when hcd
or
udc tries to register itself to otg framework. Each host or udc driver's
probe will
get this error, and re-probe after the driver which register otg has probed
successfully.

-- 
BR,
Peter Chen

[-- Attachment #2: Type: text/html, Size: 6553 bytes --]