[U-Boot] [PATCH v4] ulpi: add generic ULPI functionality
Igor Grinberg
grinberg at compulab.co.il
Sun Nov 27 09:08:13 CET 2011
Hi Simon,
On 11/27/11 06:00, Simon Glass wrote:
> Hi Jana,
>
> I am interested in this patch. It seems you could tidy the code a
> litte - sorry if I am too late with comments.
It is never too late, but please consider the following:
there are several patch series depending on this one,
so how about we make only a "no go" changes, so other patch series
could go in and then we can fix the rest?
>
> On Fri, Nov 25, 2011 at 12:05 PM, Jana Rapava <fermata7 at gmail.com> wrote:
>> Add generic functions for ULPI init and setting bits in
>> ULPI registers.
>>
>> Signed-off-by: Jana Rapava <fermata7 at gmail.com>
>> Cc: Marek Vasut <marek.vasut at gmail.com>
>> Cc: Remy Bohmer <linux at bohmer.net>
>> Cc: Stefano Babic <sbabic at denx.de>
>> Cc: Igor Grinberg <grinberg at compulab.co.il>
>> Cc: Wolfgang Grandegger <wg at denx.de>
>> ---
>> Changes for v2:
>> - make code EHCI-independent
>> - use udelay() in waiting loop
>> - mark static functions as static
>> - naming changes
>> Changes for v3:
>> - merge with patch ulpi: add generic ULPI support header file
>> - rewrite ULPI interface in more functionality-oriented way
>> Changes for v4:
>> - add error-checking
>> - add waiting for completion into ulpi_reset() function
>>
>> Makefile | 1 +
>> drivers/usb/ulpi/Makefile | 45 +++++++
>> drivers/usb/ulpi/ulpi-viewport.c | 91 ++++++++++++++
>> drivers/usb/ulpi/ulpi.c | 256 ++++++++++++++++++++++++++++++++++++++
>> include/usb/ulpi.h | 221 ++++++++++++++++++++++++++++++++
>> 5 files changed, 614 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
>> create mode 100644 include/usb/ulpi.h
>>
>> diff --git a/Makefile b/Makefile
>> index c9e2624..da7352c 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..a1a2244
>> --- /dev/null
>> +++ b/drivers/usb/ulpi/Makefile
>> @@ -0,0 +1,45 @@
>> +#
>> +# Copyright (C) 2011 Jana Rapava <fermata7 at 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
>> +COBJS-$(CONFIG_USB_ULPI_VIEWPORT) += 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..8f7d94c
>> --- /dev/null
>> +++ b/drivers/usb/ulpi/ulpi-viewport.c
>> @@ -0,0 +1,91 @@
>> +/*
>> + * Copyright (C) 2011 Jana Rapava <fermata7 at 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 <common.h>
>> +#include <asm/io.h>
>> +#include <usb/ulpi.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)
>> +
>> +static int ulpi_wait(u32 ulpi_viewport, u32 ulpi_value, u32 ulpi_mask)
>
> How about a comment as to what this function does, params and return values?
>
> Also perhaps if you pass the operation to this function it can print
> the error message (perhaps with debug()) rather than ever calling
> having to do it? Or at least decide whether messages should be in
> bottom-level functions or in top-level - and perhaps considering using
> debug() instead of printf() at lower levels.
I would like to consider this as a verbosity fix/cleanup and
have this fixed later, can we?
>
>> +{
>> + int timeout = CONFIG_USB_ULPI_TIMEOUT;
>> + u32 tmp;
>> +
>> + writel(ulpi_value, ulpi_viewport);
>
> Why do the write here? Should it be done in the write call below? At
> the very least you should rename this function.
If you move this writel() from here, then you need to add
it to every function calling this one (it is not just the write below).
It can be done, but again can we do this later?
>
>> +
>> + /* Wait for the bits in ulpi_mask to become zero. */
>> + while (--timeout) {
>> + tmp = readl(ulpi_viewport);
>> + if (!(tmp & ulpi_mask))
>> + break;
>
> return 0?
>
>> + udelay(1);
>> + }
>> +
>> + return !timeout;
>
> return 1 or -1? Or ULPI_ERROR?
As this is a static function, can we fix this later?
>
>> +}
>> +
>> +static int ulpi_wakeup(u32 ulpi_viewport)
>> +{
>> + if (readl(ulpi_viewport) & ULPI_SS)
>> + return 0; /* already awake */
>> +
>> + return ulpi_wait(ulpi_viewport, ULPI_WU, ULPI_WU);
>> +}
>> +
>> +u32 ulpi_write(u32 ulpi_viewport, u32 reg, u32 value)
>
> It seems that every caller requires a cast for the second parameter.
> Perhaps you could avoid this if you make the reg parameter u8 *
> instead?
This is what I would call a "no go"...
Jana, can you, please deal with this?
And if you do, then you can deal with *most* of other comments as well.
>
>> +{
>> + u32 tmp;
>
> blank line after declarations
>
>> + if (ulpi_wakeup(ulpi_viewport)) {
>> + printf("ULPI wakeup timed out\n");
>> + return ULPI_ERROR;
>> + }
>
> Shouldn't you do a writel() here?
if so, then also in read and wakeup...
>
>> +
>> + tmp = ulpi_wait(ulpi_viewport, ULPI_RWRUN | ULPI_RWCTRL |
>> + reg << ULPI_ADDR_SHIFT | ulpi_write_mask(value), ULPI_RWRUN);
>> + if (tmp) {
>> + printf("ULPI write timed out\n");
>> + return ULPI_ERROR;
>> + }
>> + return 0;
>> +}
>> +
>> +u32 ulpi_read(u32 ulpi_viewport, u32 reg)
>> +{
>> + if (ulpi_wakeup(ulpi_viewport)) {
>> + printf("ULPI wakeup timed out\n");
>> + return ULPI_ERROR;
>> + }
>> +
>> + if (ulpi_wait(ulpi_viewport, ULPI_RWRUN | reg << ULPI_ADDR_SHIFT,
>> + ULPI_RWRUN)) {
>> + printf("ULPI read timed out\n");
>> + return ULPI_ERROR;
>> + }
>> +
>> + return ulpi_read_mask(readl(ulpi_viewport));
>> +}
>> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
>> new file mode 100644
>> index 0000000..83a8038
>> --- /dev/null
>> +++ b/drivers/usb/ulpi/ulpi.c
>> @@ -0,0 +1,256 @@
>> +/*
>> + * Copyright (C) 2011 Jana Rapava <fermata7 at gmail.com>
>> + * Based on:
>> + * linux/drivers/usb/otg/ulpi.c
>> + * Generic ULPI USB transceiver support
>> + *
>> + * Original Copyright follow:
>> + * Copyright (C) 2009 Daniel Mack <daniel at caiaq.de>
>> + *
>> + * Based on sources from
>> + *
>> + * Sascha Hauer <s.hauer at 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.
>> + */
>> +
>> +#include <common.h>
>> +#include <exports.h>
>> +#include <usb/ulpi.h>
>> +
>> +static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
>> +
>> +/*
>> + * This is a generic ULPI interface implemenatation.
>> + * All functions return 0 in the case of success, ULPI_ERROR otherwise.
>> + */
>> +
>> +static int ulpi_integrity_check(u32 ulpi_viewport)
>> +{
>> + u32 tmp = 0;
>> + int i;
>
> blank line after decls
>
>> + for (i = 0; i < 2; i++) {
>
> Why 2? Do you need a comment here?
>
>> + ulpi_write(ulpi_viewport, (u32)&ulpi->scratch,
>> + ULPI_TEST_VALUE << i);
>> + tmp = ulpi_read(ulpi_viewport, (u32)&ulpi->scratch);
>> +
>> + if (tmp != (ULPI_TEST_VALUE << i)) {
>> + printf("ULPI integrity check failed\n");
>> + return ULPI_ERROR;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * This function is used to select transceiver speed.
>> + * Accepted parameter values are:
>> + * ULPI_FC_HIGH_SPEED, ULPI_FC_FULL_SPEED, ULPI_FC_LOW_SPEED, ULPI_FC_FS4LS
>> + * (FS transceiver for LS packets).
>> + * Default value is ULPI_FC_FULL_SPEED.
>> + */
>> +int ulpi_select_transceiver(u32 ulpi_viewport, int speed)
>> +{
>> + switch (speed) {
>> + case ULPI_FC_HIGH_SPEED:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_HIGH_SPEED))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_FULL_SPEED:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_FULL_SPEED))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_LOW_SPEED:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_LOW_SPEED))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_FS4LS:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_FS4LS))
>> + return ULPI_ERROR;
>> + break;
>
> Am I missing something?:
>
> + case ULPI_FC_HIGH_SPEED:
> + case ULPI_FC_FULL_SPEED:
> + case ULPI_FC_LOW_SPEED:
> + case ULPI_FC_FS4LS:
> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
> + speed))
> + return ULPI_ERROR;
Just:
return ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set, speed);
> + break;
>
>
>> + default:
>> + printf("ulpi_select_transceiver: unknown transceiver speed\n");
>> + return ULPI_ERROR;
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * Signals that 5V is driven to VBUS.
>> + * Ext_power_supply is 1, if we use external supply instead of
>> + * internal charge pump(which is default).
>> + * Use_ext_indicator is 1, if we use external VBUS over-current indicator.
>> + */
>> +int ulpi_drive_vbus(u32 ulpi_viewport,
>> + int ext_power_supply, int use_ext_indicator)
>> +{
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set,
>> + ULPI_OTG_DRVVBUS))
>> + return ULPI_ERROR;
>
> You could have a variable which is set to either of the 3 options,
> then call the function once.
Correct,
return ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set, flags);
>
>> +
>> + if (ext_power_supply) {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set,
>> + ULPI_OTG_DRVVBUS_EXT))
>> + return ULPI_ERROR;
>> + }
>> +
>> + if (use_ext_indicator) {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set,
>> + ULPI_OTG_EXTVBUSIND))
>> + return ULPI_ERROR;
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * If enable is 0, pull-down resistor not connected to D+ else pull-down
>> + * resistor connected to D+.
>> + * Default behaviour is as for enable equal to 1.
>> + */
>> +int ulpi_dp_pulldown(u32 ulpi_viewport, int enable)
>> +{
>> + if (enable) {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set,
>> + ULPI_OTG_DP_PULLDOWN))
>> + return ULPI_ERROR;
>> + } else {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_clear,
>> + ULPI_OTG_DP_PULLDOWN))
>> + return ULPI_ERROR;
>> + }
>> + return 0;
>
> Is something like this better?
>
> return ulpi_write(ulpi_viewport, enable ? (u32)&ulpi->otg_ctrl_set
> : (u32)&ulpi->otg_ctrl_clear,
> ULPI_OTG_DP_PULLDOWN);
I don't think so - it is unreadable, but:
u32 reg = enable ? (u32)&ulpi->otg_ctrl_set
: (u32)&ulpi->otg_ctrl_clear;
return ulpi_write(ulpi_viewport, reg, ULPI_OTG_DP_PULLDOWN);
I think will do better.
>
>> +}
>> +
>> +/*
>> + * If enable is 0, pull-down resistor not connected to D- else pull-down
>> + * resistor connected to D-.
>> + * Default behaviour is as for enable equal to 1.
>> + */
>> +int ulpi_dm_pulldown(u32 ulpi_viewport, int enable)
>> +{
>> + if (enable) {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_set,
>> + ULPI_OTG_DM_PULLDOWN))
>> + return ULPI_ERROR;
>> + } else {
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->otg_ctrl_clear,
>> + ULPI_OTG_DM_PULLDOWN))
>> + return ULPI_ERROR;
>> + }
>
> as above
>
>> + return 0;
>> +}
Both above functions should be unite into one.
>> +
>> +/*
>> + * This function is used to select bit encoding style.
>> + * Accepted parameter values are:
>> + * ULPI_FC_OPMODE_NORMAL, ULPI_FC_OPMODE_NONDRIVING,
>> + * ULPI_FC_OPMODE_DISABLE_NRZI, ULPI_FC_OPMODE_NOSYNC_NOEOP.
>> + * Default value is ULPI_FC_OPMODE_NORMAL.
>> + */
>> +int ulpi_select_opmode(u32 ulpi_viewport, int style)
I think it will make more sense:
s/style/opmode/.
>> +{
>> + switch (style) {
>> + case ULPI_FC_OPMODE_NORMAL:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_OPMODE_NORMAL))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_OPMODE_NONDRIVING:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_OPMODE_NONDRIVING))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_OPMODE_DISABLE_NRZI:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_OPMODE_DISABLE_NRZI))
>> + return ULPI_ERROR;
>> + break;
>> + case ULPI_FC_OPMODE_NOSYNC_NOEOP:
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_OPMODE_NOSYNC_NOEOP))
>> + return ULPI_ERROR;
>> + break;
>
> as above
and
return ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set, opmode);
>
>> + default:
>> + printf("ulpi_select_opmode: unknown bit encoding style\n");
>> + return ULPI_ERROR;
>> + }
>> + return 0;
>> +}
>> +
>> +/* Put PHY into low power mode. */
>> +int ulpi_suspend(u32 ulpi_viewport)
>> +{
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_clear,
>> + ULPI_FC_SUSPENDM))
>> + return ULPI_ERROR;
>> + return 0;
return ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_clear,
ULPI_FC_SUSPENDM);
>> +}
>> +
>> +/* Put PHY out of low power mode. */
>> +int ulpi_resume(u32 ulpi_viewport)
>> +{
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_SUSPENDM))
>> + return ULPI_ERROR;
>> + return 0;
same here
>> +}
>> +
>> +static int reset_wait(u32 ulpi_viewport)
>> +{
>> + int timeout = CONFIG_USB_ULPI_TIMEOUT;
>> + u32 tmp;
>> +
>> + if (ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set,
>> + ULPI_FC_RESET))
>> + return ULPI_ERROR;
here, make it:
ret = ulpi_write(ulpi_viewport, (u32)&ulpi->function_ctrl_set, ULPI_FC_RESET)
if (ret)
return ret;
so the value can propagate.
>> +
>> + /* Wait for the RESET bit to become zero. */
>> + while (--timeout) {
>> + tmp = ulpi_read(ulpi_viewport, (u32)&ulpi->function_ctrl);
>> + if (!(tmp & ULPI_FC_RESET))
>> + break;
>> + udelay(1);
>> + }
>> +
>> + return !timeout;
>> +}
>> +
>> +/* Reset transceiver, does not reset ULPI interface or ULPI register set. */
>> +int ulpi_reset(u32 ulpi_viewport)
>> +{
>> + /* Have to wait for reset to complete */
>> + if (reset_wait(ulpi_viewport))
>> + return ULPI_ERROR;
>> + return 0;
>
> Why not just
>
> return reset_wait(ulpi_viewport);
Or even more: why not make it a single function ulpi_reset()?
instead of having a wrapper that does nothing.
>
>> +}
>> +
>> +int ulpi_init(u32 ulpi_viewport)
>> +{
>> + 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(ulpi_viewport, reg) << (reg * 8);
>> +
>> + /* Split ID into vendor and product ID. */
>> + debug("Found ULPI transceiver ID %04x:%04x\n", tmp >> 16, tmp & 0xffff);
>> +
>> + if (ulpi_integrity_check(ulpi_viewport))
>> + return ULPI_ERROR;
>> +
>> + return 0;
Also, here, you can:
return ulpi_integrity_check(ulpi_viewport);
>> +}
>> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
>> new file mode 100644
>> index 0000000..806fef7
>> --- /dev/null
>> +++ b/include/usb/ulpi.h
>> @@ -0,0 +1,221 @@
>> +/*
>> + * Copyright (C) 2011 Jana Rapava <fermata7 at gmail.com>
>> + * Based on:
>> + * linux/include/linux/usb/ulpi.h
>> + * ULPI defines and function prototypes
>> + *
>> + * Original Copyrights follow:
>> + * Copyright (C) 2010 Nokia Corporation
>> + *
>> + * This software is distributed under the terms of the GNU General
>> + * Public License ("GPL") as published by the Free Software Foundation,
>> + * version 2 of that License.
>> + */
>> +
>> +#ifndef __USB_ULPI_H
>> +#define __USB_ULPI_H
>> +
>> +#define ULPI_ID_REGS_COUNT 4
>> +#define ULPI_TEST_VALUE 0x55
>> +/* value greater than 0xff indicates failure */
>> +#define ULPI_ERROR (1 << 8)
>> +
>> +/* ULPI viewport control bits */
>> +#define ULPI_WU (1 << 31)
>> +#define ULPI_SS (1 << 27)
>> +#define ULPI_RWRUN (1 << 30)
>> +#define ULPI_RWCTRL (1 << 29)
>> +
>> +/* ULPI access modes */
>> +#define ULPI_WRITE 0x00
>> +#define ULPI_SET 0x01
>> +#define ULPI_CLEAR 0x02
>> +
>> +struct ulpi_regs {
>> + /* Vendor ID and Product ID: 0x00 - 0x03 Read-only */
>> + u8 vendor_id_low;
>> + u8 vendor_id_high;
>> + u8 product_id_low;
>> + u8 product_id_high;
>> + /* Function Control: 0x04 - 0x06 Read */
>> + u8 function_ctrl; /* 0x04 Write */
>> + u8 function_ctrl_set; /* 0x05 Set */
>> + u8 function_ctrl_clear; /* 0x06 Clear */
>> + /* Interface Control: 0x07 - 0x09 Read */
>> + u8 iface_ctrl; /* 0x07 Write */
>> + u8 iface_ctrl_set; /* 0x08 Set */
>> + u8 iface_ctrl_clear; /* 0x09 Clear */
>> + /* OTG Control: 0x0A - 0x0C Read */
>> + u8 otg_ctrl; /* 0x0A Write */
>> + u8 otg_ctrl_set; /* 0x0B Set */
>> + u8 otg_ctrl_clear; /* 0x0C Clear */
>> + /* USB Interrupt Enable Rising: 0x0D - 0x0F Read */
>> + u8 usb_ie_rising; /* 0x0D Write */
>> + u8 usb_ie_rising_set; /* 0x0E Set */
>> + u8 usb_ie_rising_clear; /* 0x0F Clear */
>> + /* USB Interrupt Enable Falling: 0x10 - 0x12 Read */
>> + u8 usb_ie_falling; /* 0x10 Write */
>> + u8 usb_ie_falling_set; /* 0x11 Set */
>> + u8 usb_ie_falling_clear; /* 0x12 Clear */
>> + /* USB Interrupt Status: 0x13 Read-only */
>> + u8 usb_int_status;
>> + /* USB Interrupt Latch: 0x14 Read-only with auto-clear */
>> + u8 usb_int_latch;
>> + /* Debug: 0x15 Read-only */
>> + u8 debug;
>> + /* Scratch Register: 0x16 - 0x18 Read */
>> + u8 scratch; /* 0x16 Write */
>> + u8 scratch_set; /* 0x17 Set */
>> + u8 scratch_clear; /* 0x18 Clear */
>> + /*
>> + * Optional Carkit registers
>> + */
>> + /* Carkit Control: 0x19 - 0x1B Read */
>> + u8 carkit_ctrl; /* 0x19 Write */
>> + u8 carkit_ctrl_set; /* 0x1A Set */
>> + u8 carkit_ctrl_clear; /* 0x1B Clear */
>> + /* Carkit Interrupt Delay: 0x1C Read, Write */
>> + u8 carkit_int_delay;
>> + /* Carkit Interrupt Enable: 0x1D - 0x1F Read */
>> + u8 carkit_ie; /* 0x1D Write */
>> + u8 carkit_ie_set; /* 0x1E Set */
>> + u8 carkit_ie_clear; /* 0x1F Clear */
>> + /* Carkit Interrupt Status: 0x20 Read-only */
>> + u8 carkit_int_status;
>> + /* Carkit Interrupt Latch: 0x21 Read-only with auto-clear */
>> + u8 carkit_int_latch;
>> + /* Carkit Pulse Control: 0x22 - 0x24 Read */
>> + u8 carkit_pulse_ctrl; /* 0x22 Write */
>> + u8 carkit_pulse_ctrl_set; /* 0x23 Set */
>> + u8 carkit_pulse_ctrl_clear; /* 0x24 Clear */
>> + /*
>> + * Other optional registers
>> + */
>> + /* Transmit Positive Width: 0x25 Read, Write */
>> + u8 transmit_pos_width;
>> + /* Transmit Negative Width: 0x26 Read, Write */
>> + u8 transmit_neg_width;
>> + /* Receive Polarity Recovery: 0x27 Read, Write */
>> + u8 recv_pol_recovery;
>> + /*
>> + * Addresses 0x28 - 0x2E are reserved, so we use offsets
>> + * for immediate registers with higher addresses
>> + */
>> +};
>> +
>> +/* Access Extended Register Set (indicator) */
>> +#define ACCESS_EXT_REGS_OFFSET 0x2f /* read-write */
>> +/* Vendor-specific */
>> +#define VENDOR_SPEC_OFFSET 0x30
>> +
>> +/*
>> + * Extended Register Set
>> + *
>> + * Addresses 0x00-0x3F map directly to Immediate Register Set.
>> + * Addresses 0x40-0x7F are reserved.
>> + * Addresses 0x80-0xff are vendor-specific.
>> + */
>> +#define EXT_VENDOR_SPEC_OFFSET 0x80
>> +
>> +/*
>> + * Register Bits
>> + */
>> +
>> +/* Function Control */
>> +#define ULPI_FC_XCVRSEL (1 << 0)
>> +#define ULPI_FC_XCVRSEL_MASK (3 << 0)
>> +#define ULPI_FC_HIGH_SPEED (0 << 0)
>> +#define ULPI_FC_FULL_SPEED (1 << 0)
>> +#define ULPI_FC_LOW_SPEED (2 << 0)
>> +#define ULPI_FC_FS4LS (3 << 0)
>> +#define ULPI_FC_TERMSELECT (1 << 2)
>> +#define ULPI_FC_OPMODE (1 << 3)
>> +#define ULPI_FC_OPMODE_MASK (3 << 3)
>> +#define ULPI_FC_OPMODE_NORMAL (0 << 3)
>> +#define ULPI_FC_OPMODE_NONDRIVING (1 << 3)
>> +#define ULPI_FC_OPMODE_DISABLE_NRZI (2 << 3)
>> +#define ULPI_FC_OPMODE_NOSYNC_NOEOP (3 << 3)
>> +#define ULPI_FC_RESET (1 << 5)
>> +#define ULPI_FC_SUSPENDM (1 << 6)
>> +
>> +/* Interface Control */
>> +#define ULPI_IFACE_6_PIN_SERIAL_MODE (1 << 0)
>> +#define ULPI_IFACE_3_PIN_SERIAL_MODE (1 << 1)
>> +#define ULPI_IFACE_CARKITMODE (1 << 2)
>> +#define ULPI_IFACE_CLOCKSUSPENDM (1 << 3)
>> +#define ULPI_IFACE_AUTORESUME (1 << 4)
>> +#define ULPI_IFACE_EXTVBUS_COMPLEMENT (1 << 5)
>> +#define ULPI_IFACE_PASSTHRU (1 << 6)
>> +#define ULPI_IFACE_PROTECT_IFC_DISABLE (1 << 7)
>> +
>> +/* OTG Control */
>> +#define ULPI_OTG_ID_PULLUP (1 << 0)
>> +#define ULPI_OTG_DP_PULLDOWN (1 << 1)
>> +#define ULPI_OTG_DM_PULLDOWN (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)
>> +
>> +/*
>> + * USB Interrupt Enable Rising,
>> + * USB Interrupt Enable Falling,
>> + * USB Interrupt Status and
>> + * USB Interrupt Latch
>> + */
>> +#define ULPI_INT_HOST_DISCONNECT (1 << 0)
>> +#define ULPI_INT_VBUS_VALID (1 << 1)
>> +#define ULPI_INT_SESS_VALID (1 << 2)
>> +#define ULPI_INT_SESS_END (1 << 3)
>> +#define ULPI_INT_IDGRD (1 << 4)
>> +
>> +/* Debug */
>> +#define ULPI_DEBUG_LINESTATE0 (1 << 0)
>> +#define ULPI_DEBUG_LINESTATE1 (1 << 1)
>> +
>> +/* Carkit Control */
>> +#define ULPI_CARKIT_CTRL_CARKITPWR (1 << 0)
>> +#define ULPI_CARKIT_CTRL_IDGNDDRV (1 << 1)
>> +#define ULPI_CARKIT_CTRL_TXDEN (1 << 2)
>> +#define ULPI_CARKIT_CTRL_RXDEN (1 << 3)
>> +#define ULPI_CARKIT_CTRL_SPKLEFTEN (1 << 4)
>> +#define ULPI_CARKIT_CTRL_SPKRIGHTEN (1 << 5)
>> +#define ULPI_CARKIT_CTRL_MICEN (1 << 6)
>> +
>> +/* Carkit Interrupt Enable */
>> +#define ULPI_CARKIT_INT_EN_IDFLOAT_RISE (1 << 0)
>> +#define ULPI_CARKIT_INT_EN_IDFLOAT_FALL (1 << 1)
>> +#define ULPI_CARKIT_INT_EN_CARINTDET (1 << 2)
>> +#define ULPI_CARKIT_INT_EN_DP_RISE (1 << 3)
>> +#define ULPI_CARKIT_INT_EN_DP_FALL (1 << 4)
>> +
>> +/*
>> + * Carkit Interrupt Status and
>> + * Carkit Interrupt Latch
>> + */
>> +#define ULPI_CARKIT_INT_IDFLOAT (1 << 0)
>> +#define ULPI_CARKIT_INT_CARINTDET (1 << 1)
>> +#define ULPI_CARKIT_INT_DP (1 << 2)
>> +
>> +/* Carkit Pulse Control*/
>> +#define ULPI_CARKIT_PLS_CTRL_TXPLSEN (1 << 0)
>> +#define ULPI_CARKIT_PLS_CTRL_RXPLSEN (1 << 1)
>> +#define ULPI_CARKIT_PLS_CTRL_SPKRLEFT_BIASEN (1 << 2)
>> +#define ULPI_CARKIT_PLS_CTRL_SPKRRIGHT_BIASEN (1 << 3)
>> +
>> +u32 ulpi_write(u32 ulpi_viewport, u32 reg, u32 value);
>> +u32 ulpi_read(u32 ulpi_viewport, u32 reg);
>> +
>> +int ulpi_init(u32 ulpi_viewport);
>> +
>
> It would be nice to have some comments on this if you want people to
> understand / use the API/
If documenting takes too long - can be done later in a follow up patch.
What is important is to define it well -
the parameters comment should be fixed.
>
>> +int ulpi_select_transceiver(u32 ulpi_viewport, int speed);
>> +int ulpi_drive_vbus(u32 ulpi_viewport,
>> + int ext_power_supply, int use_ext_indicator);
>> +int ulpi_dp_pulldown(u32 ulpi_viewport, int enable);
>> +int ulpi_dm_pulldown(u32 ulpi_viewport, int enable);
>> +int ulpi_select_opmode(u32 ulpi_viewport, int style);
>> +int ulpi_suspend(u32 ulpi_viewport);
>> +int ulpi_resume(u32 ulpi_viewport);
>> +int ulpi_reset(u32 ulpi_viewport);
>> +#endif /* __USB_ULPI_H */
>> --
>> 1.7.6.3
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
--
Regards,
Igor.
More information about the U-Boot
mailing list