Dear Wolfgang,
Signed-off-by: Vipin vipin.kumar@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.
Ok. Accepted. patch-set v2 would contain the changes
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?
We also use u-boot as a firmware which runs on the board in a special mode in which the firmware fetches and burns images into NOR/NAND flashes. Under this mode, we would like to always jump to u-boot prompt. Also, the bootdelay variable should remain unchanged as we save default environment variables as well.
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?
Ok, changed to #ifndef DEBUG The standard debug facilities use printf and we also test and use this for usbtty feature. So, a serial_printf is required
+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.
No, this code is not generic.
+/*
- 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?
Again, all the code is spear specific. Probably, only the unction names are confusing
- /* 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.
Ok. Corrected globally
Too long lines.
I assume line length of 80 is acceptable. Right?
- /* 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.
Ok. Corrected 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.
Ok. Corrected globally
Best Reagrds Vipin Kumar