[U-Boot] [PATCH 4/8] OMAP3 Add usb device support
Jean-Christophe PLAGNIOL-VILLARD
plagnioj at jcrosoft.com
Sat Sep 5 02:25:27 CEST 2009
Hi,
please be carefull you have a lot's of ligne which exceed 80 chars
limit
> +
> +/* Define MUSB_DEBUG before including this file to get debug macros */
> +#ifdef MUSB_DEBUG
> +
> +static inline void MUSB_PRINT_PWR(u8 b)
please lowercase
> +{
> + serial_printf("\tpower 0x%2.2x\n", b);
maybe a macro will simplify the code
#define MUSB_FLAGS_PRINT(x, y) \
if (b & MUSB_##x_##y) \
serial_printf("\t\t"#y"\n");
static inline void musb_print_pwr(u8 b)
{
serial_printf("\tpower 0x%2.2x\n", b);
MUSB_FLAGS_PRINT(POWER, ISOUPDATE);
MUSB_FLAGS_PRINT(POWER, SOFTCONN);
MUSB_FLAGS_PRINT(POWER, HSENAB);
MUSB_FLAGS_PRINT(POWER, HSMODE);
MUSB_FLAGS_PRINT(POWER, RESET);
MUSB_FLAGS_PRINT(POWER, RESUME);
MUSB_FLAGS_PRINT(POWER, SUSPENDM);
MUSB_FLAGS_PRINT(POWER, ENSUSPEND);
}
and so on
> +
> +static inline void MUSB_PRINT_CSR0(u16 w)
please lowercase and so on
> +{
> + serial_printf("\tcsr0 0x%4.4x\n", w);
> + if (w & MUSB_CSR0_FLUSHFIFO)
> + serial_printf("\t\tFLUSHFIFO\n");
> + if (w & MUSB_CSR0_P_SVDSETUPEND)
> + serial_printf("\t\tSERV_SETUPEND\n");
> + if (w & MUSB_CSR0_P_SVDRXPKTRDY)
> + serial_printf("\t\tSERV_RXPKTRDY\n");
> + if (w & MUSB_CSR0_P_SENDSTALL)
> + serial_printf("\t\tSENDSTALL\n");
> + if (w & MUSB_CSR0_P_SETUPEND)
> + serial_printf("\t\tSETUPEND\n");
> + if (w & MUSB_CSR0_P_DATAEND)
> + serial_printf("\t\tDATAEND\n");
> + if (w & MUSB_CSR0_P_SENTSTALL)
> + serial_printf("\t\tSENTSTALL\n");
> + if (w & MUSB_CSR0_TXPKTRDY)
> + serial_printf("\t\tTXPKTRDY\n");
> + if (w & MUSB_CSR0_RXPKTRDY)
> + serial_printf("\t\tRXPKTRDY\n");
> +}
> +
> +
> +#define MAX_ENDPOINT 15
maybe we can put it configurable
> +
> +#define GET_ENDPOINT(dev,ep) \
> +(((struct usb_device_instance *)(dev))->bus->endpoint_array + ep)
> +
> +#define SET_EP0_STATE(s) \
> +do { \
> + if ((0 <= (s)) && (SET_ADDRESS >= (s))) { \
> + if ((s) != ep0_state) { \
> + if ((debug_setup) && (debug_level > 1)) \
> + serial_printf("INFO : Changing state from %s to %s in %s at line %d\n", ep0_state_strings[ep0_state], ep0_state_strings[s], __PRETTY_FUNCTION__, __LINE__); \
too long please split
> + ep0_state = s; \
> + } \
> + } else { \
> + if (debug_level > 0) \
> + serial_printf("Error at %s %d with setting state %d is invalid\n", __PRETTY_FUNCTION__, __LINE__, s); \
too long please split
> + } \
> +} while (0)
> +
> +/* static implies these initialized to 0 or NULL */
> +static int debug_setup;
> +static int debug_level;
> +static struct musb_epinfo epinfo[MAX_ENDPOINT * 2];
> +static enum ep0_state_enum { IDLE = 0, TX, RX, SET_ADDRESS } ep0_state = IDLE;
this will be better
static enum ep0_state_enum {
IDLE = 0,
TX,
RX,
SET_ADDRESS
} ep0_state = IDLE;
> +static char *ep0_state_strings[4] = {
> + "IDLE",
> + "TX",
> + "RX",
> + "SET_ADDRESS",
> +};
> +
> +static struct urb *ep0_urb;
> +struct usb_endpoint_instance *ep0_endpoint;
> +static struct usb_device_instance *udc_device;
> +static int enabled;
> +
> +#ifdef MUSB_DEBUG
> +static void musb_db_regs(void)
> +{
> + u8 b;
> + u16 w;
> +
> + b = readb(&musbr->faddr);
> + serial_printf("\tfaddr 0x%2.2x\n", b);
> +
> + b = readb(&musbr->power);
> + MUSB_PRINT_PWR(b);
> +
> + w = readw(&musbr->ep[0].ep0.csr0);
> + MUSB_PRINT_CSR0(w);
> +
> + b = readb(&musbr->devctl);
> + MUSB_PRINT_DEVCTL(b);
> +
> + b = readb(&musbr->ep[0].ep0.configdata);
> + MUSB_PRINT_CONFIG(b);
> +
> + w = readw(&musbr->frame);
> + serial_printf("\tframe 0x%4.4x\n", w);
> +
> + b = readb(&musbr->index);
> + serial_printf("\tindex 0x%2.2x\n", b);
> +
> + w = readw(&musbr->ep[1].epN.rxmaxp);
> + MUSB_PRINT_RXMAXP(w);
> +
> + w = readw(&musbr->ep[1].epN.rxcsr);
> + MUSB_PRINT_RXCSR(w);
> +
> + w = readw(&musbr->ep[1].epN.txmaxp);
> + MUSB_PRINT_TXMAXP(w);
> +
> + w = readw(&musbr->ep[1].epN.txcsr);
> + MUSB_PRINT_TXCSR(w);
> +}
> +#else
> +#define musb_db_regs()
> +#endif /* DEBUG_MUSB */
> +
> +static void musb_peri_softconnect(void)
> +{
> + u8 power, devctl;
> + u8 intrusb;
> + u16 intrrx, intrtx;
> +
> + /* Power off MUSB */
> + power = readb(&musbr->power);
> + power &= ~MUSB_POWER_SOFTCONN;
> + writeb(power, &musbr->power);
> +
> + /* Read intr to clear */
> + intrusb = readb(&musbr->intrusb);
> + intrrx = readw(&musbr->intrrx);
> + intrtx = readw(&musbr->intrtx);
> +
> + udelay(2 * 500000); /* 1 sec */
why not udelay(1000 * 1000) as other place?
> +
> + /* Power on MUSB */
> + power = readb(&musbr->power);
> + power |= MUSB_POWER_SOFTCONN;
> + /*
> + * The usb device interface is usb 1.1
> + * Disable 2.0 high speed by clearring the hsenable bit.
> + */
> + power &= ~MUSB_POWER_HSENAB;
> + writeb(power, &musbr->power);
> +
> + /* Check if device is in b-peripheral mode */
> + devctl = readb(&musbr->devctl);
> + if (!(devctl & MUSB_DEVCTL_BDEVICE) ||
> + (devctl & MUSB_DEVCTL_HM)) {
> + serial_printf("ERROR : Unsupport USB mode\n");
> + serial_printf("Check that mini-B USB cable is attached to the device\n");
evenif it's too long it's better for search
> + }
> +
> + if (debug_setup && (debug_level > 1))
> + musb_db_regs();
> +}
> +
> +static void musb_peri_reset(void)
> +{
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("INFO : %s reset\n", __PRETTY_FUNCTION__);
what do you mean y __PRETTY_FUNCTION__?
> +
> + /* the device address is reset by the event */
> + usbd_device_event_irq(udc_device, DEVICE_RESET, 0);
> + SET_EP0_STATE(IDLE);
> + if (ep0_endpoint)
> + ep0_endpoint->endpoint_address = 0xff;
> +}
> +
> +static void musb_peri_resume(void)
> +{
> + /* noop */
> +}
> +
> +static void musb_peri_ep0_stall(void)
> +{
> + u16 csr0;
please add an empty line
> + csr0 = readw(&musbr->ep[0].ep0.csr0);
> + csr0 |= MUSB_CSR0_P_SENDSTALL;
> + writew(csr0, &musbr->ep[0].ep0.csr0);
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("INFO : %s stall\n", __PRETTY_FUNCTION__);
> +}
> +
> +static void musb_peri_ep0_ack_req(void)
> +{
> + u16 csr0;
please add an empty line
> + csr0 = readw(&musbr->ep[0].ep0.csr0);
> + csr0 |= MUSB_CSR0_P_SVDRXPKTRDY;
> + writew(csr0, &musbr->ep[0].ep0.csr0);
> +}
> +
> +static void musb_ep0_tx_ready(void)
> +{
> + u16 csr0;
please add an empty line
> + csr0 = readw(&musbr->ep[0].ep0.csr0);
> + csr0 |= MUSB_CSR0_TXPKTRDY;
> + writew(csr0, &musbr->ep[0].ep0.csr0);
> +}
> +
> +static void musb_peri_ep0_last(void)
> +{
> + u16 csr0;
please add an empty line
> + csr0 = readw(&musbr->ep[0].ep0.csr0);
> + csr0 |= MUSB_CSR0_P_DATAEND;
> + writew(csr0, &musbr->ep[0].ep0.csr0);
> +}
is it not possible to create a single function which take
MUSB_CSR0_P_DATAEND, MUSB_CSR0_TXPKTRDY, MUSB_CSR0_P_SVDRXPKTRDY,
MUSB_CSR0_P_SENDSTALL etc... as paramter?
> +
> +static void musb_peri_ep0_set_address(void)
> +{
> + writeb(udc_device->address, &musbr->faddr);
> + SET_EP0_STATE(IDLE);
> + usbd_device_event_irq(udc_device, DEVICE_ADDRESS_ASSIGNED, 0);
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("INFO : %s Address set to %d\n", __PRETTY_FUNCTION__, udc_device->address);
> +}
> +
> +static void musb_peri_rx_ack(unsigned int ep)
> +{
> + u16 peri_rxcsr;
> + peri_rxcsr = readw(&musbr->ep[ep].epN.rxcsr);
> + peri_rxcsr &= ~MUSB_RXCSR_RXPKTRDY;
> + writew(peri_rxcsr, &musbr->ep[ep].epN.rxcsr);
> +}
> +
> +static void musb_peri_tx_ready(unsigned int ep)
> +{
> + u16 peri_txcsr;
> + peri_txcsr = readw(&musbr->ep[ep].epN.txcsr);
> + peri_txcsr |= MUSB_TXCSR_TXPKTRDY;
> + writew(peri_txcsr, &musbr->ep[ep].epN.txcsr);
> +}
> +
> +static void musb_peri_ep0_zero_data_request(int err)
> +{
> + musb_peri_ep0_ack_req();
> +
> + if (err) {
> + musb_peri_ep0_stall();
> + SET_EP0_STATE(IDLE);
> + } else {
> +
> + musb_peri_ep0_last();
> +
> + /* USBD state */
> + switch (ep0_urb->device_request.bRequest) {
> + case USB_REQ_SET_ADDRESS:
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("INFO : %s received set address\n", __PRETTY_FUNCTION__);
maybe you can define a debug macro to reduce the code length
and avoid the if everywhere
> + break;
> +
> + case USB_REQ_SET_CONFIGURATION:
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("INFO : %s Configured\n", __PRETTY_FUNCTION__);
> + usbd_device_event_irq(udc_device, DEVICE_CONFIGURED, 0);
> + break;
> + }
> +
> + /* EP0 state */
> + if (USB_REQ_SET_ADDRESS == ep0_urb->device_request.bRequest) {
> + SET_EP0_STATE(SET_ADDRESS);
> + } else {
> + SET_EP0_STATE(IDLE);
> + }
> + }
> +}
> +
> +static void musb_peri_ep0_rx_data_request(void)
> +{
> + /*
> + * This is the completion of the data OUT / RX
> + *
> + * Host is sending data to ep0 that is not
> + * part of setup. This comes from the cdc_recv_setup
> + * op that is device specific.
> + *
> + */
> + musb_peri_ep0_ack_req();
> +
> + ep0_endpoint->rcv_urb = ep0_urb;
> + ep0_urb->actual_length = 0;
> + SET_EP0_STATE(RX);
> +}
> +
> +static void musb_peri_ep0_tx_data_request(int err)
> +{
> + if (err) {
> + musb_peri_ep0_stall();
> + SET_EP0_STATE(IDLE);
> + } else {
> + musb_peri_ep0_ack_req();
> +
> + ep0_endpoint->tx_urb = ep0_urb;
> + ep0_endpoint->sent = 0;
> + SET_EP0_STATE(TX);
> + }
> +}
> +
> +static void musb_peri_ep0_idle(void)
> +{
> + u16 count0;
> + int err;
> + u16 csr0;
> +
> + csr0 = readw(&musbr->ep[0].ep0.csr0);
> +
> + if (!(MUSB_CSR0_RXPKTRDY & csr0))
> + goto end;
> +
> + count0 = readw(&musbr->ep[0].ep0.count0);
> + if (count0 == 0)
> + goto end;
> +
> + if (count0 != 8) {
> + if ((debug_setup) && (debug_level > 1))
> + serial_printf("WARN : %s SETUP incorrect size %d\n",
> + __PRETTY_FUNCTION__, count0);
> + musb_peri_ep0_stall();
> + goto end;
> + }
> +
> + read_fifo(0, count0, &ep0_urb->device_request);
> +
> + if (debug_level > 2) {
> + PRINT_USB_DEVICE_REQUEST(&ep0_urb->device_request);
> + }
> +
> + if (0 == ep0_urb->device_request.wLength) {
please invert the test
> + err = ep0_recv_setup(ep0_urb);
> +
> + /* Zero data request */
> + musb_peri_ep0_zero_data_request(err);
> + } else {
> + /* Is data coming or going ? */
> + u8 reqType = ep0_urb->device_request.bmRequestType;
please add an empty line
> + if (USB_REQ_DEVICE2HOST == (reqType & USB_REQ_DIRECTION_MASK)) {
> + err = ep0_recv_setup(ep0_urb);
> + /* Device to host */
> + musb_peri_ep0_tx_data_request(err);
> + } else {
> + /*
> + * Host to device
> + *
> + * The RX routine will call ep0_recv_setup
> + * when the data packet has arrived.
> + */
> + musb_peri_ep0_rx_data_request();
> + }
> + }
> +
> +end:
> + return;
why? will not be usefull to have an error return?
> +}
> +
> +
> diff --git a/include/usb.h b/include/usb.h
> index 378a23b..198d5bb 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -131,7 +131,8 @@ struct usb_device {
> #if defined(CONFIG_USB_UHCI) || defined(CONFIG_USB_OHCI) || \
> defined(CONFIG_USB_EHCI) || defined(CONFIG_USB_OHCI_NEW) || \
> defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
> - defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI)
> + defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \
> + defined(CONFIG_USB_OMAP3)
a simple CONFIG will be better
>
> int usb_lowlevel_init(void);
> int usb_lowlevel_stop(void);
Best Regards,
J.
More information about the U-Boot
mailing list