* Re: [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent
[not found] <YbTU08hzeTSJPIsp@lore-desk--annotate>
@ 2021-12-11 17:14 ` sean.wang
2021-12-13 9:53 ` Lorenzo Bianconi
0 siblings, 1 reply; 5+ messages in thread
From: sean.wang @ 2021-12-11 17:14 UTC (permalink / raw)
To: lorenzo.bianconi
Cc: nbd, sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
Mark-YW.Chen, Deren.Wu, km.lin, jenhao.yang, robin.chiu,
Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, shawnku, linux-wireless, linux-mediatek
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
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent
2021-12-11 17:14 ` [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent sean.wang
@ 2021-12-13 9:53 ` Lorenzo Bianconi
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Bianconi @ 2021-12-13 9:53 UTC (permalink / raw)
To: sean.wang
Cc: lorenzo.bianconi, nbd, Soul.Huang, YN.Chen, Leon.Yen,
Eric-SY.Chang, Mark-YW.Chen, Deren.Wu, km.lin, jenhao.yang,
robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, shawnku, linux-wireless, linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]
> 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
maybe we should just set pm->suspended = false; in mt7921_mac_reset_work() as
we do for hw_full_reset, wdyt?
Regards,
Lorenzo
>
> >> +
> >> + pm->suspended = false;
> >> +
> >> + return err;
> >> }
> >>
> >> static const struct dev_pm_ops mt7921s_pm_ops = {
> >> --
> >> 2.25.1
> >>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent
[not found] <YbcYJ47OKfh0A5Vt@lore-desk--annotate>
@ 2021-12-13 18:58 ` sean.wang
0 siblings, 0 replies; 5+ messages in thread
From: sean.wang @ 2021-12-13 18:58 UTC (permalink / raw)
To: lorenzo.bianconi
Cc: nbd, sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
Mark-YW.Chen, Deren.Wu, km.lin, jenhao.yang, robin.chiu,
Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, shawnku, linux-wireless, linux-mediatek
From: Sean Wang <sean.wang@mediatek.com>
>> 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
>
>maybe we should just set pm->suspended = false; in mt7921_mac_reset_work() as we do for hw_full_reset, wdyt?
That looks fine to me. I will submit another patch for that prior to the patch.
>
>Regards,
>Lorenzo
>
>>
>> >> +
>> >> + pm->suspended = false;
>> >> +
>> >> + return err;
>> >> }
>> >>
>> >> static const struct dev_pm_ops mt7921s_pm_ops = {
>> >> --
>> >> 2.25.1
>> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent
2021-12-11 16:31 sean.wang
@ 2021-12-11 16:41 ` Lorenzo Bianconi
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Bianconi @ 2021-12-11 16:41 UTC (permalink / raw)
To: sean.wang
Cc: nbd, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Mark-YW.Chen,
Deren.Wu, km.lin, jenhao.yang, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, shawnku,
linux-wireless, linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
> 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?
> +
> + pm->suspended = false;
> +
> + return err;
> }
>
> static const struct dev_pm_ops mt7921s_pm_ops = {
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent
@ 2021-12-11 16:31 sean.wang
2021-12-11 16:41 ` Lorenzo Bianconi
0 siblings, 1 reply; 5+ messages in thread
From: sean.wang @ 2021-12-11 16:31 UTC (permalink / raw)
To: nbd, lorenzo.bianconi
Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
Mark-YW.Chen, Deren.Wu, km.lin, jenhao.yang, robin.chiu,
Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, shawnku, linux-wireless, linux-mediatek
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);
+
+ pm->suspended = false;
+
+ return err;
}
static const struct dev_pm_ops mt7921s_pm_ops = {
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-13 18:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <YbTU08hzeTSJPIsp@lore-desk--annotate>
2021-12-11 17:14 ` [PATCH 1/2] mt76: mt7921s: make pm->suspended usage consistent sean.wang
2021-12-13 9:53 ` Lorenzo Bianconi
[not found] <YbcYJ47OKfh0A5Vt@lore-desk--annotate>
2021-12-13 18:58 ` sean.wang
2021-12-11 16:31 sean.wang
2021-12-11 16:41 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).