[PATCH] Revert "mt76: mt7921: enable aspm by default"

Philippe Schenker posted 1 patch 2 years, 5 months ago
drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
[PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Philippe Schenker 2 years, 5 months ago
This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.

This commit introduces a regression on some systems where the kernel is
crashing in different locations after a reboot was issued.

This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest firmware.

Link: https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/
Signed-off-by: Philippe Schenker <dev@pschenker.ch>
---

 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index 1a01d025bbe5..59f9ee089389 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -20,10 +20,6 @@ static const struct pci_device_id mt7921_pci_device_table[] = {
 	{ },
 };
 
-static bool mt7921_disable_aspm;
-module_param_named(disable_aspm, mt7921_disable_aspm, bool, 0644);
-MODULE_PARM_DESC(disable_aspm, "disable PCI ASPM support");
-
 static void
 mt7921_rx_poll_complete(struct mt76_dev *mdev, enum mt76_rxq_id q)
 {
@@ -280,8 +276,7 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_free_pci_vec;
 
-	if (mt7921_disable_aspm)
-		mt76_pci_disable_aspm(pdev);
+	mt76_pci_disable_aspm(pdev);
 
 	mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), &mt7921_ops,
 				 &drv_ops);
-- 
2.35.1
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Kalle Valo 2 years, 5 months ago
Philippe Schenker <dev@pschenker.ch> writes:

> This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>
> This commit introduces a regression on some systems where the kernel is
> crashing in different locations after a reboot was issued.
>
> This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest firmware.
>
> Link: https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/
> Signed-off-by: Philippe Schenker <dev@pschenker.ch>

Can I take this to wireless tree? Felix, ack?

I'll also add:

Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Deren Wu 2 years, 5 months ago
On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
> Philippe Schenker <dev@pschenker.ch> writes:
> 
> > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
> > 
> > This commit introduces a regression on some systems where the
> > kernel is
> > crashing in different locations after a reboot was issued.
> > 
> > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest
> > firmware.
> > 
> > Link: 
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
> >  
> > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> 
> Can I take this to wireless tree? Felix, ack?
> 
> I'll also add:
> 
> Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
> 

Hi Kalle,

We have a patch for a similar problem. Can you wait for the
verification by Philippe?
Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure after
reboot")
Link: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380

I can reproduce the problem in my v5.16-rc5 desktop. And the issue can
be fixed when the patch applied.


Hi Philippe,

Can you please help to check the patch in your platform?


Regards,
Deren
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Philippe Schenker 2 years, 3 months ago
On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
> On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
> > Philippe Schenker <dev@pschenker.ch> writes:
> > 
> > > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
> > > 
> > > This commit introduces a regression on some systems where the
> > > kernel is
> > > crashing in different locations after a reboot was issued.
> > > 
> > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest
> > > firmware.
> > > 
> > > Link: 
> > > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
> > >  
> > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> > 
> > Can I take this to wireless tree? Felix, ack?
> > 
> > I'll also add:
> > 
> > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
> > 
> 
> Hi Kalle,
> 
> We have a patch for a similar problem. Can you wait for the
> verification by Philippe?
> Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure
> after
> reboot")
> Link: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380
> 
> I can reproduce the problem in my v5.16-rc5 desktop. And the issue can
> be fixed when the patch applied.
> 
> 
> Hi Philippe,
> 
> Can you please help to check the patch in your platform?

Hi Kalle and Deren,

I just noticed on my system and mainline v5.18 reboots do now work
however Bluetooth is no longer accessible after a reboot.

Reverting commit bf3747ae2e25dda6a9e6c464a717c66118c588c8 on top of
v5.18 solves this problem for me.

@Deren are you aware of this bug?
@Kalle Is there a bugtracker somewhere I can submit this?

Thanks,
Philippe

> 
> 
> Regards,
> Deren
> 
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Kalle Valo 2 years, 3 months ago
Philippe Schenker <dev@pschenker.ch> writes:

