diff mbox

[U-Boot,v3,6/7] usb: ulpi: Extend the existing ulpi framework.

Message ID 1328276312-30153-7-git-send-email-govindraj.raja@ti.com
State Superseded, archived
Headers show

Commit Message

Govindraj.R Feb. 3, 2012, 1:38 p.m. UTC 
From: "Govindraj.R" <govindraj.raja@ti.com>

Extend the existing ulpi viewport framework
to pass the port number information for any ulpi
ops. Fix the usage of ulpi api's accordingly.

Tested-by: Stefano Babic <sbabic@denx.de>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 board/efikamx/efikamx-usb.c      |   24 ++++++++++------
 drivers/usb/ulpi/ulpi-viewport.c |   30 ++++++++++----------
 drivers/usb/ulpi/ulpi.c          |   54 +++++++++++++++++++------------------
 include/usb/ulpi.h               |   36 +++++++++++++++++--------
 4 files changed, 82 insertions(+), 62 deletions(-)

Comments

Igor Grinberg Feb. 6, 2012, 8:55 a.m. UTC | #1 
Hi Govindraj,

I was about to ask Tom to reorder the patches while applying, but
there are still several things to fix.
I'm also fine with fixing these by a follow up patch (after merging this).

There are several comments, you missed to fix from previous version [1].
Please, fix those.

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/123832

On 02/03/12 15:38, Govindraj.R wrote:
> From: "Govindraj.R" <govindraj.raja@ti.com>
> 
> Extend the existing ulpi viewport framework
> to pass the port number information for any ulpi
> ops. Fix the usage of ulpi api's accordingly.
> 
> Tested-by: Stefano Babic <sbabic@denx.de>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

After fixing the pointed issues:

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

> ---
>  board/efikamx/efikamx-usb.c      |   24 ++++++++++------
>  drivers/usb/ulpi/ulpi-viewport.c |   30 ++++++++++----------
>  drivers/usb/ulpi/ulpi.c          |   54 +++++++++++++++++++------------------
>  include/usb/ulpi.h               |   36 +++++++++++++++++--------
>  4 files changed, 82 insertions(+), 62 deletions(-)

[...]

> diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
> index 490fb0e..6f03f08 100644
> --- a/drivers/usb/ulpi/ulpi-viewport.c
> +++ b/drivers/usb/ulpi/ulpi-viewport.c

[...]

> -int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
> +int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
>  {
>  	u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);

You should utilize the port_num variable here, right?
something like:
val |= (ulpi_vp->port_num & 0x7) << 24;

>  
> -	return ulpi_request(ulpi_viewport, val);
> +	return ulpi_request(ulpi_vp, val);
>  }
>  
> -u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
> +u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg)
>  {
>  	int err;
>  	u32 val = ULPI_RWRUN | ((u32)reg << 16);

same here

>  
> -	err = ulpi_request(ulpi_viewport, val);
> +	err = ulpi_request(ulpi_vp, val);
>  	if (err)
>  		return err;
>  
> -	return (readl(ulpi_viewport) >> 8) & 0xff;
> +	return (readl(ulpi_vp->viewport_addr) >> 8) & 0xff;
>  }

[...]

> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index 802f077..a036bab 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -28,12 +28,23 @@
>  #endif
>  
>  /*
> + * ulpi view port address and
> + * Port_number that can be passed.
> + * Any additional data to be passed can
> + * be extended from this structure
> + */
> +struct ulpi_viewport {
> +	u32 viewport_addr;
> +	u8 port_num;
> +};

haven't we aggreed, that this will be:
u32 port_num;
?

