Dear Vipin KUMAR, In message <1260955110-5656-5-git-send-email-vipin.ku...@st.com> you wrote: > > Signed-off-by: Vipin <vipin.ku...@st.com> > --- > common/main.c | 2 + > drivers/serial/usbtty.h | 2 + > drivers/usb/gadget/Makefile | 1 + > drivers/usb/gadget/spr_udc.c | 996 > +++++++++++++++++++++++++++++++++ > include/asm-arm/arch-spear/spr_misc.h | 126 +++++ > include/usb/spr_udc.h | 227 ++++++++ > 6 files changed, 1354 insertions(+), 0 deletions(-) > mode change 100644 => 100755 drivers/serial/usbtty.h > mode change 100644 => 100755 drivers/usb/gadget/Makefile > create mode 100755 drivers/usb/gadget/spr_udc.c > create mode 100644 include/asm-arm/arch-spear/spr_misc.h > create mode 100755 include/usb/spr_udc.h
Please split into two patches: one with generic usbd driver, and the second adding support for SPEAr. > diff --git a/common/main.c b/common/main.c > index 10d8904..79f3018 100644 > --- a/common/main.c > +++ b/common/main.c > @@ -397,6 +397,7 @@ void main_loop (void) > > debug ("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>"); > > +#if !defined(CONFIG_SPEAR_USBTTY) > if (bootdelay >= 0 && s && !abortboot (bootdelay)) { > # ifdef CONFIG_AUTOBOOT_KEYED > int prev = disable_ctrlc(1); /* disable Control C checking */ > @@ -413,6 +414,7 @@ void main_loop (void) > disable_ctrlc(prev); /* restore Control C checking */ > # endif > } > +# endif Why would this be needed? > diff --git a/drivers/usb/gadget/spr_udc.c b/drivers/usb/gadget/spr_udc.c > new file mode 100755 > index 0000000..5b135c7 > --- /dev/null > +++ b/drivers/usb/gadget/spr_udc.c ... > +/* Some kind of debugging output... */ > +#if 1 > +#define UDCDBG(str) > +#define UDCDBGA(fmt, args...) > +#else > +#define UDCDBG(str) serial_printf(str "\n") > +#define UDCDBGA(fmt, args...) serial_printf(fmt "\n", ##args) > +#endif This looks wrong. Should that be a "#ifndef DEBUG" instead of "#if 1"? And cannot we use standard debug facilities? > +static struct udc_endp_regs *const outep_regs_p = > + &((struct udc_regs *const)CONFIG_SYS_USBD_BASE)->out_regs[0]; > +static struct udc_endp_regs *const inep_regs_p = > + &((struct udc_regs *const)CONFIG_SYS_USBD_BASE)->in_regs[0]; > + > +/* > + * udc_state_transition - Write the next packet to TxFIFO. > + * @initial: Initial state. > + * @final: Final state. > + * > + * Helper function to implement device state changes. The device states and > + * the events that transition between them are: > + * > + * STATE_ATTACHED > + * || /\ > + * \/ || > + * DEVICE_HUB_CONFIGURED DEVICE_HUB_RESET > + * || /\ > + * \/ || > + * STATE_POWERED > + * || /\ > + * \/ || > + * DEVICE_RESET DEVICE_POWER_INTERRUPTION > + * || /\ > + * \/ || > + * STATE_DEFAULT > + * || /\ > + * \/ || > + * DEVICE_ADDRESS_ASSIGNED DEVICE_RESET > + * || /\ > + * \/ || > + * STATE_ADDRESSED > + * || /\ > + * \/ || > + * DEVICE_CONFIGURED DEVICE_DE_CONFIGURED > + * || /\ > + * \/ || > + * STATE_CONFIGURED > + * > + * udc_state_transition transitions up (in the direction from STATE_ATTACHED > + * to STATE_CONFIGURED) from the specified initial state to the specified > final > + * state, passing through each intermediate state on the way. If the initial > + * state is at or above (i.e. nearer to STATE_CONFIGURED) the final state, > then > + * no state transitions will take place. > + * > + * udc_state_transition also transitions down (in the direction from > + * STATE_CONFIGURED to STATE_ATTACHED) from the specified initial state to > the > + * specified final state, passing through each intermediate state on the way. > + * If the initial state is at or below (i.e. nearer to STATE_ATTACHED) the > final > + * state, then no state transitions will take place. > + * > + * This function must only be called with interrupts disabled. > + */ > +static void udc_state_transition(usb_device_state_t initial, > + usb_device_state_t final) ... > +/* Stall endpoint */ > +static void udc_stall_ep(u32 ep_num) ... > +static void *get_fifo(int ep_num, int in) ... > +static short usbgetpckfromfifo(int epNum, u8 *bufp, u32 len) ... > +static void usbputpcktofifo(int epNum, u8 *bufp, u32 len) ... So far this code looks pretty generic to me. > +/* > + * spear_write_noniso_tx_fifo - Write the next packet to TxFIFO. > + * @endpoint: Endpoint pointer. > + * > + * If the endpoint has an active tx_urb, then the next packet of data from > the > + * URB is written to the tx FIFO. The total amount of data in the urb is > given > + * by urb->actual_length. The maximum amount of data that can be sent in any > + * one packet is given by endpoint->tx_packetSize. The number of data bytes > + * from this URB that have already been transmitted is given by > endpoint->sent. > + * endpoint->last is updated by this routine with the number of data bytes > + * transmitted in this packet. > + * > + */ > +static void spear_write_noniso_tx_fifo(struct usb_endpoint_instance > + *endpoint) Now her eit becomes clearly SPEAr-specific. Should this not be split into separate source files to allow reuse of the common code by other processors? > + > + /* This ensures that USBD packet fifo is accessed > + :- through word aligned pointer or > + :- through non word aligned pointer but only with a > + max length to make the next packet word aligned */ Incorrect multiline comment style. Too long lines. > + /* This rx interrupt must be for a control write data > + * stage packet. > + * > + * We don't support control write data stages. > + * We should never end up here. > + */ Incorrect multiline comment style. Please fix globally. > +struct misc_regs { ... > + u32 BIST5_RSLT_REG; /* 0x118 */ > + u32 SYST_ERROR_REG; /* 0x11C */ ... > + u32 RAS_GPP1_IN; /* 0x8000 */ Variable names must be lower case. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "Now here's something you're really going to like!" - Rocket J. Squirrel _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot