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

[Peter Chen]

On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> 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.
> 

How about this:
Moving usb_otg_register/unregister_hcd to host driver to remove
dependency hcd->otg. And moving usb_otg_get_data to common.c.

Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
returns NULL, the host/device driver's probe return -EPROBE_DEFER.
When the otg driver is probed successfully, the host/device will be
re-probed again, and usb_otg_register_hcd will be called again.

And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
otg_gadget_ops. Below build dependency issues can be fixed.
What do you think?

> 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.
-- 

Best Regards,
Peter Chen