> +
> +/*
>   * Initialize the ULPI transciever and check the interface integrity.
> - * @ulpi_viewport -  the address of the ULPI viewport register.
> + * @ulpi_viewport -  structure containing ULPI viewport data

This is ulpi_vp now.

[...]
Govindraj Feb. 6, 2012, 9:38 a.m. UTC | #2 
Hi Igor,

On Mon, Feb 6, 2012 at 2:25 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Govindraj,
>
> I was about to ask Tom to reorder the patches while applying, but
> there are still several things to fix.
> I'm also fine with fixing these by a follow up patch (after merging this).
>

[...]

>
> [...]
>
>> diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
>> index 490fb0e..6f03f08 100644
>> --- a/drivers/usb/ulpi/ulpi-viewport.c
>> +++ b/drivers/usb/ulpi/ulpi-viewport.c
>
> [...]
>
>> -int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
>> +int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
>>  {
>>       u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);
>
> You should utilize the port_num variable here, right?
> something like:
> val |= (ulpi_vp->port_num & 0x7) << 24;
>

Yes correct, Shouldn't we add something like this

[...]

#ifndef CONFIG_ULPI_PORT_SHIFT && CONFIG_ULPI_PORT_MASK
#define CONFIG_ULPI_PORT_SHIFT 24
#define CONFIG_ULPI_PORT_MASK 0x7
#endif

[...]

val |= (ulpi_vp->port_num & CONFIG_ULPI_PORT_MASK) <<
                    CONFIG_ULPI_PORT_SHIFT;


>>
>> -     return ulpi_request(ulpi_viewport, val);
>> +     return ulpi_request(ulpi_vp, val);
>>  }
>>
>> -u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
>> +u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg)
>>  {
>>       int err;
>>       u32 val = ULPI_RWRUN | ((u32)reg << 16);
>
> same here
>
>>
>> -     err = ulpi_request(ulpi_viewport, val);
>> +     err = ulpi_request(ulpi_vp, val);
>>       if (err)
>>               return err;
>>
>> -     return (readl(ulpi_viewport) >> 8) & 0xff;
>> +     return (readl(ulpi_vp->viewport_addr) >> 8) & 0xff;
>>  }
>
> [...]
>
>> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
>> index 802f077..a036bab 100644
>> --- a/include/usb/ulpi.h
>> +++ b/include/usb/ulpi.h
>> @@ -28,12 +28,23 @@
>>  #endif
>>
>>  /*
>> + * ulpi view port address and
>> + * Port_number that can be passed.
>> + * Any additional data to be passed can
>> + * be extended from this structure
>> + */
>> +struct ulpi_viewport {
>> +     u32 viewport_addr;
>> +     u8 port_num;
>> +};
>
> haven't we aggreed, that this will be:
> u32 port_num;

My bad I missed this will correct now.

