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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jun 2016 10:19:32 +0300
From:	Felipe Balbi <balbi@...nel.org>
To:	Peter Chen <hzpeterchen@...il.com>
Cc:	Roger Quadros <rogerq@...com>, peter.chen@...escale.com,
	yoshihiro.shimoda.uh@...esas.com, tony@...mide.com,
	gregkh@...uxfoundation.org, dan.j.williams@...el.com,
	mathias.nyman@...ux.intel.com, Joao.Pinto@...opsys.com,
	sergei.shtylyov@...entembedded.com, jun.li@...escale.com,
	grygorii.strashko@...com, robh@...nel.org, nsekhar@...com,
	b-liu@...com, joe@...ches.com, linux-usb@...r.kernel.org,
	linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v11 08/14] usb: otg: add OTG/dual-role core


Hi,

Peter Chen <hzpeterchen@...il.com> 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.

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

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)