Re: [PATCH v1 04/12] usb: phy: tegra: Support OTG mode programming

From: Dmitry Osipenko <digetx@gmail.com>
To: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: Thierry Reding <treding@nvidia.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Sebastian Reichel <sre@kernel.org>,
	Peter Chen <peter.chen@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@kernel.org>, David Heidelberg <david@ixit.cz>,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH v1 04/12] usb: phy: tegra: Support OTG mode programming
Date: Sat, 10 Jul 2021 00:29:11 +0300	[thread overview]
Message-ID: <2fbfc146-a563-fc58-19d2-fc1f43381fa6@gmail.com> (raw)
In-Reply-To: <YOd9GE7xM150i1XT@qmqm.qmqm.pl>

09.07.2021 01:32, Michał Mirosław пишет:
> On Thu, Jul 01, 2021 at 04:55:03PM +0300, Dmitry Osipenko wrote:
>> 01.07.2021 05:23, Dmitry Osipenko пишет:
>>>  static int tegra_usb_phy_init(struct usb_phy *u_phy)
>>> @@ -967,12 +1057,26 @@ static int tegra_usb_phy_init(struct usb_phy *u_phy)
>>>  			goto disable_vbus;
>>>  	}
>>>  
>>> +	err = tegra_usb_phy_configure_pmc(phy);
>>> +	if (err)
>>> +		goto close_phy;
>>> +
>>>  	err = tegra_usb_phy_power_on(phy);
>>>  	if (err)
>>>  		goto close_phy;
>>>  
>>> +	if (phy->irq > 0) {
>>> +		err = request_irq(phy->irq, tegra_usb_phy_isr, IRQF_SHARED,
>>> +				  dev_name(phy->u_phy.dev), phy);
>>> +		if (err)
>>> +			goto pwr_off_phy;
>>> +	}
>>
>> There were reports that this patch was casing an unhandled USB interrupt
>> event on some devices. I thought this problem was fixed already, but
>> looking again at the offending kernel log again, it still should be a
>> problem.
>>
>> The interrupt fires from the usb_add_hcd() of the CI driver before CI
>> driver have requested interrupt in ci_hdrc_probe(). So either CI driver
>> should request interrupt earlier or Tegra PHY driver should keep shared
>> interrupt disabled after requesting it, the latter variant should be
>> more robust. I'll improve it in v2.
> 
> I'd suggest the first solution, as the latter is a workaround for what
> is a normal shared interrupt behaviour. Maybe a controller reset is
> needed in CI driver before going on with PHY init?

I already implemented the second solution. The controller reset should
be okay. We could improve it all later on if will ever be needed, so far
it's unnecessary. I can't really work on improving the CI interrupt
because it requires to have a special testing setup to reproduce the
problem and I don't have that setup.