Patchwork [U-Boot] ulpi: add generic ULPI functionality

login
register
mail settings
Submitter Jana Rapava
Date Nov. 5, 2011, 8:50 p.m.
Message ID <1320526223-30474-1-git-send-email-fermata7@gmail.com>
Download mbox | patch
Permalink /patch/123889/
State Superseded
Delegated to: Remy Bohmer
Headers show

Comments

Jana Rapava - Nov. 5, 2011, 8:50 p.m.
Add generic functions for ULPI init and setting bits in 
ULPI registers. 

Signed-off-by: Jana Rapava <fermata7@gmail.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Remy Bohmer <linux@bohmer.net>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Igor Grinberg <grinberg@compulab.co.il>
---
 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++++++++++
 drivers/usb/ulpi/ulpi-viewport.c |   87 +++++++++++++++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  123 ++++++++++++++++++++++++++++++++++++++
 include/usb/ulpi.h               |   16 +++++
 5 files changed, 271 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
Marek Vasut - Nov. 5, 2011, 9:37 p.m.
> Add generic functions for ULPI init and setting bits in
> ULPI registers.
> 
> Signed-off-by: Jana Rapava <fermata7@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  Makefile                         |    1 +
>  drivers/usb/ulpi/Makefile        |   44 ++++++++++++++
>  drivers/usb/ulpi/ulpi-viewport.c |   87 +++++++++++++++++++++++++++
>  drivers/usb/ulpi/ulpi.c          |  123
> ++++++++++++++++++++++++++++++++++++++ include/usb/ulpi.h               | 
>  16 +++++
>  5 files changed, 271 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/ulpi/Makefile
>  create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
>  create mode 100644 drivers/usb/ulpi/ulpi.c
> 
> diff --git a/Makefile b/Makefile
> index 571c3eb..a475cb9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -283,6 +283,7 @@ LIBS += drivers/usb/gadget/libusb_gadget.o
>  LIBS += drivers/usb/host/libusb_host.o
>  LIBS += drivers/usb/musb/libusb_musb.o
>  LIBS += drivers/usb/phy/libusb_phy.o
> +LIBS += drivers/usb/ulpi/libusb_ulpi.o
>  LIBS += drivers/video/libvideo.o
>  LIBS += drivers/watchdog/libwatchdog.o
>  LIBS += common/libcommon.o
> diff --git a/drivers/usb/ulpi/Makefile b/drivers/usb/ulpi/Makefile
> new file mode 100644
> index 0000000..f7b6e20
> --- /dev/null
> +++ b/drivers/usb/ulpi/Makefile
> @@ -0,0 +1,44 @@
> +#
> +# Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	:= $(obj)libusb_ulpi.o
> +
> +COBJS-$(CONFIG_USB_ULPI) += ulpi.o ulpi-viewport.o
> +
> +COBJS	:= $(COBJS-y)
> +SRCS	:= $(COBJS:.o=.c)
> +OBJS	:= $(addprefix $(obj),$(COBJS))
> +
> +all:	$(LIB)
> +
> +$(LIB):	$(obj).depend $(OBJS)
> +	$(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/drivers/usb/ulpi/ulpi-viewport.c
> b/drivers/usb/ulpi/ulpi-viewport.c new file mode 100644
> index 0000000..a0c213e
> --- /dev/null
> +++ b/drivers/usb/ulpi/ulpi-viewport.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> + * Based on:
> + * linux/drivers/usb/otg/ulpi_viewport.c
> + *
> + * Original Copyright follow:
> + * Copyright (C) 2011 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <watchdog.h>
> +#include <asm/io.h>
> +#include <usb/ulpi.h>
> +#include <usb/ehci-fsl.h>
> +
> +/* ULPI viewport control bits */
> +#define ULPI_WU		(1 << 31)
> +#define ULPI_SS		(1 << 27)
> +#define ULPI_RWRUN	(1 << 30)
> +#define ULPI_RWCTRL	(1 << 29)
> +
> +#define ULPI_ADDR_SHIFT		16
> +#define ulpi_write_mask(value)	((value) & 0xff)
> +#define ulpi_read_mask(value)	(((value) >> 8) & 0xff)
> +
> +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)

static

So this works only with EHCI? How generic is it then ?

> +{
> +	int timeout = ULPI_TIMEOUT;
> +	u32 tmp;
> +
> +	writel(ulpi_value, &ehci->ulpi_viewpoint);
> +
> +	/* Wait for the bits in ulpi_mask to become zero. */
> +	while (--timeout) {
> +		tmp = readl(&ehci->ulpi_viewpoint);
> +		if (!(tmp & ulpi_mask))
> +			break;
> +		WATCHDOG_RESET();
> +	}
> +
> +	return !timeout;
> +}
> +
> +int ulpi_wakeup(struct usb_ehci *ehci)

static

