linux-kernel - Re: [PATCH v1 2/2] usb: tegra: Move UTMI-pads reset from ehci-tegra to tegra-phy

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] [day] [month] [year] [list]

Message-ID: <e6d9f648-4082-44cc-d03f-7bb02962da21@gmail.com>
Date:   Fri, 15 Dec 2017 03:33:05 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-usb@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] usb: tegra: Move UTMI-pads reset from ehci-tegra
 to tegra-phy

On 11.12.2017 16:31, Dmitry Osipenko wrote:
> On 11.12.2017 13:25, Thierry Reding wrote:
>> On Mon, Dec 11, 2017 at 02:07:38AM +0300, Dmitry Osipenko wrote:
>>> UTMI pads are shared by USB controllers and reset of UTMI pads is shared
>>> with the reset of USB1 controller. Currently reset of UTMI pads is done by
>>> the EHCI driver and ChipIdea UDC works because EHCI driver always happen
>>> to be probed first. Move reset controls from ehci-tegra to tegra-phy in
>>> order to resolve the problem.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
>>> ---
>>>  drivers/usb/host/ehci-tegra.c     | 87 ++++++++++++++++++---------------------
>>>  drivers/usb/phy/phy-tegra-usb.c   | 46 +++++++++++++++++++++
>>>  include/linux/usb/tegra_usb_phy.h |  2 +
>>>  3 files changed, 87 insertions(+), 48 deletions(-)
>>
>> I don't think we can do this. For one I don't think shared resets are
>> going to work here because you really won't ever be able to reset after
>> two devices have requested the same reset.
> 
> Ah, indeed. Originally I had the reset being done in the probe, but then changed
> it in the last minute without proper testing. Good catch! I'll revert back patch
> to the origin.
> 
>  Second, utmip_pad_close()
>> could be called at any point and it will have the side-effect of either
>> not doing a reset at all (because it is shared) or resetting the USBD
>> controller at the same time.
> 
> utmip_pad_close() is only called on tegra-phy driver removal, so it is
> absolutely fine.
> 
>> We've been over this code a great deal over the years. I'd love it to be
>> simpler, but every time we tried to simplify it, things broke.
> 
> Well, the current code is already broken quite severely because now we have two
> users of the tegra-phy: ehci-tegra and chipidea-tegra. Things brake if host
> driver is loaded after the UDC because host would reset the UDC. And also pads
> won't be reset if ehci-tegra isn't loaded at all.
> 
> Shared reset seems to be a perfect solution for us and of course it requires
> extra carefulness.

BTW, I think that the current code never did anything useful, because it resets
utmi-pads without enabling pads clk. So if non-USB1 controller gets probed
first, then it will enable non-USB1 clk and reset the pads, while it probably
should reset pads with USB1 clk being enabled.

Although, I'm not familiar with the actual HW and it could be that pads clk
isn't needed for the proper reset.