Patchwork [RFC,2/6] ARM: OMAP: USB: Add phy binding information

login
register
mail settings
Submitter Kishon Vijay Abraham I
Date Jan. 16, 2013, 3 p.m.
Message ID <1358348462-27693-3-git-send-email-kishon@ti.com>
Download mbox | patch
Permalink /patch/212552/
State Not Applicable
Headers show

Comments

Kishon Vijay Abraham I - Jan. 16, 2013, 3 p.m.
This is in preparation for the changes in PHY library to support adding
and getting multiple PHYs of the same type. In the new design, the
binding information between the PHY and the USB controller should be
specified in the platform specific initialization code. So it's been
done for OMAP platforms here.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
This kind-of binding should be done in all the platforms (I've done only
for OMAP platform). 
 arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Roger Quadros - Jan. 17, 2013, 9:39 a.m.
On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
> This is in preparation for the changes in PHY library to support adding
> and getting multiple PHYs of the same type. In the new design, the
> binding information between the PHY and the USB controller should be
> specified in the platform specific initialization code. So it's been
> done for OMAP platforms here.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> This kind-of binding should be done in all the platforms (I've done only
> for OMAP platform). 
>  arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 9d27e3f..bbe2fa5 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -24,6 +24,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/usb/musb.h>
> +#include <linux/usb/phy.h>
>  
>  #include "omap_device.h"
>  #include "soc.h"
> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>  	musb_plat.mode = board_data->mode;
>  	musb_plat.extvbus = board_data->extvbus;
>  
> -	if (cpu_is_omap44xx())
> +	if (cpu_is_omap44xx()) {
>  		musb_plat.has_mailbox = true;
> +		usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
> +	} else if (cpu_is_omap34xx()) {
> +		usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +	}

Are you sure than these OMAP platforms cannot be wired in any other way
to the PHY?

If they can be then this association must come from the board files or
device tree.

>  
>  	if (soc_is_am35xx()) {
>  		oh_name = "am35x_otg_hs";
> 

--
cheers,
-roger
Kishon Vijay Abraham I - Jan. 17, 2013, 11:03 a.m.
Hi,

On Thursday 17 January 2013 03:09 PM, Roger Quadros wrote:
> On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
>> This is in preparation for the changes in PHY library to support adding
>> and getting multiple PHYs of the same type. In the new design, the
>> binding information between the PHY and the USB controller should be
>> specified in the platform specific initialization code. So it's been
>> done for OMAP platforms here.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> This kind-of binding should be done in all the platforms (I've done only
>> for OMAP platform).
>>   arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> index 9d27e3f..bbe2fa5 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/io.h>
>>   #include <linux/usb/musb.h>
>> +#include <linux/usb/phy.h>
>>
>>   #include "omap_device.h"
>>   #include "soc.h"
>> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>>   	musb_plat.mode = board_data->mode;
>>   	musb_plat.extvbus = board_data->extvbus;
>>
>> -	if (cpu_is_omap44xx())
>> +	if (cpu_is_omap44xx()) {
>>   		musb_plat.has_mailbox = true;
>> +		usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
>> +	} else if (cpu_is_omap34xx()) {
>> +		usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +	}
>
> Are you sure than these OMAP platforms cannot be wired in any other way
> to the PHY?
>
> If they can be then this association must come from the board files or
> device tree.

I had given some thought about that and since all the existing OMAP 
platforms had it this way, I added it here.
You having pointed that out, I'll move these to board files.

Thanks
Kishon
Vivek Gautam - Jan. 17, 2013, 12:11 p.m.
Hi Kishon,


On Wed, Jan 16, 2013 at 8:30 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> This is in preparation for the changes in PHY library to support adding
> and getting multiple PHYs of the same type. In the new design, the
> binding information between the PHY and the USB controller should be
> specified in the platform specific initialization code. So it's been
> done for OMAP platforms here.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> This kind-of binding should be done in all the platforms (I've done only
> for OMAP platform).
>  arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 9d27e3f..bbe2fa5 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -24,6 +24,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/usb/musb.h>
> +#include <linux/usb/phy.h>
>
>  #include "omap_device.h"
>  #include "soc.h"
> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>         musb_plat.mode = board_data->mode;
>         musb_plat.extvbus = board_data->extvbus;
>
> -       if (cpu_is_omap44xx())
> +       if (cpu_is_omap44xx()) {
>                 musb_plat.has_mailbox = true;
> +               usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
> +       } else if (cpu_is_omap34xx()) {
> +               usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +       }

Just trying to understand things here.
Is the device name we are using here the platform device name (pdev->name) ?
If i am not wrong then for devices added through device tree
the device name would actually include the device address in the string,
wouldn't that make things somewhat weird, since we will have to add
such bindings
using this address string.

one more doubt please :)
is it possible to bind phy in the controllers' driver somehow ?

