Hari,
On Thursday, October 21, 2010 3:49 PM Hari Kanigeri wrote:
> Rene,
>
> Thanks for your comment.
>
>
>>> @@ -92,20 +92,25 @@ int omap_mbox_msg_send(struct omap_mbox *mbox,
>>> mbox_msg_t msg) struct omap_mbox_queue *mq = mbox->txq;
>>> int ret = 0, len;
>>>
>>> - spin_lock(&mq->lock);
>>> + spin_lock_bh(&mq->lock);
>>>
>>
>> Please check if this scenario looks valid to you, as discussed and depicted
>> with Fernando:
>>
>> Supposing that at this point the hw fifo is full and there are messages in
>> the kfifo.
>>
>>> if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>> ret = -ENOMEM;
>>> goto out;
>>> }
>>>
>>
>> We reach this point.
>>
>> In this scenario, the DSP or other Core reads a message from the mbox hw
>> fifo. The next happens:
>> 1.- The not full interrupt is triggered to the MPU.
>> 2.- The mbox's ISR schedules the tasklet to write the message.
>> 3.- The ISR is left and returns to here (since we still have the bh lock
>> the tasklet won't run).
>>
>>> + if (!__mbox_poll_for_space(mbox)) {\
>>
>> 4.- We check for mbox_fifo_full and we have space. so the message is
>> written.
>>
>>> + mbox_fifo_write(mbox, msg);
>>
>> At this point it looks that the FIFO order is lost. We write this message
>> before the ones in the kfifo.
>>
>
> Good point. I agree.
>
>> A solution for this could be checking if there are messages in the kfifo
>> before trying to write directly to the hw fifo, if so, just continue
>> scheduling the tasklet.
>
> A change something like this ?
>
> - if (!__mbox_poll_for_space(mbox)) {
> + if (kfifo_is_empty() && !__mbox_poll_for_space(mbox)) {
Yes, this looks good to me.
> mbox_fifo_write(mbox, msg);
Regards,
Rene--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
