[U-Boot] [PATCH 3/5] pxa255: Add USB CDC ACM driver

Marek Vasut marex at denx.de
Thu Aug 9 22:40:55 CEST 2012


Dear Łukasz Dałek,

> Signed-off-by: Łukasz Dałek <luk0104 at gmail.com>
> ---
>  drivers/serial/usbtty.h         |    2 +
>  drivers/usb/gadget/pxa25x_udc.c |  939
> +++++++++++++++++++++++++++++++++++++++ include/usb/pxa25x_udc.h        | 
>  65 +++
>  3 files changed, 1006 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/gadget/pxa25x_udc.c
>  create mode 100644 include/usb/pxa25x_udc.h
> 
> diff --git a/drivers/serial/usbtty.h b/drivers/serial/usbtty.h
> index eb670da..632b54e 100644
> --- a/drivers/serial/usbtty.h
> +++ b/drivers/serial/usbtty.h
> @@ -31,6 +31,8 @@
>  #include <usb/omap1510_udc.h>
>  #elif defined(CONFIG_MUSB_UDC)
>  #include <usb/musb_udc.h>
> +#elif defined(CONFIG_CPU_PXA25X)
> +#include <usb/pxa25x_udc.h>
>  #elif defined(CONFIG_CPU_PXA27X)
>  #include <usb/pxa27x_udc.h>
>  #elif defined(CONFIG_DW_UDC)
> diff --git a/drivers/usb/gadget/pxa25x_udc.c
> b/drivers/usb/gadget/pxa25x_udc.c new file mode 100644
> index 0000000..4ff98cc
> --- /dev/null
> +++ b/drivers/usb/gadget/pxa25x_udc.c
> @@ -0,0 +1,939 @@
> +/*
> + * PXA25x USB device driver for u-boot.
> + *
> + * Copyright (C) 2012 Łukasz Dałek <luk0104 at gmail.com>
> + *
> + * 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
> + *
> + * based on drivers/usb/gadget/pxa27x_udc.c
> + *
> + */
> +
> +#include <common.h>
> +#include <config.h>
> +#include <usb/pxa25x_udc.h>
> +#include <asm/io.h>
> +#include <asm/arch/hardware.h>
> +#include "ep0.h"
> +
> +struct pxa25x_endpoint {
> +	u8	num;
> +	u32	uddr;
> +	u32	udccs;
> +	u32	ubcr;
> +};
> +
> +static struct pxa25x_endpoint eps[] = {
> +	{ 0, UDDR0, UDCCS(0), 0 },
> +	{ 1, UDDR1, UDCCS(1), 0 },
> +	{ 2, UDDR2, UDCCS(2), UBCR2 },
> +	{ 3, UDDR3, UDCCS(3), 0 },
> +	{ 4, UDDR4, UDCCS(4), UBCR4 },
> +	{ 5, UDDR5, UDCCS(5), 0 },
> +	{ 6, UDDR6, UDCCS(6), 0 },
> +	{ 7, UDDR7, UDCCS(7), UBCR7 },
> +	{ 8, UDDR8, UDCCS(8), 0 },
> +	{ 9, UDDR9, UDCCS(9), UBCR9 },
> +	{ 10, UDDR10, UDCCS(10), 0 },
> +	{ 11, UDDR11, UDCCS(11), 0 },
> +	{ 12, UDDR12, UDCCS(12), UBCR12 },
> +	{ 13, UDDR13, UDCCS(13), 0 },
> +	{ 14, UDDR14, UDCCS(14), UBCR14 },
> +	{ 15, UDDR15, UDCCS(15), 0 },
> +};
> +
> +static struct usb_device_instance *udc_device;
> +static struct urb *ep0_urb;
> +static int ep0state = EP0_IDLE;
> +static int ep0laststate = EP0_IDLE;
> +
> +static inline void udc_set_reg(u32 reg, u32 mask)
> +{
> +	u32 val;
> +
> +	val = readl(reg);
> +	val |= mask;
> +	writel(val, reg);

set_bits_be32() does this

> +}
> +
> +static inline void udc_clear_reg(u32 reg, u32 mask)
> +{
> +	u32 val;
> +
> +	val = readl(reg);
> +	val &= ~mask;
> +	writel(val, reg);
> +}
> +
> +/* static void udc_dump_buffer(char *name, u8 *buf, int len)
> +{
> +	usbdbg("%s - buf %p, len %d", name, buf, len);
> +	print_buffer(0, buf, 1, len, 0);
> +} */
> +
> +static void udc_dump_buffer(u8 *data, int len)

Is this debug goo ? What does it do?

