devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy: work around 'phys' references to usb-phy devices
@ 2018-01-08 13:01 Arnd Bergmann
  2018-01-08 18:32 ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2018-01-08 13:01 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Stefan Wahren, devicetree, Florian Fainelli, Arnd Bergmann,
	Felipe Balbi, linux-usb, linux-kernel, stable, Eric Anholt,
	Rob Herring, Andrzej Pietrasiewicz, linux-arm-kernel

Stefan Wahren reports a problem with a warning fix that was merged
for v4.15: we had lots of device nodes with a 'phys' property pointing
to a device node that is not compliant with the binding documented in
Documentation/devicetree/bindings/phy/phy-bindings.txt

This generally works because USB HCD drivers that support both the generic
phy subsystem and the older usb-phy subsystem ignore most errors from
phy_get() and related calls and then use the usb-phy driver instead.

However, usb_add_hcd() (along with the respective functions in dwc2 and
dwc3) propagate the EPROBE_DEFER return code so we can try again whenever
the driver gets loaded. In case the driver is written for the usb-phy
subsystem (like usb-generic-phy aka usb-nop-xceiv), we will never load
a generic-phy driver for it, and keep failing here.

There is only a small number of remaining usb-phy drivers that support
device tree, so this adds a workaround by providing a full list of the
potentially affected drivers, and always failing the probe with -ENODEV
here, which is the same behavior that we used to get with incorrect
device tree files. Since we generally want older kernels to also want
to work with the fixed devicetree files, it would be good to backport
the patch into stable kernels as well (3.13+ are possibly affected).
Reverting back to the DTS sources that work would in theory fix USB
support for now, but in the long run we'd run into the same problem
again when the drivers get ported from usb-phy to generic-phy.

Fixes: 014d6da6cb25 ("ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells")
Link: https://marc.info/?l=linux-usb&m=151518314314753&w=2
Cc: stable@vger.kernel.org
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
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.
---
 drivers/phy/phy-core.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index b4964b067aec..bb4dd2a2de2d 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -387,6 +387,24 @@ int phy_calibrate(struct phy *phy)
 }
 EXPORT_SYMBOL_GPL(phy_calibrate);
 
