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

[Dmitry Osipenko <digetx@gmail.com>]

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@gmail.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.