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>,
        <Mark-YW.Chen@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>, <Eric.Liang@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/2] mt76: mt7921s: make pm->suspended usage consistent
Date:   Sun, 12 Dec 2021 01:14:42 +0800
Message-ID: <1639242882-15796-1-git-send-email-sean.wang@mediatek.com>
In-Reply-To: <YbTU08hzeTSJPIsp@lore-desk--annotate>
References: <YbTU08hzeTSJPIsp@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>
>>
>> Update pm->suspended usage to be consistent with mt7921e driver.
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  drivers/net/wireless/mediatek/mt76/mt7921/sdio.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
>> index 84be229a899d..44ee9369f6bf 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
>> @@ -278,7 +278,6 @@ static int mt7921s_resume(struct device *__dev)
>>	struct mt76_dev *mdev = &dev->mt76;
>>	int err;
>>
>> -	pm->suspended = false;
>>	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
>>
>>	err = mt7921_mcu_drv_pmctrl(dev);
>> @@ -294,7 +293,11 @@ static int mt7921s_resume(struct device *__dev)
>>	if (!pm->ds_enable)
>>		mt76_connac_mcu_set_deep_sleep(mdev, false);
>>
>> -	return mt76_connac_mcu_set_hif_suspend(mdev, false);
>> +	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
>
>should we check return value here? Something like:
>
>	if (err)
>		return err;
>
>	pm->suspended = false;
>	return 0;
>
>Or, is the chip up even if mt76_connac_mcu_set_hif_suspend() fails?

yes, chip is eventually up again by recovered with the following wifi reset

with current logic, if do so (not mark pm->suspended back as false to show suspend/resume is over),

the pm runtime would not be enabled again after the wifi reset

>> +
>> +	pm->suspended = false;
>> +
>> +	return err;
>>  }
>>
>>  static const struct dev_pm_ops mt7921s_pm_ops = {
>> --
>> 2.25.1
>>