"Bob Copeland" <me@xxxxxxxxxxxxxxx> wrote:
> On Thu, 2 Oct 2008 14:37:17 -0400, Bob Copeland wrote
>> On Thu, Oct 2, 2008 at 12:31 PM, Elias Oltmanns <eo@xxxxxxxxxxxxxx> wrote:
>
>> > Sorry, but I don't think this is safe. Checking and restoring the
>> > started flag has to be protected too, otherwise there can be races
>> > against ->stop().
>>
>> Yes, it's a bit of a mess. Even if it were serialized, a ->stop()
>> happening between suspend's call to hw_stop and actually powering down
>> the device would clear the flag. Ho hum.
>
> Okay, as usual I'm wrong here; it will clear the flag but we don't care
> since then we just wouldn't power up on resume.
>
> Since no one else chimed in, here's take two with more chewing gum and
> baling wire to fix the suspend/stop race.
>
> ---
> drivers/net/wireless/ath5k/base.c | 24 +++++++++++++++++-------
> drivers/net/wireless/ath5k/base.h | 3 ++-
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index e09ed2c..7e8fa2e 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
[...]
> @@ -666,9 +666,12 @@ ath5k_pci_resume(struct pci_dev *pdev)
> goto err_no_irq;
> }
>
> - err = ath5k_init(sc);
> - if (err)
> - goto err_irq;
> + if (test_bit(ATH_STAT_STARTED, sc->status)) {
> + err = ath5k_init(sc);
I still feel uneasy about this. Granted, I haven't thought this through
too carefully, but I'd rather not rely on the fact that ath5k_stop_hw()
will not get called between the check for ATH_STAT_STARTED and the call
to ath5k_init if I can help it. Perhaps you can add an argument `reinit'
to ath5k_init() and do something like this under the mutex:
if (reinit && !test_bit(ATH_STAT_STARTED, sc->status)) {
mutex_unlock(...);
return;
}
Regards,
Elias
--
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
