]> Zhao Yanbai Git Server - minix.git/commitdiff
Resolution of some currently fixable TODOs
authorWojciech Zajac <wzajac@jpembedded.eu>
Tue, 3 Jun 2014 12:58:48 +0000 (14:58 +0200)
committerLionel Sambuc <lionel@minix3.org>
Mon, 28 Jul 2014 15:05:50 +0000 (17:05 +0200)
drivers/usbd/README.txt
drivers/usbd/hcd/hcd.c
drivers/usbd/hcd/hcd_common.c
drivers/usbd/hcd/musb/musb_am335x.c
drivers/usbd/hcd/musb/musb_core.c
drivers/usbd/hcd/musb/musb_regs.h
drivers/usbd/include/usb/hcd_common.h

index c36055855c5508c350556e766463da1fdda4955a..265d30b984cf02405fd8995a4c166bd010f28d91 100755 (executable)
@@ -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
index 216eaa5282ee1d287f987f5033ee4a6e0a1d5ab8..ccdd53325077a1f15efcd4689c701fbc3d326800 100755 (executable)
@@ -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;
                }
        }
index 23f981f17557066a61ee75f88305d595595e5e12..d3728c0f9563f1083010d126b6ad6469fdafe9ff 100755 (executable)
@@ -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);
 
index b60848b1e5e874485bc0da6d2cabb6da20ae107e..411b76b12b5d82a28d43ba1e9465222827188325 100755 (executable)
@@ -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
 
 
  *    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);
index 854d4769df12798f51be0ff3a82b6360d680acaa..3a29a8043b2c5eb4030f10a54e1fd3a555245357 100755 (executable)
@@ -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");
 }
 
 
index ad56cfce96259ded6c6143693b5a53e041d67b6e..e0ac4a847513bcc6f6e9c9fce4b4cf4e19658d68 100755 (executable)
 #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))
                                                 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 */
index 7d3f251fbfe530c40b27adf880396df23052346f..f8a38b498439e932c8ffc139adeaaa999ebe4532 100755 (executable)
@@ -13,6 +13,8 @@
 #include <minix/usb.h>                         /* for setup structures */
 #include <minix/usb_ch9.h>                     /* for descriptor structures */
 
+#include <sys/cdefs.h>                         /* 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                                              *