> +{
> +	if (readl(&ehci->ulpi_viewpoint) & ULPI_SS)
> +		return 0; /* already awake */
> +	return ulpi_wait(ehci, ULPI_WU, ULPI_WU);
> +}
> +
> +void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value)
> +{
> +	u32 tmp;
> +	if (ulpi_wakeup(ehci)) {
> +		printf("ULPI wakeup timed out\n");
> +		return;
> +	}
> +
> +	tmp = ulpi_wait(ehci, ULPI_RWRUN | ULPI_RWCTRL |
> +		reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value), ULPI_RWRUN);
> +	if (tmp)
> +		printf("ULPI write timed out\n");
> +}
> +
> +u32 ulpi_read(struct usb_ehci *ehci, u32 reg)
> +{
> +	if (ulpi_wakeup(ehci)) {
> +		printf("ULPI wakeup timed out\n");
> +		return 0;
> +	}
> +
> +	if (ulpi_wait(ehci, ULPI_RWRUN | reg << ULPI_ADDR_SHIFT, ULPI_RWRUN)) {
> +		printf("ULPI read timed out\n");
> +		return 0;
> +	}
> +
> +	return ulpi_read_mask(readl(&ehci->ulpi_viewpoint));
> +}
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> new file mode 100644
> index 0000000..b1c482a
> --- /dev/null
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -0,0 +1,123 @@
> +/*
> + * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> + * Based on:
> + * linux/drivers/usb/otg/ulpi.c
> + * Generic ULPI USB transceiver support
> + *
> + * Original Copyright follow:
> + * Copyright (C) 2009 Daniel Mack <daniel@caiaq.de>
> + *
> + * Based on sources from
> + *
> + *   Sascha Hauer <s.hauer@pengutronix.de>
> + *   Freescale Semiconductors
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <usb/ulpi.h>
> +#include <usb/ehci-fsl.h>
> +#include <exports.h>
> +
> +#ifdef DEBUG
> +#define debug(fmt, args...)	printf(fmt, ##args)
> +#else
> +#define debug(fmt, args...)
> +#endif /* DEBUG */

#include <common.h>

> +
> +void ulpi_integrity_check(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
> +{
> +	u32 tmp = 0;
> +	int i;
> +	for (i = 0; i < 2; i++) {
> +		ulpi_write(ehci, (u32)&ulpi->scratch_write,
> +			ULPI_TEST_VALUE << i);
> +		tmp = ulpi_read(ehci, (u32)&ulpi->scratch_write);
> +
> +		if (tmp != (ULPI_TEST_VALUE << i)) {
> +			printf("ULPI integrity check failed\n");
> +			return;
> +		}
> +	}
> +}
> +
> +/*
> + * This is a family of wrapper functions which sets bits in ULPI
> registers. + * Access mode could be WRITE, SET or CLEAR.
> + * For further informations see ULPI 1.1 specification.
> + */
> +void ulpi_otg_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags)

break params in half, not past fn

> +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_clear, flags);
> +		break;
> +	}
> +}
> +
> +void ulpi_function_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags) +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_clear, flags);
> +		break;
> +	}
> +}
> +
> +void ulpi_iface_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags) +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> +		break;
> +	}
> +
> +}

Is this crap from linux or something?

> +
> +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
> +{
> +	u32 tmp = 0;
> +	int reg;
> +
> +	/* Assemble ID from four ULPI ID registers (8 bits each). */
> +	for (reg = ULPI_ID_REGS_COUNT - 1; reg >= 0; reg--)
> +		tmp |= ulpi_read(ehci, reg) << (reg * 8);
> +
> +	/* Split ID into vendor and product ID. */
> +	debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
> +
> +	ulpi_integrity_check(ehci, ulpi);
> +}
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index 1519cc5..a8625a1 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -15,6 +15,8 @@
>  #ifndef __USB_ULPI_H
>  #define __USB_ULPI_H
> 
> +#include <usb/ehci-fsl.h>
> +
>  #define ULPI_ID_REGS_COUNT	4
>  #define ULPI_TEST_VALUE		0x55
>  #define ULPI_TIMEOUT		1000 /* some reasonable value */
> @@ -25,6 +27,11 @@
>  #define ULPI_RWRUN	(1 << 30)
>  #define ULPI_RWCTRL	(1 << 29)
> 
> +/* ULPI access modes */
> +#define WRITE	0x00
> +#define SET	0x01
> +#define CLEAR	0x02
> +
>  struct ulpi_regs {
>  	/* Vendor ID and Product ID: 0x00 - 0x03 Read-only */
>  	u8	vendor_id_low;
> @@ -201,4 +208,13 @@ struct ulpi_regs {
>  void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value);
>  u32 ulpi_read(struct usb_ehci *ehci, u32 reg);
> 
> +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi);
> +
> +void ulpi_otg_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags); +void ulpi_function_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags); +void ulpi_iface_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32
> flags); +
>  #endif /* __USB_ULPI_H */
Jana Rapava - Nov. 5, 2011, 11:08 p.m.
2011/11/5 Marek Vasut <marek.vasut@gmail.com>

>
> > +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
>
> So this works only with EHCI? How generic is it then ?
>
>
I thought until now that ULPI is EHCI-dependent, but isn't... Ok, what else
should be supported? OHCI? I haven't any hardware to test it, but I could
give it a try.


>
> > +void ulpi_iface_ctrl_flags
> > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
> u32
> > flags) +{
> > +     switch (access_mode) {
> > +     case WRITE:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> > +             break;
> > +     case SET:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> > +             break;
> > +     case CLEAR:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> > +             break;
> > +     }
> > +
> > +}
>
> Is this crap from linux or something?
>
>
No, Linux has offset-based access to ULPI registers, some structure
otg_transceiver, where the driver sets the bits which it wants to be set in
ULPI registers (if I understand it well) and family of functions, which set
bits according to informations in otg_transceiver.
Marek Vasut - Nov. 5, 2011, 11:13 p.m.
> 2011/11/5 Marek Vasut <marek.vasut@gmail.com>
> 
> > > +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
> > 
> > So this works only with EHCI? How generic is it then ?
> 
> I thought until now that ULPI is EHCI-dependent, but isn't... Ok, what else
> should be supported? OHCI? I haven't any hardware to test it, but I could
> give it a try.

What about xHCI? I have no idea about OHCI, but why won't you be able to have 
OHCI and ULPI PHY?
> 
> > > +void ulpi_iface_ctrl_flags
> > > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
> > 
> > u32
> > 
> > > flags) +{
> > > +     switch (access_mode) {
> > > +     case WRITE:
> > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> > > +             break;
> > > +     case SET:
> > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> > > +             break;
> > > +     case CLEAR:
> > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> > > +             break;
> > > +     }
> > > +
> > > +}
> > 
> > Is this crap from linux or something?
> 
> No, Linux has offset-based access to ULPI registers, some structure
> otg_transceiver, where the driver sets the bits which it wants to be set in
> ULPI registers (if I understand it well) and family of functions, which set
> bits according to informations in otg_transceiver.

Ok, you have writel() functions, why do you need this switch stuff ?
Jana Rapava - Nov. 5, 2011, 11:32 p.m.
2011/11/6 Marek Vasut <marek.vasut@gmail.com>

> > 2011/11/5 Marek Vasut <marek.vasut@gmail.com>
> >
> > > > +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
> > >
> > > So this works only with EHCI? How generic is it then ?
> >
> > I thought until now that ULPI is EHCI-dependent, but isn't... Ok, what
> else
> > should be supported? OHCI? I haven't any hardware to test it, but I could
> > give it a try.
>
> What about xHCI? I have no idea about OHCI, but why won't you be able to
> have
> OHCI and ULPI PHY?
>

I'll look at it.


> >
> > > > +void ulpi_iface_ctrl_flags
> > > > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int
> access_mode,
> > >
> > > u32
> > >
> > > > flags) +{
> > > > +     switch (access_mode) {
> > > > +     case WRITE:
> > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> > > > +             break;
> > > > +     case SET:
> > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> > > > +             break;
> > > > +     case CLEAR:
> > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> > > > +             break;
> > > > +     }
> > > > +
> > > > +}
> > >
> > > Is this crap from linux or something?
> >
> > No, Linux has offset-based access to ULPI registers, some structure
> > otg_transceiver, where the driver sets the bits which it wants to be set
> in
> > ULPI registers (if I understand it well) and family of functions, which
> set
> > bits according to informations in otg_transceiver.
>
> Ok, you have writel() functions, why do you need this switch stuff ?
>

I tried to design some interface, which would allow its user care only
about register, access mode and flags and not about the exact way this huge
bunch od u8 fields called "ulpi_regs" is implemented.
Marek Vasut - Nov. 5, 2011, 11:35 p.m.
> 2011/11/6 Marek Vasut <marek.vasut@gmail.com>
> 
> > > 2011/11/5 Marek Vasut <marek.vasut@gmail.com>
> > > 
> > > > > +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32
> > > > > ulpi_mask)
> > > > 
> > > > So this works only with EHCI? How generic is it then ?
> > > 
> > > I thought until now that ULPI is EHCI-dependent, but isn't... Ok, what
> > 
> > else
> > 
> > > should be supported? OHCI? I haven't any hardware to test it, but I
> > > could give it a try.
> > 
> > What about xHCI? I have no idea about OHCI, but why won't you be able to
> > have
> > OHCI and ULPI PHY?
> 
> I'll look at it.
> 
> > > > > +void ulpi_iface_ctrl_flags
> > > > > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int
> > 
> > access_mode,
> > 
> > > > u32
> > > > 
> > > > > flags) +{
> > > > > +     switch (access_mode) {
> > > > > +     case WRITE:
> > > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write,
> > > > > flags); +             break;
> > > > > +     case SET:
> > > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> > > > > +             break;
> > > > > +     case CLEAR:
> > > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear,
> > > > > flags); +             break;
> > > > > +     }
> > > > > +
> > > > > +}
> > > > 
> > > > Is this crap from linux or something?
> > > 
> > > No, Linux has offset-based access to ULPI registers, some structure
> > > otg_transceiver, where the driver sets the bits which it wants to be
> > > set
> > 
> > in
> > 
> > > ULPI registers (if I understand it well) and family of functions, which
> > 
> > set
> > 
> > > bits according to informations in otg_transceiver.
> > 
> > Ok, you have writel() functions, why do you need this switch stuff ?
> 
> I tried to design some interface, which would allow its user care only
> about register, access mode and flags and not about the exact way this huge
> bunch od u8 fields called "ulpi_regs" is implemented.

I'd assume the interface will expose something like:

* ULPI power up port
* ULPI power down power

maybe something else, no?
Igor Grinberg - Nov. 8, 2011, 11:08 a.m.
On 11/06/11 01:32, Jana Rapava wrote:
> 
> 
> 2011/11/6 Marek Vasut <marek.vasut@gmail.com <mailto:marek.vasut@gmail.com>>
> 
>     > 2011/11/5 Marek Vasut <marek.vasut@gmail.com <mailto:marek.vasut@gmail.com>>
>     >
>     > > > +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
>     > >
>     > > So this works only with EHCI? How generic is it then ?
>     >
>     > I thought until now that ULPI is EHCI-dependent, but isn't... Ok, what else
>     > should be supported? OHCI? I haven't any hardware to test it, but I could
>     > give it a try.
> 
>     What about xHCI? I have no idea about OHCI, but why won't you be able to have
>     OHCI and ULPI PHY?
> 
> 
> I'll look at it.
>  
> 
>     >
>     > > > +void ulpi_iface_ctrl_flags
>     > > > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
>     > >
>     > > u32
>     > >
>     > > > flags) +{
>     > > > +     switch (access_mode) {
>     > > > +     case WRITE:
>     > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
>     > > > +             break;
>     > > > +     case SET:
>     > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
>     > > > +             break;
>     > > > +     case CLEAR:
>     > > > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
>     > > > +             break;
>     > > > +     }
>     > > > +
>     > > > +}
>     > >
>     > > Is this crap from linux or something?
>     >
>     > No, Linux has offset-based access to ULPI registers, some structure
>     > otg_transceiver, where the driver sets the bits which it wants to be set in
>     > ULPI registers (if I understand it well) and family of functions, which set
>     > bits according to informations in otg_transceiver.
> 
>     Ok, you have writel() functions, why do you need this switch stuff ?
> 
> 
> I tried to design some interface, which would allow its user care only about register, access mode and flags and not about the exact way this huge bunch od u8 fields called "ulpi_regs" is implemented.

User should neither care about the register, nor the access mode.
User should ask the driver to do "something useful"
(e.g. put the phy to some mode, enable vbus, etc.),
and that can be done with flags and functional API.
Igor Grinberg - Nov. 8, 2011, 11:33 a.m.
Hi Jana,

Thanks for porting this.
Several comments below.

On 11/05/11 22:50, Jana Rapava wrote:
> Add generic functions for ULPI init and setting bits in 
> ULPI registers. 
> 
> Signed-off-by: Jana Rapava <fermata7@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  Makefile                         |    1 +
>  drivers/usb/ulpi/Makefile        |   44 ++++++++++++++
>  drivers/usb/ulpi/ulpi-viewport.c |   87 +++++++++++++++++++++++++++
>  drivers/usb/ulpi/ulpi.c          |  123 ++++++++++++++++++++++++++++++++++++++
>  include/usb/ulpi.h               |   16 +++++
>  5 files changed, 271 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/ulpi/Makefile
>  create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
>  create mode 100644 drivers/usb/ulpi/ulpi.c
> 
> diff --git a/Makefile b/Makefile
> index 571c3eb..a475cb9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -283,6 +283,7 @@ LIBS += drivers/usb/gadget/libusb_gadget.o
>  LIBS += drivers/usb/host/libusb_host.o
>  LIBS += drivers/usb/musb/libusb_musb.o
>  LIBS += drivers/usb/phy/libusb_phy.o
> +LIBS += drivers/usb/ulpi/libusb_ulpi.o
>  LIBS += drivers/video/libvideo.o
>  LIBS += drivers/watchdog/libwatchdog.o
>  LIBS += common/libcommon.o
> diff --git a/drivers/usb/ulpi/Makefile b/drivers/usb/ulpi/Makefile
> new file mode 100644
> index 0000000..f7b6e20
> --- /dev/null
> +++ b/drivers/usb/ulpi/Makefile
> @@ -0,0 +1,44 @@
> +#
> +# Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	:= $(obj)libusb_ulpi.o
> +
> +COBJS-$(CONFIG_USB_ULPI) += ulpi.o ulpi-viewport.o

This switch is fine for now, but not all viewport
implementations can use the ulpi-viewport.c file.
Please split, so we can have:
COBJS-$(CONFIG_USB_ULPI) += ulpi.o
which is generic and:
COBJS-$(CONFIG_USB_ULPI_VIEWPORT) += ulpi-viewport.o
which is more hardware specific.

> +
> +COBJS	:= $(COBJS-y)
> +SRCS	:= $(COBJS:.o=.c)
> +OBJS	:= $(addprefix $(obj),$(COBJS))
> +
> +all:	$(LIB)
> +
> +$(LIB):	$(obj).depend $(OBJS)
> +	$(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
> new file mode 100644
> index 0000000..a0c213e
> --- /dev/null
> +++ b/drivers/usb/ulpi/ulpi-viewport.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> + * Based on:
> + * linux/drivers/usb/otg/ulpi_viewport.c
> + *
> + * Original Copyright follow:
> + * Copyright (C) 2011 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <watchdog.h>
> +#include <asm/io.h>
> +#include <usb/ulpi.h>
> +#include <usb/ehci-fsl.h>
> +
> +/* ULPI viewport control bits */
> +#define ULPI_WU		(1 << 31)
> +#define ULPI_SS		(1 << 27)
> +#define ULPI_RWRUN	(1 << 30)
> +#define ULPI_RWCTRL	(1 << 29)
> +
> +#define ULPI_ADDR_SHIFT		16
> +#define ulpi_write_mask(value)	((value) & 0xff)
> +#define ulpi_read_mask(value)	(((value) >> 8) & 0xff)
> +
> +int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)

As Marek already stated, ULPI can be used with various interfaces
(e.g. EHCI, OHCI, XHCI), so here I'd suggest having:
u32 *ulpi_viewpoint
which is currently generic enough instead of:
struct usb_ehci *ehci
which is EHCI specific, in all ulpi_*() functions

Also, if this function is not used outside of this file,
please mark it static.

> +{
> +	int timeout = ULPI_TIMEOUT;
> +	u32 tmp;
> +
> +	writel(ulpi_value, &ehci->ulpi_viewpoint);
> +
> +	/* Wait for the bits in ulpi_mask to become zero. */
> +	while (--timeout) {
> +		tmp = readl(&ehci->ulpi_viewpoint);
> +		if (!(tmp & ulpi_mask))
> +			break;
> +		WATCHDOG_RESET();
> +	}

This loop does not have any timing constraints, so basically
it depends on the system frequency and some arbitrary timeout
value, which can be right/wrong on variable operational frequencies?
This is not good.
You need to use one of the ?delay() functions here.

> +
> +	return !timeout;
> +}
> +
> +int ulpi_wakeup(struct usb_ehci *ehci)

same here

> +{
> +	if (readl(&ehci->ulpi_viewpoint) & ULPI_SS)
> +		return 0; /* already awake */

Please, empty line here.

> +	return ulpi_wait(ehci, ULPI_WU, ULPI_WU);
> +}
> +
> +void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value)
> +{
> +	u32 tmp;
> +	if (ulpi_wakeup(ehci)) {
> +		printf("ULPI wakeup timed out\n");
> +		return;
> +	}
> +
> +	tmp = ulpi_wait(ehci, ULPI_RWRUN | ULPI_RWCTRL |
> +		reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value), ULPI_RWRUN);
> +	if (tmp)
> +		printf("ULPI write timed out\n");
> +}
> +
> +u32 ulpi_read(struct usb_ehci *ehci, u32 reg)
> +{
> +	if (ulpi_wakeup(ehci)) {
> +		printf("ULPI wakeup timed out\n");
> +		return 0;
> +	}
> +
> +	if (ulpi_wait(ehci, ULPI_RWRUN | reg << ULPI_ADDR_SHIFT, ULPI_RWRUN)) {
> +		printf("ULPI read timed out\n");
> +		return 0;
> +	}

Is 0 the best esoteric value we can return on failure?
Can't it be a real read value?
Also, shouldn't we define it as ULPI_ERROR or something?

> +
> +	return ulpi_read_mask(readl(&ehci->ulpi_viewpoint));
> +}
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> new file mode 100644
> index 0000000..b1c482a
> --- /dev/null
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -0,0 +1,123 @@
> +/*
> + * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
> + * Based on:
> + * linux/drivers/usb/otg/ulpi.c
> + * Generic ULPI USB transceiver support
> + *
> + * Original Copyright follow:
> + * Copyright (C) 2009 Daniel Mack <daniel@caiaq.de>
> + *
> + * Based on sources from
> + *
> + *   Sascha Hauer <s.hauer@pengutronix.de>
> + *   Freescale Semiconductors
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

Are you sure that it is the right address?
May be just remove the address...

> + */
> +
> +#include <usb/ulpi.h>
> +#include <usb/ehci-fsl.h>

Should not include EHCI specific stuff here.

> +#include <exports.h>
> +
> +#ifdef DEBUG
> +#define debug(fmt, args...)	printf(fmt, ##args)
> +#else
> +#define debug(fmt, args...)
> +#endif /* DEBUG */
> +
> +void ulpi_integrity_check(struct usb_ehci *ehci, struct ulpi_regs *ulpi)

Should not be EHCI specific.

> +{
> +	u32 tmp = 0;
> +	int i;
> +	for (i = 0; i < 2; i++) {
> +		ulpi_write(ehci, (u32)&ulpi->scratch_write,
> +			ULPI_TEST_VALUE << i);
> +		tmp = ulpi_read(ehci, (u32)&ulpi->scratch_write);
> +
> +		if (tmp != (ULPI_TEST_VALUE << i)) {
> +			printf("ULPI integrity check failed\n");
> +			return;
> +		}
> +	}
> +}
> +
> +/*
> + * This is a family of wrapper functions which sets bits in ULPI registers.
> + * Access mode could be WRITE, SET or CLEAR.

What about READ?
I know it can be done from any of those registers, but it is confusing.

> + * For further informations see ULPI 1.1 specification.
> + */
> +void ulpi_otg_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
> +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_clear, flags);
> +		break;
> +	}
> +}
> +
> +void ulpi_function_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
> +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->function_ctrl_clear, flags);
> +		break;
> +	}
> +}
> +
> +void ulpi_iface_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
> +{
> +	switch (access_mode) {
> +	case WRITE:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> +		break;
> +	case SET:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> +		break;
> +	case CLEAR:
> +		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> +		break;
> +	}
> +
> +}

All the above functions are just making users hard to understand
what they should do and what information they should know and
supply to the driver.
More function oriented API would be much more useful here.

> +
> +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
> +{
> +	u32 tmp = 0;
> +	int reg;
> +
> +	/* Assemble ID from four ULPI ID registers (8 bits each). */
> +	for (reg = ULPI_ID_REGS_COUNT - 1; reg >= 0; reg--)
> +		tmp |= ulpi_read(ehci, reg) << (reg * 8);
> +
> +	/* Split ID into vendor and product ID. */
> +	debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);

What is TX? Is it transceiver or xceiver or both in one?

> +
> +	ulpi_integrity_check(ehci, ulpi);
> +}
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index 1519cc5..a8625a1 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -15,6 +15,8 @@
>  #ifndef __USB_ULPI_H
>  #define __USB_ULPI_H
>  
> +#include <usb/ehci-fsl.h>
> +

This should not be here.

>  #define ULPI_ID_REGS_COUNT	4
>  #define ULPI_TEST_VALUE		0x55
>  #define ULPI_TIMEOUT		1000 /* some reasonable value */
> @@ -25,6 +27,11 @@
>  #define ULPI_RWRUN	(1 << 30)
>  #define ULPI_RWCTRL	(1 << 29)
>  
> +/* ULPI access modes */
> +#define WRITE	0x00
> +#define SET	0x01
> +#define CLEAR	0x02

Even if we go that way (which I think is wrong), the names are
too generic.
It should be at least prefixed with ULPI_*.

> +
>  struct ulpi_regs {
>  	/* Vendor ID and Product ID: 0x00 - 0x03 Read-only */
>  	u8	vendor_id_low;
> @@ -201,4 +208,13 @@ struct ulpi_regs {
>  void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value);
>  u32 ulpi_read(struct usb_ehci *ehci, u32 reg);
>  
> +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi);
> +
> +void ulpi_otg_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
> +void ulpi_function_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
> +void ulpi_iface_ctrl_flags
> +	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
> +
>  #endif /* __USB_ULPI_H */
Jana Rapava - Nov. 12, 2011, 1:09 a.m.
2011/11/8 Igor Grinberg <grinberg@compulab.co.il>

>
> > +/*
> > + * This is a family of wrapper functions which sets bits in ULPI
> registers.
> > + * Access mode could be WRITE, SET or CLEAR.
>
> What about READ?
> I know it can be done from any of those registers, but it is confusing.
>
> > + * For further informations see ULPI 1.1 specification.
> > + */
> > +void ulpi_otg_ctrl_flags
> > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
> u32 flags)
> > +{
> > +     switch (access_mode) {
> > +     case WRITE:
> > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_write, flags);
> > +             break;
> > +     case SET:
> > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_set, flags);
> > +             break;
> > +     case CLEAR:
> > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_clear, flags);
> > +             break;
> > +     }
> > +}
> > +
> > +void ulpi_function_ctrl_flags
> > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
> u32 flags)
> > +{
> > +     switch (access_mode) {
> > +     case WRITE:
> > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_write, flags);
> > +             break;
> > +     case SET:
> > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_set, flags);
> > +             break;
> > +     case CLEAR:
> > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_clear, flags);
> > +             break;
> > +     }
> > +}
> > +
> > +void ulpi_iface_ctrl_flags
> > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode,
> u32 flags)
> > +{
> > +     switch (access_mode) {
> > +     case WRITE:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
> > +             break;
> > +     case SET:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
> > +             break;
> > +     case CLEAR:
> > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
> > +             break;
> > +     }
> > +
> > +}
>
> All the above functions are just making users hard to understand
> what they should do and what information they should know and
> supply to the driver.
> More function oriented API would be much more useful here.
>
>
Please, could you be more specific? I would be thankful, because I know
only Linux kernel ULPI API, where driver has to fill otg_transciever
structure with bits it needs to set (if I understand it well) and I would
like to use something more lightweight here.

> > +
> > +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
> > +{
> > +     u32 tmp = 0;
> > +     int reg;
> > +
> > +     /* Assemble ID from four ULPI ID registers (8 bits each). */
> > +     for (reg = ULPI_ID_REGS_COUNT - 1; reg >= 0; reg--)
> > +             tmp |= ulpi_read(ehci, reg) << (reg * 8);
> > +
> > +     /* Split ID into vendor and product ID. */
> > +     debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
>
> What is TX? Is it transceiver or xceiver or both in one?
>
>
According to ULPI 1.1 specification, it is transceiver.

--
> Regards,
> Igor.
>

Regards,
Jana Rapava
Igor Grinberg - Nov. 14, 2011, 7:40 a.m.
Hi Jana,

On 11/12/11 03:09, Jana Rapava wrote:
> 
> 
> 2011/11/8 Igor Grinberg <grinberg@compulab.co.il <mailto:grinberg@compulab.co.il>>
> 
> 
>     > +/*
>     > + * This is a family of wrapper functions which sets bits in ULPI registers.
>     > + * Access mode could be WRITE, SET or CLEAR.
> 
>     What about READ?
>     I know it can be done from any of those registers, but it is confusing.
> 
>     > + * For further informations see ULPI 1.1 specification.
>     > + */
>     > +void ulpi_otg_ctrl_flags
>     > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
>     > +{
>     > +     switch (access_mode) {
>     > +     case WRITE:
>     > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_write, flags);
>     > +             break;
>     > +     case SET:
>     > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_set, flags);
>     > +             break;
>     > +     case CLEAR:
>     > +             ulpi_write(ehci, (u32)&ulpi->otg_ctrl_clear, flags);
>     > +             break;
>     > +     }
>     > +}
>     > +
>     > +void ulpi_function_ctrl_flags
>     > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
>     > +{
>     > +     switch (access_mode) {
>     > +     case WRITE:
>     > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_write, flags);
>     > +             break;
>     > +     case SET:
>     > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_set, flags);
>     > +             break;
>     > +     case CLEAR:
>     > +             ulpi_write(ehci, (u32)&ulpi->function_ctrl_clear, flags);
>     > +             break;
>     > +     }
>     > +}
>     > +
>     > +void ulpi_iface_ctrl_flags
>     > +     (struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
>     > +{
>     > +     switch (access_mode) {
>     > +     case WRITE:
>     > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
>     > +             break;
>     > +     case SET:
>     > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
>     > +             break;
>     > +     case CLEAR:
>     > +             ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
>     > +             break;
>     > +     }
>     > +
>     > +}
> 
>     All the above functions are just making users hard to understand
>     what they should do and what information they should know and
>     supply to the driver.
>     More function oriented API would be much more useful here.
> 
> 
> Please, could you be more specific? I would be thankful, because I know only Linux kernel ULPI API, where driver has to fill otg_transciever structure with bits it needs to set (if I understand it well) and I would like to use something more lightweight here.

Please, don't write long long lines, they are extremely hard to read.

Now regarding the API:
User needs to use the *functionality* without having the need to
look into the ULPI spec to figure out the bits that are needed
for that functionality (I'm not sure it is possible, but we should try).
What I suggest is to look into your use case with mx5,
identify the functionality that is needed for it to function properly
and implement it.

Currently, in your use case, I can identify following functionalities:
1) selecting the transceiver type
2) selecting vbus indicator type
3) Dp/Dm pull downs enable/disable
4) selecting operation mode
5) suspend/resume - can be useful for usb start/stop commands
6) reset
7) drive vbus internal/external
8) ...

