[U-Boot] [PATCH 0/4] Cleanups and updates for ULPI
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@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(-)
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.
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
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.
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
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
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.
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
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
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; }
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
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
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
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
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 viathe generic ULPI layer. The generic layer accesses the ULPI PHYvia the platform viewport, so you need both the genric layer andthe viewport enabled. Currently only Chipidea/ARC basedviewport 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 andCONFIG_USB_ULPI_VIEWPORT in your board configuration file.
- MMC Support: The MMC controller on the Intel PXA is supported. To
-- 1.7.3.4
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
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 viathe generic ULPI layer. The generic layer accesses the ULPI PHYvia the platform viewport, so you need both the genric layer andthe viewport enabled. Currently only Chipidea/ARC basedviewport 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 andCONFIG_USB_ULPI_VIEWPORT in your board configuration file.
Here the configs are documented. I admit, it is not that brilliant documentation...
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:
- It is not the chip it is the controller (IP block) inside the SoC.
- 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
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:
- It is not the chip it is the controller (IP block) inside the SoC.
- 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.
From my point of view it is fine as is.
Regards, Simon
Kind regards,
Remy
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
participants (3)
-
Igor Grinberg -
Remy Bohmer -
Simon Glass