• State Resolved
  • Locked Locked
  • Replies 7 replies
  • Subscribers 99 subscribers
  • Views 616 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

omap-l138 mcbsp driver code issue

Mike Brudevold
Expert 1365 points
Other Parts Discussed in Thread: OMAP-L138

OMAP-L138, DSP/BIOS5, McBSP driver as part of the pspdrivers 1.30.01

I'm looking at the following lines of code in the Mcbsp_edma.c file, function Mcbsp_localEdmaCallback (Mcbsp_LOOPJOB_ENABLE is defined):

 

    if (TRUE != QUE_empty(&(chanHandle->queueFloatingList)))
    {
        /* should always have a packet present because this callback is       *
         * due to that packet                                                 */

        /* Get the packet from the top of the queue (atommic operation)       */
        chanHandle->tempPacket = QUE_get(&(chanHandle->queueFloatingList));

        /* get the param table information of transfer channel                */
        EDMA3_DRV_getPaRAM(chanHandle->edmaHandle,chanHandle->xferChan,&pramTbl);

        /* Handle the IOP packets appropriately in case of an breakpoint      *
         * in case of an breakpoint.either of the packets (2 link param Pkts) *
         * could have caused a callback as both of them as linkedto each other*
         * Hence we will handle that condition here                           */
        if (chanHandle->mode == IOM_INPUT)
        {
            /* Check if destination address falls into the range of 1st req   *
             * in the floating queue.                                         */
            if ((pramTbl.destAddr >= (Uint32)chanHandle->tempPacket->addr)
                && (pramTbl.destAddr < (Uint32)chanHandle->tempPacket->addr
                        + chanHandle->tempPacket->size))
            {
                /* Since we have already dequeue the 1st request, dequeue     *
                 * 2nd io request from floating queue                         */
                ioPacket = (IOM_Packet *)QUE_get(
                                      &chanHandle->queueFloatingList);

                /* Queue the tempPacket (i.e. 1st io request) as this pkt     *
                 * should be first in a queue                                 */
                QUE_put(
                    &chanHandle->queueFloatingList,
                    (Ptr)chanHandle->tempPacket);

                /* Queue the ioPacket i.e. 2nd request in floating queue      */
                QUE_put(&chanHandle->queueFloatingList,(Ptr)ioPacket);
            }
        }

 

I'm confused what exactly the purpose of this code is.  Also, it does not look like it functions properly.  It is assuming there are 2 items in the queue, and if there are not, really bad things happen (like adding the queue back to itself).  Even if there are 2 items in the queue, the chanHandle->tempPacket variable ends up pointing to an item that is still in the queue, which I don't think it should.  This code is largely not hit (and the equivalent case for IOM_OUTPUT), but when it is, things stop working.

Can someone take a look at this and let me know what the proper fix is?

  • 0
    TI__Expert 7290 points

    Mike,

    Basically this code is to handle the scenario where if the EDMA call back for the second packet comes first. This scenario will occur only when the CPU is halted for sometime because of any reason (Eg: Breakpoint, which is explained in the comment itself).

    So i suppose you have observed code hitting only when there is a breakpoint in the edma callback function.

    Are you facing any problem in this logic, if you do not put breakpoint in the EDMA callback?

    Thanks and Regards,

    Sandeep K

  • 0 in reply to Sandeep Krishnaswamy
    Expert 1365 points

    Sandeep,

    The flaw in the code is, again, that it assumes there are two packets in the floating queue.  You do need to check QUE_empty before you QUE_get the second packet, only assigning a buffer to ioPacket if one exists.  Similarly then, do not QUE_put ioPacket unless it has been assigned a buffer.  In addition, I added a line "status = IOM_ENOPACKETS;", which prevents the rest of the function from thinking there is a valid packet to return.

    I have observed this block of code being hit by placing a breakpoint INSIDE the code block, hence it had to have been hit first, and not caused by the breakpoint.  Now, as to why that is, I have no idea.  It may be related to another spurious issue I have seen, in the Mcasp driver (non-loopjob), where the edma error callback is continually reporting dma event miss, even when there are no packets being transmitted (bit clock and frame clock off).  I don't yet know enough about edma3 to have any idea why that would happen, but it could be somehow related to starting/stopping the DSP core and reloading different DSP binaries (which wouldn't reset the edma engine other than whatever bits edma3 initialization touches).

    Mike

  • 0 in reply to Mike Brudevold
    TI__Expert 5500 points

    Hi Mike,

    As you said, we need to make sure and check QUE_empty before we QUE_get the second packet. So you can just insert the following piece of code in the condition:

    if (((pramTbl.destAddr >= (Uint32)chanHandle->tempPacket->addr)
           && (pramTbl.destAddr < (Uint32)chanHandle->tempPacket->addr + chanHandle->tempPacket->size)) && (FALSE==QUE_empty(chanHandle -> queueFloatingList)))

    By doing so, with LOOPJOB_ENABLE and no breakpoints applied the part of code block should not be hit(executed). And also sssuming you are priming buffers before servicing the requests. Hope this solves your issue.

    Mike Brudevold said:
    It may be related to another spurious issue I have seen, in the Mcasp driver (non-loopjob), where the edma error callback is continually reporting dma event miss, even when there are no packets being transmitted (bit clock and frame clock off).

     

    This is not a spurious issue though. As soon as the transmit serializer is taken out of reset, XDATA in the XSTAT register is set, indicating that XBUF is empty and ready to be serviced. The XDATA status causes a DMA event AXEVT to be generated, and can cause an interrupt AXINT to be generated if it is enabled in the XINTCTL register. Now that the paramset is not set because there are no packets for servicing,continuous events are misssed by the edma. For more information, please (Refer McASP SPRU section - 2.4.1.2).

    Thanks & regards,

    Raghavendra

  • 0 in reply to Raghavendra Maddikery
    Expert 1365 points

    I'm curious about your solution as opposed to mine.  It seems to me that if chanHandle->tempPacket->addr matches the current pramTbl.{dest,src}Addr, then the packet is being serviced by the DMA (else the loopjob should be running).  Hence, we should ignore the interrupt.  This also makes no sense because we did get an interrupt, so something must have happened, hence your solution.  Does my confusion make sense?

  • 0 in reply to Mike Brudevold
    TI__Expert 7290 points

    Mike,

    According to my understanding, the code which you are refering to is only to handle the error scenario which occurs when the CPU halts (Eg: at breakpoint). So even if we ignore it at the interrupt, it should not affect the functionality of the McASP driver.

    Between, are you able to fix the issue with the solution which you have mentioned in the earlier post?  

    Thanks and Regards,

    Sandeep K

  • 0 in reply to Sandeep Krishnaswamy
    Expert 1365 points

    My fix is working great.

  • 0 in reply to Mike Brudevold
    TI__Expert 7290 points

    Thats great!!

    I guess, we can close this thread.

    Thanks and Regards,

    Sandeep K