• State Not Answered
  • Locked Locked
  • Replies 18 replies
  • Subscribers 22 subscribers
  • Views 3503 views
  • Users 0 members are here
Options
Related
How was your experience today?
This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Is Keil USB sample code weird?

herbert d

Hi,

I'm just implementing the Keil HID sample code, but I found some strange things even the compiler complains about:

In usbcore.c there are some variables used without being set before. The variables are "alt" in USB_SetConfiguration() and "msk", "old", "alt" and "ifn" in USB_SetInterface().

__inline BOOL USB_SetConfiguration(void)
{
  USB_COMMON_DESCRIPTOR *pD;
  DWORD alt, n, m;

         :

  case USB_INTERFACE_DESCRIPTOR_TYPE:
    alt = ((USB_INTERFACE_DESCRIPTOR *) pD)->bAlternateSetting;
  break;
  case USB_ENDPOINT_DESCRIPTOR_TYPE:
    if (alt == 0)
    {
         :
    }
         :
}


The code at USB_ENDPOINT_DESCRIPTOR_TYPE is executed when "alt" is 0 by chance only and setting "alt" in USB_INTERFACE_DESCRIPTOR_TYPE has no effect as "alt" is an auto-variable loosing it's contents after USB_SetConfiguration() is finished.

USB_SetInterface() shows similar things.

The source code does work in my environment, but I'm anxious that this is by chance only.

Could anybody explain me how to come to a more reliable solution?

BTW: Code parts like

  (BYTE *) pD += pD->bLength;


in a sample code scare me even after more than 20 years C programming (Realview complains as well).

Regards
Herbert

Parents
  • 0 in reply to Robert  

    Robert Rostohar wrote:
    "BTW: These functions are tested and work as expected and also pass the USB compliance test."

    Really? :-)

    A minor bug found by USBCV

    I've tested this example on USBCV (USB compliance test of USB-IF)

    LPC2368 / LPC2378 USB HID (Human Interface Device) Example
    http://www.keil.com/download/docs/335.asp

    USBCV1.3.2
    www.usb.org/.../tools

    USBCV reports this error for Ch.9 test
    - ConfigDescriptorTest_DeviceConfigured
    - ConfigDescriptorTest_DeviceAddressed
    ERROR Device Reports BUS Powered in configuration descriptor and is currently SELF powered.
    FAIL (1.2.36) A Device must report the same type of power in the configuration descriptor as in GetStatus.

    HID test completes successfully.

    Above bug is caused by this lines,

    #define USB_CONFIG_BUS_POWERED                 0x80
#define USB_CONFIG_SELF_POWERED                0xC0

usbcore.c
__inline BOOL USB_SetConfiguration (void) {
   ...
            if (((USB_CONFIGURATION_DESCRIPTOR *)pD)->bmAttributes & USB_CONFIG_SELF_POWERED) {
              USB_DeviceStatus |=  USB_GETSTATUS_SELF_POWERED;
            } else {
              USB_DeviceStatus &= ~USB_GETSTATUS_SELF_POWERED;
            }

    The condition of "if" clause is TRUE, even when USB_CONFIG_BUS_POWERED is assigned to bmAttributes, because USB_CONFIG_SELF_POWERED doesn't work as a good mask for bit 6. Then, Get_Status( DEVICE ) always returns USB_GETSTATUS_SELF_POWERED, regardless of bmAttributes.

    This bug is a minor one, it just affects Get_Status( DEVICE ).

    Tsuneo

Reply
  • 0 in reply to Robert  

    Robert Rostohar wrote:
    "BTW: These functions are tested and work as expected and also pass the USB compliance test."

    Really? :-)

    A minor bug found by USBCV

    I've tested this example on USBCV (USB compliance test of USB-IF)

    LPC2368 / LPC2378 USB HID (Human Interface Device) Example
    http://www.keil.com/download/docs/335.asp

    USBCV1.3.2
    www.usb.org/.../tools

    USBCV reports this error for Ch.9 test
    - ConfigDescriptorTest_DeviceConfigured
    - ConfigDescriptorTest_DeviceAddressed
    ERROR Device Reports BUS Powered in configuration descriptor and is currently SELF powered.
    FAIL (1.2.36) A Device must report the same type of power in the configuration descriptor as in GetStatus.

    HID test completes successfully.

    Above bug is caused by this lines,

    #define USB_CONFIG_BUS_POWERED                 0x80
#define USB_CONFIG_SELF_POWERED                0xC0

usbcore.c
__inline BOOL USB_SetConfiguration (void) {
   ...
            if (((USB_CONFIGURATION_DESCRIPTOR *)pD)->bmAttributes & USB_CONFIG_SELF_POWERED) {
              USB_DeviceStatus |=  USB_GETSTATUS_SELF_POWERED;
            } else {
              USB_DeviceStatus &= ~USB_GETSTATUS_SELF_POWERED;
            }

    The condition of "if" clause is TRUE, even when USB_CONFIG_BUS_POWERED is assigned to bmAttributes, because USB_CONFIG_SELF_POWERED doesn't work as a good mask for bit 6. Then, Get_Status( DEVICE ) always returns USB_GETSTATUS_SELF_POWERED, regardless of bmAttributes.

    This bug is a minor one, it just affects Get_Status( DEVICE ).

    Tsuneo

Children
No data