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

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

[Roger Quadros]

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 <stern@rowland.harvard.edu>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 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

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

[Roger Quadros]

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 <stern@rowland.harvard.edu>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 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

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

[Alan Stern]

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 <stern@rowland.harvard.edu>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  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 <stern@rowland.harvard.edu>

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

[Alan Stern]

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 <stern@rowland.harvard.edu>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  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

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

[Roger Quadros]

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 <stern@rowland.harvard.edu>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  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

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

[Roger Quadros]

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

CC: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
 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

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

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

[Alan Stern]

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 <stern@rowland.harvard.edu>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  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 <stern@rowland.harvard.edu>