Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   <sean.wang@mediatek.com>
To:     <lorenzo.bianconi@redhat.com>
CC:     <nbd@nbd.name>, <sean.wang@mediatek.com>,
        <Soul.Huang@mediatek.com>, <YN.Chen@mediatek.com>,
        <Leon.Yen@mediatek.com>, <Eric-SY.Chang@mediatek.com>,
        <Deren.Wu@mediatek.com>, <km.lin@mediatek.com>,
        <jenhao.yang@mediatek.com>, <robin.chiu@mediatek.com>,
        <Eddie.Chen@mediatek.com>, <ch.yeh@mediatek.com>,
        <posh.sun@mediatek.com>, <ted.huang@mediatek.com>,
        <Stella.Chang@mediatek.com>, <Tom.Chou@mediatek.com>,
        <steve.lee@mediatek.com>, <jsiuda@google.com>,
        <frankgor@google.com>, <jemele@google.com>,
        <abhishekpandit@google.com>, <shawnku@google.com>,
        <linux-wireless@vger.kernel.org>,
        <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume
Date:   Wed, 20 Jul 2022 04:44:53 +0800
Message-ID: <1658263493-14498-1-git-send-email-sean.wang@mediatek.com>
In-Reply-To: <YtXZwKyAh/MZ73rf@lore-desk--annotate>
References: <YtXZwKyAh/MZ73rf@lore-desk--annotate>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

From: Sean Wang <sean.wang@mediatek.com>

>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> It is unexpected that the reset work is running simultaneously with
>> the suspend or resume context and it is possible that reset work is

<snip>

>> @@ -363,6 +363,7 @@ static int mt7921_pci_suspend(struct device *device)
>>	int i, err;
>>
>>	pm->suspended = true;
>> +	cancel_work_sync(&dev->reset_work);
>
>should we just wait for the reset to complete here instead of cancelling? (e.g. flush_work)
>

yes, that seemed look better to me. It would wait until the queued reset request finishes.

>Regards,
>Lorenzo
>
>>	cancel_delayed_work_sync(&pm->ps_work);
>>	cancel_work_sync(&pm->wake_work);
>>
>> @@ -424,6 +425,9 @@ static int mt7921_pci_suspend(struct device
>> *device)
>>  restore_suspend:
>>	pm->suspended = false;
>>
>> +	if (err < 0)
>> +		mt7921_reset(&dev->mt76);
>> +
>>	return err;
>>  }
>>
>> @@ -437,7 +441,7 @@ static int mt7921_pci_resume(struct device
>> *device)
>>
>>	err = mt7921_mcu_drv_pmctrl(dev);
>>	if (err < 0)
>> -		return err;
>> +		goto failed;
>>
>>	mt7921_wpdma_reinit_cond(dev);
>>
>> @@ -467,11 +471,12 @@ static int mt7921_pci_resume(struct device *device)
>>		mt76_connac_mcu_set_deep_sleep(&dev->mt76, false);
>>
>>	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
>> -	if (err)
>> -		return err;
>> -
>> +failed:
>>	pm->suspended = false;
>>
>> +	if (err < 0)
>> +		mt7921_reset(&dev->mt76);
>> +
>>	return err;
>>  }
>>
>> --
>> 2.25.1
>>
>
>