> ?
>
>> +
>> +/*
>>   * Initialize the ULPI transciever and check the interface integrity.
>> - * @ulpi_viewport -  the address of the ULPI viewport register.
>> + * @ulpi_viewport -  structure containing ULPI viewport data
>
> This is ulpi_vp now.

will correct this.

--
Thanks,
Govindraj.R
Igor Grinberg Feb. 6, 2012, 10:03 a.m. UTC | #3 
On 02/06/12 11:38, Govindraj wrote:
> Hi Igor,
> 
> On Mon, Feb 6, 2012 at 2:25 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Hi Govindraj,
>>
>> I was about to ask Tom to reorder the patches while applying, but
>> there are still several things to fix.
>> I'm also fine with fixing these by a follow up patch (after merging this).
>>
> 
> [...]
> 
>>
>> [...]
>>
>>> diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
>>> index 490fb0e..6f03f08 100644
>>> --- a/drivers/usb/ulpi/ulpi-viewport.c
>>> +++ b/drivers/usb/ulpi/ulpi-viewport.c
>>
>> [...]
>>
>>> -int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
>>> +int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
>>>  {
>>>       u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);
>>
>> You should utilize the port_num variable here, right?
>> something like:
>> val |= (ulpi_vp->port_num & 0x7) << 24;
>>
> 
> Yes correct, Shouldn't we add something like this
> 
> [...]
> 
> #ifndef CONFIG_ULPI_PORT_SHIFT && CONFIG_ULPI_PORT_MASK
> #define CONFIG_ULPI_PORT_SHIFT 24
> #define CONFIG_ULPI_PORT_MASK 0x7
> #endif
> 
> [...]
> 
> val |= (ulpi_vp->port_num & CONFIG_ULPI_PORT_MASK) <<
>                     CONFIG_ULPI_PORT_SHIFT;

We have already discussed this in the previous session [1]
And the conclusion was to use plain values instead,
because otherwise, you should replace all other masks and shifts...
and that will be a mess...
IMO,
(ulpi_vp->port_num & 0x7) << 24;
is really self explanatory and intuitive, so replacing it with defines
just make the code look bad and split into multiple lines.
Also the values are viewport specific and will not change, so
there is no reason to make them a config option.

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/124222

> 
> 
>>>
>>> -     return ulpi_request(ulpi_viewport, val);
>>> +     return ulpi_request(ulpi_vp, val);
>>>  }
>>>
>>> -u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
>>> +u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg)
>>>  {
>>>       int err;
>>>       u32 val = ULPI_RWRUN | ((u32)reg << 16);
>>
>> same here
>>
>>>
>>> -     err = ulpi_request(ulpi_viewport, val);
>>> +     err = ulpi_request(ulpi_vp, val);
>>>       if (err)
>>>               return err;
>>>
>>> -     return (readl(ulpi_viewport) >> 8) & 0xff;
>>> +     return (readl(ulpi_vp->viewport_addr) >> 8) & 0xff;
>>>  }

[...]
SUBHASHINI MANNE Feb. 8, 2012, 5:42 p.m. UTC | #4 
Govindraj,
 I have Gumstix overo board with OM3730 + USB 3326 USB PHY.I want to add EHCI 
and ULPI support in u-boot.

 Once we port the EHCI_OMAP  EHCI driver How to test this driver functionality 
under u-boot environment?
  Is there any application/commands to  test ULPI functionality under u-boot 
environment?

Regards,
Subhashini
Govindraj Feb. 9, 2012, 6:32 a.m. UTC | #5 
Hi Subhashini,

On Wed, Feb 8, 2012 at 11:12 PM, SUBHASHINI MANNE <subbusiddu@gmail.com> wrote:
>
> Govindraj,
>  I have Gumstix overo board with OM3730 + USB 3326 USB PHY.I want to add EHCI
> and ULPI support in u-boot.

You can refer to the v4 patch series posted earlier:
http://www.mail-archive.com/u-boot@lists.denx.de/msg76860.html

This patch series adds support for omap4_panda using the ehci-omap framework,
You can refer to panda board file change on how to do it.

Or you also refer to this patch adding support for OMAP3: TAM3517

http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/124645

Something similar needs to be done for your board file to support ehci.

>
>  Once we port the EHCI_OMAP  EHCI driver How to test this driver functionality
> under u-boot environment?

usb start;
usb info;
usb tree;

some commands to check if your downstream device is getting enumerated.
refer to usb help in u-boot for usage.

you can also use fatls and other use case if you have a hub connected
to you root port or you can also check for tftpboot if you have
ethernet controller
over ehci (like in panda or beagle).

care to update your config file for usb ehci enabling
(refer to v4 patch series as done for panda/beagle)

>  Is there any application/commands to  test ULPI functionality under u-boot
> environment?
>

what kind of ulpi functionality do you want to test omap-ulpi-viewport
is basically for the ulpi phy access from INSNREG05_ULPI from ehci
reg map to configure and debug phy interface.
(chapter 22.2.2.3 ULPI Interfaces from omap36xx TRM v1.x vW)

You can refer to OMAP TRM chapter 22.2 High-Speed USB Host Subsystem
for reference.

--
Thanks,
Govindraj.R
diff mbox

Patch

diff --git a/board/efikamx/efikamx-usb.c b/board/efikamx/efikamx-usb.c
index 840bd9a..ac2d2e9 100644
--- a/board/efikamx/efikamx-usb.c
+++ b/board/efikamx/efikamx-usb.c
@@ -120,6 +120,7 @@  static void efika_ehci_init(struct usb_ehci *ehci, uint32_t stp_gpio,
 {
 	int ret;
 	struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+	struct ulpi_viewport ulpi_vp;
 
 	mxc_request_iomux(stp_gpio, alt0);
 	mxc_iomux_set_pad(stp_gpio, PAD_CTL_DRV_HIGH |
@@ -133,23 +134,26 @@  static void efika_ehci_init(struct usb_ehci *ehci, uint32_t stp_gpio,
 	mxc_iomux_set_pad(stp_gpio, USB_PAD_CONFIG);
 	udelay(10000);
 
-	ret = ulpi_init((u32)&ehci->ulpi_viewpoint);
+	ulpi_vp.viewport_addr = (u32)&ehci->ulpi_viewpoint;
+	ulpi_vp.port_num = 0;
+
+	ret = ulpi_init(&ulpi_vp);
 	if (ret) {
 		printf("Efika USB ULPI initialization failed\n");
 		return;
 	}
 
 	/* ULPI set flags */
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->otg_ctrl,
+	ulpi_write(&ulpi_vp, &ulpi->otg_ctrl,
 			ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN |
 			ULPI_OTG_EXTVBUSIND);
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->function_ctrl,
+	ulpi_write(&ulpi_vp, &ulpi->function_ctrl,
 			ULPI_FC_FULL_SPEED | ULPI_FC_OPMODE_NORMAL |
 			ULPI_FC_SUSPENDM);
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->iface_ctrl, 0);
+	ulpi_write(&ulpi_vp, &ulpi->iface_ctrl, 0);
 
 	/* Set VBus */
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->otg_ctrl_set,
+	ulpi_write(&ulpi_vp, &ulpi->otg_ctrl_set,
 			ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT);
 
 	/*
@@ -158,8 +162,7 @@  static void efika_ehci_init(struct usb_ehci *ehci, uint32_t stp_gpio,
 	 * NOTE: This violates USB specification, but otherwise, USB on Efika
 	 * doesn't work.
 	 */
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->otg_ctrl_set,
-			ULPI_OTG_CHRGVBUS);
+	ulpi_write(&ulpi_vp, &ulpi->otg_ctrl_set, ULPI_OTG_CHRGVBUS);
 }
 
 int board_ehci_hcd_init(int port)
