[U-Boot] [PATCH v7] Add generic ULPI layer
Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.
The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.
The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.
It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.
What is still missing, IMO: - documentation for the CONFIG_* macros (I can add it in a separate patch) - a way to make most of the initialization in one ulpi_init() call - viewport extension to be able to implement resume, reset and disabling the serial mode
The change log: 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 Changes for v5: - CodingStyle changes - add comments - simplify implemenation of the ULPI interface functions Changes for v6: - cleanup function ulpi_drive_vbus() Changes for v7: - ulpi-viewport.c: - reorder bit definitions - split ulpi_request() to two functions - reuse ulpi_wakeup() from ulpi_request() to remove duplicated calls from ulpi_{read|write}() - inline ulpi_*_mask as it is simple and used only once - ulpi.c: - move several defines into c file - rework all the functions to propagate error values - move function description comments into ulpi.h along with declarations - check arguments validity (as suggested by Simon) - fix cases when using the *_set register, bits cannot be cleared - shorten several arguments names (e.g. ulpi_set_vbus()) - add ability to disable VBUS - clean up ulpi_set_pd() - add ability to enter the serial mode - add verbosity in error cases - remove ulpi_resume() as it were wrong and must be implemented in a viewport specific way - rework ulpi_reset() as it must be implemented in a viewport specific way, but provide kind of generic implementation which should work in most of the cases - ulpi.h: - add default timeout value - remove unused defines - move several defines inside c files - add description for each function - move the API declaration to the top of the header file
Jana Rapava (1): USB: Add generic ULPI layer and a viewport
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org --- Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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 d84b350..ab7a269 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..d43b229 --- /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. +# + +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..fa2e004 --- /dev/null +++ b/drivers/usb/ulpi/ulpi-viewport.c @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2011 Jana Rapava fermata7@gmail.com + * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il> + * + * Authors: Jana Rapava fermata7@gmail.com + * Igor Grinberg grinberg@compulab.co.il + * + * 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_SS (1 << 27) +#define ULPI_RWCTRL (1 << 29) +#define ULPI_RWRUN (1 << 30) +#define ULPI_WU (1 << 31) + +/* + * Wait for the ULPI request to complete + * + * @ulpi_viewport - the address of the viewport + * @mask - expected value to wait for + * + * returns 0 on mask match, ULPI_ERROR on time out. + */ +static int ulpi_wait(u32 ulpi_viewport, u32 mask) +{ + int timeout = CONFIG_USB_ULPI_TIMEOUT; + + /* Wait for the bits in mask to become zero. */ + while (--timeout) { + if ((readl(ulpi_viewport) & mask) == 0) + return 0; + + udelay(1); + } + + return ULPI_ERROR; +} + +/* + * Wake the ULPI PHY up for communication + * + * returns 0 on success. + */ +static int ulpi_wakeup(u32 ulpi_viewport) +{ + int err; + + if (readl(ulpi_viewport) & ULPI_SS) + return 0; /* already awake */ + + writel(ULPI_WU, ulpi_viewport); + + err = ulpi_wait(ulpi_viewport, ULPI_WU); + if (err) + printf("ULPI wakeup timed out\n"); + + return err; +} + +/* + * Issue a ULPI read/write request + * + * @value - the ULPI request + */ +static int ulpi_request(u32 ulpi_viewport, u32 value) +{ + int err; + + err = ulpi_wakeup(ulpi_viewport); + if (err) + return err; + + writel(value, ulpi_viewport); + + err = ulpi_wait(ulpi_viewport, ULPI_RWRUN); + if (err) + printf("ULPI request timed out\n"); + + return err; +} + +u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value) +{ + u32 val = ULPI_RWRUN | ULPI_RWCTRL | ((u32)reg << 16) | (value & 0xff); + + return ulpi_request(ulpi_viewport, val); +} + +u32 ulpi_read(u32 ulpi_viewport, u8 *reg) +{ + u32 err; + u32 val = ULPI_RWRUN | ((u32)reg << 16); + + err = ulpi_request(ulpi_viewport, val); + if (err) + return err; + + return (readl(ulpi_viewport) >> 8) & 0xff; +} diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c new file mode 100644 index 0000000..805e29d --- /dev/null +++ b/drivers/usb/ulpi/ulpi.c @@ -0,0 +1,227 @@ +/* + * Copyright (C) 2011 Jana Rapava fermata7@gmail.com + * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il> + * + * Authors: Jana Rapava fermata7@gmail.com + * Igor Grinberg grinberg@compulab.co.il + * + * 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. + */ + +#include <common.h> +#include <exports.h> +#include <usb/ulpi.h> + +#define ULPI_ID_REGS_COUNT 4 +#define ULPI_TEST_VALUE 0x55 /* 0x55 == 0b01010101 */ + +static struct ulpi_regs *ulpi = (struct ulpi_regs *)0; + +static int ulpi_integrity_check(u32 ulpi_viewport) +{ + u32 err, val, tval = ULPI_TEST_VALUE; + int i; + + /* Use the 'special' test value to check all bits */ + for (i = 0; i < 2; i++, tval <<= 1) { + err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval); + if (err) + return err; + + val = ulpi_read(ulpi_viewport, &ulpi->scratch); + if (val != tval) { + printf("ULPI integrity check failed\n"); + return val; + } + } + + return 0; +} + +int ulpi_init(u32 ulpi_viewport) +{ + u32 val, id = 0; + u8 *reg = &ulpi->product_id_high; + int i; + + /* Assemble ID from four ULPI ID registers (8 bits each). */ + for (i = 0; i < ULPI_ID_REGS_COUNT; i++) { + val = ulpi_read(ulpi_viewport, reg - i); + if (val == ULPI_ERROR) + return val; + + id = (id << 8) | val; + } + + /* Split ID into vendor and product ID. */ + debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id & 0xffff); + + return ulpi_integrity_check(ulpi_viewport); +} + +int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed) +{ + u8 tspeed = ULPI_FC_FULL_SPEED; + u32 val; + + switch (speed) { + case ULPI_FC_HIGH_SPEED: + case ULPI_FC_FULL_SPEED: + case ULPI_FC_LOW_SPEED: + case ULPI_FC_FS4LS: + tspeed = speed; + break; + default: + printf("ULPI: %s: wrong transceiver speed specified, " + "falling back to full speed\n", __func__); + } + + val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl); + if (val == ULPI_ERROR) + return val; + + /* clear the previous speed setting */ + val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed; + + return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val); +} + +int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind) +{ + u32 flags = ULPI_OTG_DRVVBUS; + u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear; + + if (ext_power) + flags |= ULPI_OTG_DRVVBUS_EXT; + if (ext_ind) + flags |= ULPI_OTG_EXTVBUSIND; + + return ulpi_write(ulpi_viewport, reg, flags); +} + +int ulpi_set_pd(u32 ulpi_viewport, int enable) +{ + u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN; + u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear; + + return ulpi_write(ulpi_viewport, reg, val); +} + +int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode) +{ + u8 topmode = ULPI_FC_OPMODE_NORMAL; + u32 val; + + switch (opmode) { + case ULPI_FC_OPMODE_NORMAL: + case ULPI_FC_OPMODE_NONDRIVING: + case ULPI_FC_OPMODE_DISABLE_NRZI: + case ULPI_FC_OPMODE_NOSYNC_NOEOP: + topmode = opmode; + break; + default: + printf("ULPI: %s: wrong OpMode specified, " + "falling back to OpMode Normal\n", __func__); + } + + val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl); + if (val == ULPI_ERROR) + return val; + + /* clear the previous opmode setting */ + val = (val & ~ULPI_FC_OPMODE_MASK) | topmode; + + return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val); +} + +int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode) +{ + switch (smode) { + case ULPI_IFACE_6_PIN_SERIAL_MODE: + case ULPI_IFACE_3_PIN_SERIAL_MODE: + break; + default: + printf("ULPI: %s: unrecognized Serial Mode specified\n", + __func__); + return ULPI_ERROR; + } + + return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode); +} + +int ulpi_suspend(u32 ulpi_viewport) +{ + u32 err; + + err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear, + ULPI_FC_SUSPENDM); + if (err) + printf("ULPI: %s: failed writing the suspend bit\n", __func__); + + return err; +} + +/* + * Wait for ULPI PHY reset to complete. + * Actual wait for reset must be done in a view port specific way, + * because it involves checking the DIR line. + */ +static int __ulpi_reset_wait(u32 ulpi_viewport) +{ + u32 val; + int timeout = CONFIG_USB_ULPI_TIMEOUT; + + /* Wait for the RESET bit to become zero */ + while (--timeout) { + /* + * This function is generic and suppose to work + * with any viewport, so we cheat here and don't check + * for the error of ulpi_read(), if there is one, then + * there will be a timeout. + */ + val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl); + if (!(val & ULPI_FC_RESET)) + return 0; + + udelay(1); + } + + printf("ULPI: %s: reset timed out\n", __func__); + + return ULPI_ERROR; +} +int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait"))); + +int ulpi_reset(u32 ulpi_viewport) +{ + u32 err; + + err = ulpi_write(ulpi_viewport, + &ulpi->function_ctrl_set, ULPI_FC_RESET); + if (err) { + printf("ULPI: %s: failed writing reset bit\n", __func__); + return err; + } + + return ulpi_reset_wait(ulpi_viewport); +} diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h new file mode 100644 index 0000000..d871290 --- /dev/null +++ b/include/usb/ulpi.h @@ -0,0 +1,298 @@ +/* + * Generic ULPI interface. + * + * Copyright (C) 2011 Jana Rapava fermata7@gmail.com + * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il> + * + * Authors: Jana Rapava fermata7@gmail.com + * Igor Grinberg grinberg@compulab.co.il + * + * Register offsets taken from: + * linux/include/linux/usb/ulpi.h + * + * 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_ERROR (1 << 8) /* overflow from any register value */ + +#ifndef CONFIG_USB_ULPI_TIMEOUT +#define CONFIG_USB_ULPI_TIMEOUT 1000 /* timeout in us */ +#endif + +/* + * Initialize the ULPI transciever and check the interface integrity. + * @ulpi_viewport - the address of the ULPI viewport register. + * + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_init(u32 ulpi_viewport); + +/* + * Select transceiver speed. + * @speed - ULPI_FC_HIGH_SPEED, ULPI_FC_FULL_SPEED (default), + * ULPI_FC_LOW_SPEED, ULPI_FC_FS4LS + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed); + +/* + * Enable/disable VBUS. + * @ext_power - external VBUS supply is used (default is false) + * @ext_indicator - external VBUS over-current indicator is used + * + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_enable_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind); + +/* + * Enable/disable pull-down resistors on D+ and D- USB lines. + * + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_set_pd(u32 ulpi_viewport, int enable); + +/* + * Select OpMode. + * @opmode - ULPI_FC_OPMODE_NORMAL (default), ULPI_FC_OPMODE_NONDRIVING, + * ULPI_FC_OPMODE_DISABLE_NRZI, ULPI_FC_OPMODE_NOSYNC_NOEOP + * + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode); + +/* + * Switch to Serial Mode. + * @smode - ULPI_IFACE_6_PIN_SERIAL_MODE or ULPI_IFACE_3_PIN_SERIAL_MODE + * + * returns 0 on success, ULPI_ERROR on failure. + * + * Notes: + * Switches immediately to Serial Mode. + * To return from Serial Mode, STP line needs to be asserted. + */ +int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode); + +/* + * Put PHY into low power mode. + * + * returns 0 on success, ULPI_ERROR on failure. + * + * Notes: + * STP line must be driven low to keep the PHY in suspend. + * To resume the PHY, STP line needs to be asserted. + */ +int ulpi_suspend(u32 ulpi_viewport); + +/* + * Reset the transceiver. ULPI interface and registers are not affected. + * + * returns 0 on success, ULPI_ERROR on failure. + */ +int ulpi_reset(u32 ulpi_viewport); + + +/* ULPI access methods below must be implemented for each ULPI viewport. */ + +/* + * Write to the ULPI PHY register via the viewport. + * @reg - the ULPI register (one of the fields in struct ulpi_regs). + * @value - the value - only 8 lower bits are used, others ignored. + * + * returns 0 on success, ULPI_ERROR on failure. + */ +u32 ulpi_write(u32 ulpi_viewport, u8 *reg, u32 value); + +/* + * Read the ULPI PHY register content via the viewport. + * @reg - the ULPI register (one of the fields in struct ulpi_regs). + * + * returns register content on success, ULPI_ERROR on failure. + */ +u32 ulpi_read(u32 ulpi_viewport, u8 *reg); + +/* + * Wait for the reset to complete. + * The Link must not attempt to access the PHY until the reset has + * completed and DIR line is de-asserted. + */ +int ulpi_reset_wait(u32 ulpi_viewport); + +/* 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 + +/* ULPI registers, bits and offsets definitions */ +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 + */ +}; + +/* + * Register Bits + */ + +/* Function Control */ +#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_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 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) + + +#endif /* __USB_ULPI_H__ */
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
For some reason, I don't see Remy in the Cc of the mail (in the email headers). Detlev, can you comment on that please?
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++
This would benefit from additions to the README describing the two new CONFIGs you add.
5 files changed, 688 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/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c new file mode 100644 index 0000000..805e29d --- /dev/null +++ b/drivers/usb/ulpi/ulpi.c @@ -0,0 +1,227 @@ +/*
- Copyright (C) 2011 Jana Rapava fermata7@gmail.com
- Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
- Authors: Jana Rapava fermata7@gmail.com
- Igor Grinberg grinberg@compulab.co.il
- 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.
- */
+#include <common.h> +#include <exports.h> +#include <usb/ulpi.h>
+#define ULPI_ID_REGS_COUNT 4 +#define ULPI_TEST_VALUE 0x55 /* 0x55 == 0b01010101 */
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+static int ulpi_integrity_check(u32 ulpi_viewport) +{
- u32 err, val, tval = ULPI_TEST_VALUE;
- int i;
- /* Use the 'special' test value to check all bits */
- for (i = 0; i < 2; i++, tval <<= 1) {
- err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);
- if (err)
- return err;
- val = ulpi_read(ulpi_viewport, &ulpi->scratch);
- if (val != tval) {
- printf("ULPI integrity check failed\n");
- return val;
- }
- }
- return 0;
+}
+int ulpi_init(u32 ulpi_viewport) +{
- u32 val, id = 0;
- u8 *reg = &ulpi->product_id_high;
- int i;
- /* Assemble ID from four ULPI ID registers (8 bits each). */
- for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {
- val = ulpi_read(ulpi_viewport, reg - i);
- if (val == ULPI_ERROR)
- return val;
- id = (id << 8) | val;
- }
- /* Split ID into vendor and product ID. */
- debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id & 0xffff);
- return ulpi_integrity_check(ulpi_viewport);
+}
+int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
Is there any point in making the argument u8? How about just unsigned? I think this adds a mask to the call = larger code size. You check the arg with the switch() below anyway.
+{
- u8 tspeed = ULPI_FC_FULL_SPEED;
- u32 val;
- switch (speed) {
- case ULPI_FC_HIGH_SPEED:
- case ULPI_FC_FULL_SPEED:
- case ULPI_FC_LOW_SPEED:
- case ULPI_FC_FS4LS:
- tspeed = speed;
- break;
- default:
- printf("ULPI: %s: wrong transceiver speed specified, "
- "falling back to full speed\n", __func__);
- }
- val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
- if (val == ULPI_ERROR)
- return val;
- /* clear the previous speed setting */
- val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;
- return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind) +{
- u32 flags = ULPI_OTG_DRVVBUS;
- u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
- if (ext_power)
- flags |= ULPI_OTG_DRVVBUS_EXT;
- if (ext_ind)
- flags |= ULPI_OTG_EXTVBUSIND;
- return ulpi_write(ulpi_viewport, reg, flags);
+}
+int ulpi_set_pd(u32 ulpi_viewport, int enable) +{
- u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;
- u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
- return ulpi_write(ulpi_viewport, reg, val);
+}
+int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
suggest unsigned for arg instead of u8
+{
- u8 topmode = ULPI_FC_OPMODE_NORMAL;
- u32 val;
- switch (opmode) {
- case ULPI_FC_OPMODE_NORMAL:
- case ULPI_FC_OPMODE_NONDRIVING:
- case ULPI_FC_OPMODE_DISABLE_NRZI:
- case ULPI_FC_OPMODE_NOSYNC_NOEOP:
- topmode = opmode;
- break;
- default:
- printf("ULPI: %s: wrong OpMode specified, "
- "falling back to OpMode Normal\n", __func__);
- }
- val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
- if (val == ULPI_ERROR)
- return val;
- /* clear the previous opmode setting */
- val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;
- return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
and again
+{
- switch (smode) {
- case ULPI_IFACE_6_PIN_SERIAL_MODE:
- case ULPI_IFACE_3_PIN_SERIAL_MODE:
- break;
- default:
- printf("ULPI: %s: unrecognized Serial Mode specified\n",
- __func__);
- return ULPI_ERROR;
- }
- return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);
+}
+int ulpi_suspend(u32 ulpi_viewport) +{
- u32 err;
This function returns int but err is u32. Which do you want? I think function return values should be native types, int or unsigned in this case.
- err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
- ULPI_FC_SUSPENDM);
- if (err)
- printf("ULPI: %s: failed writing the suspend bit\n", __func__);
- return err;
+}
+/*
- Wait for ULPI PHY reset to complete.
- Actual wait for reset must be done in a view port specific way,
- because it involves checking the DIR line.
- */
+static int __ulpi_reset_wait(u32 ulpi_viewport) +{
- u32 val;
- int timeout = CONFIG_USB_ULPI_TIMEOUT;
- /* Wait for the RESET bit to become zero */
- while (--timeout) {
- /*
- * This function is generic and suppose to work
- * with any viewport, so we cheat here and don't check
- * for the error of ulpi_read(), if there is one, then
- * there will be a timeout.
- */
- val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
- if (!(val & ULPI_FC_RESET))
- return 0;
- udelay(1);
- }
- printf("ULPI: %s: reset timed out\n", __func__);
- return ULPI_ERROR;
+} +int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
+int ulpi_reset(u32 ulpi_viewport) +{
- u32 err;
same as above function
- err = ulpi_write(ulpi_viewport,
- &ulpi->function_ctrl_set, ULPI_FC_RESET);
- if (err) {
- printf("ULPI: %s: failed writing reset bit\n", __func__);
- return err;
- }
- return ulpi_reset_wait(ulpi_viewport);
+}
In general if you have time you could check that you document all the args in your comments rather than just some. But I think in all cases it is pretty clear anyway.
Just out of interest, is it possible to test this? How would I plumb it in?
Regards, Simon
On 12/07/11 03:42, Simon Glass wrote:
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++
This would benefit from additions to the README describing the two new CONFIGs you add.
5 files changed, 688 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/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c new file mode 100644 index 0000000..805e29d --- /dev/null +++ b/drivers/usb/ulpi/ulpi.c @@ -0,0 +1,227 @@ +/*
- Copyright (C) 2011 Jana Rapava fermata7@gmail.com
- Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
- Authors: Jana Rapava fermata7@gmail.com
Igor Grinberg <grinberg@compulab.co.il>
- 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.
- */
+#include <common.h> +#include <exports.h> +#include <usb/ulpi.h>
+#define ULPI_ID_REGS_COUNT 4 +#define ULPI_TEST_VALUE 0x55 /* 0x55 == 0b01010101 */
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+static int ulpi_integrity_check(u32 ulpi_viewport) +{
u32 err, val, tval = ULPI_TEST_VALUE;int i;/* Use the 'special' test value to check all bits */for (i = 0; i < 2; i++, tval <<= 1) {err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);if (err)return err;val = ulpi_read(ulpi_viewport, &ulpi->scratch);if (val != tval) {printf("ULPI integrity check failed\n");return val;}}return 0;+}
+int ulpi_init(u32 ulpi_viewport) +{
u32 val, id = 0;u8 *reg = &ulpi->product_id_high;int i;/* Assemble ID from four ULPI ID registers (8 bits each). */for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {val = ulpi_read(ulpi_viewport, reg - i);if (val == ULPI_ERROR)return val;id = (id << 8) | val;}/* Split ID into vendor and product ID. */debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id & 0xffff);return ulpi_integrity_check(ulpi_viewport);+}
+int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
Is there any point in making the argument u8?
Not really...
How about just unsigned?
Sounds sane
I think this adds a mask to the call = larger code size. You check the arg with the switch() below anyway.
Haven't thought about that... I don't know how is this exactly works, but I don't see any reason why shouldn't I use unsigned here.
+{
u8 tspeed = ULPI_FC_FULL_SPEED;u32 val;switch (speed) {case ULPI_FC_HIGH_SPEED:case ULPI_FC_FULL_SPEED:case ULPI_FC_LOW_SPEED:case ULPI_FC_FS4LS:tspeed = speed;break;default:printf("ULPI: %s: wrong transceiver speed specified, ""falling back to full speed\n", __func__);}val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (val == ULPI_ERROR)return val;/* clear the previous speed setting */val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);+}
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind) +{
u32 flags = ULPI_OTG_DRVVBUS;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power)flags |= ULPI_OTG_DRVVBUS_EXT;if (ext_ind)flags |= ULPI_OTG_EXTVBUSIND;return ulpi_write(ulpi_viewport, reg, flags);+}
+int ulpi_set_pd(u32 ulpi_viewport, int enable) +{
u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;return ulpi_write(ulpi_viewport, reg, val);+}
+int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
suggest unsigned for arg instead of u8
Ok
+{
u8 topmode = ULPI_FC_OPMODE_NORMAL;u32 val;switch (opmode) {case ULPI_FC_OPMODE_NORMAL:case ULPI_FC_OPMODE_NONDRIVING:case ULPI_FC_OPMODE_DISABLE_NRZI:case ULPI_FC_OPMODE_NOSYNC_NOEOP:topmode = opmode;break;default:printf("ULPI: %s: wrong OpMode specified, ""falling back to OpMode Normal\n", __func__);}val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (val == ULPI_ERROR)return val;/* clear the previous opmode setting */val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);+}
+int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
and again
+{
switch (smode) {case ULPI_IFACE_6_PIN_SERIAL_MODE:case ULPI_IFACE_3_PIN_SERIAL_MODE:break;default:printf("ULPI: %s: unrecognized Serial Mode specified\n",__func__);return ULPI_ERROR;}return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);+}
+int ulpi_suspend(u32 ulpi_viewport) +{
u32 err;This function returns int but err is u32. Which do you want? I think function return values should be native types, int or unsigned in this case.
Yep, missed this... it also applies in some other places.
err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,ULPI_FC_SUSPENDM);if (err)printf("ULPI: %s: failed writing the suspend bit\n", __func__);return err;+}
+/*
- Wait for ULPI PHY reset to complete.
- Actual wait for reset must be done in a view port specific way,
- because it involves checking the DIR line.
- */
+static int __ulpi_reset_wait(u32 ulpi_viewport) +{
u32 val;int timeout = CONFIG_USB_ULPI_TIMEOUT;/* Wait for the RESET bit to become zero */while (--timeout) {/** This function is generic and suppose to work* with any viewport, so we cheat here and don't check* for the error of ulpi_read(), if there is one, then* there will be a timeout.*/val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (!(val & ULPI_FC_RESET))return 0;udelay(1);}printf("ULPI: %s: reset timed out\n", __func__);return ULPI_ERROR;+} +int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
+int ulpi_reset(u32 ulpi_viewport) +{
u32 err;same as above function
err = ulpi_write(ulpi_viewport,&ulpi->function_ctrl_set, ULPI_FC_RESET);if (err) {printf("ULPI: %s: failed writing reset bit\n", __func__);return err;}return ulpi_reset_wait(ulpi_viewport);+}
In general if you have time you could check that you document all the args in your comments rather than just some. But I think in all cases it is pretty clear anyway.
Me too, that's exactly the reason I haven't documented them all around - I don't like the stupid copy/paste work...
Just out of interest, is it possible to test this? How would I plumb it in?
Well, from my experience with ULPI hardware, I think the controller specific glue looks like the right place for putting the ULPI layer calls in.
In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.
I think, Marek has patches that supposed to use this framework on efikamx board.
On 12/07/11 03:42, Simon Glass wrote:
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++
This would benefit from additions to the README describing the two new CONFIGs you add.
5 files changed, 688 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/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c new file mode 100644 index 0000000..805e29d --- /dev/null +++ b/drivers/usb/ulpi/ulpi.c @@ -0,0 +1,227 @@ +/*
- Copyright (C) 2011 Jana Rapava fermata7@gmail.com
- Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
- Authors: Jana Rapava fermata7@gmail.com
Igor Grinberg <grinberg@compulab.co.il>
- 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.
- */
+#include <common.h> +#include <exports.h> +#include <usb/ulpi.h>
+#define ULPI_ID_REGS_COUNT 4 +#define ULPI_TEST_VALUE 0x55 /* 0x55 == 0b01010101 */
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+static int ulpi_integrity_check(u32 ulpi_viewport) +{
u32 err, val, tval = ULPI_TEST_VALUE;int i;/* Use the 'special' test value to check all bits */for (i = 0; i < 2; i++, tval <<= 1) {err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);if (err)return err;val = ulpi_read(ulpi_viewport, &ulpi->scratch);if (val != tval) {printf("ULPI integrity check failed\n");return val;}}return 0;+}
+int ulpi_init(u32 ulpi_viewport) +{
u32 val, id = 0;u8 *reg = &ulpi->product_id_high;int i;/* Assemble ID from four ULPI ID registers (8 bits each). */for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {val = ulpi_read(ulpi_viewport, reg - i);if (val == ULPI_ERROR)return val;id = (id << 8) | val;}/* Split ID into vendor and product ID. */debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id &0xffff); +
return ulpi_integrity_check(ulpi_viewport);+}
+int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)
Is there any point in making the argument u8?
Not really...
How about just unsigned?
Sounds sane
I think this adds a mask to the call = larger code size. You check the arg with the switch() below anyway.
Haven't thought about that... I don't know how is this exactly works, but I don't see any reason why shouldn't I use unsigned here.
+{
u8 tspeed = ULPI_FC_FULL_SPEED;u32 val;switch (speed) {case ULPI_FC_HIGH_SPEED:case ULPI_FC_FULL_SPEED:case ULPI_FC_LOW_SPEED:case ULPI_FC_FS4LS:tspeed = speed;break;default:printf("ULPI: %s: wrong transceiver speed specified, ""falling back to full speed\n", __func__);}val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (val == ULPI_ERROR)return val;/* clear the previous speed setting */val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);+}
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind) +{
u32 flags = ULPI_OTG_DRVVBUS;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power)flags |= ULPI_OTG_DRVVBUS_EXT;if (ext_ind)flags |= ULPI_OTG_EXTVBUSIND;return ulpi_write(ulpi_viewport, reg, flags);+}
+int ulpi_set_pd(u32 ulpi_viewport, int enable) +{
u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;return ulpi_write(ulpi_viewport, reg, val);+}
+int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)
suggest unsigned for arg instead of u8
Ok
+{
u8 topmode = ULPI_FC_OPMODE_NORMAL;u32 val;switch (opmode) {case ULPI_FC_OPMODE_NORMAL:case ULPI_FC_OPMODE_NONDRIVING:case ULPI_FC_OPMODE_DISABLE_NRZI:case ULPI_FC_OPMODE_NOSYNC_NOEOP:topmode = opmode;break;default:printf("ULPI: %s: wrong OpMode specified, ""falling back to OpMode Normal\n", __func__);}val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (val == ULPI_ERROR)return val;/* clear the previous opmode setting */val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);+}
+int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)
and again
+{
switch (smode) {case ULPI_IFACE_6_PIN_SERIAL_MODE:case ULPI_IFACE_3_PIN_SERIAL_MODE:break;default:printf("ULPI: %s: unrecognized Serial Mode specified\n",__func__);return ULPI_ERROR;}return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);+}
+int ulpi_suspend(u32 ulpi_viewport) +{
u32 err;This function returns int but err is u32. Which do you want? I think function return values should be native types, int or unsigned in this case.
Yep, missed this... it also applies in some other places.
err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,ULPI_FC_SUSPENDM);if (err)printf("ULPI: %s: failed writing the suspend bit\n",__func__); +
return err;+}
+/*
- Wait for ULPI PHY reset to complete.
- Actual wait for reset must be done in a view port specific way,
- because it involves checking the DIR line.
- */
+static int __ulpi_reset_wait(u32 ulpi_viewport) +{
u32 val;int timeout = CONFIG_USB_ULPI_TIMEOUT;/* Wait for the RESET bit to become zero */while (--timeout) {/** This function is generic and suppose to work* with any viewport, so we cheat here and don't check* for the error of ulpi_read(), if there is one, then* there will be a timeout.*/val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);if (!(val & ULPI_FC_RESET))return 0;udelay(1);}printf("ULPI: %s: reset timed out\n", __func__);return ULPI_ERROR;+} +int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait"))); + +int ulpi_reset(u32 ulpi_viewport) +{
u32 err;same as above function
err = ulpi_write(ulpi_viewport,&ulpi->function_ctrl_set, ULPI_FC_RESET);if (err) {printf("ULPI: %s: failed writing reset bit\n",__func__); + return err;
}return ulpi_reset_wait(ulpi_viewport);+}
In general if you have time you could check that you document all the args in your comments rather than just some. But I think in all cases it is pretty clear anyway.
Me too, that's exactly the reason I haven't documented them all around - I don't like the stupid copy/paste work...
Just out of interest, is it possible to test this? How would I plumb it in?
Well, from my experience with ULPI hardware, I think the controller specific glue looks like the right place for putting the ULPI layer calls in.
In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.
I think, Marek has patches that supposed to use this framework on efikamx board.
I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there. I had to fall back to basic ulpi_read()/ulpi_write() calls :-( I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this (or redesigned version) in, but I don't know if that's a good idea.
M
Hi Marek,
On 12/07/11 19:27, Marek Vasut wrote:
On 12/07/11 03:42, Simon Glass wrote:
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
[...]
Just out of interest, is it possible to test this? How would I plumb it in?
Well, from my experience with ULPI hardware, I think the controller specific glue looks like the right place for putting the ULPI layer calls in.
In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.
I think, Marek has patches that supposed to use this framework on efikamx board.
I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.
May I ask you why? Isn't it because of that nasty VBUS bug efikamx has? You can't say the design is wrong if it is more generic then you want...
I had to fall back to basic ulpi_read()/ulpi_write() calls :-(
That's too bad. Because ulpi_{read|write}() is only a viewport implementation and it is not following the ULPI spec.
I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this
Good.
(or redesigned version) in, but I don't know if that's a good idea.
I don't think it should be redesigned. Currently, it is generic and abstracts the ULPI specification nicely. It can be used on any architecture. I have already stated in the cover letter, what IMO is missing to improve usability, but that will not be a problem.
Do you have the efikamx patches somewhere I can look at?
Hi Marek,
On 12/07/11 19:27, Marek Vasut wrote:
On 12/07/11 03:42, Simon Glass wrote:
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
[...]
Just out of interest, is it possible to test this? How would I plumb it in?
Well, from my experience with ULPI hardware, I think the controller specific glue looks like the right place for putting the ULPI layer calls in.
In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.
I think, Marek has patches that supposed to use this framework on efikamx board.
I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.
May I ask you why? Isn't it because of that nasty VBUS bug efikamx has? You can't say the design is wrong if it is more generic then you want...
I think it's overengineered. Basically, to achieve what I needed, I either didn't find the right function or I had to use multiple functions. Therefore I had to fall back to plain simple ulpi_read/write().
I had to fall back to basic ulpi_read()/ulpi_write() calls :-(
That's too bad. Because ulpi_{read|write}() is only a viewport implementation and it is not following the ULPI spec.
Well ... we'll need to rethink this.
I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this
Good.
(or redesigned version) in, but I don't know if that's a good idea.
I don't think it should be redesigned. Currently, it is generic and abstracts the ULPI specification nicely.
Nicely maybe, but try using it on top of some hardware that has ULPI chip.
It can be used on any architecture. I have already stated in the cover letter, what IMO is missing to improve usability, but that will not be a problem.
Do you have the efikamx patches somewhere I can look at?
Submitted to ML just a while ago.
M
On 12/07/11 20:36, Marek Vasut wrote:
Hi Marek,
On 12/07/11 19:27, Marek Vasut wrote:
On 12/07/11 03:42, Simon Glass wrote:
Hi Igor,
Looks good - a few comments from me.
On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
From: Jana Rapava fermata7@gmail.com
Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context. Add a viewport implementation for Chipidea/ARC based controllers.
Signed-off-by: Jana Rapava fermata7@gmail.com Signed-off-by: Igor Grinberg grinberg@compulab.co.il Cc: Remy Bohmer linux@bohmer.net Cc: Stefano Babic sbabic@denx.de Cc: Wolfgang Grandegger wg@denx.de Cc: Simon Glass sjg@chromium.org
[...]
Just out of interest, is it possible to test this? How would I plumb it in?
Well, from my experience with ULPI hardware, I think the controller specific glue looks like the right place for putting the ULPI layer calls in.
In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.
I think, Marek has patches that supposed to use this framework on efikamx board.
I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.
May I ask you why? Isn't it because of that nasty VBUS bug efikamx has? You can't say the design is wrong if it is more generic then you want...
I think it's overengineered. Basically, to achieve what I needed, I either didn't find the right function or I had to use multiple functions. Therefore I had to fall back to plain simple ulpi_read/write().
So now it is plain simple multiple ulpi_read/write() calls? This is no more as simple as ulpi_<layer function>() calls... Instead of forging masks and writing arbitrary values, use the functional API.
I don't think is is over engineered, because it is simple and in the same time helps prevent mistakes and most of the cases, you don't need to be familiar with all the ULPI bits.
I had to fall back to basic ulpi_read()/ulpi_write() calls :-(
That's too bad. Because ulpi_{read|write}() is only a viewport implementation and it is not following the ULPI spec.
Well ... we'll need to rethink this.
I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this
Good.
(or redesigned version) in, but I don't know if that's a good idea.
I don't think it should be redesigned. Currently, it is generic and abstracts the ULPI specification nicely.
Nicely maybe, but try using it on top of some hardware that has ULPI chip.
What's the problem? It is better then just using ulpi_read/write() calls... May be you are just lazy to read the documentation...
It can be used on any architecture. I have already stated in the cover letter, what IMO is missing to improve usability, but that will not be a problem.
Do you have the efikamx patches somewhere I can look at?
Submitted to ML just a while ago.
M
Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.
The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.
The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.
It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.
What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate
patch)
- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode
The change log: 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
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
to remove duplicated calls from ulpi_{read|write}()
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
along with declarations
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h
bits cannot be cleared
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register,
must be implemented in a viewport specific way
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and
viewport specific way, but provide kind of generic implementation which should work in most of the cases
- rework ulpi_reset() as it must be implemented in a
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file
Jana Rapava (1): USB: Add generic ULPI layer and a viewport
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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
Igor, please add Cc annotations to this patch too.
M
Hi Marek,
On 12/07/11 01:49, Marek Vasut wrote:
Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.
The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.
The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.
It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.
What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate
patch)
- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode
The change log: 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
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
to remove duplicated calls from ulpi_{read|write}()
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
along with declarations
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h
bits cannot be cleared
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register,
must be implemented in a viewport specific way
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and
viewport specific way, but provide kind of generic implementation which should work in most of the cases
- rework ulpi_reset() as it must be implemented in a
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file
Jana Rapava (1): USB: Add generic ULPI layer and a viewport
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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
Igor, please add Cc annotations to this patch too.
Well, I indeed forgot this, but I've send a forward of this email to everybody supposed to be in Cc. Adding Cc once again...
On 12/07/11 14:59, Igor Grinberg wrote:
Hi Marek,
On 12/07/11 01:49, Marek Vasut wrote:
Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.
The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.
The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.
It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.
What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate
patch)
- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode
The change log: 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
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
to remove duplicated calls from ulpi_{read|write}()
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
along with declarations
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h
bits cannot be cleared
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register,
must be implemented in a viewport specific way
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and
viewport specific way, but provide kind of generic implementation which should work in most of the cases
- rework ulpi_reset() as it must be implemented in a
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file
Jana Rapava (1): USB: Add generic ULPI layer and a viewport
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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
Igor, please add Cc annotations to this patch too.
Well, I indeed forgot this, but I've send a forward of this email to everybody supposed to be in Cc. Adding Cc once again...
That's interesting... I've added much more people to Cc, than those listed currently... It looks like the bounce from the list "eats" the Cc partially...
On 12/07/11 14:59, Igor Grinberg wrote:
Hi Marek,
On 12/07/11 01:49, Marek Vasut wrote:
Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.
The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.
The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.
It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.
What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate
patch)
a way to make most of the initialization in one ulpi_init() call
viewport extension to be able to implement resume,
reset and disabling the serial mode
The change log:
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
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
to remove duplicated calls from ulpi_{read|write}() - inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h
along with declarations - check arguments validity (as suggested by Simon) - fix cases when using the *_set register, bits cannot be cleared - shorten several arguments names (e.g. ulpi_set_vbus()) - add ability to disable VBUS - clean up ulpi_set_pd() - add ability to enter the serial mode - add verbosity in error cases - remove ulpi_resume() as it were wrong and must be implemented in a viewport specific way - rework ulpi_reset() as it must be implemented in a viewport specific way, but provide kind of generic implementation which should work in most of the cases
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file
Jana Rapava (1): USB: Add generic ULPI layer and a viewport
Makefile | 1 + drivers/usb/ulpi/Makefile | 44 ++++++ drivers/usb/ulpi/ulpi-viewport.c | 118 +++++++++++++++ drivers/usb/ulpi/ulpi.c | 227 +++++++++++++++++++++++++++++ include/usb/ulpi.h | 298
++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 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
Igor, please add Cc annotations to this patch too.
Well, I indeed forgot this, but I've send a forward of this email to everybody supposed to be in Cc. Adding Cc once again...
That's interesting... I've added much more people to Cc, than those listed currently... It looks like the bounce from the list "eats" the Cc partially...
Exactly, I noticed this earlier in some other emails too. Detlev, can you please verify?
M
participants (3)
-
Igor Grinberg -
Marek Vasut -
Simon Glass