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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot