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]

Message-ID: <87shw5lnrs.fsf@linux.intel.com>
Date:	Wed, 22 Jun 2016 09:51:19 +0300
From:	Felipe Balbi <balbi@...nel.org>
To:	Peter Chen <hzpeterchen@...il.com>,
	yoshihiro.shimoda.uh@...esas.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:
>> >> >> > 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.
>> >> >> 
>> >> >
>> >> > Since nxp SoC claims they are OTG compliance, we need to pass usb.org
>> >> > test. The internal bsp has passed PET test, and formal compliance test
>> >> > is on the way (should pass too). 
>> >> >
>> >> > The dual-role and OTG compliance use the same zImage, but different
>> >> > dtb.
>> >> 
>> >> okay, that's good to know. Now, the question really is: considering we
>> >> only have one user for this generic OTG FSM layer, do we really need to
>> >> make it generic at all? I mean, just look at how invasive a change that
>> >> is.
>> >
>> > If the chipidea is the only user for this roger's framework, I don't
>> > think it is necessary. In fact, Roger introduces this framework, and
>> > the first user is dwc3, we think it can be used for others. Let's
>> 
>> Right, we need to look at the history of dwc3 to figure out why the
>> conclusion that dwc3 needs this was made.
>> 
>> Roger started working on this framework when Power on Reset section of
>> databook had some details which weren't always clear and, for safety, we
>> always had reset asserted for a really long time. It was so long (about
>> 400 ms) that resetting dwc3 for each role swap was just too much.
>> 
>> Coupled with that, the OTG chapter wasn't very clear either on
>> expections from Host and Peripheral side initialization in OTG/DRD
>> systems.
>> 
>> More recent version of dwc3 databook have a much better description of
>> how and which reset bits _must_ be asserted and which shouldn't be
>> touched unless it's for debugging purposes. When I implemented that, our
>> ->probe() went from 400ms down to about 50us.
>> 
>> Coupled with that, the OTG chapter also became a lot clearer to the
>> point that it states you just don't initialize anything other than the
>> OTG block, and just wait for OTG interrupt to do whatever it is you need
>> to do.
>> 
>> This meant that we could actually afford to do full reinitialization of
>> dwc3 on role swap (it's now only 50us anyway) and we knew how to swap
>> roles properly.
>> 
>> (The reason for needing soft-reset during role swap is kinda long. But
>> in summary dwc3 shadows register writes to both host and peripheral
>> sides)
>> 
>> > just discuss if it is necessary for dual-role switch.
>> 
>> fair. However, if we have a single user we don't have a Generic
>> layer. There's not enough variance to come up with truly generic
>> architecture for this.
>> 
>> -- 
>
> I have put some points in my last reply [1], I summery it here to
> see if a generic framework is deserved or not?
>
> 1. If there are some parts we can use during the role switch
> - The common start/stop host and peripheral operation
> eg, when switch from host to peripheral, all drivers can use
> usb_remove_hcd to finish it.

a UDC such as dwc3 already implements start/stop for peripheral and
host. Why would go through and indirection layer that just comes back to
us? (well, dwc3's host side, start/stop translates to adding/removing
xhci-plat's device)

> - A common workqueue to handle vbus and id event

I already have a threaded IRQ handler. Why do I need a workqueue?

> - sysfs for role switch

A generic sysfs is desirable, but I really don't know where to put it.
Maybe it's enough to go down the hwmon route and just have an agreement
of filename and contents to be written to.

-- 
balbi

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