@@ -177,9 +180,12 @@  void ehci_powerup_fixup(uint32_t *status_reg, uint32_t *reg)
 	uint32_t port = OTG_BASE_ADDR + (0x200 * CONFIG_MXC_USB_PORT);
 	struct usb_ehci *ehci = (struct usb_ehci *)port;
 	struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+	struct ulpi_viewport ulpi_vp;
+
+	ulpi_vp.viewport_addr = (u32)&ehci->ulpi_viewpoint;
+	ulpi_vp.port_num = 0;
 
-	ulpi_write((u32)&ehci->ulpi_viewpoint, &ulpi->otg_ctrl_set,
-			ULPI_OTG_CHRGVBUS);
+	ulpi_write(&ulpi_vp, &ulpi->otg_ctrl_set, ULPI_OTG_CHRGVBUS);
 
 	wait_ms(50);
 
diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
index 490fb0e..6f03f08 100644
--- a/drivers/usb/ulpi/ulpi-viewport.c
+++ b/drivers/usb/ulpi/ulpi-viewport.c
@@ -40,13 +40,13 @@ 
  *
  * returns 0 on mask match, ULPI_ERROR on time out.
  */
-static int ulpi_wait(u32 ulpi_viewport, u32 mask)
+static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask)
 {
 	int timeout = CONFIG_USB_ULPI_TIMEOUT;
 
 	/* Wait for the bits in mask to become zero. */
 	while (--timeout) {
-		if ((readl(ulpi_viewport) & mask) == 0)
+		if ((readl(ulpi_vp->viewport_addr) & mask) == 0)
 			return 0;
 
 		udelay(1);
@@ -60,16 +60,16 @@  static int ulpi_wait(u32 ulpi_viewport, u32 mask)
  *
  * returns 0 on success.
  */
-static int ulpi_wakeup(u32 ulpi_viewport)
+static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp)
 {
 	int err;
 
-	if (readl(ulpi_viewport) & ULPI_SS)
+	if (readl(ulpi_vp->viewport_addr) & ULPI_SS)
 		return 0; /* already awake */
 
-	writel(ULPI_WU, ulpi_viewport);
+	writel(ULPI_WU, ulpi_vp->viewport_addr);
 
-	err = ulpi_wait(ulpi_viewport, ULPI_WU);
+	err = ulpi_wait(ulpi_vp, ULPI_WU);
 	if (err)
 		printf("ULPI wakeup timed out\n");
 
@@ -81,38 +81,38 @@  static int ulpi_wakeup(u32 ulpi_viewport)
  *
  * @value - the ULPI request
  */
-static int ulpi_request(u32 ulpi_viewport, u32 value)
+static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value)
 {
 	int err;
 
-	err = ulpi_wakeup(ulpi_viewport);
+	err = ulpi_wakeup(ulpi_vp);
 	if (err)
 		return err;
 
-	writel(value, ulpi_viewport);
+	writel(value, ulpi_vp->viewport_addr);
 
-	err = ulpi_wait(ulpi_viewport, ULPI_RWRUN);
+	err = ulpi_wait(ulpi_vp, ULPI_RWRUN);
 	if (err)
 		printf("ULPI request timed out\n");
 
 	return err;
 }
 
-int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
+int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
 {
 	u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);
 
-	return ulpi_request(ulpi_viewport, val);
+	return ulpi_request(ulpi_vp, val);
 }
 
