On Tue, Jan 25, 2011 at 08:50:34PM -0500, Bob Copeland wrote:
> From: Bob Copeland <me@xxxxxxxxxxxxxxx>
> Date: Mon, 24 Jan 2011 09:25:11 -0500
> Subject: [PATCH] ath5k: fix error handling in ath5k_hw_dma_stop
>
> Review spotted a problem with the error handling in ath5k_hw_dma_stop:
> a successful return from ath5k_hw_stop_tx_dma will be treated as
> an error, so we always bail out of the loop after processing a single
> active queue. As a result, we may not actually stop some queues during
> reset.
>
> Signed-off-by: Bob Copeland <me@xxxxxxxxxxxxxxx>
> ---
>
> > Patch is good, but does not make code fully correct. When last queue
> > is inactive, we return -EINVAL from ath5_hw_dma_stop(). So we need
> > also:
> >
> > if (err && err != -EINVAL)
> > return err;
> > + err = 0;
> > }
>
> > But perhaps, would be better just return 0 from
> > ath5k_hw_stop_tx_dma() when queue is inactive.
>
> How about this version instead?
It's better :)
> diff --git a/drivers/net/wireless/ath/ath5k/dma.c b/drivers/net/wireless/ath/ath5k/dma.c
> index 0064be7..21091c2 100644
> --- a/drivers/net/wireless/ath/ath5k/dma.c
> +++ b/drivers/net/wireless/ath/ath5k/dma.c
> @@ -838,9 +838,9 @@ int ath5k_hw_dma_stop(struct ath5k_hw *ah)
> for (i = 0; i < qmax; i++) {
> err = ath5k_hw_stop_tx_dma(ah, i);
> /* -EINVAL -> queue inactive */
> - if (err != -EINVAL)
> + if (err && err != -EINVAL)
> return err;
> }
>
> - return err;
> + return 0;
> }
Reviewed-by: Stanislaw Gruszka <sgruszka@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
