• AVR Freaks

Question(s) about USB Stack v2013-02-15

Author
Neiwiertz
Super Member
  • Total Posts : 2094
  • Reward points : 0
  • Joined: 2004/09/01 02:58:52
  • Status: offline
2013/07/28 07:53:16 (permalink)
0

Question(s) about USB Stack v2013-02-15

[Question-01__IsSolved code UnChanged]  A1 A2 A3 A4
At File: usb_device.h  Line: 1422, can this section below rewrite as...,
probarly not i think due STAT.UOWN role is out of play then which could be 0 or 1,
don't know if this is intended... that handle And STAT.UOWN bit,
are Both Two different items to determine if an USB handle is busy or not?
BOOL USBHandleBusy(USB_HANDLE handle);    
/*DOM-IGNORE-BEGIN*/
//#define USBHandleBusy(handle) (handle==0?0:((volatile BDT_ENTRY*)handle)->STAT.UOWN)
#define USBHandleBusy(handle) (handle==0?0:1)

post edited by Neiwiertz - 2013/08/08 03:03:06

Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
#1

17 Replies Related Threads

    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/28 09:47:58 (permalink)
    0
    Hmm, why would you want to rule out the hardware...?

    GENOVA :D :D ! GODO
    #2
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 06:38:54 (permalink)
    0
    I don't want to rule the hardware (never be good), therefore i think this is intended purpose how it works for the second part of the if, for the first part handle==0 i guess this one is meant also to catch startup initialization where in and out handle(s) are declared and initialized to zero see line 356 of main.c
     
    I am try to understand this piece of code the detection if handle is busy or not for normal operation and initialized from startup = 0 will be not busy regarding UOWN bit which is zero at startup i guess, i just read at the datasheet the user should initialize at startup the UOWN bit to desired value before enable usb module
     
    USBConfigureEndpoint could initialize UOWN through
      USBEnableEndpoint
      USBCBInitEP
      USER_USB_CALLBACK_EVENT_HANDLER
     
    But i see at code many other(s) can modify UOWN like the one(s) STAT.Val
    Complicated this UOWN thing but it works hopefull it's handled with care
     
    Regards
    post edited by Neiwiertz - 2013/07/29 06:55:19

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #3
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 07:24:09 (permalink)
    0
    [Question-02]  A1
    At USBCheckHIDRequest of usb_function_hid.c i noticed this if is commented out and the code between two brace will be called if the case of the switch is matched even though, comment out those two brace(s) as well should not make a difference, compared to older hid driver the size is now fixed as well should this if statement stay being commented out? (see second case DSC_RPT)
     
    v2013-02-15 (see from v2012-04-03 vs v2012-07-18 where change is made)
            switch(SetupPkt.bDescriptorType)     
            {
                case DSC_HID: //HID Descriptor         
                    if(USBActiveConfiguration == 1)
                    {
                        USBEP0SendROMPtr(
                            (ROM BYTE*)&configDescriptor1 + 18,        //18 is a magic number.  It is the offset from start of the configuration descriptor to the start of the HID descriptor.
                            sizeof(USB_HID_DSC)+3,
                            USB_EP0_INCLUDE_ZERO);
                    }
                    break;
                case DSC_RPT:  //Report Descriptor          
                    //if(USBActiveConfiguration == 1)
                    {
                        USBEP0SendROMPtr(
                            (ROM BYTE*)&hid_rpt01,
                            HID_RPT01_SIZE,     //See usbcfg.h
                            USB_EP0_INCLUDE_ZERO);
                    }
                    break;
                case DSC_PHY:  //Physical Descriptor
    Older
            switch(SetupPkt.bDescriptorType)     
            {
                case DSC_HID: //HID Descriptor         
                    if(USBActiveConfiguration == 1)
                    {
                        USBEP0SendROMPtr(
                            (ROM BYTE*)&configDescriptor1 + 18,        //18 is a magic number.  It is the offset from start of the configuration descriptor to the start of the HID descriptor.
                            sizeof(USB_HID_DSC)+3,
                            USB_EP0_INCLUDE_ZERO);
                    }
                    break;
                case DSC_RPT:  //Report Descriptor          
                    if(USBActiveConfiguration == 1)
                    {
                        USBEP0SendROMPtr(
                            (ROM BYTE*)&hid_rpt01,
                            sizeof(hid_rpt01),     //See usbcfg.h
                            USB_EP0_INCLUDE_ZERO);
                    }
                    break;
                case DSC_PHY:  //Physical Descriptor

    post edited by Neiwiertz - 2013/08/08 02:58:45

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #4
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 07:51:13 (permalink)
    0
    [Question-03__IsSolved code UnChanged]  A1
    ghost bracket(s) at USBStdGetStatusHandler case section should those being deleted? see ubc_device.c  line 2313 and 2318 see third and last case section
    hmmm i think those bracket(s) where part of an if statement at some point
    previous in time at the past i go looking
        switch(SetupPkt.Recipient)     
        {
            case USB_SETUP_RECIPIENT_DEVICE_BITFIELD:
                inPipes[0].info.bits.busy = 1;
                /*
                 * [0]: bit0: Self-Powered Status [0] Bus-Powered [1] Self-Powered
                 *      bit1: RemoteWakeup        [0] Disabled    [1] Enabled
                 */
                if(self_power == 1) // self_power is defined in HardwareProfile.h
                {
                    CtrlTrfData[0]|=0x01;
                }

                if(RemoteWakeup == TRUE)
                {
                    CtrlTrfData[0]|=0x02;
                }
                break;
            case USB_SETUP_RECIPIENT_INTERFACE_BITFIELD:
                inPipes[0].info.bits.busy = 1;     // No data to update
                break;
            case USB_SETUP_RECIPIENT_ENDPOINT_BITFIELD:
                inPipes[0].info.bits.busy = 1;
                /*
                 * [0]: bit0: Halt Status [0] Not Halted [1] Halted
                 */
                {
                    BDT_ENTRY *p;

                    if(SetupPkt.EPDir == 0)
                    {
                        p = (BDT_ENTRY*)pBDTEntryOut[SetupPkt.EPNum];
                    }
                    else
                    {
                        p = (BDT_ENTRY*)pBDTEntryIn[SetupPkt.EPNum];
                    }

                    if((p->STAT.UOWN == 1) && (p->STAT.BSTALL == 1))
                        CtrlTrfData[0]=0x01;    // Set bit0
                    break;
                }
        }//end switch

    post edited by Neiwiertz - 2013/08/08 03:04:48

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #5
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 08:00:47 (permalink)
    0
    [Question-04__IsSolved]  A1
    At USBCheckStdRequest line 2881 of usb_device.c
    a missing break noticed and potential un implemented case section
            case USB_REQUEST_SYNCH_FRAME:    
            default:
                break;
        }//end switch
    }//end USBCheckStdRequest
    Should be ?
            case USB_REQUEST_SYNCH_FRAME:    
                break;
            default:
                break;
        }//end switch
    }//end USBCheckStdRequest

    post edited by Neiwiertz - 2013/08/08 03:07:08

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #6
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 08:20:03 (permalink)
    0
    [Question-05]
    After reading Tsuneo post here, i searched for USBSetBDTAddress this one is declared at #define USBSetBDTAddress(addr) at usb_hal_pic18 line 509 but i could not find any body implementation for this macro is it an empty one ?
    macro USBPowerModule seems to be empty as well am i missing some thing?
    post edited by Neiwiertz - 2013/08/08 03:07:57

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #7
    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 11:34:22 (permalink)
    0
    Neiwiertz

    ghost bracket(s) at USBStdGetStatusHandler case section should those being deleted? see ubc_device.c  line 2313


     
    They should be there because new variables are declared in the middle of code, and older C versions did not allow that (C89 maybe)

    GENOVA :D :D ! GODO
    #8
    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 11:44:15 (permalink)
    0
    Neiwiertz

    At USBCheckStdRequest line 2881 of usb_device.c
    a missing break noticed and potential un implemented case section


     
    I call it "removing redundant code" grin

    GENOVA :D :D ! GODO
    #9
    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 11:46:11 (permalink)
    0
    Neiwiertz

    At USBCheckHIDRequest of usb_function_hid.c i noticed this if is commented out and the code between two brace will be called if the case of the switch is matched even though, comment out those two brace(s) as well should not make a difference, compared to older hid driver the size is now fixed as well should this if statement stay being commented out? (see second case DSC_RPT)

     
    I noticed this as well.
    I am not sure need a more expert expert Smile
    In my opinion, it will make differences if there are more Configurations... I'd keep it..

    GENOVA :D :D ! GODO
    #10
    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 11:47:22 (permalink)
    0
    Neiwiertz

    I don't want to rule the hardware (never be good), therefore i think this is intended purpose how it works for the second part of the if, for the first part handle==0 i guess this one is meant also to catch startup initialization where in and out handle(s) are declared and initialized to zero see line 356 of main.c

    I am try to understand this piece of code the detection if handle is busy or not for normal operation and initialized from startup = 0 will be not busy regarding UOWN bit which is zero at startup i guess, i just read at the datasheet the user should initialize at startup the UOWN bit to desired value before enable usb module

    USBConfigureEndpoint could initialize UOWN through
    USBEnableEndpoint
    USBCBInitEP
    USER_USB_CALLBACK_EVENT_HANDLER

    But i see at code many other(s) can modify UOWN like the one(s) STAT.Val
    Complicated this UOWN thing but it works hopefull it's handled with care

    Regards

     
     
    Now I am not sure then. Indeed the check for the Handle and then the HArdware flag makes sense, generally speaking...

    GENOVA :D :D ! GODO
    #11
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 14:46:25 (permalink)
    0
    Thanks Dario
     
    All your reply i agree and understand, the two brackets is new for me :) now i noticed the variable declare, i noticed the two brackets previous at the usb stack aswell at the potmeter demo i believe there was a variable declared
     
    "In my opinion, it will make differences if there are more Configurations... I'd keep it.."
    Yes i would keep it too and and re activate the if statement, Hopefull Tsuneo can clarify
    post edited by Neiwiertz - 2013/07/29 17:35:45

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #12
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/29 23:22:07 (permalink)
    +4 (2)
    I agree with Dario.

    Until a configuration is established by Set_Configuration, device should return STALL to such Requests, that target to a specific interface or endpoint (other than the default).

    In practice, HID specific requests, including Get_Descriptor(HID / HID Report), are issued just by host HID class driver. The HID class driver is assigned to the device, after a configuration is chosen (Select Configuration). Therefore, we shouldn't see any HID class Request before Set_Configuration.

    Tsuneo
    #13
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/30 03:14:07 (permalink)
    0
    Hello Tsuneo
     
    I added this one at E16 list, about Question-2 the commented if statement, Dario had reply here,
    i also think about to re-activate this if statement "if(USBActiveConfiguration == 1)" don't know if that is wisdom?
     
    Due Now for report descriptor the section will always being perfomed if the belonging case value is equal to the switch.
    Additional the report descriptor section uses fixed size HID_RPT01_SIZE vs
    sizeof(hid_rpt01) at the older implementation.
    post edited by Neiwiertz - 2013/07/30 03:16:50

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #14
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/07/30 11:10:51 (permalink)
    0
    [Question-06__IsSolved code UnChanged]  A1
    I managed to get this usb stack build after two days, this means re-add just only the needed include(s) anyway at usb_device.c about EVENT_DEVICE_STACK_BASE this one is set by = EVENT_DEVICE_STACK_BASE at typedef enum USB_EVENT.
    Which also throw(s) a compile error, this compile error was not thrown previously ;)
    i looked at all usb_device.c files and everywhere the same i think it should be changed like this:
     
    Additional this same name is used at typedef enum USB_DEVICE_STACK_EVENTS at usb_common.h aswell,
    search for EVENT_DEVICE_STACK_BASE.
     
    At usb_device.h
    /* USB device stack events description here - DWF */      
    typedef enum
    {
        // Notification that a SET_CONFIGURATION() command was received (device)
        EVENT_CONFIGURED
        /*DOM-IGNORE-BEGIN*/    = EVENT_DEVICE_STACK_BASE        /*DOM-IGNORE-END*/,

        // A SET_DESCRIPTOR request was received (device)
        EVENT_SET_DESCRIPTOR, // etc...
    Should be like this to have a seperate event for EVENT_DEVICE_STACK_BASE?
    and Second possible improve will be to initialize all event members of enum?
    typedef enum      
    {
        EVENT_CONFIGURED=0,
        EVENT_DEVICE_STACK_BASE=1,
        EVENT_SET_DESCRIPTOR=2,
        EVENT_EP0_REQUEST=3,
        EVENT_ATTACH=4,
        EVENT_TRANSFER_TERMINATED=5
    } USB_DEVICE_STACK_EVENTS;

    post edited by Neiwiertz - 2013/08/08 03:14:35

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #15
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/08/01 12:18:17 (permalink)
    0
    Any comment about Question-06 are welcome, i certain think it is a pitfall and one event was missing, still wondering how it is possible the stack could compile succesfull due the event was not declared at the enum only used for initialize which is wrong my point of view, the event was used at code but if a define is un declared the compiler remains silent due it never can decide for us, it is upon our self to ensure everyting is declared before start try to use it

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #16
    DarioG
    Allmächtig.
    • Total Posts : 54081
    • Reward points : 0
    • Joined: 2006/02/25 08:58:22
    • Location: Oesterreich
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/08/01 12:27:29 (permalink)
    0
    I must be sincere and admit that I did not understand that  Smile
    I looked into my sources, too...

    GENOVA :D :D ! GODO
    #17
    Neiwiertz
    Super Member
    • Total Posts : 2094
    • Reward points : 0
    • Joined: 2004/09/01 02:58:52
    • Status: offline
    Re:Question(s) about USB Stack v2013-02-15 2013/08/01 13:21:08 (permalink)
    +2 (1)
    I am also still doubt about this one, i will see if can create support ticket
     
    From microchip ticket answer, my short explain,
    do Not change code at usb_device.h a code change can break stack operation,
    It is the way intended to use EVENT_DEVICE_STACK_BASE as initializer for EVENT_CONFIGURED,
    therefore Not to overlap
    post edited by Neiwiertz - 2013/08/08 03:13:50

    Flying With --|Explorer 16|HardWare|SoftWare|-- Fav(s) Gallery Lists
    #18
    Jump to:
    © 2019 APG vNext Commercial Version 4.5