+static struct of_device_id __maybe_unused legacy_usbphy[] = {
+	{ .compatible = "fsl,imx23-usbphy" },
+	{ .compatible = "fsl,imx6q-usbphy" },
+	{ .compatible = "fsl,imx6sl-usbphy" },
+	{ .compatible = "fsl,imx6sx-usbphy" },
+	{ .compatible = "fsl,imx6ul-usbphy" },
+	{ .compatible = "fsl,vf610-usbphy" },
+	{ .compatible = "nvidia,tegra20-usb-phy" },
+	{ .compatible = "nvidia,tegra30-usb-phy" },
+	{ .compatible = "nxp,isp1301" },
+	{ .compatible = "ti,am335x-usb-ctrl-module" },
+	{ .compatible = "ti,am335x-usb-phy" },
+	{ .compatible = "ti,keystone-usbphy" },
+	{ .compatible = "ti,twl6030-usb" },
+	{ .compatible = "usb-nop-xceiv" },
+	{},
+};
+
 /**
  * _of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @np: device_node for which to get the phy
@@ -410,6 +428,15 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
 	if (ret)
 		return ERR_PTR(-ENODEV);
 
+	/*
+	 * Some USB host controllers use a "phys" property to refer to
+	 * a device that does not have a generic phy driver but that
+	 * has a driver for the older usb-phy framework.
+	 * We must not return -EPROBE_DEFER for those, so bail out early.
+	 */
+	if (of_match_node(legacy_usbphy, args.np))
+		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)) {
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
  2018-01-08 13:01 [PATCH] phy: work around 'phys' references to usb-phy devices Arnd Bergmann
@ 2018-01-08 18:32 ` Kishon Vijay Abraham I
  2018-01-10 20:57   ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Kishon Vijay Abraham I @ 2018-01-08 18:32 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Eric Anholt, Florian Fainelli, devicetree, linux-arm-kernel,
	Rob Herring, linux-usb, stable, Stefan Wahren, Felipe Balbi,
	Andrzej Pietrasiewicz, linux-kernel

Hi Arnd,

On Monday 08 January 2018 06:31 PM, Arnd Bergmann wrote:
> Stefan Wahren reports a problem with a warning fix that was merged
> for v4.15: we had lots of device nodes with a 'phys' property pointing
> to a device node that is not compliant with the binding documented in
> Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> This generally works because USB HCD drivers that support both the generic
> phy subsystem and the older usb-phy subsystem ignore most errors from
> phy_get() and related calls and then use the usb-phy driver instead.
> 
> However, usb_add_hcd() (along with the respective functions in dwc2 and
> dwc3) propagate the EPROBE_DEFER return code so we can try again whenever
> the driver gets loaded. In case the driver is written for the usb-phy
> subsystem (like usb-generic-phy aka usb-nop-xceiv), we will never load
> a generic-phy driver for it, and keep failing here.
> 
> There is only a small number of remaining usb-phy drivers that support
> device tree, so this adds a workaround by providing a full list of the
> potentially affected drivers, and always failing the probe with -ENODEV
> here, which is the same behavior that we used to get with incorrect
> device tree files. Since we generally want older kernels to also want
> to work with the fixed devicetree files, it would be good to backport
> the patch into stable kernels as well (3.13+ are possibly affected).
> Reverting back to the DTS sources that work would in theory fix USB
> support for now, but in the long run we'd run into the same problem
> again when the drivers get ported from usb-phy to generic-phy.
> 
> Fixes: 014d6da6cb25 ("ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells")
> Link: https://marc.info/?l=linux-usb&m=151518314314753&w=2
> Cc: stable@vger.kernel.org
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> 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?
> ---
>  drivers/phy/phy-core.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index b4964b067aec..bb4dd2a2de2d 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -387,6 +387,24 @@ int phy_calibrate(struct phy *phy)
>  }
>  EXPORT_SYMBOL_GPL(phy_calibrate);
>  
> +static struct of_device_id __maybe_unused legacy_usbphy[] = {
> +	{ .compatible = "fsl,imx23-usbphy" },
> +	{ .compatible = "fsl,imx6q-usbphy" },
> +	{ .compatible = "fsl,imx6sl-usbphy" },
> +	{ .compatible = "fsl,imx6sx-usbphy" },
> +	{ .compatible = "fsl,imx6ul-usbphy" },
> +	{ .compatible = "fsl,vf610-usbphy" },
> +	{ .compatible = "nvidia,tegra20-usb-phy" },
> +	{ .compatible = "nvidia,tegra30-usb-phy" },
> +	{ .compatible = "nxp,isp1301" },
> +	{ .compatible = "ti,am335x-usb-ctrl-module" },
> +	{ .compatible = "ti,am335x-usb-phy" },
> +	{ .compatible = "ti,keystone-usbphy" },
> +	{ .compatible = "ti,twl6030-usb" },
> +	{ .compatible = "usb-nop-xceiv" },
> +	{},

"ti,am335x-usb-ctrl-module" and "ti,twl6030-usb" are not phys.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
  2018-01-08 18:32 ` Kishon Vijay Abraham I
@ 2018-01-10 20:57   ` Arnd Bergmann
       [not found]     ` <CAK8P3a0F4sK=ou8HXmSktq+0S22juYaCaY48RStDKwk8QGgviQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2018-01-10 20:57 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Eric Anholt, Florian Fainelli, DTML, Linux ARM, Rob Herring,
	linux-usb, # 3.4.x, Stefan Wahren, Felipe Balbi,
	Andrzej Pietrasiewicz, Linux Kernel Mailing List

On Mon, Jan 8, 2018 at 7:32 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi Arnd,
>
> On Monday 08 January 2018 06:31 PM, Arnd Bergmann wrote:
>> Stefan Wahren reports a problem with a warning fix that was merged
>> for v4.15: we had lots of device nodes with a 'phys' property pointing
>> to a device node that is not compliant with the binding documented in
>> Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> This generally works because USB HCD drivers that support both the generic
>> phy subsystem and the older usb-phy subsystem ignore most errors from
>> phy_get() and related calls and then use the usb-phy driver instead.
>>
>> However, usb_add_hcd() (along with the respective functions in dwc2 and
>> dwc3) propagate the EPROBE_DEFER return code so we can try again whenever
>> the driver gets loaded. In case the driver is written for the usb-phy
>> subsystem (like usb-generic-phy aka usb-nop-xceiv), we will never load
>> a generic-phy driver for it, and keep failing here.
>>
>> There is only a small number of remaining usb-phy drivers that support
>> device tree, so this adds a workaround by providing a full list of the
>> potentially affected drivers, and always failing the probe with -ENODEV
>> here, which is the same behavior that we used to get with incorrect
>> device tree files. Since we generally want older kernels to also want
>> to work with the fixed devicetree files, it would be good to backport
>> the patch into stable kernels as well (3.13+ are possibly affected).
>> Reverting back to the DTS sources that work would in theory fix USB
>> support for now, but in the long run we'd run into the same problem
>> again when the drivers get ported from usb-phy to generic-phy.
>>
>> Fixes: 014d6da6cb25 ("ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells")
>> Link: https://marc.info/?l=linux-usb&m=151518314314753&w=2
>> Cc: stable@vger.kernel.org
>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>> Cc: Felipe Balbi <balbi@kernel.org>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> 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.

>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index b4964b067aec..bb4dd2a2de2d 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -387,6 +387,24 @@ int phy_calibrate(struct phy *phy)
>>  }
>>  EXPORT_SYMBOL_GPL(phy_calibrate);
>>
>> +static struct of_device_id __maybe_unused legacy_usbphy[] = {
>> +     { .compatible = "fsl,imx23-usbphy" },
>> +     { .compatible = "fsl,imx6q-usbphy" },
>> +     { .compatible = "fsl,imx6sl-usbphy" },
>> +     { .compatible = "fsl,imx6sx-usbphy" },
>> +     { .compatible = "fsl,imx6ul-usbphy" },
>> +     { .compatible = "fsl,vf610-usbphy" },
>> +     { .compatible = "nvidia,tegra20-usb-phy" },
>> +     { .compatible = "nvidia,tegra30-usb-phy" },
>> +     { .compatible = "nxp,isp1301" },
>> +     { .compatible = "ti,am335x-usb-ctrl-module" },
>> +     { .compatible = "ti,am335x-usb-phy" },
>> +     { .compatible = "ti,keystone-usbphy" },
>> +     { .compatible = "ti,twl6030-usb" },
>> +     { .compatible = "usb-nop-xceiv" },
>> +     {},
>
> "ti,am335x-usb-ctrl-module" and "ti,twl6030-usb" are not phys.

Ok, I see.

       Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
       [not found]     ` <CAK8P3a0F4sK=ou8HXmSktq+0S22juYaCaY48RStDKwk8QGgviQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-01-11 13:30       ` Kishon Vijay Abraham I
       [not found]         ` <6dd37865-9c71-29f9-16b4-26e51e6b1c70-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Kishon Vijay Abraham I @ 2018-01-11 13:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Eric Anholt, Florian Fainelli, DTML, Linux ARM, Rob Herring,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, # 3.4.x, Stefan Wahren,
	Felipe Balbi, Andrzej Pietrasiewicz, Linux Kernel Mailing List

Hi Arnd,

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-l0cyMroinI0@public.gmane.org> wrote:
>> Hi Arnd,
>>
>> On Monday 08 January 2018 06:31 PM, Arnd Bergmann wrote:
>>> Stefan Wahren reports a problem with a warning fix that was merged
>>> for v4.15: we had lots of device nodes with a 'phys' property pointing
>>> to a device node that is not compliant with the binding documented in
>>> Documentation/devicetree/bindings/phy/phy-bindings.txt
>>>
>>> This generally works because USB HCD drivers that support both the generic
>>> phy subsystem and the older usb-phy subsystem ignore most errors from
>>> phy_get() and related calls and then use the usb-phy driver instead.
>>>
>>> However, usb_add_hcd() (along with the respective functions in dwc2 and
>>> dwc3) propagate the EPROBE_DEFER return code so we can try again whenever
>>> the driver gets loaded. In case the driver is written for the usb-phy
>>> subsystem (like usb-generic-phy aka usb-nop-xceiv), we will never load
>>> a generic-phy driver for it, and keep failing here.
>>>
>>> There is only a small number of remaining usb-phy drivers that support
>>> device tree, so this adds a workaround by providing a full list of the
>>> potentially affected drivers, and always failing the probe with -ENODEV
>>> here, which is the same behavior that we used to get with incorrect
>>> device tree files. Since we generally want older kernels to also want
>>> to work with the fixed devicetree files, it would be good to backport
>>> the patch into stable kernels as well (3.13+ are possibly affected).
>>> Reverting back to the DTS sources that work would in theory fix USB
>>> support for now, but in the long run we'd run into the same problem
>>> again when the drivers get ported from usb-phy to generic-phy.
>>>
>>> Fixes: 014d6da6cb25 ("ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells")
>>> Link: https://marc.info/?l=linux-usb&m=151518314314753&w=2
>>> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>> Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>> ---
>>> 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.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
       [not found]         ` <6dd37865-9c71-29f9-16b4-26e51e6b1c70-l0cyMroinI0@public.gmane.org>
@ 2018-01-11 15:20           ` Arnd Bergmann
       [not found]             ` <CAK8P3a0NSGCR8=MzHAPqyhWMsUWAxJjtstYo8gszKcXUCaupgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2018-01-11 15:20 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Eric Anholt, Florian Fainelli, DTML, Linux ARM, Rob Herring,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, # 3.4.x, Stefan Wahren,
	Felipe Balbi, Andrzej Pietrasiewicz, Linux Kernel Mailing List

On Thu, Jan 11, 2018 at 2:30 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> 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-r2nGTMty4D4@public.gmane.org>

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)) {
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
       [not found]             ` <CAK8P3a0NSGCR8=MzHAPqyhWMsUWAxJjtstYo8gszKcXUCaupgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-01-11 18:16               ` Eric Anholt
       [not found]                 ` <87d12guups.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Anholt @ 2018-01-11 18:16 UTC (permalink / raw)
  To: Arnd Bergmann, Kishon Vijay Abraham I
  Cc: Florian Fainelli, DTML, Linux ARM, Rob Herring,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, # 3.4.x, Stefan Wahren,
	Felipe Balbi, Andrzej Pietrasiewicz, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 3907 bytes --]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:

> On Thu, Jan 11, 2018 at 2:30 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> 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-r2nGTMty4D4@public.gmane.org>
>
> 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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: work around 'phys' references to usb-phy devices
       [not found]                 ` <87d12guups.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
@ 2018-01-12  6:00                   ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 7+ messages in thread
From: Kishon Vijay Abraham I @ 2018-01-12  6:00 UTC (permalink / raw)
  To: Eric Anholt, Arnd Bergmann
  Cc: Florian Fainelli, DTML, Linux ARM, Rob Herring,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, # 3.4.x, Stefan Wahren,
	Felipe Balbi, Andrzej Pietrasiewicz, Linux Kernel Mailing List

Hi Arnd,

On Thursday 11 January 2018 11:46 PM, Eric Anholt wrote:
> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
> 
>> On Thu, Jan 11, 2018 at 2:30 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> 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?

yeah, this looks simpler.
>>
>> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

In case you want to take this patch yourself
Acked-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>

(or let me know if I have to create a separate pull request for Greg)

Thanks
Kishon

>>
>> 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!
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-12  6:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 13:01 [PATCH] phy: work around 'phys' references to usb-phy devices Arnd Bergmann
2018-01-08 18:32 ` Kishon Vijay Abraham I
2018-01-10 20:57   ` Arnd Bergmann
     [not found]     ` <CAK8P3a0F4sK=ou8HXmSktq+0S22juYaCaY48RStDKwk8QGgviQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-11 13:30       ` Kishon Vijay Abraham I
     [not found]         ` <6dd37865-9c71-29f9-16b4-26e51e6b1c70-l0cyMroinI0@public.gmane.org>
2018-01-11 15:20           ` Arnd Bergmann
     [not found]             ` <CAK8P3a0NSGCR8=MzHAPqyhWMsUWAxJjtstYo8gszKcXUCaupgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-11 18:16               ` Eric Anholt
     [not found]                 ` <87d12guups.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
2018-01-12  6:00                   ` Kishon Vijay Abraham I

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).