Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 19 Apr 2018 09:43:53 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc:     Johan Hovold <johan@kernel.org>, Bin Liu <b-liu@ti.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Arnd Bergmann <arnd@arndb.de>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] USB: musb: dsps: phy fix and DT-topology support
Message-ID: <20180419074353.GK9198@localhost>
References: <20180413151505.32663-1-johan@kernel.org>
 <CAFBinCBYvi2RXEujcc-mKHq0iROeEThGaCe7ca_S-nWbkC3ttw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFBinCBYvi2RXEujcc-mKHq0iROeEThGaCe7ca_S-nWbkC3ttw@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 18, 2018 at 09:18:30PM +0200, Martin Blumenstingl wrote:
> Hi Johan,
> 
> On Fri, Apr 13, 2018 at 5:15 PM, Johan Hovold <johan@kernel.org> wrote:
> > I've been carrying a patch out-of-tree since my work on improving the
> > USB device-tree support which is needed to be able to describe USB
> > topologies for musb based controllers.
> >
> > This patch, which associates the platform controller device with the
> > glue device device-tree node, did not play well with the recent changes
> > which added generic phy support to USB core however.
> I'm the one who added this
> 
> > Like the recent dwc2 regression fixed by Arnd after the device-tree
> > #phy-cell changes, the generic phy code in USB core can now also fail
> > indefinitly with -EPROBE_DEFER when the controller uses a legacy USB
> > phy.
> >
> > The second patch addresses this for musb, which handles its own (legacy
> > and generic) phys, but something more may possibly now be needed for
> > other platforms with legacy phys.
> I'm not sure if I understand the problem yet - could you please
> explain with your words what "legacy PHYs" are and how the "conflict"
> with the PHY handling in USB core?
> 
> I am aware of two PHY subsystems:
> - drivers/phy
> -- also called "generic PHY framework"
> -- uses a "phys" property

Right, and these are sometimes referred to as generic PHYs, as opposed
to...

> - drivers/usb/phy
> -- also called "USB PHY framework"
> -- AFAIK this should not be used for new drivers

...the legacy ones.

> -- uses an "usb-phy" property

This is unfortunately not always the case however; some legacy USB phys
are also referred to using a "phys" property...

> the new PHY handling in USB core only parses the "phys" property and
> thus should not conflict with "usb-phy" (the legacy property)

> however, I probably missed something so I'd appreciate an explanation
> how things can break

...and that is the problem. Specifically, since last fall when a number
of legacy-phy nodes had a #phy-cells property added to them (to silence
DTC warnings), the generic PHY subsubsystem can now return -EPROBE_DEFER
indefinitely when looking up a phy as it finds a matching device-tree
node, but for which there will never be a generic phy registered (since
it's handled by a legacy-phy driver).

I referred to Arnd's workaround for "usb-nop-xceiv" devices

	b7563e2796f8 ("phy: work around 'phys' references to
	usb-nop-xceiv devices")

which has some more background on this.

So if we have any other host controllers out there using
"phys"-properties with legacy phys other than "usb-nop-xceiv", then
these will now fail to register (with -EPROBE_DEFER) unless
hcd->skip_phy_initialization is set (or we blacklist them as well in the
generic phy code).

I'm not aware of any further examples, but we're sure to find out soon
enough if there are.

Perhaps we should blacklist also "ti,am335x-usb-phy" in the generic phy
code even if hcd->skip_phy_initialization is still needed for musb for
the time being anyway.

Johan