[rt2x00-users] [RFC] rt2x00: Implement tx queue watchdog for rt2800pci

[Helmut Schaa]

Am Dienstag 10 August 2010 schrieb Ivo Van Doorn:
> On Tue, Aug 10, 2010 at 5:03 PM, Helmut Schaa
> <helmut.schaa at googlemail.com> wrote:
> > Am Dienstag 10 August 2010 schrieb Ivo Van Doorn:
> >> Hi,
> >>
> >> On Tue, Aug 10, 2010 at 3:54 PM, Helmut Schaa
> >> <helmut.schaa at googlemail.com> wrote:
> >> > On rt2800pci it can happen that a tx status report gets lost which would
> >> > mean that a tx queue entry remains unfreed. However, the device also
> >> > also generates DMA done interrupts when it finishes a DMA transfer.
> >> > Hence, introduce a new tx queue index called Q_INDEX_DMA_DONE which
> >> > always points to the next tx queue entry that wasn't transferred to the
> >> > hardware yet.
> >> >
> >> > The rt2800pci_watchdog function checks if a frame was queued for
> >> > transmission but the according tx status wasn't reported within 100ms.
> >> > If that condition appears all remaining tx entries till the
> >> > Q_INDEX_DMA_DONE index will get freed.
> >> >
> >> > Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
> >> > ---
> >>
> >
> > [...]
> >
> >> > +static void rt2800pci_watchdog(struct rt2x00_dev *rt2x00dev)
> >> > +{
> >> > +       struct data_queue *queue;
> >> > +
> >> > +       tx_queue_for_each(rt2x00dev, queue) {
> >> > +               if (time_after(jiffies, queue->last_index + (HZ / 10)) &&
> >>
> >> Perhaps something like rt2x00queue_timeout(), do we really need
> >> jiffies here instead of another time_t variable in the queue (last_index_done)?
> >
> > Ok, checked that again. I cannot use rt2x00queue_timeout here since it refers to
> > the time between the last index access and the last done index access.
> >
> > What I want to check is if the time between the last dma done access and the
> > last done index access greater then 100ms. However, I can simply add another
> > timeout method rt2x00queue_status_timeout?
> 
> I would call it something like dma_timeout, but yes. :)
> And perhaps instead of jiffies you can add another time_t variable to the
> queue which is updated when the DMA_DONE is increased?
> 
> Because jiffies isn't the best approach, consider a low traffic network:

Ok, I'll respin the patch ...

> Then the last DMA could be 200ms ago, and the last TXdone to complete
> the queue was done 150 ms ago. Then if you compare it against jiffies
> you have the timeout triggered while there is no packet in the hardware.

Yeah, that's why I had the "queue->index[Q_INDEX_DONE] != queue->index[Q_INDEX_DMA_DONE]"
check in there but that can be most likely removed then.

Helmut