All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI
@ 2011-12-12 10:08 Igor Grinberg
  2011-12-12 10:08 ` [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned Igor Grinberg
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Igor Grinberg @ 2011-12-12 10:08 UTC (permalink / raw)
  To: u-boot

This series encorporates comments from Simon Glass [1] regarding
the arguments types and return error types used in generic ULPI code.
Adds a documentation for the new CONFIG_* options.
In addition it improves the verbosity of the error path.

[1]     http://www.mail-archive.com/u-boot at lists.denx.de/msg72060.html

Igor Grinberg (4):
  USB: ULPI: switch argument type from u8 to unsigned
  USB: ULPI: clean a mixup of return types
  USB: ULPI: increase error case verbosity
  README: add documentation for CONFIG_USB_ULPI*

 README                           |    8 ++++++++
 drivers/usb/ulpi/ulpi-viewport.c |    4 ++--
 drivers/usb/ulpi/ulpi.c          |   30 +++++++++++++++---------------
 include/usb/ulpi.h               |    8 ++++----
 4 files changed, 29 insertions(+), 21 deletions(-)

-- 
1.7.3.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned
  2011-12-12 10:08 [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI Igor Grinberg
@ 2011-12-12 10:08 ` Igor Grinberg
  2011-12-14  0:25   ` Simon Glass
  2011-12-12 10:08 ` [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types Igor Grinberg
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Igor Grinberg @ 2011-12-12 10:08 UTC (permalink / raw)
  To: u-boot

There is no benefit in usign u8, so switch to unsinged to reduce the
binary image size (by 20 bytes).

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 drivers/usb/ulpi/ulpi.c |   10 +++++-----
 include/usb/ulpi.h      |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
index 805e29d..1371aa6 100644
--- a/drivers/usb/ulpi/ulpi.c
+++ b/drivers/usb/ulpi/ulpi.c
@@ -79,9 +79,9 @@ int ulpi_init(u32 ulpi_viewport)
 	return ulpi_integrity_check(ulpi_viewport);
 }
 
