2013-04-15 11:50:46

by Roger Quadros

Subject: [PATCH 1/2] USB: ehci-omap: Don't select any PHY driver

Don't select NOP_USB_XCEIV. Instead, board config
must select USB_PHY and the appropriate PHY driver.

Also add a hint in Kconfig so that users enabling
this driver manually enable the right PHY drivers as well.

Gets rid of the below warnings when USB_EHCI_HCD_OMAP
is enabled.

warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)

CC: Alan Stern <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/host/Kconfig | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index c0be25c..c558472 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -150,11 +150,13 @@ config USB_EHCI_MXC
config USB_EHCI_HCD_OMAP
tristate "EHCI support for OMAP3 and later chips"
depends on ARCH_OMAP
- select NOP_USB_XCEIV
default y
---help---
Enables support for the on-chip EHCI controller on
OMAP3 and later chips.
+ If your system uses a PHY on the USB port, you will need to
+ enable USB_PHY and the appropriate PHY driver as well. Most
+ boards need the NOP_USB_XCEIV PHY driver.

config USB_EHCI_HCD_ORION
tristate "Support for Marvell EBU on-chip EHCI USB controller"
--
1.7.4.1

2013-04-15 11:50:50

by Roger Quadros

Subject: [PATCH 2/2] USB: ehci-omap: Improve PHY error handling

As the USB PHY layer never returns NULL we don't need
to check for that condition.

If we fail to get the PHY device it could be due
to missing USB PHY drivers. Give this hint to the user
in the error message.