> On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
>> On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
>> > Philippe Schenker <dev@pschenker.ch> writes:
>> > 
>> > > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>> > > 
>> > > This commit introduces a regression on some systems where the
>> > > kernel is
>> > > crashing in different locations after a reboot was issued.
>> > > 
>> > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest
>> > > firmware.
>> > > 
>> > > Link: 
>> > > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
>> > >  
>> > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
>> > 
>> > Can I take this to wireless tree? Felix, ack?
>> > 
>> > I'll also add:
>> > 
>> > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
>> > 
>> 
>> Hi Kalle,
>> 
>> We have a patch for a similar problem. Can you wait for the
>> verification by Philippe?
>> Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure
>> after
>> reboot")
>> Link: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380
>> 
>> I can reproduce the problem in my v5.16-rc5 desktop. And the issue can
>> be fixed when the patch applied.
>> 
>> 
>> Hi Philippe,
>> 
>> Can you please help to check the patch in your platform?
>
> Hi Kalle and Deren,
>
> I just noticed on my system and mainline v5.18 reboots do now work
> however Bluetooth is no longer accessible after a reboot.
>
> Reverting commit bf3747ae2e25dda6a9e6c464a717c66118c588c8 on top of
> v5.18 solves this problem for me.
>
> @Deren are you aware of this bug?
> @Kalle Is there a bugtracker somewhere I can submit this?

For regressions the best is to submit it to the regressions list, CCed
it now.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Deren Wu 2 years, 3 months ago
On Wed, 2022-06-01 at 11:58 +0300, Kalle Valo wrote:
> Philippe Schenker <dev@pschenker.ch> writes:
> 
> > On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
> > > On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
> > > > Philippe Schenker <dev@pschenker.ch> writes:
> > > > 
> > > > > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
> > > > > 
> > > > > This commit introduces a regression on some systems where the
> > > > > kernel is
> > > > > crashing in different locations after a reboot was issued.
> > > > > 
> > > > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with
> > > > > latest
> > > > > firmware.
> > > > > 
> > > > > Link: 
> > > > > 
https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
> > > > >  
> > > > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> > > > 
> > > > Can I take this to wireless tree? Felix, ack?
> > > > 
> > > > I'll also add:
> > > > 
> > > > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
> > > > 
> > > 
> > > Hi Kalle,
> > > 
> > > We have a patch for a similar problem. Can you wait for the
> > > verification by Philippe?
> > > Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure
> > > after
> > > reboot")
> > > Link: 
> > > 
https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380__;!!CTRNKA9wMg0ARbw!zCYyDcufJ-OLqQV6leCegA5SkNOOVjAIo-jzTHTk6HUWT9Gjt-bvSz8lr81Zv95u$
> > >  
> > > 
> > > I can reproduce the problem in my v5.16-rc5 desktop. And the
> > > issue can
> > > be fixed when the patch applied.
> > > 
> > > 
> > > Hi Philippe,
> > > 
> > > Can you please help to check the patch in your platform?
> > 
> > Hi Kalle and Deren,
> > 
> > I just noticed on my system and mainline v5.18 reboots do now work
> > however Bluetooth is no longer accessible after a reboot.
> > 
> > Reverting commit bf3747ae2e25dda6a9e6c464a717c66118c588c8 on top of
> > v5.18 solves this problem for me.
> > 
> > @Deren are you aware of this bug?
> > @Kalle Is there a bugtracker somewhere I can submit this?
> 
> For regressions the best is to submit it to the regressions list,
> CCed
> it now.
> 
Hi Philippe,

Tried your test with v5.18.0 on my desktop and both wifi/bt are still
avaible after reboot. The only problem is I need to insert btusb module
by command "modprobe btusb" to make BT workable.

I will check the issue on different platforms. If there are any
finding, I will let you know.

