[rt2x00-users] [PATCH 4/4] rt2x00: Implement watchdog monitoring

Helmut Schaa helmut.schaa at googlemail.com
Mon Jul 5 16:30:05 AEST 2010


Am Sonntag 04 Juli 2010 schrieb Ivo van Doorn:
> Implement watchdog monitoring for USB devices (PCI support can
> be added later). This will determine if URBs being uploaded to
> the hardware are actually returning. Both rt2500usb and rt2800usb
> have shown that URBs being uploaded can remain hanging without
> being released by the hardware.
> By using this watchdog, a queue can be reset when this occurs.
> For rt2800usb it has been tested that the connection is preserved
> even though this interruption.

Yeah, I like that idea but see some comments below ...

[...]

> diff --git a/drivers/net/wireless/rt2x00/rt2x00link.c b/drivers/net/wireless/rt2x00/rt2x00link.c
> index d1948ea..38995d2 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00link.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00link.c
> @@ -385,6 +385,12 @@ static void rt2x00link_tuner(struct work_struct *work)
>  		rt2x00dev->ops->lib->link_tuner(rt2x00dev, qual, link->count);
>  
>  	/*
> + 	 * Perform watchdog checkings to determine the health of the hardware.
> +	 */
> +	if (test_bit(DRIVER_SUPPORT_WATCHDOG, &rt2x00dev->flags))
> +		rt2x00dev->ops->lib->watchdog(rt2x00dev);
> +
> +	/*
>  	 * Send a signal to the led to update the led signal strength.
>  	 */
>  	rt2x00leds_led_quality(rt2x00dev, qual->rssi);

Could we move the watchdog into a different periodic function? If you'd like
to keep it here rt2x00link_tuner could be renamed to do_periodic_stuff (or
something more appropriate but you get the idea ;) ).

One more thing: since link tuning is disabled in AP mode (see patch "rt2x00:
Disable link tuning in AP mode") we should move the AP mode check into the
periodic function mode instead of having it refuse to start the periodic work
in rt2x00link_start_tuner (oh, and maybe rt2x00link_start_tuner should be
renamed too). Because I think the watchdog functionality would be useful in
AP and AdHoc mode as well.

[...]

> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
> index a22837c..a630a26 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -292,6 +292,37 @@ void rt2x00usb_kill_tx_queue(struct rt2x00_dev *rt2x00dev,
>  }
>  EXPORT_SYMBOL_GPL(rt2x00usb_kill_tx_queue);
>  
> +void rt2x00usb_watchdog(struct rt2x00_dev *rt2x00dev)
> +{
> +	struct data_queue *queue;
> +	struct queue_entry_priv_usb *entry_priv;
> +	u32 i;
> +	u32 limit;
> +
> +	tx_queue_for_each(rt2x00dev, queue) {
> +		if (rt2x00queue_timeout(queue)) {
> +			WARNING(rt2x00dev, "TX queue %d timed out, invoke reset", queue->qid);

The warning is only printed when compiled with debug, right? Would it make
sense to attract more attention if the watchdog triggers? Not sure though.

Helmut




More information about the users mailing list