[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Eric Bénard]

without this patch, the initialization of the OTG port in host mode
fails with :
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
ULPI transceiver vendor/product ID 0x0000/0x0000
ULPI ID does not match any known transceiver.
mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 drivers/usb/otg/ulpi.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c
index ccc8195..e802e44 100644
--- a/drivers/usb/otg/ulpi.c
+++ b/drivers/usb/otg/ulpi.c
@@ -34,7 +34,10 @@
 /* ULPI hardcoded IDs, used for probing */
 static unsigned int ulpi_ids[] = {
 	ULPI_ID(0x04cc, 0x1504),	/* NXP ISP1504 */
-	ULPI_ID(0x0424, 0x0006),        /* SMSC USB3319 */
+	ULPI_ID(0x0424, 0x0006),	/* SMSC USB3319 */
+#if (defined(CONFIG_ARCH_MX25) || defined(CONFIG_ARCH_MX35))
+	ULPI_ID(0x0000, 0x0000),	/* i.MX25 & i.MX35 internal PHY */
+#endif
 };
 
 static int ulpi_set_otg_flags(struct otg_transceiver *otg)
-- 
1.7.0.4

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Sergei Shtylyov]

Hello.

Eric B?nard wrote:

> without this patch, the initialization of the OTG port in host mode
> fails with :
> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
> ULPI transceiver vendor/product ID 0x0000/0x0000
> ULPI ID does not match any known transceiver.
> mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing

> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  drivers/usb/otg/ulpi.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)

    You should have CC'ed linux-usb at vger.kernel.org.

> diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c
> index ccc8195..e802e44 100644
> --- a/drivers/usb/otg/ulpi.c
> +++ b/drivers/usb/otg/ulpi.c
> @@ -34,7 +34,10 @@
>  /* ULPI hardcoded IDs, used for probing */
>  static unsigned int ulpi_ids[] = {
>  	ULPI_ID(0x04cc, 0x1504),	/* NXP ISP1504 */
> -	ULPI_ID(0x0424, 0x0006),        /* SMSC USB3319 */
> +	ULPI_ID(0x0424, 0x0006),	/* SMSC USB3319 */
> +#if (defined(CONFIG_ARCH_MX25) || defined(CONFIG_ARCH_MX35))
> +	ULPI_ID(0x0000, 0x0000),	/* i.MX25 & i.MX35 internal PHY */
> +#endif

    I don't know why this table is at all used in the first place. IMO, checking 
for the vendor/device ID serves no purpose...

WBR, Sergei

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Igor Grinberg]

 On 10/04/10 16:54, Sergei Shtylyov wrote:
> Hello.
>
> Eric B?nard wrote:
>
>> without this patch, the initialization of the OTG port in host mode
>> fails with :
>> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
>> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
>> ULPI transceiver vendor/product ID 0x0000/0x0000
>> ULPI ID does not match any known transceiver.
>> mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing
>
>> Signed-off-by: Eric B?nard <eric@eukrea.com>
>> ---
>>  drivers/usb/otg/ulpi.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>
>    You should have CC'ed linux-usb at vger.kernel.org.
>
>> diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c
>> index ccc8195..e802e44 100644
>> --- a/drivers/usb/otg/ulpi.c
>> +++ b/drivers/usb/otg/ulpi.c
>> @@ -34,7 +34,10 @@
>>  /* ULPI hardcoded IDs, used for probing */
>>  static unsigned int ulpi_ids[] = {
>>      ULPI_ID(0x04cc, 0x1504),    /* NXP ISP1504 */
>> -    ULPI_ID(0x0424, 0x0006),        /* SMSC USB3319 */
>> +    ULPI_ID(0x0424, 0x0006),    /* SMSC USB3319 */
>> +#if (defined(CONFIG_ARCH_MX25) || defined(CONFIG_ARCH_MX35))
>> +    ULPI_ID(0x0000, 0x0000),    /* i.MX25 & i.MX35 internal PHY */
>> +#endif
>
>    I don't know why this table is at all used in the first place. IMO, checking for the vendor/device ID serves no purpose...

Well, it was there in first place, when Daniel merged this.
May be it was taken from Sascha.
I think its purpose to check if there is a ulpi transceiver out there,
although, ulpi defines a special scratch register for testing purposes.

>
> WBR, Sergei
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Regards,
Igor.

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Daniel Mack]