Regards,
Deren
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Philippe Schenker 2 years, 3 months ago
On Thu, 2022-06-02 at 00:55 +0800, Deren Wu wrote:
> On Wed, 2022-06-01 at 11:58 +0300, Kalle Valo wrote:
> > Philippe Schenker <dev@pschenker.ch> writes:
> > 
> > > On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
> > > > On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
> > > > > Philippe Schenker <dev@pschenker.ch> writes:
> > > > > 
> > > > > > This reverts commit
> > > > > > bf3747ae2e25dda6a9e6c464a717c66118c588c8.
> > > > > > 
> > > > > > This commit introduces a regression on some systems where
> > > > > > the
> > > > > > kernel is
> > > > > > crashing in different locations after a reboot was issued.
> > > > > > 
> > > > > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with
> > > > > > latest
> > > > > > firmware.
> > > > > > 
> > > > > > Link: 
> > > > > > 
> https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
> > > > > >  
> > > > > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> > > > > 
> > > > > Can I take this to wireless tree? Felix, ack?
> > > > > 
> > > > > I'll also add:
> > > > > 
> > > > > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
> > > > > 
> > > > 
> > > > Hi Kalle,
> > > > 
> > > > We have a patch for a similar problem. Can you wait for the
> > > > verification by Philippe?
> > > > Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe
> > > > failure
> > > > after
> > > > reboot")
> > > > Link: 
> > > > 
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380__;!!CTRNKA9wMg0ARbw!zCYyDcufJ-OLqQV6leCegA5SkNOOVjAIo-jzTHTk6HUWT9Gjt-bvSz8lr81Zv95u$
> > > >  
> > > > 
> > > > I can reproduce the problem in my v5.16-rc5 desktop. And the
> > > > issue can
> > > > be fixed when the patch applied.
> > > > 
> > > > 
> > > > Hi Philippe,
> > > > 
> > > > Can you please help to check the patch in your platform?
> > > 
> > > Hi Kalle and Deren,
> > > 
> > > I just noticed on my system and mainline v5.18 reboots do now work
> > > however Bluetooth is no longer accessible after a reboot.
> > > 
> > > Reverting commit bf3747ae2e25dda6a9e6c464a717c66118c588c8 on top
> > > of
> > > v5.18 solves this problem for me.
> > > 
> > > @Deren are you aware of this bug?
> > > @Kalle Is there a bugtracker somewhere I can submit this?
> > 
> > For regressions the best is to submit it to the regressions list,
> > CCed
> > it now.
> > 
> Hi Philippe,
> 
> Tried your test with v5.18.0 on my desktop and both wifi/bt are still
> avaible after reboot. The only problem is I need to insert btusb
> module
> by command "modprobe btusb" to make BT workable.
> 
> I will check the issue on different platforms. If there are any
> finding, I will let you know.

Thanks for your tests, I did test again on my platform. This time with a
hand-built v5.18 straight from torvalds/linux. And I can confirm my
findings I even loaded btusb (removed and reloaded) nothing helped. I
always get the message

No default controller available

In this case I guess it could be rather a BIOS issue. In this testing
round also some USB ports did not work.

If it helps any my system is a Lenovo P14s Gen2. I believe then the
driver is good.

Regards,
Philippe

> 
> Regards,
> Deren
> 
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Philippe Schenker 2 years, 5 months ago
On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
> On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
> > Philippe Schenker <dev@pschenker.ch> writes:
> > 
> > > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
> > > 
> > > This commit introduces a regression on some systems where the
> > > kernel is
> > > crashing in different locations after a reboot was issued.
> > > 
> > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest
> > > firmware.
> > > 
> > > Link: 
> > > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
> > >  
> > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> > 
> > Can I take this to wireless tree? Felix, ack?
> > 
> > I'll also add:
> > 
> > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
> > 
> 
> Hi Kalle,
> 
> We have a patch for a similar problem. Can you wait for the
> verification by Philippe?
> Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure
> after
> reboot")
> Link: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380
> 
> I can reproduce the problem in my v5.16-rc5 desktop. And the issue can
> be fixed when the patch applied.
> 
> 
> Hi Philippe,
> 
> Can you please help to check the patch in your platform?