-u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
+u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg)
 {
 	int err;
 	u32 val = ULPI_RWRUN | ((u32)reg << 16);
 
-	err = ulpi_request(ulpi_viewport, val);
+	err = ulpi_request(ulpi_vp, val);
 	if (err)
 		return err;
 
-	return (readl(ulpi_viewport) >> 8) & 0xff;
+	return (readl(ulpi_vp->viewport_addr) >> 8) & 0xff;
 }
diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
index 6202227..dde2585 100644
--- a/drivers/usb/ulpi/ulpi.c
+++ b/drivers/usb/ulpi/ulpi.c
@@ -37,18 +37,18 @@ 
 
 static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
 
-static int ulpi_integrity_check(u32 ulpi_viewport)
+static int ulpi_integrity_check(struct ulpi_viewport *ulpi_vp)
 {
 	u32 val, tval = ULPI_TEST_VALUE;
 	int err, i;
 
 	/* Use the 'special' test value to check all bits */
 	for (i = 0; i < 2; i++, tval <<= 1) {
-		err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);
+		err = ulpi_write(ulpi_vp, &ulpi->scratch, tval);
 		if (err)
 			return err;
 
-		val = ulpi_read(ulpi_viewport, &ulpi->scratch);
+		val = ulpi_read(ulpi_vp, &ulpi->scratch);
 		if (val != tval) {
 			printf("ULPI integrity check failed\n");
 			return val;
@@ -58,7 +58,7 @@  static int ulpi_integrity_check(u32 ulpi_viewport)
 	return 0;
 }
 
-int ulpi_init(u32 ulpi_viewport)
+int ulpi_init(struct ulpi_viewport *ulpi_vp)
 {
 	u32 val, id = 0;
 	u8 *reg = &ulpi->product_id_high;
@@ -66,7 +66,7 @@  int ulpi_init(u32 ulpi_viewport)
 
 	/* Assemble ID from four ULPI ID registers (8 bits each). */
 	for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {
-		val = ulpi_read(ulpi_viewport, reg - i);
+		val = ulpi_read(ulpi_vp, reg - i);
 		if (val == ULPI_ERROR)
 			return val;
 
@@ -76,10 +76,10 @@  int ulpi_init(u32 ulpi_viewport)
 	/* Split ID into vendor and product ID. */
 	debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id & 0xffff);
 
-	return ulpi_integrity_check(ulpi_viewport);
+	return ulpi_integrity_check(ulpi_vp);
 }
 
-int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
+int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)
 {
 	u32 tspeed = ULPI_FC_FULL_SPEED;
 	u32 val;
@@ -96,17 +96,18 @@  int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
 			"falling back to full speed\n", __func__, speed);
 	}
 