You are writing a generic driver, right?
Then you should also go through the ULPI spec and see what
functionalities it provides.
Of course you don't have to implement all the functionalities
described by the spec (although it would be very nice of you),
but at least those that your board uses and in a generic way.

> 
>     > +
>     > +void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
>     > +{
>     > +     u32 tmp = 0;
>     > +     int reg;
>     > +
>     > +     /* Assemble ID from four ULPI ID registers (8 bits each). */
>     > +     for (reg = ULPI_ID_REGS_COUNT - 1; reg >= 0; reg--)
>     > +             tmp |= ulpi_read(ehci, reg) << (reg * 8);
>     > +
>     > +     /* Split ID into vendor and product ID. */
>     > +     debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
> 
>     What is TX? Is it transceiver or xceiver or both in one?
> 
> 
> According to ULPI 1.1 specification, it is transceiver.

then can it be transceiver instead of TX?

Patch

diff --git a/Makefile b/Makefile
index 571c3eb..a475cb9 100644
--- a/Makefile
+++ b/Makefile
@@ -283,6 +283,7 @@  LIBS += drivers/usb/gadget/libusb_gadget.o
 LIBS += drivers/usb/host/libusb_host.o
 LIBS += drivers/usb/musb/libusb_musb.o
 LIBS += drivers/usb/phy/libusb_phy.o
+LIBS += drivers/usb/ulpi/libusb_ulpi.o
 LIBS += drivers/video/libvideo.o
 LIBS += drivers/watchdog/libwatchdog.o
 LIBS += common/libcommon.o
diff --git a/drivers/usb/ulpi/Makefile b/drivers/usb/ulpi/Makefile
new file mode 100644
index 0000000..f7b6e20
--- /dev/null
+++ b/drivers/usb/ulpi/Makefile
@@ -0,0 +1,44 @@ 
+#
+# Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	:= $(obj)libusb_ulpi.o
+
+COBJS-$(CONFIG_USB_ULPI) += ulpi.o ulpi-viewport.o
+
+COBJS	:= $(COBJS-y)
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+all:	$(LIB)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/drivers/usb/ulpi/ulpi-viewport.c b/drivers/usb/ulpi/ulpi-viewport.c
new file mode 100644
index 0000000..a0c213e
--- /dev/null
+++ b/drivers/usb/ulpi/ulpi-viewport.c
@@ -0,0 +1,87 @@ 
+/*
+ * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
+ * Based on:
+ * linux/drivers/usb/otg/ulpi_viewport.c
+ *
+ * Original Copyright follow:
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <watchdog.h>
+#include <asm/io.h>
+#include <usb/ulpi.h>
+#include <usb/ehci-fsl.h>
+
+/* ULPI viewport control bits */
+#define ULPI_WU		(1 << 31)
+#define ULPI_SS		(1 << 27)
+#define ULPI_RWRUN	(1 << 30)
+#define ULPI_RWCTRL	(1 << 29)
+
+#define ULPI_ADDR_SHIFT		16
+#define ulpi_write_mask(value)	((value) & 0xff)
+#define ulpi_read_mask(value)	(((value) >> 8) & 0xff)
+
+int ulpi_wait(struct usb_ehci *ehci, u32 ulpi_value, u32 ulpi_mask)
+{
+	int timeout = ULPI_TIMEOUT;
+	u32 tmp;
+
+	writel(ulpi_value, &ehci->ulpi_viewpoint);
+
+	/* Wait for the bits in ulpi_mask to become zero. */
+	while (--timeout) {
+		tmp = readl(&ehci->ulpi_viewpoint);
+		if (!(tmp & ulpi_mask))
+			break;
+		WATCHDOG_RESET();
+	}
+
+	return !timeout;
+}
+
+int ulpi_wakeup(struct usb_ehci *ehci)
+{
+	if (readl(&ehci->ulpi_viewpoint) & ULPI_SS)
+		return 0; /* already awake */
+	return ulpi_wait(ehci, ULPI_WU, ULPI_WU);
+}
+
+void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value)
+{
+	u32 tmp;
+	if (ulpi_wakeup(ehci)) {
+		printf("ULPI wakeup timed out\n");
+		return;
+	}
+
+	tmp = ulpi_wait(ehci, ULPI_RWRUN | ULPI_RWCTRL |
+		reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value), ULPI_RWRUN);
+	if (tmp)
+		printf("ULPI write timed out\n");
+}
+
+u32 ulpi_read(struct usb_ehci *ehci, u32 reg)
+{
+	if (ulpi_wakeup(ehci)) {
+		printf("ULPI wakeup timed out\n");
+		return 0;
+	}
+
+	if (ulpi_wait(ehci, ULPI_RWRUN | reg << ULPI_ADDR_SHIFT, ULPI_RWRUN)) {
+		printf("ULPI read timed out\n");
+		return 0;
+	}
+
+	return ulpi_read_mask(readl(&ehci->ulpi_viewpoint));
+}
diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
new file mode 100644
index 0000000..b1c482a
--- /dev/null
+++ b/drivers/usb/ulpi/ulpi.c
@@ -0,0 +1,123 @@ 
+/*
+ * Copyright (C) 2011 Jana Rapava <fermata7@gmail.com>
+ * Based on:
+ * linux/drivers/usb/otg/ulpi.c
+ * Generic ULPI USB transceiver support
+ *
+ * Original Copyright follow:
+ * Copyright (C) 2009 Daniel Mack <daniel@caiaq.de>
+ *
+ * Based on sources from
+ *
+ *   Sascha Hauer <s.hauer@pengutronix.de>
+ *   Freescale Semiconductors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <usb/ulpi.h>
+#include <usb/ehci-fsl.h>
+#include <exports.h>
+
+#ifdef DEBUG
+#define debug(fmt, args...)	printf(fmt, ##args)
+#else
+#define debug(fmt, args...)
+#endif /* DEBUG */
+
+void ulpi_integrity_check(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
+{
+	u32 tmp = 0;
+	int i;
+	for (i = 0; i < 2; i++) {
+		ulpi_write(ehci, (u32)&ulpi->scratch_write,
+			ULPI_TEST_VALUE << i);
+		tmp = ulpi_read(ehci, (u32)&ulpi->scratch_write);
+
+		if (tmp != (ULPI_TEST_VALUE << i)) {
+			printf("ULPI integrity check failed\n");
+			return;
+		}
+	}
+}
+
+/*
+ * This is a family of wrapper functions which sets bits in ULPI registers.
+ * Access mode could be WRITE, SET or CLEAR.
+ * For further informations see ULPI 1.1 specification.
+ */
+void ulpi_otg_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
+{
+	switch (access_mode) {
+	case WRITE:
+		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_write, flags);
+		break;
+	case SET:
+		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_set, flags);
+		break;
+	case CLEAR:
+		ulpi_write(ehci, (u32)&ulpi->otg_ctrl_clear, flags);
+		break;
+	}
+}
+
+void ulpi_function_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
+{
+	switch (access_mode) {
+	case WRITE:
+		ulpi_write(ehci, (u32)&ulpi->function_ctrl_write, flags);
+		break;
+	case SET:
+		ulpi_write(ehci, (u32)&ulpi->function_ctrl_set, flags);
+		break;
+	case CLEAR:
+		ulpi_write(ehci, (u32)&ulpi->function_ctrl_clear, flags);
+		break;
+	}
+}
+
+void ulpi_iface_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags)
+{
+	switch (access_mode) {
+	case WRITE:
+		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_write, flags);
+		break;
+	case SET:
+		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_set, flags);
+		break;
+	case CLEAR:
+		ulpi_write(ehci, (u32)&ulpi->iface_ctrl_clear, flags);
+		break;
+	}
+
+}
+
+void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi)
+{
+	u32 tmp = 0;
+	int reg;
+
+	/* Assemble ID from four ULPI ID registers (8 bits each). */
+	for (reg = ULPI_ID_REGS_COUNT - 1; reg >= 0; reg--)
+		tmp |= ulpi_read(ehci, reg) << (reg * 8);
+
+	/* Split ID into vendor and product ID. */
+	debug("Found ULPI TX, ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
+
+	ulpi_integrity_check(ehci, ulpi);
+}
diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
index 1519cc5..a8625a1 100644
--- a/include/usb/ulpi.h
+++ b/include/usb/ulpi.h
@@ -15,6 +15,8 @@ 
 #ifndef __USB_ULPI_H
 #define __USB_ULPI_H
 
+#include <usb/ehci-fsl.h>
+
 #define ULPI_ID_REGS_COUNT	4
 #define ULPI_TEST_VALUE		0x55
 #define ULPI_TIMEOUT		1000 /* some reasonable value */
@@ -25,6 +27,11 @@ 
 #define ULPI_RWRUN	(1 << 30)
 #define ULPI_RWCTRL	(1 << 29)
 
+/* ULPI access modes */
+#define WRITE	0x00
+#define SET	0x01
+#define CLEAR	0x02
+
 struct ulpi_regs {
 	/* Vendor ID and Product ID: 0x00 - 0x03 Read-only */
 	u8	vendor_id_low;
@@ -201,4 +208,13 @@  struct ulpi_regs {
 void ulpi_write(struct usb_ehci *ehci, u32 reg, u32 value);
 u32 ulpi_read(struct usb_ehci *ehci, u32 reg);
 
+void ulpi_init(struct usb_ehci *ehci, struct ulpi_regs *ulpi);
+
+void ulpi_otg_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
+void ulpi_function_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
+void ulpi_iface_ctrl_flags
+	(struct usb_ehci *ehci, struct ulpi_regs *ulpi, int access_mode, u32 flags);
+
 #endif /* __USB_ULPI_H */