* [PATCH 0/3] Generic ULPI driver extention
@ 2010-07-15 13:00 Igor Grinberg
2010-07-15 13:00 ` [PATCH 1/3] usb/otg/ulpi: remove unused macro Igor Grinberg
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Igor Grinberg @ 2010-07-15 13:00 UTC (permalink / raw)
To: linux-arm-kernel
This patch series add support for SMSC USB3319 ulpi phy,
introduces the ulpi specific flags for control over the
ulpi phy and extends the generic ulpi driver with
support for Function and Interface control of upli phy.
Depends on:
From: Ajay Kumar Gupta <ajay.gupta@ti.com>
Date: Thu, 8 Jul 2010 14:03:01 +0530
Subject: USB: ulpi: fix compilation warning
From: Eric B?nard <eric@eukrea.com>
Date: Thu, 15 Jul 2010 09:20:19 +0200
Subject: [PATCH v2] otg/ulpi.c : fix register write
Igor Grinberg (3):
usb/otg/ulpi: remove unused macro
usb/otg/ulpi: add support for SMSC USB3319 ulpi phy
usb/otg/ulpi: extend the generic ulpi driver.
arch/arm/mach-mx2/mach-pca100.c | 4 +-
arch/arm/mach-mx3/mach-armadillo5x0.c | 4 +-
arch/arm/mach-mx3/mach-mx31lilly.c | 4 +-
arch/arm/mach-mx3/mach-mx31lite.c | 2 +-
arch/arm/mach-mx3/mach-mx31moboard.c | 2 +-
arch/arm/mach-mx3/mach-pcm037.c | 4 +-
arch/arm/mach-mx3/mach-pcm043.c | 2 +-
arch/arm/mach-mx3/mx31moboard-smartbot.c | 2 +-
drivers/usb/otg/Kconfig | 2 -
drivers/usb/otg/ulpi.c | 130 +++++++++++++++++++++++++++---
include/linux/usb/otg.h | 7 --
include/linux/usb/ulpi.h | 39 +++++++++
12 files changed, 169 insertions(+), 33 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] usb/otg/ulpi: remove unused macro 2010-07-15 13:00 [PATCH 0/3] Generic ULPI driver extention Igor Grinberg @ 2010-07-15 13:00 ` Igor Grinberg 2010-07-15 13:00 ` [PATCH 2/3] usb/otg/ulpi: add support for SMSC USB3319 ulpi phy Igor Grinberg ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Igor Grinberg @ 2010-07-15 13:00 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- drivers/usb/otg/ulpi.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c index 10a1df6..448d643 100644 --- a/drivers/usb/otg/ulpi.c +++ b/drivers/usb/otg/ulpi.c @@ -31,8 +31,6 @@ #define ULPI_ID(vendor, product) (((vendor) << 16) | (product)) -#define TR_FLAG(flags, a, b) (((flags) & a) ? b : 0) - /* ULPI hardcoded IDs, used for probing */ static unsigned int ulpi_ids[] = { ULPI_ID(0x04cc, 0x1504), /* NXP ISP1504 */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] usb/otg/ulpi: add support for SMSC USB3319 ulpi phy 2010-07-15 13:00 [PATCH 0/3] Generic ULPI driver extention Igor Grinberg 2010-07-15 13:00 ` [PATCH 1/3] usb/otg/ulpi: remove unused macro Igor Grinberg @ 2010-07-15 13:00 ` Igor Grinberg 2010-07-15 13:00 ` [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver Igor Grinberg 2010-07-21 14:47 ` [PATCH 0/3] Generic ULPI driver extention Igor Grinberg 3 siblings, 0 replies; 11+ messages in thread From: Igor Grinberg @ 2010-07-15 13:00 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- drivers/usb/otg/Kconfig | 2 -- drivers/usb/otg/ulpi.c | 1 + 2 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig index 3d2d3e5..3b12895 100644 --- a/drivers/usb/otg/Kconfig +++ b/drivers/usb/otg/Kconfig @@ -49,8 +49,6 @@ config USB_ULPI Enable this to support ULPI connected USB OTG transceivers which are likely found on embedded boards. - The only chip currently supported is NXP's ISP1504 - config TWL4030_USB tristate "TWL4030 USB Transceiver Driver" depends on TWL4030_CORE && REGULATOR_TWL4030 diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c index 448d643..ef7dbe4 100644 --- a/drivers/usb/otg/ulpi.c +++ b/drivers/usb/otg/ulpi.c @@ -34,6 +34,7 @@ /* ULPI hardcoded IDs, used for probing */ static unsigned int ulpi_ids[] = { ULPI_ID(0x04cc, 0x1504), /* NXP ISP1504 */ + ULPI_ID(0x0424, 0x0006), /* SMSC USB3319 */ }; static int ulpi_set_flags(struct otg_transceiver *otg) -- 1.7.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-07-15 13:00 [PATCH 0/3] Generic ULPI driver extention Igor Grinberg 2010-07-15 13:00 ` [PATCH 1/3] usb/otg/ulpi: remove unused macro Igor Grinberg 2010-07-15 13:00 ` [PATCH 2/3] usb/otg/ulpi: add support for SMSC USB3319 ulpi phy Igor Grinberg @ 2010-07-15 13:00 ` Igor Grinberg 2010-08-13 13:27 ` Heikki Krogerus 2010-07-21 14:47 ` [PATCH 0/3] Generic ULPI driver extention Igor Grinberg 3 siblings, 1 reply; 11+ messages in thread From: Igor Grinberg @ 2010-07-15 13:00 UTC (permalink / raw) To: linux-arm-kernel 1) Introduce ulpi specific flags for control of the ulpi phy 2) Extend the generic ulpi driver with support for Function and Interface control of upli phy 3) Update the platforms using the generic ulpi driver with new ulpi flags 4) Remove the otg control flags not in use Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- arch/arm/mach-mx2/mach-pca100.c | 4 +- arch/arm/mach-mx3/mach-armadillo5x0.c | 4 +- arch/arm/mach-mx3/mach-mx31lilly.c | 4 +- arch/arm/mach-mx3/mach-mx31lite.c | 2 +- arch/arm/mach-mx3/mach-mx31moboard.c | 2 +- arch/arm/mach-mx3/mach-pcm037.c | 4 +- arch/arm/mach-mx3/mach-pcm043.c | 2 +- arch/arm/mach-mx3/mx31moboard-smartbot.c | 2 +- drivers/usb/otg/ulpi.c | 127 +++++++++++++++++++++++++++--- include/linux/usb/otg.h | 7 -- include/linux/usb/ulpi.h | 39 +++++++++ 11 files changed, 168 insertions(+), 29 deletions(-) diff --git a/arch/arm/mach-mx2/mach-pca100.c b/arch/arm/mach-mx2/mach-pca100.c index a87422e..09f0d7c 100644 --- a/arch/arm/mach-mx2/mach-pca100.c +++ b/arch/arm/mach-mx2/mach-pca100.c @@ -357,13 +357,13 @@ static void __init pca100_init(void) #if defined(CONFIG_USB_ULPI) if (otg_mode_host) { otg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_otg_host, &otg_pdata); } usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_usbh2, &usbh2_pdata); #endif diff --git a/arch/arm/mach-mx3/mach-armadillo5x0.c b/arch/arm/mach-mx3/mach-armadillo5x0.c index 5f72ec9..b400686 100644 --- a/arch/arm/mach-mx3/mach-armadillo5x0.c +++ b/arch/arm/mach-mx3/mach-armadillo5x0.c @@ -552,9 +552,9 @@ static void __init armadillo5x0_init(void) /* USB */ #if defined(CONFIG_USB_ULPI) usbotg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_otg_host, &usbotg_pdata); mxc_register_device(&mxc_usbh2, &usbh2_pdata); diff --git a/arch/arm/mach-mx3/mach-mx31lilly.c b/arch/arm/mach-mx3/mach-mx31lilly.c index b2c7f51..6187988 100644 --- a/arch/arm/mach-mx3/mach-mx31lilly.c +++ b/arch/arm/mach-mx3/mach-mx31lilly.c @@ -249,9 +249,9 @@ static struct mxc_usbh_platform_data usbh2_pdata = { static void lilly1131_usb_init(void) { usbotg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_usbh1, &usbh1_pdata); mxc_register_device(&mxc_usbh2, &usbh2_pdata); diff --git a/arch/arm/mach-mx3/mach-mx31lite.c b/arch/arm/mach-mx3/mach-mx31lite.c index 2b6d114..4bee3c5 100644 --- a/arch/arm/mach-mx3/mach-mx31lite.c +++ b/arch/arm/mach-mx3/mach-mx31lite.c @@ -261,7 +261,7 @@ static void __init mxc_board_init(void) #if defined(CONFIG_USB_ULPI) /* USB */ usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_usbh2, &usbh2_pdata); #endif diff --git a/arch/arm/mach-mx3/mach-mx31moboard.c b/arch/arm/mach-mx3/mach-mx31moboard.c index 62b5e40..8821672 100644 --- a/arch/arm/mach-mx3/mach-mx31moboard.c +++ b/arch/arm/mach-mx3/mach-mx31moboard.c @@ -405,7 +405,7 @@ static struct mxc_usbh_platform_data usbh2_pdata = { static int __init moboard_usbh2_init(void) { usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); return mxc_register_device(&mxc_usbh2, &usbh2_pdata); } diff --git a/arch/arm/mach-mx3/mach-pcm037.c b/arch/arm/mach-mx3/mach-pcm037.c index cce4106..9718688 100644 --- a/arch/arm/mach-mx3/mach-pcm037.c +++ b/arch/arm/mach-mx3/mach-pcm037.c @@ -658,13 +658,13 @@ static void __init mxc_board_init(void) #if defined(CONFIG_USB_ULPI) if (otg_mode_host) { otg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_otg_host, &otg_pdata); } usbh2_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_usbh2, &usbh2_pdata); #endif diff --git a/arch/arm/mach-mx3/mach-pcm043.c b/arch/arm/mach-mx3/mach-pcm043.c index 78d9185..5c80242 100644 --- a/arch/arm/mach-mx3/mach-pcm043.c +++ b/arch/arm/mach-mx3/mach-pcm043.c @@ -380,7 +380,7 @@ static void __init mxc_board_init(void) #if defined(CONFIG_USB_ULPI) if (otg_mode_host) { otg_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); mxc_register_device(&mxc_otg_host, &otg_pdata); } diff --git a/arch/arm/mach-mx3/mx31moboard-smartbot.c b/arch/arm/mach-mx3/mx31moboard-smartbot.c index 293eea6..53c9ad5 100644 --- a/arch/arm/mach-mx3/mx31moboard-smartbot.c +++ b/arch/arm/mach-mx3/mx31moboard-smartbot.c @@ -138,7 +138,7 @@ static struct mxc_usbh_platform_data otg_host_pdata = { static int __init smartbot_otg_host_init(void) { otg_host_pdata.otg = otg_ulpi_create(&mxc_ulpi_access_ops, - USB_OTG_DRV_VBUS | USB_OTG_DRV_VBUS_EXT); + ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT); return mxc_register_device(&mxc_otg_host, &otg_host_pdata); } diff --git a/drivers/usb/otg/ulpi.c b/drivers/usb/otg/ulpi.c index ef7dbe4..ccc8195 100644 --- a/drivers/usb/otg/ulpi.c +++ b/drivers/usb/otg/ulpi.c @@ -37,25 +37,106 @@ static unsigned int ulpi_ids[] = { ULPI_ID(0x0424, 0x0006), /* SMSC USB3319 */ }; -static int ulpi_set_flags(struct otg_transceiver *otg) +static int ulpi_set_otg_flags(struct otg_transceiver *otg) { - unsigned int flags = 0; + unsigned int flags = ULPI_OTG_CTRL_DP_PULLDOWN | + ULPI_OTG_CTRL_DM_PULLDOWN; - if (otg->flags & USB_OTG_PULLUP_ID) + if (otg->flags & ULPI_OTG_ID_PULLUP) flags |= ULPI_OTG_CTRL_ID_PULLUP; - if (otg->flags & USB_OTG_PULLDOWN_DM) - flags |= ULPI_OTG_CTRL_DM_PULLDOWN; + /* + * ULPI Specification rev.1.1 default + * for Dp/DmPulldown is enabled. + */ + if (otg->flags & ULPI_OTG_DP_PULLDOWN_DIS) + flags &= ~ULPI_OTG_CTRL_DP_PULLDOWN; - if (otg->flags & USB_OTG_PULLDOWN_DP) - flags |= ULPI_OTG_CTRL_DP_PULLDOWN; + if (otg->flags & ULPI_OTG_DM_PULLDOWN_DIS) + flags &= ~ULPI_OTG_CTRL_DM_PULLDOWN; - if (otg->flags & USB_OTG_EXT_VBUS_INDICATOR) + if (otg->flags & ULPI_OTG_EXTVBUSIND) flags |= ULPI_OTG_CTRL_EXTVBUSIND; return otg_io_write(otg, flags, ULPI_OTG_CTRL); } +static int ulpi_set_fc_flags(struct otg_transceiver *otg) +{ + unsigned int flags = 0; + + /* + * ULPI Specification rev.1.1 default + * for XcvrSelect is Full Speed. + */ + if (otg->flags & ULPI_FC_HS) + flags |= ULPI_FUNC_CTRL_HIGH_SPEED; + else if (otg->flags & ULPI_FC_LS) + flags |= ULPI_FUNC_CTRL_LOW_SPEED; + else if (otg->flags & ULPI_FC_FS4LS) + flags |= ULPI_FUNC_CTRL_FS4LS; + else + flags |= ULPI_FUNC_CTRL_FULL_SPEED; + + if (otg->flags & ULPI_FC_TERMSEL) + flags |= ULPI_FUNC_CTRL_TERMSELECT; + + /* + * ULPI Specification rev.1.1 default + * for OpMode is Normal Operation. + */ + if (otg->flags & ULPI_FC_OP_NODRV) + flags |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING; + else if (otg->flags & ULPI_FC_OP_DIS_NRZI) + flags |= ULPI_FUNC_CTRL_OPMODE_DISABLE_NRZI; + else if (otg->flags & ULPI_FC_OP_NSYNC_NEOP) + flags |= ULPI_FUNC_CTRL_OPMODE_NOSYNC_NOEOP; + else + flags |= ULPI_FUNC_CTRL_OPMODE_NORMAL; + + /* + * ULPI Specification rev.1.1 default + * for SuspendM is Powered. + */ + flags |= ULPI_FUNC_CTRL_SUSPENDM; + + return otg_io_write(otg, flags, ULPI_FUNC_CTRL); +} + +static int ulpi_set_ic_flags(struct otg_transceiver *otg) +{ + unsigned int flags = 0; + + if (otg->flags & ULPI_IC_AUTORESUME) + flags |= ULPI_IFC_CTRL_AUTORESUME; + + if (otg->flags & ULPI_IC_EXTVBUS_INDINV) + flags |= ULPI_IFC_CTRL_EXTERNAL_VBUS; + + if (otg->flags & ULPI_IC_IND_PASSTHRU) + flags |= ULPI_IFC_CTRL_PASSTHRU; + + if (otg->flags & ULPI_IC_PROTECT_DIS) + flags |= ULPI_IFC_CTRL_PROTECT_IFC_DISABLE; + + return otg_io_write(otg, flags, ULPI_IFC_CTRL); +} + +static int ulpi_set_flags(struct otg_transceiver *otg) +{ + int ret; + + ret = ulpi_set_otg_flags(otg); + if (ret) + return ret; + + ret = ulpi_set_ic_flags(otg); + if (ret) + return ret; + + return ulpi_set_fc_flags(otg); +} + static int ulpi_init(struct otg_transceiver *otg) { int i, vid, pid, ret; @@ -80,6 +161,31 @@ static int ulpi_init(struct otg_transceiver *otg) return -ENODEV; } +static int ulpi_set_host(struct otg_transceiver *otg, struct usb_bus *host) +{ + unsigned int flags = otg_io_read(otg, ULPI_IFC_CTRL); + + if (!host) { + otg->host = NULL; + return 0; + } + + otg->host = host; + + flags &= ~(ULPI_IFC_CTRL_6_PIN_SERIAL_MODE | + ULPI_IFC_CTRL_3_PIN_SERIAL_MODE | + ULPI_IFC_CTRL_CARKITMODE); + + if (otg->flags & ULPI_IC_6PIN_SERIAL) + flags |= ULPI_IFC_CTRL_6_PIN_SERIAL_MODE; + else if (otg->flags & ULPI_IC_3PIN_SERIAL) + flags |= ULPI_IFC_CTRL_3_PIN_SERIAL_MODE; + else if (otg->flags & ULPI_IC_CARKIT) + flags |= ULPI_IFC_CTRL_CARKITMODE; + + return otg_io_write(otg, flags, ULPI_IFC_CTRL); +} + static int ulpi_set_vbus(struct otg_transceiver *otg, bool on) { unsigned int flags = otg_io_read(otg, ULPI_OTG_CTRL); @@ -87,10 +193,10 @@ static int ulpi_set_vbus(struct otg_transceiver *otg, bool on) flags &= ~(ULPI_OTG_CTRL_DRVVBUS | ULPI_OTG_CTRL_DRVVBUS_EXT); if (on) { - if (otg->flags & USB_OTG_DRV_VBUS) + if (otg->flags & ULPI_OTG_DRVVBUS) flags |= ULPI_OTG_CTRL_DRVVBUS; - if (otg->flags & USB_OTG_DRV_VBUS_EXT) + if (otg->flags & ULPI_OTG_DRVVBUS_EXT) flags |= ULPI_OTG_CTRL_DRVVBUS_EXT; } @@ -111,6 +217,7 @@ otg_ulpi_create(struct otg_io_access_ops *ops, otg->flags = flags; otg->io_ops = ops; otg->init = ulpi_init; + otg->set_host = ulpi_set_host; otg->set_vbus = ulpi_set_vbus; return otg; diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index f8302d0..3dfc3b7 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -43,13 +43,6 @@ enum usb_xceiv_events { USB_EVENT_ENUMERATED, /* gadget driver enumerated */ }; -#define USB_OTG_PULLUP_ID (1 << 0) -#define USB_OTG_PULLDOWN_DP (1 << 1) -#define USB_OTG_PULLDOWN_DM (1 << 2) -#define USB_OTG_EXT_VBUS_INDICATOR (1 << 3) -#define USB_OTG_DRV_VBUS (1 << 4) -#define USB_OTG_DRV_VBUS_EXT (1 << 5) - struct otg_transceiver; /* for transceivers connected thru an ULPI interface, the user must diff --git a/include/linux/usb/ulpi.h b/include/linux/usb/ulpi.h index 900d97b..82b1507 100644 --- a/include/linux/usb/ulpi.h +++ b/include/linux/usb/ulpi.h @@ -15,6 +15,41 @@ /*-------------------------------------------------------------------------*/ /* + * ULPI Flags + */ +#define ULPI_OTG_ID_PULLUP (1 << 0) +#define ULPI_OTG_DP_PULLDOWN_DIS (1 << 1) +#define ULPI_OTG_DM_PULLDOWN_DIS (1 << 2) +#define ULPI_OTG_DISCHRGVBUS (1 << 3) +#define ULPI_OTG_CHRGVBUS (1 << 4) +#define ULPI_OTG_DRVVBUS (1 << 5) +#define ULPI_OTG_DRVVBUS_EXT (1 << 6) +#define ULPI_OTG_EXTVBUSIND (1 << 7) + +#define ULPI_IC_6PIN_SERIAL (1 << 8) +#define ULPI_IC_3PIN_SERIAL (1 << 9) +#define ULPI_IC_CARKIT (1 << 10) +#define ULPI_IC_CLKSUSPM (1 << 11) +#define ULPI_IC_AUTORESUME (1 << 12) +#define ULPI_IC_EXTVBUS_INDINV (1 << 13) +#define ULPI_IC_IND_PASSTHRU (1 << 14) +#define ULPI_IC_PROTECT_DIS (1 << 15) + +#define ULPI_FC_HS (1 << 16) +#define ULPI_FC_FS (1 << 17) +#define ULPI_FC_LS (1 << 18) +#define ULPI_FC_FS4LS (1 << 19) +#define ULPI_FC_TERMSEL (1 << 20) +#define ULPI_FC_OP_NORM (1 << 21) +#define ULPI_FC_OP_NODRV (1 << 22) +#define ULPI_FC_OP_DIS_NRZI (1 << 23) +#define ULPI_FC_OP_NSYNC_NEOP (1 << 24) +#define ULPI_FC_RST (1 << 25) +#define ULPI_FC_SUSPM (1 << 26) + +/*-------------------------------------------------------------------------*/ + +/* * Macros for Set and Clear * See ULPI 1.1 specification to find the registers with Set and Clear offsets */ @@ -59,6 +94,10 @@ /*-------------------------------------------------------------------------*/ +/* + * Register Bits + */ + /* Function Control */ #define ULPI_FUNC_CTRL_XCVRSEL (1 << 0) #define ULPI_FUNC_CTRL_XCVRSEL_MASK (3 << 0) -- 1.7.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-07-15 13:00 ` [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver Igor Grinberg @ 2010-08-13 13:27 ` Heikki Krogerus 2010-08-15 7:29 ` Igor Grinberg 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2010-08-13 13:27 UTC (permalink / raw) To: linux-arm-kernel Hi, This patch has already been applied and I have totally missed it, sorry about that, but I have to ask.. On Thu, Jul 15, 2010 at 04:00:16PM +0300, Igor Grinberg wrote: > 1) Introduce ulpi specific flags for control of the ulpi phy > 2) Extend the generic ulpi driver with support for Function and > Interface control of upli phy > 3) Update the platforms using the generic ulpi driver with new ulpi > flags > 4) Remove the otg control flags not in use > > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > Signed-off-by: Mike Rapoport <mike@compulab.co.il> <snip> > diff --git a/include/linux/usb/ulpi.h b/include/linux/usb/ulpi.h > index 900d97b..82b1507 100644 > --- a/include/linux/usb/ulpi.h > +++ b/include/linux/usb/ulpi.h > @@ -15,6 +15,41 @@ > /*-------------------------------------------------------------------------*/ > > /* > + * ULPI Flags > + */ > +#define ULPI_OTG_ID_PULLUP (1 << 0) > +#define ULPI_OTG_DP_PULLDOWN_DIS (1 << 1) > +#define ULPI_OTG_DM_PULLDOWN_DIS (1 << 2) > +#define ULPI_OTG_DISCHRGVBUS (1 << 3) > +#define ULPI_OTG_CHRGVBUS (1 << 4) > +#define ULPI_OTG_DRVVBUS (1 << 5) > +#define ULPI_OTG_DRVVBUS_EXT (1 << 6) > +#define ULPI_OTG_EXTVBUSIND (1 << 7) Why are you redefining these? If you look through the file you'll find the same bits are already there for OTG Control? > +#define ULPI_IC_6PIN_SERIAL (1 << 8) > +#define ULPI_IC_3PIN_SERIAL (1 << 9) > +#define ULPI_IC_CARKIT (1 << 10) > +#define ULPI_IC_CLKSUSPM (1 << 11) > +#define ULPI_IC_AUTORESUME (1 << 12) > +#define ULPI_IC_EXTVBUS_INDINV (1 << 13) > +#define ULPI_IC_IND_PASSTHRU (1 << 14) > +#define ULPI_IC_PROTECT_DIS (1 << 15) Why bit offsets starting from (1 << 8)? I took a look@ulpi.c and you are still using ULPI_IFC_CTRL register, so these offsets can't possibly work. There are also existing definitions for ULPI Interface Control bits in any case. > +#define ULPI_FC_HS (1 << 16) > +#define ULPI_FC_FS (1 << 17) > +#define ULPI_FC_LS (1 << 18) > +#define ULPI_FC_FS4LS (1 << 19) > +#define ULPI_FC_TERMSEL (1 << 20) > +#define ULPI_FC_OP_NORM (1 << 21) > +#define ULPI_FC_OP_NODRV (1 << 22) > +#define ULPI_FC_OP_DIS_NRZI (1 << 23) > +#define ULPI_FC_OP_NSYNC_NEOP (1 << 24) > +#define ULPI_FC_RST (1 << 25) > +#define ULPI_FC_SUSPM (1 << 26) Same here? I'm asking in case I'm missing something and there is a reason the above, before sending any patches. br, -- heikki ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-08-13 13:27 ` Heikki Krogerus @ 2010-08-15 7:29 ` Igor Grinberg 2010-08-16 9:37 ` Heikki Krogerus 0 siblings, 1 reply; 11+ messages in thread From: Igor Grinberg @ 2010-08-15 7:29 UTC (permalink / raw) To: linux-arm-kernel On 08/13/10 16:27, Heikki Krogerus wrote: > Hi, > > This patch has already been applied and I have totally missed it, > sorry about that, but I have to ask.. > Hi, It was on the list more then a week, I think, and then in linux-next. I really wanted some eyes on it involved, so may be we could design something better then what I've done, but it is never too late ;) > On Thu, Jul 15, 2010 at 04:00:16PM +0300, Igor Grinberg wrote: > >> 1) Introduce ulpi specific flags for control of the ulpi phy >> 2) Extend the generic ulpi driver with support for Function and >> Interface control of upli phy >> 3) Update the platforms using the generic ulpi driver with new ulpi >> flags >> 4) Remove the otg control flags not in use >> >> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> >> Signed-off-by: Mike Rapoport <mike@compulab.co.il> >> > <snip> > > >> diff --git a/include/linux/usb/ulpi.h b/include/linux/usb/ulpi.h >> index 900d97b..82b1507 100644 >> --- a/include/linux/usb/ulpi.h >> +++ b/include/linux/usb/ulpi.h >> @@ -15,6 +15,41 @@ >> /*-------------------------------------------------------------------------*/ >> >> /* >> + * ULPI Flags >> + */ >> +#define ULPI_OTG_ID_PULLUP (1 << 0) >> +#define ULPI_OTG_DP_PULLDOWN_DIS (1 << 1) >> +#define ULPI_OTG_DM_PULLDOWN_DIS (1 << 2) >> +#define ULPI_OTG_DISCHRGVBUS (1 << 3) >> +#define ULPI_OTG_CHRGVBUS (1 << 4) >> +#define ULPI_OTG_DRVVBUS (1 << 5) >> +#define ULPI_OTG_DRVVBUS_EXT (1 << 6) >> +#define ULPI_OTG_EXTVBUSIND (1 << 7) >> > Why are you redefining these? If you look through the file you'll find > the same bits are already there for OTG Control? > Well, trust me I went through the file and more then one time. If you overlook the changes in the ulpi.h, you can see that I've added the comment before register bits, so it will be clearer. The idea of the flags is to provide a control of the ulpi phy by the ulpi driver only, so whoever wants to use it (any platform, not just imx), will only specify the flags different from the default of the ulpi spec. and the ulpi driver will take care of the rest (perfect case). This makes any platform independent from the ulpi registers, thus separating the interface from the implementation. Makes possible for some changes (if ever) in ulpi spec. be transparent to the ulpi driver users. Lowers the legitimation for using the ulpi registers directly in the platform code (like it is currently done in imx ehci glue). Lets ulpi_create() method stay only with one parameter (flags) instead of adding two more parameters for IC and FC. Moreover, the best thing to do is to place the register bits only inside ulpi.c and provide functional methods (like the ulpi_set_host/peripheral(), etc..) and the flags above to control their behavior. I wanted to do this in first place, but currently, the ulpi driver we have is not ready to deal with all the control of the ulpi phy (it will never be, if nobody will contribute to it), I cannot afford to myself this amount of work right now and I don't have the imx hardware to test the changes in imx-ehci glue. > >> +#define ULPI_IC_6PIN_SERIAL (1 << 8) >> +#define ULPI_IC_3PIN_SERIAL (1 << 9) >> +#define ULPI_IC_CARKIT (1 << 10) >> +#define ULPI_IC_CLKSUSPM (1 << 11) >> +#define ULPI_IC_AUTORESUME (1 << 12) >> +#define ULPI_IC_EXTVBUS_INDINV (1 << 13) >> +#define ULPI_IC_IND_PASSTHRU (1 << 14) >> +#define ULPI_IC_PROTECT_DIS (1 << 15) >> > Why bit offsets starting from (1 << 8)? This is because they are not bits, but flags, so they can start wherever you want. This provides the separation between the interface and the implementation (I've already explained it above). > I took a look at ulpi.c and > you are still using ULPI_IFC_CTRL register, so these offsets can't > possibly work. Of course I use the registers, it is the ulpi spec. is what defines them, but the right thing is to move the registers and bits into the ulpi driver (as I've explained above). > There are also existing definitions for ULPI Interface > Control bits in any case. > I wanted to make the flags somewhat consistent with the registers bits and also fit one u32 parameter variable of ulpi_create(). > >> +#define ULPI_FC_HS (1 << 16) >> +#define ULPI_FC_FS (1 << 17) >> +#define ULPI_FC_LS (1 << 18) >> +#define ULPI_FC_FS4LS (1 << 19) >> +#define ULPI_FC_TERMSEL (1 << 20) >> +#define ULPI_FC_OP_NORM (1 << 21) >> +#define ULPI_FC_OP_NODRV (1 << 22) >> +#define ULPI_FC_OP_DIS_NRZI (1 << 23) >> +#define ULPI_FC_OP_NSYNC_NEOP (1 << 24) >> +#define ULPI_FC_RST (1 << 25) >> +#define ULPI_FC_SUSPM (1 << 26) >> > Same here? > > I'm asking in case I'm missing something and there is a reason the > above, before sending any patches. > I hope the reason is clear now :) Also, I'm trying to get more attention of the imx guys. There is a ulpi driver, please use/develop it where possible/appropriate instead of locking the code in platforms (e.g. imx-ehci glue). This way more platforms can benefit from your contribution. > br, > > -- Regards, Igor. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-08-15 7:29 ` Igor Grinberg @ 2010-08-16 9:37 ` Heikki Krogerus 2010-08-17 16:17 ` Igor Grinberg 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2010-08-16 9:37 UTC (permalink / raw) To: linux-arm-kernel Hi, On Sun, Aug 15, 2010 at 09:29:54AM +0200, ext Igor Grinberg wrote: >On 08/13/10 16:27, Heikki Krogerus wrote: >> This patch has already been applied and I have totally missed it, >> sorry about that, but I have to ask.. > > It was on the list more then a week, I think, and then in linux-next. > I really wanted some eyes on it involved, so may be we could design > something better then what I've done, but it is never too late ;) I was on vacation, and only now saw your patches. Sorry about the late comments. >> On Thu, Jul 15, 2010 at 04:00:16PM +0300, Igor Grinberg wrote: >>> /* >>> + * ULPI Flags >>> + */ >>> +#define ULPI_OTG_ID_PULLUP (1 << 0) >>> +#define ULPI_OTG_DP_PULLDOWN_DIS (1 << 1) >>> +#define ULPI_OTG_DM_PULLDOWN_DIS (1 << 2) >>> +#define ULPI_OTG_DISCHRGVBUS (1 << 3) >>> +#define ULPI_OTG_CHRGVBUS (1 << 4) >>> +#define ULPI_OTG_DRVVBUS (1 << 5) >>> +#define ULPI_OTG_DRVVBUS_EXT (1 << 6) >>> +#define ULPI_OTG_EXTVBUSIND (1 << 7) >>> >> Why are you redefining these? If you look through the file you'll find >> the same bits are already there for OTG Control? >> > > Well, trust me I went through the file and more then one time. > If you overlook the changes in the ulpi.h, you can see that I've added > the comment before register bits, so it will be clearer. > > The idea of the flags is to provide a control of the ulpi phy by the ulpi > driver only, so whoever wants to use it (any platform, not just imx), > will only specify the flags different from the default of the ulpi spec. > and the ulpi driver will take care of the rest (perfect case). > > This makes any platform independent from the ulpi registers, > thus separating the interface from the implementation. > Makes possible for some changes (if ever) in ulpi spec. be > transparent to the ulpi driver users. > Lowers the legitimation for using the ulpi registers directly in > the platform code (like it is currently done in imx ehci glue). > Lets ulpi_create() method stay only with one parameter (flags) > instead of adding two more parameters for IC and FC. > > Moreover, the best thing to do is to place the register bits only > inside ulpi.c and provide functional methods (like the > ulpi_set_host/peripheral(), etc..) and the flags above to control > their behavior. > I wanted to do this in first place, but currently, the ulpi driver > we have is not ready to deal with all the control of the ulpi phy > (it will never be, if nobody will contribute to it), I cannot afford > to myself this amount of work right now and I don't have > the imx hardware to test the changes in imx-ehci glue. OK, I get your idea now with the flags now. Should have read the code more carefully. I'm not sure if it's the right way though, but if you can show that they are beneficial in more then one case, then I'm sure there is no problem with them. Please consider the following. It seems that many ulpi transceivers are autonomous as they say. There is little if any need to access ulpi registers expect the vendor specific ones. The vendor specific stuff will require register access in any case and in some cases they need the transceivers to be programmed in a specific way. Right now to me using the flags for the common parts and then separately programming the vendor specific stuff does not seem very efficient. I need to re-send the charger driver for isp170x. These transceivers are fairly commonly used with musb, and most platform probable only need nop-usb-xceiv.c with them. However, the charger detection needs access to the common ulpi registers as well as the vendor specific ones. If you see that you can make this kind of functions work with the flags, then I'm OK with them. The twl4030-usb has a few places where it needs to program some common ulpi registers. Those would need to be removed, or the file would need separate definitions for the ulpi registers if you want to only define the register only in ulpi.c. Maybe you could give it a quick look and see how would it be handled with the flags. br, -- heikki ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-08-16 9:37 ` Heikki Krogerus @ 2010-08-17 16:17 ` Igor Grinberg 2010-08-18 9:49 ` Heikki Krogerus 0 siblings, 1 reply; 11+ messages in thread From: Igor Grinberg @ 2010-08-17 16:17 UTC (permalink / raw) To: linux-arm-kernel On 08/16/10 12:37, Heikki Krogerus wrote: > Hi, > > On Sun, Aug 15, 2010 at 09:29:54AM +0200, ext Igor Grinberg wrote: >> On 08/13/10 16:27, Heikki Krogerus wrote: >>> This patch has already been applied and I have totally missed it, >>> sorry about that, but I have to ask.. >> It was on the list more then a week, I think, and then in linux-next. >> I really wanted some eyes on it involved, so may be we could design >> something better then what I've done, but it is never too late ;) > I was on vacation, and only now saw your patches. Sorry about the late > comments. > >>> On Thu, Jul 15, 2010 at 04:00:16PM +0300, Igor Grinberg wrote: >>>> /* >>>> + * ULPI Flags >>>> + */ >>>> +#define ULPI_OTG_ID_PULLUP (1 << 0) >>>> +#define ULPI_OTG_DP_PULLDOWN_DIS (1 << 1) >>>> +#define ULPI_OTG_DM_PULLDOWN_DIS (1 << 2) >>>> +#define ULPI_OTG_DISCHRGVBUS (1 << 3) >>>> +#define ULPI_OTG_CHRGVBUS (1 << 4) >>>> +#define ULPI_OTG_DRVVBUS (1 << 5) >>>> +#define ULPI_OTG_DRVVBUS_EXT (1 << 6) >>>> +#define ULPI_OTG_EXTVBUSIND (1 << 7) >>>> >>> Why are you redefining these? If you look through the file you'll find >>> the same bits are already there for OTG Control? >>> >> Well, trust me I went through the file and more then one time. >> If you overlook the changes in the ulpi.h, you can see that I've added >> the comment before register bits, so it will be clearer. >> >> The idea of the flags is to provide a control of the ulpi phy by the ulpi >> driver only, so whoever wants to use it (any platform, not just imx), >> will only specify the flags different from the default of the ulpi spec. >> and the ulpi driver will take care of the rest (perfect case). >> >> This makes any platform independent from the ulpi registers, >> thus separating the interface from the implementation. >> Makes possible for some changes (if ever) in ulpi spec. be >> transparent to the ulpi driver users. >> Lowers the legitimation for using the ulpi registers directly in >> the platform code (like it is currently done in imx ehci glue). >> Lets ulpi_create() method stay only with one parameter (flags) >> instead of adding two more parameters for IC and FC. >> >> Moreover, the best thing to do is to place the register bits only >> inside ulpi.c and provide functional methods (like the >> ulpi_set_host/peripheral(), etc..) and the flags above to control >> their behavior. >> I wanted to do this in first place, but currently, the ulpi driver >> we have is not ready to deal with all the control of the ulpi phy >> (it will never be, if nobody will contribute to it), I cannot afford >> to myself this amount of work right now and I don't have >> the imx hardware to test the changes in imx-ehci glue. > OK, I get your idea now with the flags now. Should have read the code > more carefully. I'm not sure if it's the right way though, but if you > can show that they are beneficial in more then one case, then I'm sure > there is no problem with them. Indeed, this can be not the best way, but at least it is better then letting everyone around access the ulpi defined registers in a way they like. There is a ulpi standard and kernel should provide a framework/driver that follows the standard (at least for the ulpi defined registers). > Please consider the following. > > It seems that many ulpi transceivers are autonomous as they say. > There is little if any need to access ulpi registers expect the > vendor specific ones. The vendor specific stuff will require register > access in any case and in some cases they need the transceivers to be > programmed in a specific way. Right now to me using the flags for the > common parts and then separately programming the vendor specific stuff > does not seem very efficient. Well, vendor specific registers are very important issue. This is how I see it: platform core <-------wired--------> ulpi phy / / \ / ulpi generic regs | vendor specific regs / / \ ulpi access ops <-----> ulpi generic driver <---> ulpi phy specific driver(s) / \ otg framework \ \ / usb subsystems e.g. musb, ehci... If the ulpi transceiver is autonomous and platform does not need to do any setup of the transceiver, then I think you don't need to access the transceiver in any way, just setup the platform core, which can be done in platform specific code. If there is a need to access the ulpi transceiver, then it should be done by using the ulpi driver. ulpi transceiver usually is a chip, so if it needs to be setup, then it deserves a driver, I think. Anyway, we need a good generic ulpi driver, which will provide some kind of interface for the ulpi specific drivers that are aware of the vendor specific registers and setup. This interface should make the task of developing ulpi specific driver easy. The driver does not have to end up with flags. The intension of those flags is to give a start for something bigger... otherwise people will continue to setup the hardware specific stuff in the generic drivers (like the recent ulpi phy reset code in omap-ehci glue) instead of contributing to the ulpi driver, so other platforms could reuse it. > I need to re-send the charger driver for isp170x. These transceivers > are fairly commonly used with musb, and most platform probable only > need nop-usb-xceiv.c with them. However, the charger detection needs > access to the common ulpi registers as well as the vendor specific > ones. If you see that you can make this kind of functions work with > the flags, then I'm OK with them. Is this series (part of it) you need to resend: http://www.spinics.net/lists/linux-usb/msg29643.html isp1704_test_ulpi() - can be done fully by the generic ulpi driver, in fact it is already reads the id. May be we should think of a way to export it, so drivers like this one can do the matching. isp1704_charger_detect() - isp170x specific ulpi registers accesses. isp1704_charger_verify() - I don't fully understand the purpose of this, I see the reset (can be done by the generic driver) and then some ulpi generic and specific accesses to the transceiver. I think it can be handled by the ulpi generic and specific driver and with help of the flags. > The twl4030-usb has a few places where it needs to program some common > ulpi registers. Those would need to be removed, or the file would need > separate definitions for the ulpi registers if you want to only define > the register only in ulpi.c. Maybe you could give it a quick look > and see how would it be handled with the flags. This one is a bit tricky... :) Are those calls in twl4030_usb_set_mode() have to be in that specific order? Otherwise, I think it can use the generic and specific driver (after someone prepares it). > br, -- Regards, Igor. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-08-17 16:17 ` Igor Grinberg @ 2010-08-18 9:49 ` Heikki Krogerus 2010-08-18 14:50 ` Igor Grinberg 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2010-08-18 9:49 UTC (permalink / raw) To: linux-arm-kernel On Tue, Aug 17, 2010 at 06:17:09PM +0200, ext Igor Grinberg wrote: >On 08/16/10 12:37, Heikki Krogerus wrote: >> OK, I get your idea now with the flags now. Should have read the code >> more carefully. I'm not sure if it's the right way though, but if you >> can show that they are beneficial in more then one case, then I'm sure >> there is no problem with them. > > Indeed, this can be not the best way, but at least it is better then letting > everyone around access the ulpi defined registers in a way they like. > There is a ulpi standard and kernel should provide a framework/driver > that follows the standard (at least for the ulpi defined registers). > >> Please consider the following. >> >> It seems that many ulpi transceivers are autonomous as they say. >> There is little if any need to access ulpi registers expect the >> vendor specific ones. The vendor specific stuff will require register >> access in any case and in some cases they need the transceivers to be >> programmed in a specific way. Right now to me using the flags for the >> common parts and then separately programming the vendor specific stuff >> does not seem very efficient. > > Well, vendor specific registers are very important issue. > This is how I see it: > > platform core <-------wired--------> ulpi phy > / / \ > / ulpi generic regs | vendor specific regs > / / \ > ulpi access ops <-----> ulpi generic driver <---> ulpi phy specific driver(s) > / \ > otg framework \ > \ / > usb subsystems e.g. musb, ehci... > > > If the ulpi transceiver is autonomous and platform does not need to > do any setup of the transceiver, then I think you don't need to > access the transceiver in any way, just setup the platform core, > which can be done in platform specific code. > > If there is a need to access the ulpi transceiver, then it should be done > by using the ulpi driver. ulpi transceiver usually is a chip, so if it needs > to be setup, then it deserves a driver, I think. > > Anyway, we need a good generic ulpi driver, which will provide some > kind of interface for the ulpi specific drivers that are aware of the > vendor specific registers and setup. This interface should make the > task of developing ulpi specific driver easy. I'm totally _not_ against a generic ulpi driver, and see it can be something very useful. > The driver does not have to end up with flags. The intension of those > flags is to give a start for something bigger... otherwise people will > continue to setup the hardware specific stuff in the generic drivers > (like the recent ulpi phy reset code in omap-ehci glue) instead of > contributing to the ulpi driver, so other platforms could reuse it. Unfortunately this is not generating enough interest in people. I guess this is one of those cases where people prefer to wait for someone else to make the ulpi driver more mature instead of contributing themselves, and in the meantime, hack their existing or new driver (like me) :(. Someone simply needs to put effort in developing it, but as I understood, you are too busy at the moment, and so am I. >> I need to re-send the charger driver for isp170x. These transceivers >> are fairly commonly used with musb, and most platform probable only >> need nop-usb-xceiv.c with them. However, the charger detection needs >> access to the common ulpi registers as well as the vendor specific >> ones. If you see that you can make this kind of functions work with >> the flags, then I'm OK with them. > > Is this series (part of it) you need to resend: > http://www.spinics.net/lists/linux-usb/msg29643.html > > isp1704_test_ulpi() - can be done fully by the generic ulpi driver, > in fact it is already reads the id. May be we should think of a way to export > it, so drivers like this one can do the matching. Yes it would make sense for now. > isp1704_charger_detect() - isp170x specific ulpi registers accesses. > > isp1704_charger_verify() - I don't fully understand the purpose of this, > I see the reset (can be done by the generic driver) and then some > ulpi generic and specific accesses to the transceiver. > I think it can be handled by the ulpi generic and specific driver > and with help of the flags. This transceiver is a bit difficult as RX51 platform uses it as it's second transceiver. So the primary transceiver provides the struct otg_transceiver, and from software's point of view only charger detection is done with isp170x, so I was unable to take advantage of the ulpi driver. This kind of oddities make things even more complicated. The otg framework is not suited for multiple transceivers (maybe it newer will), but the platform needs to be considered in any case :(. Unfortunately I have to send the driver as it is, and can't spend any time with it right now. I need to put more though into this at one point. br, -- heikki ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver. 2010-08-18 9:49 ` Heikki Krogerus @ 2010-08-18 14:50 ` Igor Grinberg 0 siblings, 0 replies; 11+ messages in thread From: Igor Grinberg @ 2010-08-18 14:50 UTC (permalink / raw) To: linux-arm-kernel On 08/18/10 12:49, Heikki Krogerus wrote: > On Tue, Aug 17, 2010 at 06:17:09PM +0200, ext Igor Grinberg wrote: >> On 08/16/10 12:37, Heikki Krogerus wrote: >>> OK, I get your idea now with the flags now. Should have read the code >>> more carefully. I'm not sure if it's the right way though, but if you >>> can show that they are beneficial in more then one case, then I'm sure >>> there is no problem with them. >> Indeed, this can be not the best way, but at least it is better then letting >> everyone around access the ulpi defined registers in a way they like. >> There is a ulpi standard and kernel should provide a framework/driver >> that follows the standard (at least for the ulpi defined registers). >> >>> Please consider the following. >>> >>> It seems that many ulpi transceivers are autonomous as they say. >>> There is little if any need to access ulpi registers expect the >>> vendor specific ones. The vendor specific stuff will require register >>> access in any case and in some cases they need the transceivers to be >>> programmed in a specific way. Right now to me using the flags for the >>> common parts and then separately programming the vendor specific stuff >>> does not seem very efficient. >> Well, vendor specific registers are very important issue. >> This is how I see it: >> >> platform core <-------wired--------> ulpi phy >> / / \ >> / ulpi generic regs | vendor specific regs >> / / \ >> ulpi access ops <-----> ulpi generic driver <---> ulpi phy specific driver(s) >> / \ >> otg framework \ >> \ / >> usb subsystems e.g. musb, ehci... >> >> >> If the ulpi transceiver is autonomous and platform does not need to >> do any setup of the transceiver, then I think you don't need to >> access the transceiver in any way, just setup the platform core, >> which can be done in platform specific code. >> >> If there is a need to access the ulpi transceiver, then it should be done >> by using the ulpi driver. ulpi transceiver usually is a chip, so if it needs >> to be setup, then it deserves a driver, I think. >> >> Anyway, we need a good generic ulpi driver, which will provide some >> kind of interface for the ulpi specific drivers that are aware of the >> vendor specific registers and setup. This interface should make the >> task of developing ulpi specific driver easy. > I'm totally _not_ against a generic ulpi driver, and see it can be > something very useful. > >> The driver does not have to end up with flags. The intension of those >> flags is to give a start for something bigger... otherwise people will >> continue to setup the hardware specific stuff in the generic drivers >> (like the recent ulpi phy reset code in omap-ehci glue) instead of >> contributing to the ulpi driver, so other platforms could reuse it. > Unfortunately this is not generating enough interest in people. I > guess this is one of those cases where people prefer to wait for > someone else to make the ulpi driver more mature instead of > contributing themselves, and in the meantime, hack their existing or > new driver (like me) :(. Someone simply needs to put effort in > developing it, but as I understood, you are too busy at the moment, > and so am I. Well, it's a pity, but that is the reality... I think even small contributions could make a difference. Lets say, a reset or the pm, are not so time consuming and are very reusable. >>> I need to re-send the charger driver for isp170x. These transceivers >>> are fairly commonly used with musb, and most platform probable only >>> need nop-usb-xceiv.c with them. However, the charger detection needs >>> access to the common ulpi registers as well as the vendor specific >>> ones. If you see that you can make this kind of functions work with >>> the flags, then I'm OK with them. >> Is this series (part of it) you need to resend: >> http://www.spinics.net/lists/linux-usb/msg29643.html >> >> isp1704_test_ulpi() - can be done fully by the generic ulpi driver, >> in fact it is already reads the id. May be we should think of a way to export >> it, so drivers like this one can do the matching. > Yes it would make sense for now. > >> isp1704_charger_detect() - isp170x specific ulpi registers accesses. >> >> isp1704_charger_verify() - I don't fully understand the purpose of this, >> I see the reset (can be done by the generic driver) and then some >> ulpi generic and specific accesses to the transceiver. >> I think it can be handled by the ulpi generic and specific driver >> and with help of the flags. > This transceiver is a bit difficult as RX51 platform uses it as it's > second transceiver. So the primary transceiver provides the struct > otg_transceiver, and from software's point of view only charger > detection is done with isp170x, so I was unable to take advantage of > the ulpi driver. > > This kind of oddities make things even more complicated. The otg > framework is not suited for multiple transceivers (maybe it newer > will), but the platform needs to be considered in any case :(. As far as I know, the OTG standard does not permit more then one OTG port (and thus transceiver) on the system, but we can have multiple ulpi transceivers, which are not necessarily used for OTG. > Unfortunately I have to send the driver as it is, and can't spend any > time with it right now. I need to put more though into this at one > point. Yeah, I see that. Hope that point in time will come... > br, -- Regards, Igor. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] Generic ULPI driver extention 2010-07-15 13:00 [PATCH 0/3] Generic ULPI driver extention Igor Grinberg ` (2 preceding siblings ...) 2010-07-15 13:00 ` [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver Igor Grinberg @ 2010-07-21 14:47 ` Igor Grinberg 3 siblings, 0 replies; 11+ messages in thread From: Igor Grinberg @ 2010-07-21 14:47 UTC (permalink / raw) To: linux-arm-kernel Greg, Daniel, Sascha, Any comments? > This patch series add support for SMSC USB3319 ulpi phy, > introduces the ulpi specific flags for control over the > ulpi phy and extends the generic ulpi driver with > support for Function and Interface control of upli phy. > > Depends on: > From: Ajay Kumar Gupta <ajay.gupta@ti.com> > Date: Thu, 8 Jul 2010 14:03:01 +0530 > Subject: USB: ulpi: fix compilation warning > > From: Eric B?nard <eric@eukrea.com> > Date: Thu, 15 Jul 2010 09:20:19 +0200 > Subject: [PATCH v2] otg/ulpi.c : fix register write > > Igor Grinberg (3): > usb/otg/ulpi: remove unused macro > usb/otg/ulpi: add support for SMSC USB3319 ulpi phy > usb/otg/ulpi: extend the generic ulpi driver. > > arch/arm/mach-mx2/mach-pca100.c | 4 +- > arch/arm/mach-mx3/mach-armadillo5x0.c | 4 +- > arch/arm/mach-mx3/mach-mx31lilly.c | 4 +- > arch/arm/mach-mx3/mach-mx31lite.c | 2 +- > arch/arm/mach-mx3/mach-mx31moboard.c | 2 +- > arch/arm/mach-mx3/mach-pcm037.c | 4 +- > arch/arm/mach-mx3/mach-pcm043.c | 2 +- > arch/arm/mach-mx3/mx31moboard-smartbot.c | 2 +- > drivers/usb/otg/Kconfig | 2 - > drivers/usb/otg/ulpi.c | 130 +++++++++++++++++++++++++++--- > include/linux/usb/otg.h | 7 -- > include/linux/usb/ulpi.h | 39 +++++++++ > 12 files changed, 169 insertions(+), 33 deletions(-) > > > -- Regards, Igor. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-08-18 14:50 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-15 13:00 [PATCH 0/3] Generic ULPI driver extention Igor Grinberg 2010-07-15 13:00 ` [PATCH 1/3] usb/otg/ulpi: remove unused macro Igor Grinberg 2010-07-15 13:00 ` [PATCH 2/3] usb/otg/ulpi: add support for SMSC USB3319 ulpi phy Igor Grinberg 2010-07-15 13:00 ` [PATCH 3/3] usb/otg/ulpi: extend the generic ulpi driver Igor Grinberg 2010-08-13 13:27 ` Heikki Krogerus 2010-08-15 7:29 ` Igor Grinberg 2010-08-16 9:37 ` Heikki Krogerus 2010-08-17 16:17 ` Igor Grinberg 2010-08-18 9:49 ` Heikki Krogerus 2010-08-18 14:50 ` Igor Grinberg 2010-07-21 14:47 ` [PATCH 0/3] Generic ULPI driver extention Igor Grinberg
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.