-	val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+	val = ulpi_read(ulpi_vp, &ulpi->function_ctrl);
 	if (val == ULPI_ERROR)
 		return val;
 
 	/* clear the previous speed setting */
 	val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;
 
-	return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+	return ulpi_write(ulpi_vp, &ulpi->function_ctrl, val);
 }
 
-int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind)
+int ulpi_set_vbus(struct ulpi_viewport *ulpi_vp, int on, int ext_power,
+			int ext_ind)
 {
 	u32 flags = ULPI_OTG_DRVVBUS;
 	u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
@@ -116,18 +117,18 @@  int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind)
 	if (ext_ind)
 		flags |= ULPI_OTG_EXTVBUSIND;
 
-	return ulpi_write(ulpi_viewport, reg, flags);
+	return ulpi_write(ulpi_vp, reg, flags);
 }
 
-int ulpi_set_pd(u32 ulpi_viewport, int enable)
+int ulpi_set_pd(struct ulpi_viewport *ulpi_vp, int enable)
 {
 	u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;
 	u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
 
-	return ulpi_write(ulpi_viewport, reg, val);
+	return ulpi_write(ulpi_vp, reg, val);
 }
 
-int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
+int ulpi_opmode_sel(struct ulpi_viewport *ulpi_vp, unsigned opmode)
 {
 	u32 topmode = ULPI_FC_OPMODE_NORMAL;
 	u32 val;
@@ -144,17 +145,17 @@  int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
 			"falling back to OpMode Normal\n", __func__, opmode);
 	}
 
-	val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+	val = ulpi_read(ulpi_vp, &ulpi->function_ctrl);
 	if (val == ULPI_ERROR)
 		return val;
 
 	/* clear the previous opmode setting */
 	val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;
 
-	return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+	return ulpi_write(ulpi_vp, &ulpi->function_ctrl, val);
 }
 
-int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
+int ulpi_serial_mode_enable(struct ulpi_viewport *ulpi_vp, unsigned smode)
 {
 	switch (smode) {
 	case ULPI_IFACE_6_PIN_SERIAL_MODE:
@@ -166,14 +167,14 @@  int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
 		return ULPI_ERROR;
 	}
 
-	return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);
+	return ulpi_write(ulpi_vp, &ulpi->iface_ctrl_set, smode);
 }
 