On Mon, Oct 04, 2010 at 05:15:35PM +0200, Igor Grinberg wrote:
>  On 10/04/10 16:54, Sergei Shtylyov wrote:
> > Hello.
> >
> > Eric B?nard wrote:
> >
> >> without this patch, the initialization of the OTG port in host mode
> >> fails with :
> >> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> >> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
> >> ULPI transceiver vendor/product ID 0x0000/0x0000
> >> ULPI ID does not match any known transceiver.
> >> mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing
> >
> >> Signed-off-by: Eric B?nard <eric@eukrea.com>
> >> ---
> >>  drivers/usb/otg/ulpi.c |    5 ++++-
> >>  1 files changed, 4 insertions(+), 1 deletions(-)
> >
> >    You should have CC'ed linux-usb at vger.kernel.org.
> >
> >> diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c
> >> index ccc8195..e802e44 100644
> >> --- a/drivers/usb/otg/ulpi.c
> >> +++ b/drivers/usb/otg/ulpi.c
> >> @@ -34,7 +34,10 @@
> >>  /* ULPI hardcoded IDs, used for probing */
> >>  static unsigned int ulpi_ids[] = {
> >>      ULPI_ID(0x04cc, 0x1504),    /* NXP ISP1504 */
> >> -    ULPI_ID(0x0424, 0x0006),        /* SMSC USB3319 */
> >> +    ULPI_ID(0x0424, 0x0006),    /* SMSC USB3319 */
> >> +#if (defined(CONFIG_ARCH_MX25) || defined(CONFIG_ARCH_MX35))
> >> +    ULPI_ID(0x0000, 0x0000),    /* i.MX25 & i.MX35 internal PHY */
> >> +#endif
> >
> >    I don't know why this table is at all used in the first place. IMO, checking for the vendor/device ID serves no purpose...
> 
> Well, it was there in first place, when Daniel merged this.
> May be it was taken from Sascha.
> I think its purpose to check if there is a ulpi transceiver out there,
> although, ulpi defines a special scratch register for testing purposes.

Correct, its primary purpose was to see whether the ULPI low-level
communication works at all, and to bail out when it doesn't.

I wasn't aware of that scratch register. If we don't need that table any
more, feel free to drop it.

Thanks,
Daniel

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Igor Grinberg]

 Hi,