Aah, so I have been a bit late with my painful bisecting. Should have
checked -next before... Whatever, your patch works just fine. I cherry
picked your patch on top mainline v5.17 and it works just fine with that
one.

Thank you very much Deren!

Sorry Kalle for the overlapping revert, please do not apply it.

Best Regards,
Philippe

> 
> 
> Regards,
> Deren
> 

Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Kalle Valo 2 years, 5 months ago
Philippe Schenker <dev@pschenker.ch> writes:

> On Tue, 2022-04-12 at 19:06 +0800, Deren Wu wrote:
>> On Tue, 2022-04-12 at 12:37 +0300, Kalle Valo wrote:
>> > Philippe Schenker <dev@pschenker.ch> writes:
>> > 
>> > > This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>> > > 
>> > > This commit introduces a regression on some systems where the
>> > > kernel is
>> > > crashing in different locations after a reboot was issued.
>> > > 
>> > > This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest
>> > > firmware.
>> > > 
>> > > Link: 
>> > > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/__;!!CTRNKA9wMg0ARbw!09tjyaQlMci3fVI3yiNiDJKUW_qwNA_CbVhoAraeIX96B99Q14J4iDycWA9cq36Y$
>> > >  
>> > > Signed-off-by: Philippe Schenker <dev@pschenker.ch>
>> > 
>> > Can I take this to wireless tree? Felix, ack?
>> > 
>> > I'll also add:
>> > 
>> > Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
>> > 
>> 
>> Hi Kalle,
>> 
>> We have a patch for a similar problem. Can you wait for the
>> verification by Philippe?
>> Commit 602cc0c9618a81 ("mt76: mt7921e: fix possible probe failure
>> after
>> reboot")
>> Link: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/wireless/mediatek/mt76?id=602cc0c9618a819ab00ea3c9400742a0ca318380
>> 
>> I can reproduce the problem in my v5.16-rc5 desktop. And the issue can
>> be fixed when the patch applied.
>> 
>> 
>> Hi Philippe,
>> 
>> Can you please help to check the patch in your platform?
>
> Aah, so I have been a bit late with my painful bisecting. Should have
> checked -next before... 

Actually commit 602cc0c9618a is already in Linus' tree and it was
included in v5.18-rc1 release.

> Whatever, your patch works just fine. I cherry picked your patch on
> top mainline v5.17 and it works just fine with that one.
>
> Thank you very much Deren!

And thank you Philippe for quickly testing this!

> Sorry Kalle for the overlapping revert, please do not apply it.

Ok, I'll drop your revert.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Thorsten Leemhuis 2 years, 5 months ago
On 12.04.22 11:37, Kalle Valo wrote:
> Philippe Schenker <dev@pschenker.ch> writes:
> 
>> This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>>
>> This commit introduces a regression on some systems where the kernel is
>> crashing in different locations after a reboot was issued.
>>
>> This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest firmware.
>>
>> Link: https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/
>> Signed-off-by: Philippe Schenker <dev@pschenker.ch>
> 
> Can I take this to wireless tree? Felix, ack?
> 
> I'll also add:
> 
> Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")

Sorry, stupid questions from the regression tracker: wouldn't this cause
a regression for users of kernel versions post-bf3747ae2e25, as the
power consumption is likely to increase for them? Without having dug
into the backstory much: would disabling ASPM for this particular
machine using a quirk be the better approach? Or are we assuming a lot
of machines are affected?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Kalle Valo 2 years, 5 months ago
Thorsten Leemhuis <regressions@leemhuis.info> writes:

> On 12.04.22 11:37, Kalle Valo wrote:
>> Philippe Schenker <dev@pschenker.ch> writes:
>> 
>>> This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>>>
>>> This commit introduces a regression on some systems where the kernel is
>>> crashing in different locations after a reboot was issued.
>>>
>>> This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest firmware.
>>>
>>> Link:
>>> https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/
>>> Signed-off-by: Philippe Schenker <dev@pschenker.ch>
>> 
>> Can I take this to wireless tree? Felix, ack?
>> 
>> I'll also add:
>> 
>> Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
>
> Sorry, stupid questions from the regression tracker: wouldn't this cause
> a regression for users of kernel versions post-bf3747ae2e25, as the
> power consumption is likely to increase for them? Without having dug
> into the backstory much: would disabling ASPM for this particular
> machine using a quirk be the better approach? Or are we assuming a lot
> of machines are affected?
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.

BTW, maybe you could add that boilerplace text after P.S. into the
signature (ie. under "-- " line)? That way your mails would more
readable and make it more clear that you didn't write the boilerplate
text specifically for this mail.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Thorsten Leemhuis 2 years, 4 months ago
On 12.04.22 12:36, Kalle Valo wrote:
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> [...]
>> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
>> reports on my table. I can only look briefly into most of them and lack
>> knowledge about most of the areas they concern. I thus unfortunately
>> will sometimes get things wrong or miss something important. I hope
>> that's not the case here; if you think it is, don't hesitate to tell me
>> in a public reply, it's in everyone's interest to set the public record
>> straight.
> BTW, maybe you could add that boilerplace text after P.S. into the
> signature (ie. under "-- " line)? That way your mails would more
> readable and make it more clear that you didn't write the boilerplate
> text specifically for this mail.

Late reply:

FYI, I thought back and forth about the boilerplace text and how to
handle that when I started using it. I deliberately decided against
putting it under a "-- " line, as that wouldn't work well for some of
the mails I write -- for example those where I deliberately use
top-posting (which I hate and kinda feels wrong, but nevertheless right
at the same time) to make this as easy to grasp as possible.

After your comment I have thought about it again for a while but in the
end for now decided to mostly stick to the approach I used, but your
comment made me shorten the text a bit.

Ciao, Thorsten
Re: [PATCH] Revert "mt76: mt7921: enable aspm by default"
Posted by Kalle Valo 2 years, 5 months ago
Thorsten Leemhuis <regressions@leemhuis.info> writes:

> On 12.04.22 11:37, Kalle Valo wrote:
>> Philippe Schenker <dev@pschenker.ch> writes:
>> 
>>> This reverts commit bf3747ae2e25dda6a9e6c464a717c66118c588c8.
>>>
>>> This commit introduces a regression on some systems where the kernel is
>>> crashing in different locations after a reboot was issued.
>>>
>>> This issue was bisected on a Thinkpad P14s Gen2 (AMD) with latest firmware.
>>>
>>> Link:
>>> https://lore.kernel.org/linux-wireless/5077a953487275837e81bdf1808ded00b9676f9f.camel@pschenker.ch/
>>> Signed-off-by: Philippe Schenker <dev@pschenker.ch>
>> 
>> Can I take this to wireless tree? Felix, ack?
>> 
>> I'll also add:
>> 
>> Fixes: bf3747ae2e25 ("mt76: mt7921: enable aspm by default")
>
> Sorry, stupid questions from the regression tracker: wouldn't this cause
> a regression for users of kernel versions post-bf3747ae2e25, as the
> power consumption is likely to increase for them? Without having dug
> into the backstory much: would disabling ASPM for this particular
> machine using a quirk be the better approach? Or are we assuming a lot
> of machines are affected?

Kernel crashing is far more serious than increased power consumption. If
there's a better fix available in the next day or two of course that can
be considered. But if there's no such fix available, we have to revert
the commit.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches