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

[Felipe Balbi]

Hi,

Peter Chen <hzpeterchen@xxxxxxxxx> writes:
>> Peter Chen <hzpeterchen@xxxxxxxxx> writes:
>> >> >>> +
>> >> >>> +		/* start host */
>> >> >>> +		ret = hcd_ops->add(otg->primary_hcd.hcd,
>> >> >>> +				   otg->primary_hcd.irqnum,
>> >> >>> +				   otg->primary_hcd.irqflags);
>> >> >> 
>> >> >> this is usb_add_hcd(), is it not? Why add an indirection?
>> >> >
>> >> > I've introduced the host and gadget ops interface to get around the
>> >> > circular dependency issue we can't avoid.
>> >> > otg needs to call host/gadget functions and host/gadget also needs to
>> >> > call otg functions.
>> >> 
>> >> IMO, this shows a fragility of your design. You're, now, lying to
>> >> usb_hcd and usb_udc and making them register into a virtual layer that
>> >> doesn't exist. And that layer will end up calling the real registration
>> >> function when some magic event happens.
>> >> 
>> >> This is only really needed for quirky devices like dwc3 (but see more on
>> >> dwc3 below) where host and peripheral registers shadow each
>> >> other. Otherwise we would be able to always keep hcd and udc always
>> >> registered. They would get different interrupt statuses anyway and
>> >> nothing would ever break.
>> >> 
>> >> However, when it comes to dwc3, we already have all the code necessary
>> >> to workaround this issue by destroying the XHCI pdev when OTG interrupt
>> >> says we should be peripheral (and vice-versa). DWC3 also keeps track of
>> >> the OTG states for those folks who really care about OTG (Hint: nobody
>> >> has cared for the past 10 years, why would they do so now?) and we don't
>> >> need a SW state machine when the HW handles that for us, right?
>> >> 
>> >> As for chipidea, IIRC, that doesn't need a SW state machine either, but
>> >> I know very little about that IP and don't even have documentation on
>> >> it. My understanding, however, is that chipidea behaves kinda like MUSB,
>> >> which changes roles automatically in HW based on ID pin state.
>> >
>> > Chipidea needs to set register for USB role manually.
>> 
>> okay, so chipidea has private control of role. Much like dwc3. That's good.
>> 
>> >> >>> + * @otg_dev: OTG controller device, if needs to be used with OTG core.
>> >> >> 
>> >> >> do you really know of any platform which has a separate OTG controller?
>> >> >> 
>> >> >
>> >> > Andrew had pointed out in [1] that Tegra210 has separate blocks for OTG, host
>> >> > and gadget.
>> >> >
>> >> > [1] http://article.gmane.org/gmane.linux.ports.tegra/22969
>> >> 
>> >> that's not an OTG controller, it's just a mux. No different than Intel's
>> >> mux for swapping between XHCI and peripheral-only DWC3.
>> >> 
>> >> frankly, I would NEVER talk about OTG when type-C comes into play. They
>> >> are two competing standards and, apparently, type-C is winning when it
>> >> comes to role-swapping.
>> >> 
>> >
>> > In fact, OTG is mis-used by people. Currently, if the port is dual-role,
>> > It will be considered as an OTG port.
>> 
>> That's because "dual-role" is a non-standard OTG. Seen as people really
>> didn't care about OTG, we (linux-usb folks) ended up naturally referring
>> to "non-standard OTG" as "dual-role". Just to avoid confusion.
>
> So, unless we use OTG FSM defined in OTG spec, we should not mention
> "OTG" in Linux, right?

to avoid confusion with the terminology, yes. With that settled, let's
figure out how you can deliver what your marketting guys are asking of
you.

>> > You are right, if the connector is type-c, it will be called as "type-c
>> > port" by people :)
>> 
>> oh no, that's not what I'm talking about. If you read Type-C and PD
>> specs, they define their own method for data role swapping. USB OTG
>> doesn't fit on top of a Type-C environment. It's not about what people
>> will call it, it's really that OTG can't work on top of type-c. For
>> starters, there's no ID pin ;-)
>
> I know type-c, yes, there is no relationship between OTG and type-c.

okay, thanks

-- 
balbi

Attachment: signature.asc
Description: PGP signature