On 10/05/10 07:42, Daniel Mack wrote:
> On Mon, Oct 04, 2010 at 05:15:35PM +0200, Igor Grinberg wrote:
>>  On 10/04/10 16:54, Sergei Shtylyov wrote:
>>> Hello.
>>>
>>> Eric B?nard wrote:
>>>
>>>> without this patch, the initialization of the OTG port in host mode
>>>> fails with :
>>>> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
>>>> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
>>>> ULPI transceiver vendor/product ID 0x0000/0x0000
>>>> ULPI ID does not match any known transceiver.
>>>> mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing
>>>> Signed-off-by: Eric B?nard <eric@eukrea.com>
>>>> ---
>>>>  drivers/usb/otg/ulpi.c |    5 ++++-
>>>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>>    You should have CC'ed linux-usb at vger.kernel.org.
>>>
>>>> diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c
>>>> index ccc8195..e802e44 100644
>>>> --- a/drivers/usb/otg/ulpi.c
>>>> +++ b/drivers/usb/otg/ulpi.c
>>>> @@ -34,7 +34,10 @@
>>>>  /* ULPI hardcoded IDs, used for probing */
>>>>  static unsigned int ulpi_ids[] = {
>>>>      ULPI_ID(0x04cc, 0x1504),    /* NXP ISP1504 */
>>>> -    ULPI_ID(0x0424, 0x0006),        /* SMSC USB3319 */
>>>> +    ULPI_ID(0x0424, 0x0006),    /* SMSC USB3319 */
>>>> +#if (defined(CONFIG_ARCH_MX25) || defined(CONFIG_ARCH_MX35))
>>>> +    ULPI_ID(0x0000, 0x0000),    /* i.MX25 & i.MX35 internal PHY */
>>>> +#endif
>>>    I don't know why this table is at all used in the first place. IMO, checking for the vendor/device ID serves no purpose...
>> Well, it was there in first place, when Daniel merged this.
>> May be it was taken from Sascha.
>> I think its purpose to check if there is a ulpi transceiver out there,
>> although, ulpi defines a special scratch register for testing purposes.
> Correct, its primary purpose was to see whether the ULPI low-level
> communication works at all, and to bail out when it doesn't.
>
> I wasn't aware of that scratch register. If we don't need that table any
> more, feel free to drop it.

This driver is still in development stage and is not fully functional,
so I'd like to keep the table and ID info printing, but I really don't like
trivial IDs, like that of Eric.

How about something like the attached patch?

Eric, can you test if it works for you?

If it's good, I can send a prettier version of it for review and merging.

-- 
Regards,
Igor.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ulpi.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101005/87e8f647/attachment.ksh>

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Eric Bénard]

Hi Igor,

Le 05/10/2010 14:59, Igor Grinberg a ?crit :
> This driver is still in development stage and is not fully functional,
> so I'd like to keep the table and ID info printing, but I really don't like
> trivial IDs, like that of Eric.
>
> How about something like the attached patch?
>
> Eric, can you test if it works for you?
>
> If it's good, I can send a prettier version of it for review and merging.
>
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
ULPI transceiver vendor/product ID 0x0000/0x0000
ULPI integrity check: failed!

In fact, that's my fault, the internal PHY is not ULPI but UTMI so we 
should not use these functions here and it was working until now by 
chance as there are dedicated registers to control the internal PHY 
which are initialized with the right values as a default.

Sorry for the noise and forget my patch which is wrong, I'll rework the 
platform data as in the present case we should not provide use ulpi when 
using the internal PHY.

Eric

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Igor Grinberg]

 Hi,

On 10/05/10 17:12, Eric B?nard wrote:
> Hi Igor,
>
> Le 05/10/2010 14:59, Igor Grinberg a ?crit :
>> This driver is still in development stage and is not fully functional,
>> so I'd like to keep the table and ID info printing, but I really don't like
>> trivial IDs, like that of Eric.
>>
>> How about something like the attached patch?
>>
>> Eric, can you test if it works for you?
>>
>> If it's good, I can send a prettier version of it for review and merging.
>>
> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller
> ULPI transceiver vendor/product ID 0x0000/0x0000
> ULPI integrity check: failed!

Ah! It works ;)

>
> In fact, that's my fault, the internal PHY is not ULPI but UTMI so we should not use these functions here and it was working until now by chance as there are dedicated registers to control the internal PHY which are initialized with the right values as a default.

Too bad... I hope, I was useful in your discovery...

>
> Sorry for the noise and forget my patch which is wrong, I'll rework the platform data as in the present case we should not provide use ulpi when using the internal PHY.

OK.
Nevertheless, I want to resend a version of my patch properly.
It proves useful after all...
Any comments/suggestions will be appreciated.

-- 
Regards,
Igor.

[PATCH] ulpi: add i.MX25 & 35 internal PHY ID

[Eric Bénard]

Hi Igor,

Le 05/10/2010 17:42, Igor Grinberg a ?crit :
>> In fact, that's my fault, the internal PHY is not ULPI but UTMI so we should not use these functions here and it was working until now by chance as there are dedicated registers to control the internal PHY which are initialized with the right values as a default.
>
> Too bad... I hope, I was useful in your discovery...
>
yes as your patch would have worked if this was really an ULPI PHY ;-)
>>
>> Sorry for the noise and forget my patch which is wrong, I'll rework the platform data as in the present case we should not provide use ulpi when using the internal PHY.
>
> OK.
> Nevertheless, I want to resend a version of my patch properly.
> It proves useful after all...
> Any comments/suggestions will be appreciated.
>
I'll test it asap on other platforms really having ULPI.

Thanks
Eric

[PATCH] cpuimx25 & cpuimx35: fix OTG port registration in host mode

[Eric Bénard]

the PHY is UTMI so don't create an ULPI viewpoint.

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 arch/arm/mach-mx25/mach-cpuimx25.c |   14 ++++----------
 arch/arm/mach-mx3/mach-cpuimx35.c  |   14 ++++----------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-mx25/mach-cpuimx25.c b/arch/arm/mach-mx25/mach-cpuimx25.c
index e064bb3..c530c8a 100644
--- a/arch/arm/mach-mx25/mach-cpuimx25.c
+++ b/arch/arm/mach-mx25/mach-cpuimx25.c
@@ -41,7 +41,6 @@
 #include <mach/mxc_nand.h>
 #include <mach/imxfb.h>
 #include <mach/mxc_ehci.h>
-#include <mach/ulpi.h>
 #include <mach/iomux-mx25.h>
 
 #include "devices-imx25.h"
@@ -135,18 +134,13 @@ static void __init eukrea_cpuimx25_init(void)
 				ARRAY_SIZE(eukrea_cpuimx25_i2c_devices));
 	imx25_add_imx_i2c0(&eukrea_cpuimx25_i2c0_data);
 
-#if defined(CONFIG_USB_ULPI)
-	if (otg_mode_host) {
-		otg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops,
-				ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT);
-
+	if (otg_mode_host)
 		mxc_register_device(&mxc_otg, &otg_pdata);
-	}
-	mxc_register_device(&mxc_usbh2, &usbh2_pdata);
-#endif
-	if (!otg_mode_host)
+	else
 		mxc_register_device(&otg_udc_device, &otg_device_pdata);
 
+	mxc_register_device(&mxc_usbh2, &usbh2_pdata);
+
 #ifdef CONFIG_MACH_EUKREA_MBIMXSD25_BASEBOARD
 	eukrea_mbimxsd25_baseboard_init();
 #endif
diff --git a/arch/arm/mach-mx3/mach-cpuimx35.c b/arch/arm/mach-mx3/mach-cpuimx35.c
index d16bec8..8ba5321 100644
--- a/arch/arm/mach-mx3/mach-cpuimx35.c
+++ b/arch/arm/mach-mx3/mach-cpuimx35.c
@@ -44,7 +44,6 @@
 #include <mach/iomux-mx35.h>
 #include <mach/mxc_nand.h>
 #include <mach/mxc_ehci.h>
-#include <mach/ulpi.h>
 
 #include "devices-imx35.h"
 #include "devices.h"
@@ -168,18 +167,13 @@ static void __init mxc_board_init(void)
 			ARRAY_SIZE(eukrea_cpuimx35_i2c_devices));
 	imx35_add_imx_i2c0(&eukrea_cpuimx35_i2c0_data);
 
-#if defined(CONFIG_USB_ULPI)
-	if (otg_mode_host) {
-		otg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops,
-				ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT);
-
+	if (otg_mode_host)
 		mxc_register_device(&mxc_otg_host, &otg_pdata);
-	}
-	mxc_register_device(&mxc_usbh1, &usbh1_pdata);
-#endif
-	if (!otg_mode_host)
+	else
 		mxc_register_device(&mxc_otg_udc_device, &otg_device_pdata);
 
+	mxc_register_device(&mxc_usbh1, &usbh1_pdata);
+
 #ifdef CONFIG_MACH_EUKREA_MBIMXSD35_BASEBOARD
 	eukrea_mbimxsd35_baseboard_init();
 #endif
-- 
1.7.0.4