>
>         if (soc_is_am35xx()) {
>                 oh_name = "am35x_otg_hs";
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I - Jan. 17, 2013, 12:31 p.m.
Hi,

On Thursday 17 January 2013 05:41 PM, Vivek Gautam wrote:
> Hi Kishon,
>
>
> On Wed, Jan 16, 2013 at 8:30 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> This is in preparation for the changes in PHY library to support adding
>> and getting multiple PHYs of the same type. In the new design, the
>> binding information between the PHY and the USB controller should be
>> specified in the platform specific initialization code. So it's been
>> done for OMAP platforms here.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> This kind-of binding should be done in all the platforms (I've done only
>> for OMAP platform).
>>   arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> index 9d27e3f..bbe2fa5 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/io.h>
>>   #include <linux/usb/musb.h>
>> +#include <linux/usb/phy.h>
>>
>>   #include "omap_device.h"
>>   #include "soc.h"
>> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>>          musb_plat.mode = board_data->mode;
>>          musb_plat.extvbus = board_data->extvbus;
>>
>> -       if (cpu_is_omap44xx())
>> +       if (cpu_is_omap44xx()) {
>>                  musb_plat.has_mailbox = true;
>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
>> +       } else if (cpu_is_omap34xx()) {
>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +       }
>
> Just trying to understand things here.
> Is the device name we are using here the platform device name (pdev->name) ?

it's pdev->dev.name.
> If i am not wrong then for devices added through device tree
> the device name would actually include the device address in the string,

Ideally for device tree, devm_usb_get_phy_by_phandle() introduced in the 
5th patch of this series should be used.
usb_bind_phy should be called from platform specific initialization 
files (like your board file), which won't get executed in dt boot.
> wouldn't that make things somewhat weird, since we will have to add
> such bindings
> using this address string.
>
> one more doubt please :)
> is it possible to bind phy in the controllers' driver somehow ?
Thats not going to be good. Why do you need to do that?

Thanks
Kishon
Vivek Gautam - Jan. 17, 2013, 1:11 p.m.
Hi Kishon,


Thanks for the explanation.

On Thu, Jan 17, 2013 at 6:01 PM, kishon <kishon@ti.com> wrote:
> Hi,
>
>
> On Thursday 17 January 2013 05:41 PM, Vivek Gautam wrote:
>>
>> Hi Kishon,
>>
>>
>> On Wed, Jan 16, 2013 at 8:30 PM, Kishon Vijay Abraham I <kishon@ti.com>
>> wrote:
>>>
>>> This is in preparation for the changes in PHY library to support adding
>>> and getting multiple PHYs of the same type. In the new design, the
>>> binding information between the PHY and the USB controller should be
>>> specified in the platform specific initialization code. So it's been
>>> done for OMAP platforms here.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>> This kind-of binding should be done in all the platforms (I've done only
>>> for OMAP platform).
>>>   arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/usb-musb.c
>>> b/arch/arm/mach-omap2/usb-musb.c
>>> index 9d27e3f..bbe2fa5 100644
>>> --- a/arch/arm/mach-omap2/usb-musb.c
>>> +++ b/arch/arm/mach-omap2/usb-musb.c
>>> @@ -24,6 +24,7 @@
>>>   #include <linux/dma-mapping.h>
>>>   #include <linux/io.h>
>>>   #include <linux/usb/musb.h>
>>> +#include <linux/usb/phy.h>
>>>
>>>   #include "omap_device.h"
>>>   #include "soc.h"
>>> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data
>>> *musb_board_data)
>>>          musb_plat.mode = board_data->mode;
>>>          musb_plat.extvbus = board_data->extvbus;
>>>
>>> -       if (cpu_is_omap44xx())
>>> +       if (cpu_is_omap44xx()) {
>>>                  musb_plat.has_mailbox = true;
>>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
>>> +       } else if (cpu_is_omap34xx()) {
>>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>>> +       }
>>
>>
>> Just trying to understand things here.
>> Is the device name we are using here the platform device name (pdev->name)
>> ?
>
>
> it's pdev->dev.name.
>
>> If i am not wrong then for devices added through device tree
>> the device name would actually include the device address in the string,
>
>
> Ideally for device tree, devm_usb_get_phy_by_phandle() introduced in the 5th
> patch of this series should be used.

Ok.
> usb_bind_phy should be called from platform specific initialization files
> (like your board file), which won't get executed in dt boot.
>
Right.

>> wouldn't that make things somewhat weird, since we will have to add
>> such bindings
>> using this address string.
>>
>> one more doubt please :)
>> is it possible to bind phy in the controllers' driver somehow ?
>
> Thats not going to be good. Why do you need to do that?
>
I was certainly missing something while understanding your patches.
That's why got some doubt on this. :)


> Thanks
> Kishon

Patch

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 9d27e3f..bbe2fa5 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -24,6 +24,7 @@ 
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/usb/musb.h>
+#include <linux/usb/phy.h>
 
 #include "omap_device.h"
 #include "soc.h"
@@ -85,8 +86,12 @@  void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
 	musb_plat.mode = board_data->mode;
 	musb_plat.extvbus = board_data->extvbus;
 
-	if (cpu_is_omap44xx())
+	if (cpu_is_omap44xx()) {
 		musb_plat.has_mailbox = true;
+		usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
+	} else if (cpu_is_omap34xx()) {
+		usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	}
 
 	if (soc_is_am35xx()) {
 		oh_name = "am35x_otg_hs";