| Message ID | 20130520085326.GA21067@panicking |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Michael, Sorry for missing the subject last time... For the subject: In the square brackets, it should be stated just [PATCH vX] Afterwards, should come the subsystem/platform and then short yet meaningful patch description. For example: [PATCH v2] usb: omap: ulpi: fix ulpi transceiver access Also, the patch submission rules are available at: http://www.denx.de/wiki/view/U-Boot/Patches#General_Patch_Submission_Rules I think it is worth reading. On 05/20/13 11:53, Michael Trimarchi wrote: > This patch fix the omap access to the transceiver configuration registers > using the ulpi bus. As reported by the documentation the bit31 is used > only to check if the transaction is done or still running and the reading > and writing operation have different offset and have different values. > What we need to do at the end of a transaction is leave the bus in done state. > Anyway an error using the ulpi omap register is not recoverable so any error > give out the usage of this interface. > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> You can add the below tag to your future submissions of this patch (if you decide to fix the subject). Reviewed-by: Igor Grinberg <grinberg@compulab.co.il> > --- > > Changes for V2: > - Fix commit message > > --- > drivers/usb/ulpi/omap-ulpi-viewport.c | 40 +++++++-------------------------- > 1 file changed, 8 insertions(+), 32 deletions(-) > > diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c > index 3c1ea1a..2a42033 100644 > --- a/drivers/usb/ulpi/omap-ulpi-viewport.c > +++ b/drivers/usb/ulpi/omap-ulpi-viewport.c > @@ -22,18 +22,19 @@ > #include <asm/io.h> > #include <usb/ulpi.h> > > -#define OMAP_ULPI_WR_OPSEL (3 << 21) > -#define OMAP_ULPI_ACCESS (1 << 31) > +#define OMAP_ULPI_WR_OPSEL (2 << 22) > +#define OMAP_ULPI_RD_OPSEL (3 << 22) > +#define OMAP_ULPI_START (1 << 31) > > /* > - * Wait for the ULPI Access to complete > + * Wait for having ulpi in done state > */ > static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) > { > int timeout = CONFIG_USB_ULPI_TIMEOUT; > > while (--timeout) { > - if ((readl(ulpi_vp->viewport_addr) & mask)) > + if (!(readl(ulpi_vp->viewport_addr) & mask)) > return 0; > > udelay(1); > @@ -43,40 +44,15 @@ static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) > } > > /* > - * Wake the ULPI PHY up for communication > - * > - * returns 0 on success. > - */ > -static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp) > -{ > - int err; > - > - if (readl(ulpi_vp->viewport_addr) & OMAP_ULPI_ACCESS) > - return 0; /* already awake */ > - > - writel(OMAP_ULPI_ACCESS, ulpi_vp->viewport_addr); > - > - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); > - if (err) > - debug("ULPI wakeup timed out\n"); > - > - return err; > -} > - > -/* > * Issue a ULPI read/write request > */ > static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) > { > int err; > > - err = ulpi_wakeup(ulpi_vp); > - if (err) > - return err; > - > writel(value, ulpi_vp->viewport_addr); > > - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); > + err = ulpi_wait(ulpi_vp, OMAP_ULPI_START); > if (err) > debug("ULPI request timed out\n"); > > @@ -85,7 +61,7 @@ static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) > > int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value) > { > - u32 val = ((ulpi_vp->port_num & 0xf) << 24) | > + u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) | > OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff); > > return ulpi_request(ulpi_vp, val); > @@ -95,7 +71,7 @@ u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg) > { > int err; > u32 val = ((ulpi_vp->port_num & 0xf) << 24) | > - OMAP_ULPI_WR_OPSEL | ((u32)reg << 16); > + OMAP_ULPI_RD_OPSEL | ((u32)reg << 16); > > err = ulpi_request(ulpi_vp, val); > if (err) >
Hi On 20/05/13 12:05, Igor Grinberg wrote: > Hi Michael, > > Sorry for missing the subject last time... > For the subject: > In the square brackets, it should be stated just [PATCH vX] > Afterwards, should come the subsystem/platform > and then short yet meaningful patch description. For example: > [PATCH v2] usb: omap: ulpi: fix ulpi transceiver access > > Also, the patch submission rules are available at: > http://www.denx.de/wiki/view/U-Boot/Patches#General_Patch_Submission_Rules > I think it is worth reading. > Sorry, I know but I'm so busy that I have almost edit the last one. I will re-submit :( Michael > > On 05/20/13 11:53, Michael Trimarchi wrote: >> This patch fix the omap access to the transceiver configuration registers >> using the ulpi bus. As reported by the documentation the bit31 is used >> only to check if the transaction is done or still running and the reading >> and writing operation have different offset and have different values. >> What we need to do at the end of a transaction is leave the bus in done state. >> Anyway an error using the ulpi omap register is not recoverable so any error >> give out the usage of this interface. >> >> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > > You can add the below tag to your future submissions of this patch > (if you decide to fix the subject). > > Reviewed-by: Igor Grinberg <grinberg@compulab.co.il> > >> --- >> >> Changes for V2: >> - Fix commit message >> >> --- >> drivers/usb/ulpi/omap-ulpi-viewport.c | 40 +++++++-------------------------- >> 1 file changed, 8 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c >> index 3c1ea1a..2a42033 100644 >> --- a/drivers/usb/ulpi/omap-ulpi-viewport.c >> +++ b/drivers/usb/ulpi/omap-ulpi-viewport.c >> @@ -22,18 +22,19 @@ >> #include <asm/io.h> >> #include <usb/ulpi.h> >> >> -#define OMAP_ULPI_WR_OPSEL (3 << 21) >> -#define OMAP_ULPI_ACCESS (1 << 31) >> +#define OMAP_ULPI_WR_OPSEL (2 << 22) >> +#define OMAP_ULPI_RD_OPSEL (3 << 22) >> +#define OMAP_ULPI_START (1 << 31) >> >> /* >> - * Wait for the ULPI Access to complete >> + * Wait for having ulpi in done state >> */ >> static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) >> { >> int timeout = CONFIG_USB_ULPI_TIMEOUT; >> >> while (--timeout) { >> - if ((readl(ulpi_vp->viewport_addr) & mask)) >> + if (!(readl(ulpi_vp->viewport_addr) & mask)) >> return 0; >> >> udelay(1); >> @@ -43,40 +44,15 @@ static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) >> } >> >> /* >> - * Wake the ULPI PHY up for communication >> - * >> - * returns 0 on success. >> - */ >> -static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp) >> -{ >> - int err; >> - >> - if (readl(ulpi_vp->viewport_addr) & OMAP_ULPI_ACCESS) >> - return 0; /* already awake */ >> - >> - writel(OMAP_ULPI_ACCESS, ulpi_vp->viewport_addr); >> - >> - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); >> - if (err) >> - debug("ULPI wakeup timed out\n"); >> - >> - return err; >> -} >> - >> -/* >> * Issue a ULPI read/write request >> */ >> static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) >> { >> int err; >> >> - err = ulpi_wakeup(ulpi_vp); >> - if (err) >> - return err; >> - >> writel(value, ulpi_vp->viewport_addr); >> >> - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); >> + err = ulpi_wait(ulpi_vp, OMAP_ULPI_START); >> if (err) >> debug("ULPI request timed out\n"); >> >> @@ -85,7 +61,7 @@ static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) >> >> int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value) >> { >> - u32 val = ((ulpi_vp->port_num & 0xf) << 24) | >> + u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) | >> OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff); >> >> return ulpi_request(ulpi_vp, val); >> @@ -95,7 +71,7 @@ u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg) >> { >> int err; >> u32 val = ((ulpi_vp->port_num & 0xf) << 24) | >> - OMAP_ULPI_WR_OPSEL | ((u32)reg << 16); >> + OMAP_ULPI_RD_OPSEL | ((u32)reg << 16); >> >> err = ulpi_request(ulpi_vp, val); >> if (err) >> >
diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c index 3c1ea1a..2a42033 100644 --- a/drivers/usb/ulpi/omap-ulpi-viewport.c +++ b/drivers/usb/ulpi/omap-ulpi-viewport.c @@ -22,18 +22,19 @@ #include <asm/io.h> #include <usb/ulpi.h> -#define OMAP_ULPI_WR_OPSEL (3 << 21) -#define OMAP_ULPI_ACCESS (1 << 31) +#define OMAP_ULPI_WR_OPSEL (2 << 22) +#define OMAP_ULPI_RD_OPSEL (3 << 22) +#define OMAP_ULPI_START (1 << 31) /* - * Wait for the ULPI Access to complete + * Wait for having ulpi in done state */ static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) { int timeout = CONFIG_USB_ULPI_TIMEOUT; while (--timeout) { - if ((readl(ulpi_vp->viewport_addr) & mask)) + if (!(readl(ulpi_vp->viewport_addr) & mask)) return 0; udelay(1); @@ -43,40 +44,15 @@ static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask) } /* - * Wake the ULPI PHY up for communication - * - * returns 0 on success. - */ -static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp) -{ - int err; - - if (readl(ulpi_vp->viewport_addr) & OMAP_ULPI_ACCESS) - return 0; /* already awake */ - - writel(OMAP_ULPI_ACCESS, ulpi_vp->viewport_addr); - - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); - if (err) - debug("ULPI wakeup timed out\n"); - - return err; -} - -/* * Issue a ULPI read/write request */ static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) { int err; - err = ulpi_wakeup(ulpi_vp); - if (err) - return err; - writel(value, ulpi_vp->viewport_addr); - err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS); + err = ulpi_wait(ulpi_vp, OMAP_ULPI_START); if (err) debug("ULPI request timed out\n"); @@ -85,7 +61,7 @@ static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value) int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value) { - u32 val = ((ulpi_vp->port_num & 0xf) << 24) | + u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) | OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff); return ulpi_request(ulpi_vp, val); @@ -95,7 +71,7 @@ u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg) { int err; u32 val = ((ulpi_vp->port_num & 0xf) << 24) | - OMAP_ULPI_WR_OPSEL | ((u32)reg << 16); + OMAP_ULPI_RD_OPSEL | ((u32)reg << 16); err = ulpi_request(ulpi_vp, val); if (err)
This patch fix the omap access to the transceiver configuration registers using the ulpi bus. As reported by the documentation the bit31 is used only to check if the transaction is done or still running and the reading and writing operation have different offset and have different values. What we need to do at the end of a transaction is leave the bus in done state. Anyway an error using the ulpi omap register is not recoverable so any error give out the usage of this interface. Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- Changes for V2: - Fix commit message --- drivers/usb/ulpi/omap-ulpi-viewport.c | 40 +++++++-------------------------- 1 file changed, 8 insertions(+), 32 deletions(-)