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 diffe= rent
>>> cable type is inserted.
>>>
>>> As we don't need most of the complex OTG states and OTG ti= mers
>>> we implement a lean DRD state machine in usb-otg.c.
>>> The DRD state machine is only interested in 2 hardware inputs<= br> >>> 'id' and 'b_sess_vld'.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>=C2=A0 drivers/usb/common/Makefile=C2=A0 |=C2=A0 =C2=A0 2 +- >>>=C2=A0 drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++= ++++++++++++++++++++++
>>>=C2=A0 drivers/usb/core/Kconfig=C2=A0 =C2=A0 =C2=A0|=C2=A0 =C2= =A0 4 +-
>>>=C2=A0 include/linux/usb/gadget.h=C2=A0 =C2=A0|=C2=A0 =C2=A0 2 = +
>>>=C2=A0 include/linux/usb/hcd.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2= =A0 1 +
>>>=C2=A0 include/linux/usb/otg-fsm.h=C2=A0 |=C2=A0 =C2=A0 7 +
>>>=C2=A0 include/linux/usb/otg.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 154 = ++++++-
>>>=C2=A0 7 files changed, 1206 insertions(+), 6 deletions(-)
>>>=C2=A0 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,
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct usb_otg_config *config)
>>> +{
>>> +=C2=A0 =C2=A0struct usb_otg *otg;
>>> +=C2=A0 =C2=A0struct otg_wait_data *wait;
>>> +=C2=A0 =C2=A0int ret =3D 0;
>>> +
>>> +=C2=A0 =C2=A0if (!dev || !config || !config->fsm_ops)
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ERR_PTR(-EINV= AL);
>>> +
>>> +=C2=A0 =C2=A0/* already in list? */
>>> +=C2=A0 =C2=A0mutex_lock(&otg_list_mutex);
>>> +=C2=A0 =C2=A0if (usb_otg_get_data(dev)) {
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(dev, "o= tg: %s: device already in otg list\n",
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0__func__);
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D -EINVAL;
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto unlock;
>>> +=C2=A0 =C2=A0}
>>> +
>>> +=C2=A0 =C2=A0/* allocate and add to list */
>>> +=C2=A0 =C2=A0otg =3D kzalloc(sizeof(*otg), GFP_KERNEL);
>>> +=C2=A0 =C2=A0if (!otg) {
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D -ENOMEM;
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto unlock;
>>> +=C2=A0 =C2=A0}
>>> +
>>> +=C2=A0 =C2=A0otg->dev =3D dev;
>>> +=C2=A0 =C2=A0otg->caps =3D config->otg_caps;
>>
>> Here, we should be checking if user needs to disable any OTG featu= res. So,
>>
>>=C2=A0 =C2=A0 =C2=A0 if (dev->of_node)
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 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 l= ocal copy instead
>> of the one passed by the OTG controller.
>
> Why can't modify the one from OTG controller directly?
>

=C2=A0
udc_bind_to_driver gets called only when both, udc driver and functi= on driver
are available. Defer probing will have to be done in usb_add_gadget_udc_rel= ease().

Yes, you are r= ight.

=C2=A0

>
> 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<= br> yet probed?

No, =C2=A0you only need= to return =C2=A0-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