-int ulpi_suspend(u32 ulpi_viewport)
+int ulpi_suspend(struct ulpi_viewport *ulpi_vp)
 {
 	int err;
 
-	err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
+	err = ulpi_write(ulpi_vp, &ulpi->function_ctrl_clear,
 			ULPI_FC_SUSPENDM);
 	if (err)
 		printf("ULPI: %s: failed writing the suspend bit\n", __func__);
@@ -186,7 +187,7 @@  int ulpi_suspend(u32 ulpi_viewport)
  * Actual wait for reset must be done in a view port specific way,
  * because it involves checking the DIR line.
  */
-static int __ulpi_reset_wait(u32 ulpi_viewport)
+static int __ulpi_reset_wait(struct ulpi_viewport *ulpi_vp)
 {
 	u32 val;
 	int timeout = CONFIG_USB_ULPI_TIMEOUT;
@@ -199,7 +200,7 @@  static int __ulpi_reset_wait(u32 ulpi_viewport)
 		 * for the error of ulpi_read(), if there is one, then
 		 * there will be a timeout.
 		 */
-		val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+		val = ulpi_read(ulpi_vp, &ulpi->function_ctrl);
 		if (!(val & ULPI_FC_RESET))
 			return 0;
 
@@ -210,18 +211,19 @@  static int __ulpi_reset_wait(u32 ulpi_viewport)
 
 	return ULPI_ERROR;
 }
-int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
+int ulpi_reset_wait(struct ulpi_viewport *ulpi_vp)
+	__attribute__((weak, alias("__ulpi_reset_wait")));
 
-int ulpi_reset(u32 ulpi_viewport)
+int ulpi_reset(struct ulpi_viewport *ulpi_vp)
 {
 	int err;
 
-	err = ulpi_write(ulpi_viewport,
+	err = ulpi_write(ulpi_vp,
 			&ulpi->function_ctrl_set, ULPI_FC_RESET);
 	if (err) {
 		printf("ULPI: %s: failed writing reset bit\n", __func__);
 		return err;
 	}
 
-	return ulpi_reset_wait(ulpi_viewport);
+	return ulpi_reset_wait(ulpi_vp);
 }
diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
index 802f077..a036bab 100644
--- a/include/usb/ulpi.h
+++ b/include/usb/ulpi.h
@@ -28,12 +28,23 @@ 
 #endif
 
 /*
+ * ulpi view port address and
+ * Port_number that can be passed.
+ * Any additional data to be passed can
+ * be extended from this structure
+ */
+struct ulpi_viewport {
+	u32 viewport_addr;
+	u8 port_num;
+};
+
+/*
  * Initialize the ULPI transciever and check the interface integrity.
- * @ulpi_viewport -  the address of the ULPI viewport register.
+ * @ulpi_viewport -  structure containing ULPI viewport data
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_init(u32 ulpi_viewport);
+int ulpi_init(struct ulpi_viewport *ulpi_vp);
 
 /*
  * Select transceiver speed.
@@ -41,7 +52,7 @@  int ulpi_init(u32 ulpi_viewport);
  *                ULPI_FC_LOW_SPEED,  ULPI_FC_FS4LS
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
+int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);
 
 /*
  * Enable/disable VBUS.
@@ -50,14 +61,15 @@  int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_enable_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind);
+int ulpi_enable_vbus(struct ulpi_viewport *ulpi_vp,
+			int on, int ext_power, int ext_ind);
 
 /*
  * Enable/disable pull-down resistors on D+ and D- USB lines.
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_set_pd(u32 ulpi_viewport, int enable);
+int ulpi_set_pd(struct ulpi_viewport *ulpi_vp, int enable);
 
 /*
  * Select OpMode.
@@ -66,7 +78,7 @@  int ulpi_set_pd(u32 ulpi_viewport, int enable);
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
+int ulpi_opmode_sel(struct ulpi_viewport *ulpi_vp, unsigned opmode);
 
 /*
  * Switch to Serial Mode.
@@ -78,7 +90,7 @@  int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
  * Switches immediately to Serial Mode.
  * To return from Serial Mode, STP line needs to be asserted.
  */
-int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
+int ulpi_serial_mode_enable(struct ulpi_viewport *ulpi_vp, unsigned smode);
 
 /*
  * Put PHY into low power mode.
@@ -89,14 +101,14 @@  int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
  * STP line must be driven low to keep the PHY in suspend.
  * To resume the PHY, STP line needs to be asserted.
  */
-int ulpi_suspend(u32 ulpi_viewport);
+int ulpi_suspend(struct ulpi_viewport *ulpi_vp);
 
 /*
  * Reset the transceiver. ULPI interface and registers are not affected.
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_reset(u32 ulpi_viewport);
+int ulpi_reset(struct ulpi_viewport *ulpi_vp);
 
 
 /* ULPI access methods below must be implemented for each ULPI viewport. */
@@ -108,7 +120,7 @@  int ulpi_reset(u32 ulpi_viewport);
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
+int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value);
 
 /*
  * Read the ULPI PHY register content via the viewport.
@@ -116,14 +128,14 @@  int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
  *
  * returns register content on success, ULPI_ERROR on failure.
  */
-u32 ulpi_read(u32 ulpi_viewport, u8 *reg);
+u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg);
 
 /*
  * Wait for the reset to complete.
  * The Link must not attempt to access the PHY until the reset has
  * completed and DIR line is de-asserted.
  */
-int ulpi_reset_wait(u32 ulpi_viewport);
+int ulpi_reset_wait(struct ulpi_viewport *ulpi_vp);
 
 /* Access Extended Register Set (indicator) */
 #define ACCESS_EXT_REGS_OFFSET	0x2f	/* read-write */