-int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
+int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
 {
-	u8 tspeed = ULPI_FC_FULL_SPEED;
+	u32 tspeed = ULPI_FC_FULL_SPEED;
 	u32 val;
 
 	switch (speed) {
@@ -127,9 +127,9 @@ int ulpi_set_pd(u32 ulpi_viewport, int enable)
 	return ulpi_write(ulpi_viewport, reg, val);
 }
 
-int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
+int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
 {
-	u8 topmode = ULPI_FC_OPMODE_NORMAL;
+	u32 topmode = ULPI_FC_OPMODE_NORMAL;
 	u32 val;
 
 	switch (opmode) {
@@ -154,7 +154,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
 	return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
 }
 
-int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
+int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
 {
 	switch (smode) {
 	case ULPI_IFACE_6_PIN_SERIAL_MODE:
diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
index d871290..dc78a59 100644
--- a/include/usb/ulpi.h
+++ b/include/usb/ulpi.h
@@ -41,7 +41,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, u8 speed);
+int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
 
 /*
  * Enable/disable VBUS.
@@ -66,7 +66,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, u8 opmode);
+int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
 
 /*
  * Switch to Serial Mode.
@@ -78,7 +78,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 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, u8 smode);
+int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
 
 /*
  * Put PHY into low power mode.
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types
  2011-12-12 10:08 [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI Igor Grinberg
  2011-12-12 10:08 ` [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned Igor Grinberg
@ 2011-12-12 10:08 ` Igor Grinberg
  2011-12-14  0:26   ` Simon Glass
  2011-12-16 20:04   ` Remy Bohmer
  2011-12-12 10:08 ` [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity Igor Grinberg
  2011-12-12 10:08 ` [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI* Igor Grinberg
  3 siblings, 2 replies; 20+ messages in thread
From: Igor Grinberg @ 2011-12-12 10:08 UTC (permalink / raw)
  To: u-boot

Clean a mixup between u32 and int as a return type
for functions returning error values.
Use int as it is native (and widely used) return type.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 drivers/usb/ulpi/ulpi-viewport.c |    4 ++--
 drivers/usb/ulpi/ulpi.c          |    8 ++++----
 include/usb/ulpi.h               |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
index fa2e004..490fb0e 100644
--- a/drivers/usb/ulpi/ulpi-viewport.c
+++ b/drivers/usb/ulpi/ulpi-viewport.c
@@ -98,7 +98,7 @@ static int ulpi_request(u32 ulpi_viewport, u32 value)
 	return err;
 }
 
-u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
+int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
 {
 	u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);
 
@@ -107,7 +107,7 @@ u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
 
 u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
 {
-	u32 err;
+	int err;
 	u32 val = ULPI_RWRUN | ((u32)reg << 16);
 
 	err = ulpi_request(ulpi_viewport, val);
diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
index 1371aa6..f3f293f 100644
--- a/drivers/usb/ulpi/ulpi.c
+++ b/drivers/usb/ulpi/ulpi.c
@@ -39,8 +39,8 @@ static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
 
 static int ulpi_integrity_check(u32 ulpi_viewport)
 {
-	u32 err, val, tval = ULPI_TEST_VALUE;
-	int i;
+	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) {
@@ -171,7 +171,7 @@ int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
 
 int ulpi_suspend(u32 ulpi_viewport)
 {
-	u32 err;
+	int err;
 
 	err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
 			ULPI_FC_SUSPENDM);
@@ -214,7 +214,7 @@ int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
 
 int ulpi_reset(u32 ulpi_viewport)
 {
-	u32 err;
+	int err;
 
 	err = ulpi_write(ulpi_viewport,
 			&ulpi->function_ctrl_set, ULPI_FC_RESET);
diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
index dc78a59..802f077 100644
--- a/include/usb/ulpi.h
+++ b/include/usb/ulpi.h
@@ -108,7 +108,7 @@ int ulpi_reset(u32 ulpi_viewport);
  *
  * returns 0 on success, ULPI_ERROR on failure.
  */
-u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
+int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
 
 /*
  * Read the ULPI PHY register content via the viewport.
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity
  2011-12-12 10:08 [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI Igor Grinberg
  2011-12-12 10:08 ` [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned Igor Grinberg
  2011-12-12 10:08 ` [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types Igor Grinberg
@ 2011-12-12 10:08 ` Igor Grinberg
  2011-12-14  0:27   ` Simon Glass
  2011-12-16 20:06   ` Remy Bohmer
  2011-12-12 10:08 ` [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI* Igor Grinberg
  3 siblings, 2 replies; 20+ messages in thread
From: Igor Grinberg @ 2011-12-12 10:08 UTC (permalink / raw)
  To: u-boot

Add the argument value to the error message.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 drivers/usb/ulpi/ulpi.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
index f3f293f..6202227 100644
--- a/drivers/usb/ulpi/ulpi.c
+++ b/drivers/usb/ulpi/ulpi.c
@@ -92,8 +92,8 @@ int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
 		tspeed = speed;
 		break;
 	default:
-		printf("ULPI: %s: wrong transceiver speed specified, "
-			"falling back to full speed\n", __func__);
+		printf("ULPI: %s: wrong transceiver speed specified: %u, "
+			"falling back to full speed\n", __func__, speed);
 	}
 
 	val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
@@ -140,8 +140,8 @@ int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
 		topmode = opmode;
 		break;
 	default:
-		printf("ULPI: %s: wrong OpMode specified, "
-			"falling back to OpMode Normal\n", __func__);
+		printf("ULPI: %s: wrong OpMode specified: %u, "
+			"falling back to OpMode Normal\n", __func__, opmode);
 	}
 
 	val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
@@ -161,8 +161,8 @@ int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
 	case ULPI_IFACE_3_PIN_SERIAL_MODE:
 		break;
 	default:
-		printf("ULPI: %s: unrecognized Serial Mode specified\n",
-			__func__);
+		printf("ULPI: %s: unrecognized Serial Mode specified: %u\n",
+			__func__, smode);
 		return ULPI_ERROR;
 	}
 
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-12 10:08 [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI Igor Grinberg
                   ` (2 preceding siblings ...)
  2011-12-12 10:08 ` [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity Igor Grinberg
@ 2011-12-12 10:08 ` Igor Grinberg
  2011-12-14  0:28   ` Simon Glass
  3 siblings, 1 reply; 20+ messages in thread
From: Igor Grinberg @ 2011-12-12 10:08 UTC (permalink / raw)
  To: u-boot

Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
configuration options.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 README |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/README b/README
index ff72e47..6fe1e0f 100644
--- a/README
+++ b/README
@@ -1185,6 +1185,14 @@ The following options need to be configured:
 			for your device
 			- CONFIG_USBD_PRODUCTID 0xFFFF
 
+- ULPI Layer Support:
+		The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
+		the generic ULPI layer. The generic layer accesses the ULPI PHY
+		via the platform viewport, so you need both the genric layer and
+		the viewport enabled. Currently only Chipidea/ARC based
+		viewport is supported.
+		To enable the ULPI layer support, define CONFIG_USB_ULPI and
+		CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
 
 - MMC Support:
 		The MMC controller on the Intel PXA is supported. To
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned
  2011-12-12 10:08 ` [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned Igor Grinberg
@ 2011-12-14  0:25   ` Simon Glass
  2011-12-14  6:16     ` [U-Boot] [PATCH v2 " Igor Grinberg
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2011-12-14  0:25 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> There is no benefit in usign u8, so switch to unsinged to reduce the
> binary image size (by 20 bytes).
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>

Some typos in commit msg, but anyway:

Acked-by: Simon Glass <sjg@chromium.org>

> ---
> ?drivers/usb/ulpi/ulpi.c | ? 10 +++++-----
> ?include/usb/ulpi.h ? ? ?| ? ?6 +++---
> ?2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> index 805e29d..1371aa6 100644
> --- a/drivers/usb/ulpi/ulpi.c
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -79,9 +79,9 @@ int ulpi_init(u32 ulpi_viewport)
> ? ? ? ?return ulpi_integrity_check(ulpi_viewport);
> ?}
>
> -int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
> +int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
> ?{
> - ? ? ? u8 tspeed = ULPI_FC_FULL_SPEED;
> + ? ? ? u32 tspeed = ULPI_FC_FULL_SPEED;
> ? ? ? ?u32 val;
>
> ? ? ? ?switch (speed) {
> @@ -127,9 +127,9 @@ int ulpi_set_pd(u32 ulpi_viewport, int enable)
> ? ? ? ?return ulpi_write(ulpi_viewport, reg, val);
> ?}
>
> -int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
> +int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
> ?{
> - ? ? ? u8 topmode = ULPI_FC_OPMODE_NORMAL;
> + ? ? ? u32 topmode = ULPI_FC_OPMODE_NORMAL;
> ? ? ? ?u32 val;
>
> ? ? ? ?switch (opmode) {
> @@ -154,7 +154,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
> ? ? ? ?return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
> ?}
>
> -int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
> +int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
> ?{
> ? ? ? ?switch (smode) {
> ? ? ? ?case ULPI_IFACE_6_PIN_SERIAL_MODE:
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index d871290..dc78a59 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -41,7 +41,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, u8 speed);
> +int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
>
> ?/*
> ?* Enable/disable VBUS.
> @@ -66,7 +66,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, u8 opmode);
> +int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
>
> ?/*
> ?* Switch to Serial Mode.
> @@ -78,7 +78,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 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, u8 smode);
> +int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
>
> ?/*
> ?* Put PHY into low power mode.
> --
> 1.7.3.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types
  2011-12-12 10:08 ` [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types Igor Grinberg
@ 2011-12-14  0:26   ` Simon Glass
  2011-12-16 20:04   ` Remy Bohmer
  1 sibling, 0 replies; 20+ messages in thread
From: Simon Glass @ 2011-12-14  0:26 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Clean a mixup between u32 and int as a return type
> for functions returning error values.
> Use int as it is native (and widely used) return type.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>

Acked-by: Simon Glass <sjg@chromium.org>

> ---
> ?drivers/usb/ulpi/ulpi-viewport.c | ? ?4 ++--
> ?drivers/usb/ulpi/ulpi.c ? ? ? ? ?| ? ?8 ++++----
> ?include/usb/ulpi.h ? ? ? ? ? ? ? | ? ?2 +-
> ?3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
> index fa2e004..490fb0e 100644
> --- a/drivers/usb/ulpi/ulpi-viewport.c
> +++ b/drivers/usb/ulpi/ulpi-viewport.c
> @@ -98,7 +98,7 @@ static int ulpi_request(u32 ulpi_viewport, u32 value)
> ? ? ? ?return err;
> ?}
>
> -u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
> +int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
> ?{
> ? ? ? ?u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff);
>
> @@ -107,7 +107,7 @@ u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value)
>
> ?u32 ulpi_read(u32 ulpi_viewport, u8 *reg)
> ?{
> - ? ? ? u32 err;
> + ? ? ? int err;
> ? ? ? ?u32 val = ULPI_RWRUN | ((u32)reg << 16);
>
> ? ? ? ?err = ulpi_request(ulpi_viewport, val);
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> index 1371aa6..f3f293f 100644
> --- a/drivers/usb/ulpi/ulpi.c
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -39,8 +39,8 @@ static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
>
> ?static int ulpi_integrity_check(u32 ulpi_viewport)
> ?{
> - ? ? ? u32 err, val, tval = ULPI_TEST_VALUE;
> - ? ? ? int i;
> + ? ? ? 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) {
> @@ -171,7 +171,7 @@ int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
>
> ?int ulpi_suspend(u32 ulpi_viewport)
> ?{
> - ? ? ? u32 err;
> + ? ? ? int err;
>
> ? ? ? ?err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
> ? ? ? ? ? ? ? ? ? ? ? ?ULPI_FC_SUSPENDM);
> @@ -214,7 +214,7 @@ int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
>
> ?int ulpi_reset(u32 ulpi_viewport)
> ?{
> - ? ? ? u32 err;
> + ? ? ? int err;
>
> ? ? ? ?err = ulpi_write(ulpi_viewport,
> ? ? ? ? ? ? ? ? ? ? ? ?&ulpi->function_ctrl_set, ULPI_FC_RESET);
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index dc78a59..802f077 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -108,7 +108,7 @@ int ulpi_reset(u32 ulpi_viewport);
> ?*
> ?* returns 0 on success, ULPI_ERROR on failure.
> ?*/
> -u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
> +int ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value);
>
> ?/*
> ?* Read the ULPI PHY register content via the viewport.
> --
> 1.7.3.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity
  2011-12-12 10:08 ` [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity Igor Grinberg
@ 2011-12-14  0:27   ` Simon Glass
  2011-12-16 20:06   ` Remy Bohmer
  1 sibling, 0 replies; 20+ messages in thread
From: Simon Glass @ 2011-12-14  0:27 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Add the argument value to the error message.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>

Acked-by: Simon Glass <sjg@chromium.org>

(I do wonder if this should be debug() rather than printf(), but that's fine)

> ---
> ?drivers/usb/ulpi/ulpi.c | ? 12 ++++++------
> ?1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> index f3f293f..6202227 100644
> --- a/drivers/usb/ulpi/ulpi.c
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -92,8 +92,8 @@ int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
> ? ? ? ? ? ? ? ?tspeed = speed;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?default:
> - ? ? ? ? ? ? ? printf("ULPI: %s: wrong transceiver speed specified, "
> - ? ? ? ? ? ? ? ? ? ? ? "falling back to full speed\n", __func__);
> + ? ? ? ? ? ? ? printf("ULPI: %s: wrong transceiver speed specified: %u, "
> + ? ? ? ? ? ? ? ? ? ? ? "falling back to full speed\n", __func__, speed);
> ? ? ? ?}
>
> ? ? ? ?val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
> @@ -140,8 +140,8 @@ int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
> ? ? ? ? ? ? ? ?topmode = opmode;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?default:
> - ? ? ? ? ? ? ? printf("ULPI: %s: wrong OpMode specified, "
> - ? ? ? ? ? ? ? ? ? ? ? "falling back to OpMode Normal\n", __func__);
> + ? ? ? ? ? ? ? printf("ULPI: %s: wrong OpMode specified: %u, "
> + ? ? ? ? ? ? ? ? ? ? ? "falling back to OpMode Normal\n", __func__, opmode);
> ? ? ? ?}
>
> ? ? ? ?val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
> @@ -161,8 +161,8 @@ int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
> ? ? ? ?case ULPI_IFACE_3_PIN_SERIAL_MODE:
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?default:
> - ? ? ? ? ? ? ? printf("ULPI: %s: unrecognized Serial Mode specified\n",
> - ? ? ? ? ? ? ? ? ? ? ? __func__);
> + ? ? ? ? ? ? ? printf("ULPI: %s: unrecognized Serial Mode specified: %u\n",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, smode);
> ? ? ? ? ? ? ? ?return ULPI_ERROR;
> ? ? ? ?}
>
> --
> 1.7.3.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-12 10:08 ` [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI* Igor Grinberg
@ 2011-12-14  0:28   ` Simon Glass
  2011-12-14  5:51     ` Igor Grinberg
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2011-12-14  0:28 UTC (permalink / raw)
  To: u-boot

Hi Igor,

On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
> configuration options.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> ?README | ? ?8 ++++++++
> ?1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/README b/README
> index ff72e47..6fe1e0f 100644
> --- a/README
> +++ b/README
> @@ -1185,6 +1185,14 @@ The following options need to be configured:
> ? ? ? ? ? ? ? ? ? ? ? ?for your device
> ? ? ? ? ? ? ? ? ? ? ? ?- CONFIG_USBD_PRODUCTID 0xFFFF
>
> +- ULPI Layer Support:
> + ? ? ? ? ? ? ? The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
> + ? ? ? ? ? ? ? the generic ULPI layer. The generic layer accesses the ULPI PHY
> + ? ? ? ? ? ? ? via the platform viewport, so you need both the genric layer and
> + ? ? ? ? ? ? ? the viewport enabled. Currently only Chipidea/ARC based
> + ? ? ? ? ? ? ? viewport is supported.

Where does it say that only this one is supported in the code? What is
specific to that device?

Regards,
Simon

> + ? ? ? ? ? ? ? To enable the ULPI layer support, define CONFIG_USB_ULPI and
> + ? ? ? ? ? ? ? CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>
> ?- MMC Support:
> ? ? ? ? ? ? ? ?The MMC controller on the Intel PXA is supported. To
> --
> 1.7.3.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-14  0:28   ` Simon Glass
@ 2011-12-14  5:51     ` Igor Grinberg
  2011-12-14 19:26       ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Igor Grinberg @ 2011-12-14  5:51 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 12/14/11 02:28, Simon Glass wrote:
> Hi Igor,
> 
> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>> configuration options.
>>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>>  README |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/README b/README
>> index ff72e47..6fe1e0f 100644
>> --- a/README
>> +++ b/README
>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>                        for your device
>>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>>
>> +- ULPI Layer Support:
>> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
>> +               via the platform viewport, so you need both the genric layer and
>> +               the viewport enabled. Currently only Chipidea/ARC based
>> +               viewport is supported.
> 
> Where does it say that only this one is supported in the code?

You mean comments or the code?

> What is specific to that device?

The viewport bits? It is not a part of the ULPI spec.
Other vendors do not have to comply with those.
For example PXA310 has those bits placed and named in some other way...

> 
> Regards,
> Simon
> 
>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>
>>  - MMC Support:
>>                The MMC controller on the Intel PXA is supported. To
>> --
>> 1.7.3.4
>>
> 

-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2 1/4] USB: ULPI: switch argument type from u8 to unsigned
  2011-12-14  0:25   ` Simon Glass
@ 2011-12-14  6:16     ` Igor Grinberg
  2011-12-16  1:18       ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Igor Grinberg @ 2011-12-14  6:16 UTC (permalink / raw)
  To: u-boot

There is no benefit in using u8, so switch to unsigned to reduce the
binary image size (by 20 bytes).

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
Acked-by: Simon Glass <sjg@chromium.org>
---
v2:	no functional changes - fix typos in the commit message
	and add Simon's ack.

 drivers/usb/ulpi/ulpi.c |   10 +++++-----
 include/usb/ulpi.h      |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
index 805e29d..1371aa6 100644
--- a/drivers/usb/ulpi/ulpi.c
+++ b/drivers/usb/ulpi/ulpi.c
@@ -79,9 +79,9 @@ int ulpi_init(u32 ulpi_viewport)
 	return ulpi_integrity_check(ulpi_viewport);
 }
 
-int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
+int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
 {
-	u8 tspeed = ULPI_FC_FULL_SPEED;
+	u32 tspeed = ULPI_FC_FULL_SPEED;
 	u32 val;
 
 	switch (speed) {
@@ -127,9 +127,9 @@ int ulpi_set_pd(u32 ulpi_viewport, int enable)
 	return ulpi_write(ulpi_viewport, reg, val);
 }
 
-int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
+int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
 {
-	u8 topmode = ULPI_FC_OPMODE_NORMAL;
+	u32 topmode = ULPI_FC_OPMODE_NORMAL;
 	u32 val;
 
 	switch (opmode) {
@@ -154,7 +154,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
 	return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
 }
 
-int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
+int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
 {
 	switch (smode) {
 	case ULPI_IFACE_6_PIN_SERIAL_MODE:
diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
index d871290..dc78a59 100644
--- a/include/usb/ulpi.h
+++ b/include/usb/ulpi.h
@@ -41,7 +41,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, u8 speed);
+int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
 
 /*
  * Enable/disable VBUS.
@@ -66,7 +66,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, u8 opmode);
+int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
 
 /*
  * Switch to Serial Mode.
@@ -78,7 +78,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 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, u8 smode);
+int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
 
 /*
  * Put PHY into low power mode.
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-14  5:51     ` Igor Grinberg
@ 2011-12-14 19:26       ` Simon Glass
  2011-12-15  9:06         ` Igor Grinberg
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2011-12-14 19:26 UTC (permalink / raw)
  To: u-boot

Hi Igor,

On Tue, Dec 13, 2011 at 9:51 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 12/14/11 02:28, Simon Glass wrote:
>> Hi Igor,
>>
>> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>>> configuration options.
>>>
>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>> ---
>>> ?README | ? ?8 ++++++++
>>> ?1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/README b/README
>>> index ff72e47..6fe1e0f 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>> ? ? ? ? ? ? ? ? ? ? ? ?for your device
>>> ? ? ? ? ? ? ? ? ? ? ? ?- CONFIG_USBD_PRODUCTID 0xFFFF
>>>
>>> +- ULPI Layer Support:
>>> + ? ? ? ? ? ? ? The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>>> + ? ? ? ? ? ? ? the generic ULPI layer. The generic layer accesses the ULPI PHY
>>> + ? ? ? ? ? ? ? via the platform viewport, so you need both the genric layer and
>>> + ? ? ? ? ? ? ? the viewport enabled. Currently only Chipidea/ARC based
>>> + ? ? ? ? ? ? ? viewport is supported.
>>
>> Where does it say that only this one is supported in the code?
>
> You mean comments or the code?

Well the filename seems generic and not specific to that chip. Are
viewports something that other chips can support?

COBJS-$(CONFIG_USB_ULPI)		+= ulpi.o
COBJS-$(CONFIG_USB_ULPI_VIEWPORT)	+= ulpi-viewport.o

It would be good if you could mention the two new CONFIG options in the README.

>
>> What is specific to that device?
>
> The viewport bits? It is not a part of the ULPI spec.
> Other vendors do not have to comply with those.
> For example PXA310 has those bits placed and named in some other way...

OK I didn't realise that.

>>> + ? ? ? ? ? ? ? To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>> + ? ? ? ? ? ? ? CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>>
>>> ?- MMC Support:
>>> ? ? ? ? ? ? ? ?The MMC controller on the Intel PXA is supported. To
>>> --
>>> 1.7.3.4
>>>
>>
>
> --
> Regards,
> Igor.

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-14 19:26       ` Simon Glass
@ 2011-12-15  9:06         ` Igor Grinberg
  2011-12-16 20:10           ` Remy Bohmer
  0 siblings, 1 reply; 20+ messages in thread
From: Igor Grinberg @ 2011-12-15  9:06 UTC (permalink / raw)
  To: u-boot

On 12/14/11 21:26, Simon Glass wrote:
> Hi Igor,
> 
> On Tue, Dec 13, 2011 at 9:51 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Hi Simon,
>>
>> On 12/14/11 02:28, Simon Glass wrote:
>>> Hi Igor,
>>>
>>> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>>>> configuration options.
>>>>
>>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>>> ---
>>>>  README |    8 ++++++++
>>>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/README b/README
>>>> index ff72e47..6fe1e0f 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>>>                        for your device
>>>>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>>>>
>>>> +- ULPI Layer Support:
>>>> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>>>> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
>>>> +               via the platform viewport, so you need both the genric layer and
>>>> +               the viewport enabled. Currently only Chipidea/ARC based
>>>> +               viewport is supported.
>>>
>>> Where does it say that only this one is supported in the code?
>>
>> You mean comments or the code?
> 
> Well the filename seems generic and not specific to that chip. Are
> viewports something that other chips can support?

Let me clarify:
1) It is not the chip it is the controller (IP block) inside the SoC.
2) viewport is just the register name inside the SoC that provides
   and interface of the controller to access the ULPI PHY.

I think every SoC that uses that controller has the viewport setup
this way, but I might be wrong (and that's why the viewport is
separated from the generic ULPI spec implementation).

Regarding the name... yeah it could be renamed, but it follows Linux
naming currently and it is the first one submitted,
so IMO it can be named that generically.

> 
> COBJS-$(CONFIG_USB_ULPI)		+= ulpi.o
> COBJS-$(CONFIG_USB_ULPI_VIEWPORT)	+= ulpi-viewport.o
> 
> It would be good if you could mention the two new CONFIG options in the README.

I did, see below...

> 
>>
>>> What is specific to that device?
>>
>> The viewport bits? It is not a part of the ULPI spec.
>> Other vendors do not have to comply with those.
>> For example PXA310 has those bits placed and named in some other way...
> 
> OK I didn't realise that.

I think same stand for OMAP, but I'm not sure.
OMAP still does arbitrary register writes for accessing ULPI.

> 
>>>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.

Here the configs are documented.
I admit, it is not that brilliant documentation...


-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2 1/4] USB: ULPI: switch argument type from u8 to unsigned
  2011-12-14  6:16     ` [U-Boot] [PATCH v2 " Igor Grinberg
@ 2011-12-16  1:18       ` Simon Glass
  2011-12-16 20:02         ` Remy Bohmer
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2011-12-16  1:18 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 13, 2011 at 10:16 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> There is no benefit in using u8, so switch to unsigned to reduce the
> binary image size (by 20 bytes).
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> Acked-by: Simon Glass <sjg@chromium.org>

Looks good thanks

> ---
> v2: ? ? no functional changes - fix typos in the commit message
> ? ? ? ?and add Simon's ack.
>
> ?drivers/usb/ulpi/ulpi.c | ? 10 +++++-----
> ?include/usb/ulpi.h ? ? ?| ? ?6 +++---
> ?2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> index 805e29d..1371aa6 100644
> --- a/drivers/usb/ulpi/ulpi.c
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -79,9 +79,9 @@ int ulpi_init(u32 ulpi_viewport)
> ? ? ? ?return ulpi_integrity_check(ulpi_viewport);
> ?}
>
> -int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
> +int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed)
> ?{
> - ? ? ? u8 tspeed = ULPI_FC_FULL_SPEED;
> + ? ? ? u32 tspeed = ULPI_FC_FULL_SPEED;
> ? ? ? ?u32 val;
>
> ? ? ? ?switch (speed) {
> @@ -127,9 +127,9 @@ int ulpi_set_pd(u32 ulpi_viewport, int enable)
> ? ? ? ?return ulpi_write(ulpi_viewport, reg, val);
> ?}
>
> -int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
> +int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode)
> ?{
> - ? ? ? u8 topmode = ULPI_FC_OPMODE_NORMAL;
> + ? ? ? u32 topmode = ULPI_FC_OPMODE_NORMAL;
> ? ? ? ?u32 val;
>
> ? ? ? ?switch (opmode) {
> @@ -154,7 +154,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
> ? ? ? ?return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
> ?}
>
> -int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
> +int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode)
> ?{
> ? ? ? ?switch (smode) {
> ? ? ? ?case ULPI_IFACE_6_PIN_SERIAL_MODE:
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index d871290..dc78a59 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -41,7 +41,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, u8 speed);
> +int ulpi_select_transceiver(u32 ulpi_viewport, unsigned speed);
>
> ?/*
> ?* Enable/disable VBUS.
> @@ -66,7 +66,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, u8 opmode);
> +int ulpi_opmode_sel(u32 ulpi_viewport, unsigned opmode);
>
> ?/*
> ?* Switch to Serial Mode.
> @@ -78,7 +78,7 @@ int ulpi_opmode_sel(u32 ulpi_viewport, u8 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, u8 smode);
> +int ulpi_serial_mode_enable(u32 ulpi_viewport, unsigned smode);
>
> ?/*
> ?* Put PHY into low power mode.
> --
> 1.7.3.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2 1/4] USB: ULPI: switch argument type from u8 to unsigned
  2011-12-16  1:18       ` Simon Glass
@ 2011-12-16 20:02         ` Remy Bohmer
  0 siblings, 0 replies; 20+ messages in thread
From: Remy Bohmer @ 2011-12-16 20:02 UTC (permalink / raw)
  To: u-boot

Hi,

2011/12/16 Simon Glass <sjg@chromium.org>:
> On Tue, Dec 13, 2011 at 10:16 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> There is no benefit in using u8, so switch to unsigned to reduce the
>> binary image size (by 20 bytes).
>>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> Acked-by: Simon Glass <sjg@chromium.org>
>
> Looks good thanks
>
>> ---
>> v2: ? ? no functional changes - fix typos in the commit message
>> ? ? ? ?and add Simon's ack.
>>
>> ?drivers/usb/ulpi/ulpi.c | ? 10 +++++-----
>> ?include/usb/ulpi.h ? ? ?| ? ?6 +++---
>> ?2 files changed, 8 insertions(+), 8 deletions(-)

Applied to u-boot-usb.

Thanks.

Remy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types
  2011-12-12 10:08 ` [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types Igor Grinberg
  2011-12-14  0:26   ` Simon Glass
@ 2011-12-16 20:04   ` Remy Bohmer
  1 sibling, 0 replies; 20+ messages in thread
From: Remy Bohmer @ 2011-12-16 20:04 UTC (permalink / raw)
  To: u-boot

Hi,

2011/12/12 Igor Grinberg <grinberg@compulab.co.il>:
> Clean a mixup between u32 and int as a return type
> for functions returning error values.
> Use int as it is native (and widely used) return type.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> ?drivers/usb/ulpi/ulpi-viewport.c | ? ?4 ++--
> ?drivers/usb/ulpi/ulpi.c ? ? ? ? ?| ? ?8 ++++----
> ?include/usb/ulpi.h ? ? ? ? ? ? ? | ? ?2 +-
> ?3 files changed, 7 insertions(+), 7 deletions(-)

Applied to u-boot-usb. Thanks.

Kind regards,

Remy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity
  2011-12-12 10:08 ` [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity Igor Grinberg
  2011-12-14  0:27   ` Simon Glass
@ 2011-12-16 20:06   ` Remy Bohmer
  1 sibling, 0 replies; 20+ messages in thread
From: Remy Bohmer @ 2011-12-16 20:06 UTC (permalink / raw)
  To: u-boot

Hi,

2011/12/12 Igor Grinberg <grinberg@compulab.co.il>:
> Add the argument value to the error message.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> ?drivers/usb/ulpi/ulpi.c | ? 12 ++++++------
> ?1 files changed, 6 insertions(+), 6 deletions(-)

Applied to -u-boot-usb. Thanks.

Kind regards,

Remy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-15  9:06         ` Igor Grinberg
@ 2011-12-16 20:10           ` Remy Bohmer
  2011-12-16 20:29             ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Remy Bohmer @ 2011-12-16 20:10 UTC (permalink / raw)
  To: u-boot

Hi Igor,

2011/12/15 Igor Grinberg <grinberg@compulab.co.il>:
>>>> Where does it say that only this one is supported in the code?
>>>
>>> You mean comments or the code?
>>
>> Well the filename seems generic and not specific to that chip. Are
>> viewports something that other chips can support?
>
> Let me clarify:
> 1) It is not the chip it is the controller (IP block) inside the SoC.
> 2) viewport is just the register name inside the SoC that provides
> ? and interface of the controller to access the ULPI PHY.
>
> I think every SoC that uses that controller has the viewport setup
> this way, but I might be wrong (and that's why the viewport is
> separated from the generic ULPI spec implementation).
>
> Regarding the name... yeah it could be renamed, but it follows Linux
> naming currently and it is the first one submitted,
> so IMO it can be named that generically.
>
>>
>> COBJS-$(CONFIG_USB_ULPI) ? ? ? ? ? ? ?+= ulpi.o
>> COBJS-$(CONFIG_USB_ULPI_VIEWPORT) ? ? += ulpi-viewport.o
>>
>> It would be good if you could mention the two new CONFIG options in the README.
>
> I did, see below...
>
>>
>>>
>>>> What is specific to that device?
>>>
>>> The viewport bits? It is not a part of the ULPI spec.
>>> Other vendors do not have to comply with those.
>>> For example PXA310 has those bits placed and named in some other way...
>>
>> OK I didn't realise that.
>
> I think same stand for OMAP, but I'm not sure.
> OMAP still does arbitrary register writes for accessing ULPI.
>
>>
>>>>> + ? ? ? ? ? ? ? To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>>> + ? ? ? ? ? ? ? CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>
> Here the configs are documented.
> I admit, it is not that brilliant documentation...

Are you planning to post an update of this patch? The rest of the
series I already pulled into the USB tree.

Kind regards,

Remy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-16 20:10           ` Remy Bohmer
@ 2011-12-16 20:29             ` Simon Glass
  2011-12-16 20:35               ` Remy Bohmer
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2011-12-16 20:29 UTC (permalink / raw)
  To: u-boot

Hi Remy,

On Fri, Dec 16, 2011 at 12:10 PM, Remy Bohmer <linux@bohmer.net> wrote:
> Hi Igor,
>
> 2011/12/15 Igor Grinberg <grinberg@compulab.co.il>:
>>>>> Where does it say that only this one is supported in the code?
>>>>
>>>> You mean comments or the code?
>>>
>>> Well the filename seems generic and not specific to that chip. Are
>>> viewports something that other chips can support?
>>
>> Let me clarify:
>> 1) It is not the chip it is the controller (IP block) inside the SoC.
>> 2) viewport is just the register name inside the SoC that provides
>> ? and interface of the controller to access the ULPI PHY.
>>
>> I think every SoC that uses that controller has the viewport setup
>> this way, but I might be wrong (and that's why the viewport is
>> separated from the generic ULPI spec implementation).
>>
>> Regarding the name... yeah it could be renamed, but it follows Linux
>> naming currently and it is the first one submitted,
>> so IMO it can be named that generically.
>>
>>>
>>> COBJS-$(CONFIG_USB_ULPI) ? ? ? ? ? ? ?+= ulpi.o
>>> COBJS-$(CONFIG_USB_ULPI_VIEWPORT) ? ? += ulpi-viewport.o
>>>
>>> It would be good if you could mention the two new CONFIG options in the README.
>>
>> I did, see below...
>>
>>>
>>>>
>>>>> What is specific to that device?
>>>>
>>>> The viewport bits? It is not a part of the ULPI spec.
>>>> Other vendors do not have to comply with those.
>>>> For example PXA310 has those bits placed and named in some other way...
>>>
>>> OK I didn't realise that.
>>
>> I think same stand for OMAP, but I'm not sure.
>> OMAP still does arbitrary register writes for accessing ULPI.
>>
>>>
>>>>>> + ? ? ? ? ? ? ? To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>>>> + ? ? ? ? ? ? ? CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>
>> Here the configs are documented.
>> I admit, it is not that brilliant documentation...
>
> Are you planning to post an update of this patch? The rest of the
> series I already pulled into the USB tree.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI*
  2011-12-16 20:29             ` Simon Glass
@ 2011-12-16 20:35               ` Remy Bohmer
  0 siblings, 0 replies; 20+ messages in thread
From: Remy Bohmer @ 2011-12-16 20:35 UTC (permalink / raw)
  To: u-boot

Hi,

2011/12/16 Simon Glass <sjg@chromium.org>:
> Hi Remy,
>
> On Fri, Dec 16, 2011 at 12:10 PM, Remy Bohmer <linux@bohmer.net> wrote:
>> Hi Igor,
>>
>>> Here the configs are documented.
>>> I admit, it is not that brilliant documentation...
>>
>> Are you planning to post an update of this patch? The rest of the
>> series I already pulled into the USB tree.
>
> From my point of view it is fine as is.

In that case i will pull it in. Thanks.

Kind regards,

Remy

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-12-16 20:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-12 10:08 [U-Boot] [PATCH 0/4] Cleanups and updates for ULPI Igor Grinberg
2011-12-12 10:08 ` [U-Boot] [PATCH 1/4] USB: ULPI: switch argument type from u8 to unsigned Igor Grinberg
2011-12-14  0:25   ` Simon Glass
2011-12-14  6:16     ` [U-Boot] [PATCH v2 " Igor Grinberg
2011-12-16  1:18       ` Simon Glass
2011-12-16 20:02         ` Remy Bohmer
2011-12-12 10:08 ` [U-Boot] [PATCH 2/4] USB: ULPI: clean a mixup of return types Igor Grinberg
2011-12-14  0:26   ` Simon Glass
2011-12-16 20:04   ` Remy Bohmer
2011-12-12 10:08 ` [U-Boot] [PATCH 3/4] USB: ULPI: increase error case verbosity Igor Grinberg
2011-12-14  0:27   ` Simon Glass
2011-12-16 20:06   ` Remy Bohmer
2011-12-12 10:08 ` [U-Boot] [PATCH 4/4] README: add documentation for CONFIG_USB_ULPI* Igor Grinberg
2011-12-14  0:28   ` Simon Glass
2011-12-14  5:51     ` Igor Grinberg
2011-12-14 19:26       ` Simon Glass
2011-12-15  9:06         ` Igor Grinberg
2011-12-16 20:10           ` Remy Bohmer
2011-12-16 20:29             ` Simon Glass
2011-12-16 20:35               ` Remy Bohmer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.