> +{
> +	u8 buff[8 * 5 + 1]; /* 8 characters 0x??_ + null */
> +	int i;
> +
> +	for (i = 0; i < len; ++i) {
> +		int n = i % 8;
> +		buff[n * 5 + 0] = '0';
> +		buff[n * 5 + 1] = 'x';
> +
> +		u8 ch = data[i] >> 4;
> +		buff[n * 5 + 2] = (ch < 10)?(ch + '0'):(ch - 10 + 'a');
> +		ch = data[i] & 0xf;
> +		buff[n * 5 + 3] = (ch < 10)?(ch + '0'):(ch - 10 + 'a');
> +		buff[n * 5 + 4] = ' ';
> +
> +		buff[n * 5 + 5] = 0x0;
> +
> +		if (n == 7)
> +			usbdbg("%s", buff);
> +	}
> +
> +}
> +
> +static void udc_flush_fifo(struct usb_endpoint_instance *endpoint)
> +{
> +	int ep_num = endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK,

Ok, this probably doesn't pass tools/checkpatch.pl right? Also, define it on two 
separate lines.

> +	    isout =
> +		    (endpoint->endpoint_address & USB_ENDPOINT_DIR_MASK) == 
USB_DIR_OUT;
> +	int ep_type;
> +	u32 val;
> +
> +	if (ep_num > 15) {
> +		usberr("%s: endpoint out of range %d", __func__, ep_num);

printf() or debug(), choose one.

> +		return ;
> +	}
> +
> +	if (!ep_num) {
> +		while (readl(UDCCS0) & UDCCS_CTRL_RNE)
> +			readl(UDDR0);

How does this loop work? And no endless loops please.

> +		writel(UDCCS_CTRL_FTF, UDCCS0);
> +		usbdbg("flushed endpoint 0 (udccs0 0x%02X)",
> +			readl(UDCCS0) & 0xff);

debug()

> +		return ;
> +	}
> +
> +	if (isout) {
> +		while (readl(eps[ep_num].udccs) & UDCCS_BULK_OUT_RNE)
> +			readl(eps[ep_num].uddr);
> +		usbdbg("out endpoint %d flushed", ep_num);
> +		return ;
> +	}
> +
> +	ep_type = endpoint->tx_attributes;
> +
> +	val = UDCCS_BULK_IN_TPC | UDCCS_BULK_IN_FTF | UDCCS_BULK_IN_TUR;
> +	if (ep_type == USB_ENDPOINT_XFER_BULK ||
> +			ep_type == USB_ENDPOINT_XFER_INT) {
> +		val |= UDCCS_BULK_IN_SST;
> +	}
> +	writel(val, eps[ep_num].udccs);
> +	usbdbg("in endpoint %d flushed", ep_num);
> +}
> +
> +/* static void udc_stall_ep(int num)
> +{
> +	writel(UDCCS_BULK_IN_FST, eps[num].udccs);
> +	usbdbg("forced stall on ep %d", num);
> +} */

Dead code

> +static int udc_write_urb(struct usb_endpoint_instance *endpoint)
> +{
> +	int ep_num = endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
> +	int i, length;
> +	struct urb *urb = endpoint->tx_urb;
> +	u8 *data;
> +	/* int timeout = 2000; */
> +
> +	if (!urb)
> +		return -1;
> +
> +	if (!urb->actual_length) {
> +		usbdbg("clearing tpc and tur bits");
> +		writel(UDCCS_BULK_IN_TPC | UDCCS_BULK_IN_TUR, 
eps[ep_num].udccs);
> +		return -1;
> +	}
> +
> +
> +	/* FIXME: check TFS bit for udc transsmion ready */

So fix it

> +	usbdbg("urb->actual_length %d, endpoint->sent %d, endpoint 0x%p, "
> +		"urb 0x%p", urb->actual_length, endpoint->sent,
> +		endpoint, urb);
> +
> +	length = MIN(urb->actual_length - endpoint->sent,
> endpoint->tx_packetSize); +	if (length <= 0) {
> +		usbdbg("nothing to sent");
> +		return -1;
> +	}
> +
> +	/* clear tpc and tur bit */
> +	writel(UDCCS_BULK_IN_TPC | UDCCS_BULK_IN_TUR, eps[ep_num].udccs);
> +
> +	data = (u8 *)urb->buffer + endpoint->sent;
> +	for (i = 0; i < length; ++i)
> +		writel(data[i], eps[ep_num].uddr);
> +
> +	usbdbg("prepare to sent %d bytes on ep %d, udccs 0x%02X",
> +		length, ep_num, readl(eps[ep_num].udccs));
> +
> +	/* short packet? */
> +	if (length != endpoint->tx_packetSize) {
> +		usbdbg("ep_num %d, sending short packet", ep_num);
> +		writel(UDCCS_BULK_IN_TSP, eps[ep_num].udccs);
> +	}
> +
> +	/* wait for data */
> +#if 0
> +	while ( !(readl(eps[ep_num].udccs) & UDCCS_BULK_IN_TPC) ) {

This passed checkpatch?

C++ comments up ahead etc. I don't review any further, please lint the patches 
with checkpatch and resubmit.

Thanks!

[...]




More information about the U-Boot mailing list