CC: Alan Stern <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/host/ehci-omap.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 5de3e43..2e34ddd 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -175,13 +175,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
else
phy = devm_usb_get_phy_dev(dev, i);
- if (IS_ERR(phy) || !phy) {
+ if (IS_ERR(phy)) {
/* Don't bail out if PHY is not absolutely necessary */
if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
continue;

- ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
- dev_err(dev, "Can't get PHY device for port %d: %d\n",
+ ret = PTR_ERR(phy);
+ dev_err(dev, "Can't get PHY device for port %d: %d. Is USB_PHY driver enabled?\n",
i, ret);
goto err_phy;
}
--
1.7.4.1

2013-04-16 15:30:59

by Alan Stern

Subject: Re: [PATCH 1/2] USB: ehci-omap: Don't select any PHY driver

On Mon, 15 Apr 2013, Roger Quadros wrote:

> Don't select NOP_USB_XCEIV. Instead, board config
> must select USB_PHY and the appropriate PHY driver.
>
> Also add a hint in Kconfig so that users enabling
> this driver manually enable the right PHY drivers as well.
>
> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
> is enabled.
>
> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>
> CC: Alan Stern <[email protected]>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> drivers/usb/host/Kconfig | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index c0be25c..c558472 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -150,11 +150,13 @@ config USB_EHCI_MXC
> config USB_EHCI_HCD_OMAP
> tristate "EHCI support for OMAP3 and later chips"
> depends on ARCH_OMAP
> - select NOP_USB_XCEIV
> default y
> ---help---
> Enables support for the on-chip EHCI controller on
> OMAP3 and later chips.
> + If your system uses a PHY on the USB port, you will need to
> + enable USB_PHY and the appropriate PHY driver as well. Most
> + boards need the NOP_USB_XCEIV PHY driver.
>
> config USB_EHCI_HCD_ORION
> tristate "Support for Marvell EBU on-chip EHCI USB controller"

Acked-by: Alan Stern <[email protected]>

2013-04-16 15:32:22

by Alan Stern

Subject: Re: [PATCH 2/2] USB: ehci-omap: Improve PHY error handling

On Mon, 15 Apr 2013, Roger Quadros wrote:

> As the USB PHY layer never returns NULL we don't need
> to check for that condition.
>
> If we fail to get the PHY device it could be due
> to missing USB PHY drivers. Give this hint to the user
> in the error message.
>
> CC: Alan Stern <[email protected]>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> drivers/usb/host/ehci-omap.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 5de3e43..2e34ddd 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -175,13 +175,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
> else
> phy = devm_usb_get_phy_dev(dev, i);
> - if (IS_ERR(phy) || !phy) {
> + if (IS_ERR(phy)) {
> /* Don't bail out if PHY is not absolutely necessary */
> if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
> continue;
>
> - ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
> - dev_err(dev, "Can't get PHY device for port %d: %d\n",
> + ret = PTR_ERR(phy);
> + dev_err(dev, "Can't get PHY device for port %d: %d. Is USB_PHY driver enabled?\n",
> i, ret);
> goto err_phy;
> }

Getting rid of the !phy test is good. But I'm doubtful about the
change to the error message. Are you going to make a similar change to
every platform driver? There doesn't seem to be any reason to do this
for ehci-omap but not the others.

Alan Stern

2013-04-17 08:12:55

by Roger Quadros

Subject: Re: [PATCH 2/2] USB: ehci-omap: Improve PHY error handling

On 04/16/2013 06:32 PM, Alan Stern wrote:
> On Mon, 15 Apr 2013, Roger Quadros wrote:
>
>> As the USB PHY layer never returns NULL we don't need
>> to check for that condition.
>>
>> If we fail to get the PHY device it could be due
>> to missing USB PHY drivers. Give this hint to the user
>> in the error message.
>>
>> CC: Alan Stern <[email protected]>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>> drivers/usb/host/ehci-omap.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
>> index 5de3e43..2e34ddd 100644
>> --- a/drivers/usb/host/ehci-omap.c
>> +++ b/drivers/usb/host/ehci-omap.c
>> @@ -175,13 +175,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>> phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
>> else
>> phy = devm_usb_get_phy_dev(dev, i);
>> - if (IS_ERR(phy) || !phy) {
>> + if (IS_ERR(phy)) {
>> /* Don't bail out if PHY is not absolutely necessary */
>> if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
>> continue;
>>
>> - ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
>> - dev_err(dev, "Can't get PHY device for port %d: %d\n",
>> + ret = PTR_ERR(phy);
>> + dev_err(dev, "Can't get PHY device for port %d: %d. Is USB_PHY driver enabled?\n",
>> i, ret);
>> goto err_phy;
>> }
>
> Getting rid of the !phy test is good. But I'm doubtful about the
> change to the error message. Are you going to make a similar change to
> every platform driver? There doesn't seem to be any reason to do this
> for ehci-omap but not the others.

I'm not very sure about that. If using the PHY API while USB_PHY is disabled
is to be treated as a universal failure, then the print might as well come from
the dummy stub for xx_usb_get_phy_xx().

For this patch, I'll just undo the print message change and resend. That could be
done in a separate patch instead.

cheers,
-roger

2013-04-17 08:24:39

by Roger Quadros

Subject: [PATCH v2 2/2] USB: ehci-omap: Improve PHY error handling

As the USB PHY layer never returns NULL we don't need
to check for that condition.

CC: Alan Stern <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/host/ehci-omap.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 5de3e43..3d1491b 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -175,12 +175,12 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
else
phy = devm_usb_get_phy_dev(dev, i);
- if (IS_ERR(phy) || !phy) {
+ if (IS_ERR(phy)) {
/* Don't bail out if PHY is not absolutely necessary */
if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
continue;

- ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
+ ret = PTR_ERR(phy);
dev_err(dev, "Can't get PHY device for port %d: %d\n",
i, ret);
goto err_phy;
--
1.7.4.1

2013-04-17 14:15:09

by Alan Stern

Subject: Re: [PATCH v2 2/2] USB: ehci-omap: Improve PHY error handling

On Wed, 17 Apr 2013, Roger Quadros wrote:

> As the USB PHY layer never returns NULL we don't need
> to check for that condition.
>
> CC: Alan Stern <[email protected]>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> drivers/usb/host/ehci-omap.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 5de3e43..3d1491b 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -175,12 +175,12 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
> else
> phy = devm_usb_get_phy_dev(dev, i);
> - if (IS_ERR(phy) || !phy) {
> + if (IS_ERR(phy)) {
> /* Don't bail out if PHY is not absolutely necessary */
> if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
> continue;
>
> - ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
> + ret = PTR_ERR(phy);
> dev_err(dev, "Can't get PHY device for port %d: %d\n",
> i, ret);
> goto err_phy;

Acked-by: Alan Stern <[email protected]>