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

[Jun Li]

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

I think hcd and gadget are independent each other, now

Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)

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)

Li Jun

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