From: Wojciech Zajac Date: Tue, 3 Jun 2014 12:58:48 +0000 (+0200) Subject: Resolution of some currently fixable TODOs X-Git-Tag: v3.3.0~208 X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/man.arpaname.html?a=commitdiff_plain;h=9f2204a8ad6ceb4f427d507a381c7134a1c4b39e;p=minix.git Resolution of some currently fixable TODOs --- diff --git a/drivers/usbd/README.txt b/drivers/usbd/README.txt index c36055855..265d30b98 100755 --- a/drivers/usbd/README.txt +++ b/drivers/usbd/README.txt @@ -17,3 +17,5 @@ created march-june 2014, JPEmbedded (info@jpembedded.eu) - URBs are enqueued in DDEKit but not in USBD itself - DDEKit way of handling interface numbers is not explicitly defined, bitmask formatting for it, is therefore hardcoded into USBD +- Waiting for USB0 clock to leave IDLEST.Disable state, by nanosleep, was + removed, as this should be implemented for all clocks in clkconf_set \ No newline at end of file diff --git a/drivers/usbd/hcd/hcd.c b/drivers/usbd/hcd/hcd.c index 216eaa528..ccdd53325 100755 --- a/drivers/usbd/hcd/hcd.c +++ b/drivers/usbd/hcd/hcd.c @@ -202,7 +202,8 @@ hcd_enumerate(hcd_device_state * this_device) return EXIT_FAILURE; } - /* TODO: dynamic device address when more devices are available */ + /* TODO: Dynamic device addressing should be added here, when more + * than one device can be handled at a time */ /* Set address */ if (EXIT_SUCCESS != hcd_set_address(this_device, HCD_ATTACHED_ADDR)) { @@ -219,9 +220,12 @@ hcd_enumerate(hcd_device_state * this_device) return EXIT_FAILURE; } - /* TODO: always first configuration */ + /* TODO: Always use first configuration, as there is no support for + * multiple configurations in DDEKit/devman and devices rarely have + * more than one anyway */ /* Set configuration */ - if (EXIT_SUCCESS != hcd_set_configuration(this_device, 0x01)) { + if (EXIT_SUCCESS != hcd_set_configuration(this_device, + HCD_SET_CONFIG_NUM(HCD_DEFAULT_CONFIG))) { USB_MSG("Failed to set configuration"); return EXIT_FAILURE; } @@ -346,12 +350,12 @@ hcd_get_descriptor_tree(hcd_device_state * this_device) completed = 0; do { - /* TODO: configuration 0 is hard-coded + /* TODO: Default configuration is hard-coded * but others are rarely used anyway */ /* TODO: magic numbers, no header for these */ setup.bRequestType = 0x80; /* IN */ setup.bRequest = 0x06; /* Get descriptor */ - setup.wValue = 0x0200; /* Configuration 0 */ + setup.wValue = 0x0200 | HCD_DEFAULT_CONFIG; setup.wIndex = 0x0000; setup.wLength = buffer_length; @@ -712,8 +716,9 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup) } } else { - /* TODO: unimplemented */ - USB_MSG("Illegal non-zero length OUT setup packet"); + /* TODO: Unimplemented OUT DATA stage */ + d->out_data_stage(d->private_data); + return EXIT_FAILURE; } } diff --git a/drivers/usbd/hcd/hcd_common.c b/drivers/usbd/hcd/hcd_common.c index 23f981f17..d3728c0f9 100755 --- a/drivers/usbd/hcd/hcd_common.c +++ b/drivers/usbd/hcd/hcd_common.c @@ -179,8 +179,9 @@ hcd_os_nanosleep(int nanosec) nanotm.tv_nsec = nanosec; } - /* TODO: since it is not likely to be ever interrupted, we do not try + /* TODO: Since it is not likely to be ever interrupted, we do not try * to sleep for a remaining time in case of signal handling */ + /* Signal handling will most likely end up with termination anyway */ USB_ASSERT(EXIT_SUCCESS == nanosleep(&nanotm, NULL), "Calling nanosleep() failed"); } @@ -225,7 +226,8 @@ hcd_disconnect_device(hcd_device_state * this_device) hcd_tree_cleanup(&(this_device->config_tree)); - /* TODO: spilled resources */ + /* TODO: Spilled resources */ + /* DDEKit does no resource deallocation, when terminating thread */ ddekit_thread_terminate(this_device->thread); ddekit_sem_deinit(this_device->lock); diff --git a/drivers/usbd/hcd/musb/musb_am335x.c b/drivers/usbd/hcd/musb/musb_am335x.c index b60848b1e..411b76b12 100755 --- a/drivers/usbd/hcd/musb/musb_am335x.c +++ b/drivers/usbd/hcd/musb/musb_am335x.c @@ -11,7 +11,9 @@ #include "musb_core.h" -/* TODO: BeagleBone white uses USB0 for PC connection as peripheral */ +/* TODO: BeagleBone white uses USB0 for PC connection as peripheral, + * so this is disabled by default */ +/* Should USB0 ever be used on BBB/BBW, one can change it to '#define': */ #undef AM335X_USE_USB0 @@ -197,8 +199,7 @@ * AM335x clocking register defines * *===========================================================================*/ /* Clock module registers offsets */ -#define AM335X_CM_PER_BASE_OFFSET 0x00u -#define AM335X_REG_CM_PER_USB0_CLKCTRL (AM335X_CM_PER_BASE_OFFSET + 28u) +#define AM335X_REG_CM_PER_USB0_CLKCTRL_OFFSET 0x1Cu /* Possible values to be set */ #define AM335X_VAL_CM_PER_USB0_CLKCTRL_MODULEMODE_ENABLE 0x2u @@ -267,7 +268,7 @@ static int musb_am335x_internal_init(void); static void musb_am335x_internal_deinit(void); /* Interrupt related */ -static void musb_am335x_irq_init(void *); /* TODO: required by DDEKit */ +static void musb_am335x_irq_init(void *); static void musb_am335x_usbss_isr(void *); static void musb_am335x_usbx_isr(void *); static int musb_am335x_irqstat0_to_ep(int); @@ -420,15 +421,11 @@ musb_am335x_internal_init(void) DEBUG_DUMP; /* Configure clocking */ - if (hcd_os_clkconf(AM335X_REG_CM_PER_USB0_CLKCTRL, + if (hcd_os_clkconf(AM335X_REG_CM_PER_USB0_CLKCTRL_OFFSET, AM335X_VAL_CM_PER_USB0_CLKCTRL_MODULEMODE_ENABLE, AM335X_CLKCONF_FULL_VAL)) return EXIT_FAILURE; - /* TODO: time to stabilize? */ - /* Sleep 25msec */ - hcd_os_nanosleep(HCD_NANOSLEEP_MSEC(25)); - /* Read and dump revision register */ USB_MSG("MUSB revision (REVREG): %08X", (unsigned int)HCD_RD4(am335x.ss.regs, AM335X_REG_REVREG)); @@ -493,6 +490,11 @@ static void musb_am335x_irq_init(void * UNUSED(unused)) { DEBUG_DUMP; + + /* TODO: This function does nothing and is not needed by MUSB but + * is still required by DDEKit for initialization, as NULL pointer + * cannot be passed to ddekit_interrupt_attach */ + return; } @@ -541,7 +543,8 @@ musb_am335x_usbx_isr(void * data) irqstat0 = HCD_RD4(r, AM335X_REG_USBXIRQSTAT0); irqstat1 = HCD_RD4(r, AM335X_REG_USBXIRQSTAT1); - /* TODO: priority of interrupts */ + /* Interrupts, seem to appear one at a time + * Check which bit is set to resolve event */ if (irqstat1 & AM335X_VAL_USBXIRQSTAT1_DRVVBUS) { USB_DBG("DRVVBUS level changed"); CLEAR_IRQ1(AM335X_VAL_USBXIRQSTAT1_DRVVBUS); diff --git a/drivers/usbd/hcd/musb/musb_core.c b/drivers/usbd/hcd/musb/musb_core.c index 854d4769d..3a29a8043 100755 --- a/drivers/usbd/hcd/musb/musb_core.c +++ b/drivers/usbd/hcd/musb/musb_core.c @@ -325,7 +325,8 @@ musb_core_stop(void * cfg) r = ((musb_core_config *)cfg)->regs; - /* TODO: add hardware interrupt disable */ + /* Disable all interrupts */ + HCD_WR1(r, MUSB_REG_INTRUSBE, MUSB_VAL_INTRUSBE_NONE); /* Stop session */ devctl = HCD_RD1(r, MUSB_REG_DEVCTL); @@ -695,8 +696,9 @@ musb_out_data_stage(void * cfg) /* Set EP and device address to be used in this command */ musb_set_state((musb_core_config *)cfg); - /* TODO: not needed for enumeration but may be needed later */ - USB_MSG("Setup packet's 'DATA OUT' stage not implemented"); + /* TODO: Not needed for enumeration but may be needed later, if + * additional control transfers are implemented */ + USB_ASSERT(0, "Setup packet's 'DATA OUT' stage not implemented"); } diff --git a/drivers/usbd/hcd/musb/musb_regs.h b/drivers/usbd/hcd/musb/musb_regs.h index ad56cfce9..e0ac4a847 100755 --- a/drivers/usbd/hcd/musb/musb_regs.h +++ b/drivers/usbd/hcd/musb/musb_regs.h @@ -101,6 +101,7 @@ #define MUSB_VAL_INTRUSBE_DISCON HCD_BIT(5) #define MUSB_VAL_INTRUSBE_SESSREQ HCD_BIT(6) #define MUSB_VAL_INTRUSBE_VBUSERR HCD_BIT(7) +#define MUSB_VAL_INTRUSBE_NONE 0x00u /* HOST_TYPE0 */ #define MUSB_VAL_HOST_TYPE0_MASK (HCD_BIT(6) | HCD_BIT(7)) @@ -141,7 +142,9 @@ HCD_BIT(3)) /* HOST_RXINTERVAL/HOST_TXINTERVAL */ -/* TODO: Default NAK limit */ +/* Default NAK limit for non-control transfer + * When too big this may cause driver to wait for + * quite long in case of NAK error */ #define MUSB_VAL_HOST_XXINTERVAL_DEFAULT 0x10u /* HOST_RXCSR */ diff --git a/drivers/usbd/include/usb/hcd_common.h b/drivers/usbd/include/usb/hcd_common.h index 7d3f251fb..f8a38b498 100755 --- a/drivers/usbd/include/usb/hcd_common.h +++ b/drivers/usbd/include/usb/hcd_common.h @@ -13,6 +13,8 @@ #include /* for setup structures */ #include /* for descriptor structures */ +#include /* for __aligned() */ + /*===========================================================================* * USB register handling defines * @@ -143,9 +145,8 @@ typedef struct hcd_device_state { /* Number of bytes received/transmitted in last transfer */ int data_len; - /* TODO: forcefully align buffer to make things clear? */ - /* Buffer for each device to hold transfered data */ - hcd_reg1 buffer[MAX_WTOTALLENGTH]; + /* Word aligned buffer for each device to hold transfered data */ + hcd_reg1 buffer[MAX_WTOTALLENGTH] __aligned(sizeof(hcd_reg4)); } hcd_device_state; @@ -219,10 +220,14 @@ typedef struct hcd_datarequest hcd_datarequest; /* Default USB communication parameters */ #define HCD_DEFAULT_EP 0x00 #define HCD_DEFAULT_ADDR 0x00 +#define HCD_DEFAULT_CONFIG 0x00 -/* TODO: one device */ +/* TODO: One device only */ #define HCD_ATTACHED_ADDR 0x01 +/* Translates configuration number for 'set configuration' */ +#define HCD_SET_CONFIG_NUM(num) ((num)+0x01u) + /*===========================================================================* * Operating system specific *