[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87d12guups.fsf@anholt.net>
Date: Thu, 11 Jan 2018 10:16:31 -0800
From: Eric Anholt <eric@...olt.net>
To: Arnd Bergmann <arnd@...db.de>,
Kishon Vijay Abraham I <kishon@...com>
Cc: Florian Fainelli <f.fainelli@...il.com>,
DTML <devicetree@...r.kernel.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Rob Herring <robh+dt@...nel.org>, linux-usb@...r.kernel.org,
"# 3.4.x" <stable@...r.kernel.org>,
Stefan Wahren <stefan.wahren@...e.com>,
Felipe Balbi <balbi@...nel.org>,
Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] phy: work around 'phys' references to usb-phy devices
Arnd Bergmann <arnd@...db.de> writes:
> On Thu, Jan 11, 2018 at 2:30 PM, Kishon Vijay Abraham I <kishon@...com> wrote:
>> On Thursday 11 January 2018 02:27 AM, Arnd Bergmann wrote:
>>> On Mon, Jan 8, 2018 at 7:32 PM, Kishon Vijay Abraham I <kishon@...com> wrote:
>>>> On Monday 08 January 2018 06:31 PM, Arnd Bergmann wrote:
>>>>> Stefan Wahren reports a problem with a warning fix that was merged
>>>>> ---
>>>>> This obviously needs to be tested, I wrote this up as a reply to
>>>>> Stefan's bug report. I'm fairly sure that I covered all usb-phy
>>>>> driver strings here. My goal is to have a fix merged into 4.15
>>>>> rather than reverting all the DT fixes.
>>>>
>>>> Shouldn't the fix be in phy consumer drivers to not return error if it's able
>>>> to find the phy either using usb-phy or generic phy?
>>>
>>> Stefan has posted a patch to that effect now, but I fear that might be
>>> a little fragile, in particular this short before the release with the
>>> regression
>>> in place.
>>>
>>> The main problem is that we'd have to change the generic
>>> usb_add_hcd() function in addition to dwc2 and dwc3 to ignore
>>> -EPROBE_DEFER from phy_get() whenever usb_get_phy_dev()
>>> has already succeeded.
>>>
>>> If there is any HCD that relies on usb_add_hcd() to get both the
>>> usb_phy and the phy structures, and it may need to defer probing
>>> when the latter one isn't ready yet, that fix would break another
>>> driver.
>>
>> hmm.. IMO the better thing right now would be to revert the dt patch which adds
>> #phy-cells.
>> We have to see if there are better fixes in order to add #phy-cells warning fix
>> in stable tree.
>
> Let's see which patches that would be, I think this is the full list of
> nodes that got an extra #phy-cells:
>
> c22fe696157d ARM: dts: Fix dm814x missing phy-cells property
> f0e11ff8ff65 ARM: dts: am33xx: Add missing #phy-cells to ti,am335x-usb-phy
> c5bbf358b790 arm: dts: nspire: Add missing #phy-cells to usb-nop-xceiv
> 44e5dced2ef6 arm: dts: marvell: Add missing #phy-cells to usb-nop-xceiv
> 014d6da6cb25 ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells
> f568f6f554b8 ARM: dts: omap: Add missing #phy-cells to usb-nop-xceiv
>
> plus a couple in linux-next:
>
> d745d5f277bf ARM: dts: imx51-zii-rdu1: Add missing #phy-cells to usb-nop-xceiv
> 915fbe59cbf2 ARM: dts: imx: Add missing #phy-cells to usb-nop-xceiv
>
> It's a lot of patches to revert, and I guess it would get us back to hundreds
> of warnings in an allmodconfig build, so I'd first try to come up with
> ways to prove that at least some of them can stay.
>
> Almost all the warnings are about "usb-nop-xceiv" phys, the only exceptions
> I could find are the OMAP ones (the first two patches), which use
> "ti,am335x-usb-phy" and are referenced from a "ti,musb-am33xx". That
> particular driver is not affected by the bug, so we can leave that in.
>
> To deal with all the "usb-nop-xceiv" references including the one that
> Stefan reported, we could use a much simpler version of my earlier
> patch, do you think this is any better?
>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index b4964b067aec..f056d8fb3921 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -410,6 +410,10 @@ static struct phy *_of_phy_get(struct device_node
> *np, int index)
> if (ret)
> return ERR_PTR(-ENODEV);
>
> + /* This phy type handled by the usb-phy subsystem for now */
> + if (of_device_is_compatible("usb-nop-xceiv"))
> + return ERR_PTR(-ENODEV);
> +
> mutex_lock(&phy_provider_mutex);
> phy_provider = of_phy_provider_lookup(args.np);
> if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
This seems like a nice workaround!
Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)
Powered by blists - more mailing lists