Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1403476

Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core

From Roger Quadros <rogerq@ti.com>
Newsgroups linux.kernel
Subject Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
Date 2016-05-19 09:40 +0200
Message-ID <rAqIN-5bD-1@gated-at.bofh.it> (permalink)
References (8 earlier) <rzIRr-2gu-3@gated-at.bofh.it> <rA95f-2dr-17@gated-at.bofh.it> <rAa1k-2O5-35@gated-at.bofh.it> <rAa1k-2O5-33@gated-at.bofh.it> <rAaXn-3oK-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 18/05/16 17:46, Jun Li wrote:
> 
> 
>>>>
>>>> I didn't want to have complex Kconfig so decided to have otg as
>>>> built-in only.
>>>> What do you want me to change in existing code? and why?
>>>
>>> Remove those stuff which only for pass diff driver config Like every
>>> controller driver need a duplicated
>>>
>>> static struct otg_hcd_ops ci_hcd_ops = {
>>>     ...
>>> }
>>
>> This is an exception only. Every controller driver doesn't need to
>> implement hcd_ops. It is implemented in the hcd core.
>>
>>>
>>> And here is another example, for gadget connect, otg driver can
>>> directly call to usb_udc_vbus_handler() in drd state machine, but you
>>> create another interface:
>>>
>>> .connect_control = usb_gadget_connect_control,
>>>
>>> If the symbol is defined in one driver which is 'm', another driver
>>> reference it should be 'm' as well, then there is no this kind of
>>> problem as my understanding.
>>
>> That is fine as long as all are 'm'. but how do you solve the case when
>> Gadget is built in and host is 'm'? OTG has to be built-in and you will
>> need an hcd to gadget interface.
> 
> Hcd to gadget interface? Or you want to say otg to host interface?

Sorry, I meant to say host to otg interface.

> 
> I think hcd and gadget are independent each other, now
> 
> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)

It is actually a circular dependency for both.
 hcd <--> otg and gadget <--> otg

hcd -> otg for usb_otg_register/unregister_hcd
otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, usb_hub_find_child

gadget -> otg for usb_otg_register/unregister_gadget
otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler

Now consider what will happen if I get rid of the otg_hcd and otg_gadget interfaces.
'y' means built-in, 'm' means module.

1) hcd 'y', gadget 'y'
otg has to be 'y' for proper build.

2) hcd 'm', gadget 'm'
otg has to be 'm' for proper build.

3) hcd 'y', gadget 'm'
Build will fail always.
If otg is 'y', otg build will fail due to dependency on gadget.
If otg is 'm', hcd build will fail due to dependency on otg.

4) hcd 'm', gadget 'y'
Build will fail always.
If otg is 'y', otg build will fail due to dependency on hcd.
If otg is 'm', gadget build will fails due to dependency on otg.

So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
to remove otg->hcd and otg->gadget dependency.

Now we can address 3) and 4) like so

3) hcd 'y', gadget 'm'
otg has to be 'y' for proper build.

4) hcd 'm', gadget 'y'
otg has to be 'y' for proper build.

> 
> If you directly call to usb_udc_vbus_handler() in drd state machine
> 
> Then:
> 
> Hcd --> otg; and gadget <--> otg, (gadget and otg will refer to symbol of each other)

It's not so easy. We are again creating a circular dependency and all
build configurations don't work like I pointed above.

The only optimization I could do is make CONFIG_OTG tristate and
allow it to be built as 'm' if both hcd and gadget are 'm'.
i.e. case (2).

cheers,
-roger

>>
>> Do you have any ideas to solve that case?
>>
>> cheers,
>> -roger
>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>  	return 0;
>>>>>>>>>>>> @@ -660,9 +830,15 @@ static ssize_t
>>>>>>>>>>>> usb_udc_softconn_store(struct
>>>>>>>> device *dev,
>>>>>>>>>>>>  		return -EOPNOTSUPP;
>>>>>>>>>>>>  	}
>>>>>>>>>>>>
>>>>>>>>>>>> +	/* In OTG mode we don't support softconnect, but
>> b_bus_req */
>>>>>>>>>>>> +	if (udc->gadget->otg_dev) {
>>>>>>>>>>>> +		dev_err(dev, "soft-connect not supported in OTG
>> mode\n");
>>>>>>>>>>>> +		return -EOPNOTSUPP;
>>>>>>>>>>>> +	}
>>>>>>>>>>>> +
>>>>>>>>>>>
>>>>>>>>>>> The soft-connect can be supported at dual-role mode currently,
>>>>>>>>>>> we can use b_bus_req entry once it is implemented later.
>>>>>>>>>>
>>>>>>>>>> Soft-connect should be done via sysfs handling within the OTG
>> core.
>>>>>>>>>> This can be added later. I don't want anything outside the OTG
>>>>>>>>>> core to handle soft-connect behaviour as it will be hard to
>>>>>>>>>> keep things in sync.
>>>>>>>>>>
>>>>>>>>>> I can update the comment to something like this.
>>>>>>>>>>
>>>>>>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
>>>>>>>>>> core */
>>>>>>>>>
>>>>>>>>> Ok, let's Felipe decide it.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>  	if (sysfs_streq(buf, "connect")) {
>>>>>>>>>>>>  		usb_gadget_udc_start(udc);
>>>>>>>>>>>> -		usb_gadget_connect(udc->gadget);
>>>>>>>>>>>> +		usb_udc_connect_control(udc);
>>>>>>>>>>>
>>>>>>>>>>> This line seems to be not related with this patch.
>>>>>>>>>>>
>>>>>>>>>> Right. I'll remove it.
>>>>>>>>>>
>>>>>>>>>> cheers,
>>>>>>>>>> -roger
>>>>>>>>>

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[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

csiph-web