[PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Heiner Kallweit]

Referenced commit missed that for chip versions 42 and 43 ASPM
remained disabled in the respective rtl_hw_start_...() routines.
This resulted in problems as described in [0].
Therefore re-instantiate the previous logic.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=217635

Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9445f04f8..2b3aa6b45 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2747,6 +2747,13 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 		return;
 
 	if (enable) {
+		/* On these chip versions ASPM can even harm
+		 * bus communication of other PCI devices.
+		 */
+		if (tp->mac_version == RTL_GIGA_MAC_VER_42 ||
+		    tp->mac_version == RTL_GIGA_MAC_VER_43)
+			return;
+
 		rtl_mod_config5(tp, 0, ASPM_en);
 		rtl_mod_config2(tp, 0, ClkReqEn);
 
-- 
2.41.0

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Linux regression tracking (Thorsten Leemhuis)]

On 13.07.23 21:46, Heiner Kallweit wrote:
> Referenced commit missed that for chip versions 42 and 43 ASPM

Thanks again for taking care of this.

> [0] https://bugzilla.kernel.org/show_bug.cgi?id=217635

That line should be a proper Link: or Closes: tag, as explained by
Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html) and thus be in the
signed-off-by area, for example like this (without the space upfront):

> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
 Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]

A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
ensure the backport there).

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> […]

#regzbot ^backmonitor: https://bugzilla.kernel.org/show_bug.cgi?id=217635

Ciao, Thorsten

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Heiner Kallweit]

On 14.07.2023 05:31, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 13.07.23 21:46, Heiner Kallweit wrote:
>> Referenced commit missed that for chip versions 42 and 43 ASPM
> 
> Thanks again for taking care of this.
> 
>> [0] https://bugzilla.kernel.org/show_bug.cgi?id=217635
> 
> That line should be a proper Link: or Closes: tag, as explained by
> Documentation/process/submitting-patches.rst

OK

> (http://docs.kernel.org/process/submitting-patches.html) and
> Documentation/process/5.Posting.rst
> (https://docs.kernel.org/process/5.Posting.html) and thus be in the
> signed-off-by area, for example like this (without the space upfront):
> 
>> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
>  Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]
> 
> A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
> 6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
> ensure the backport there).
> 

That's different in the net subsystem. The net (vs. net-next) annotation
ensures the backport.

>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> […]
> 
> #regzbot ^backmonitor: https://bugzilla.kernel.org/show_bug.cgi?id=217635
> 
> Ciao, Thorsten

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Linux regression tracking (Thorsten Leemhuis)]

On 14.07.23 07:34, Heiner Kallweit wrote:
> On 14.07.2023 05:31, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 13.07.23 21:46, Heiner Kallweit wrote:
>
>>> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
>>  Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]
>> A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
>> 6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
>> ensure the backport there).
> That's different in the net subsystem. The net (vs. net-next) annotation
> ensures the backport.

Huh, how does that work? I thought "net" currently means "for 6.5" while
"net-next" implies 6.6?

Ciao, Thorsten

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Heiner Kallweit]

On 14.07.2023 08:30, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 14.07.23 07:34, Heiner Kallweit wrote:
>> On 14.07.2023 05:31, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 13.07.23 21:46, Heiner Kallweit wrote:
>>
>>>> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
>>>  Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]
>>> A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
>>> 6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
>>> ensure the backport there).
>> That's different in the net subsystem. The net (vs. net-next) annotation
>> ensures the backport.
> 
> Huh, how does that work? I thought "net" currently means "for 6.5" while
> "net-next" implies 6.6?
> 

https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt

See question:
I see a network patch and I think it should be backported to stable.
Should I request it via "stable@vger.kernel.org" like the references in
the kernel's Documentation/process/stable-kernel-rules.rst file say?

> Ciao, Thorsten

Heiner

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Linux regression tracking (Thorsten Leemhuis)]

[ccing greg]

On 14.07.23 08:34, Heiner Kallweit wrote:
> On 14.07.2023 08:30, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 14.07.23 07:34, Heiner Kallweit wrote:
>>> On 14.07.2023 05:31, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>> On 13.07.23 21:46, Heiner Kallweit wrote:
>>>
>>>>> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
>>>>  Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]
>>>> A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
>>>> 6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
>>>> ensure the backport there).
>>> That's different in the net subsystem. The net (vs. net-next) annotation
>>> ensures the backport.
>>
>> Huh, how does that work? I thought "net" currently means "for 6.5" while
>> "net-next" implies 6.6?
> 
> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
> 
> See question:
> I see a network patch and I think it should be backported to stable.
> Should I request it via "stable@vger.kernel.org" like the references in
> the kernel's Documentation/process/stable-kernel-rules.rst file say?

Ahh, thx. Hmm, the patchworks queue mentioned a bit higher (
http://patchwork.ozlabs.org/bundle/davem/stable/?state=* ) seems to be
mostly unused since Oct 2020. Is there a newer one?

Also a "git log --oneline --no-merges v6.0.. --grep 'stable@vger'
drivers/net/ net/  | wc -l" returns "196", but I assume all those where
mistakes?

Ciao, Thorsten

Re: [PATCH net] r8169: fix ASPM-related problem for chip version 42 and 43

[Linux regression tracking (Thorsten Leemhuis)]

[Short version: sorry for the noise, a stale file sent us sideways.]

On 14.07.23 08:58, Linux regression tracking (Thorsten Leemhuis) wrote:
> [ccing greg]
> 
> On 14.07.23 08:34, Heiner Kallweit wrote:
>> On 14.07.2023 08:30, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 14.07.23 07:34, Heiner Kallweit wrote:
>>>> On 14.07.2023 05:31, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>>> On 13.07.23 21:46, Heiner Kallweit wrote:
>>>>
>>>>>> Fixes: 5fc3f6c90cca ("r8169: consolidate disabling ASPM before EPHY access")
>>>>>  Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217635 # [0]
>>>>> A "Cc: stable@vger.kernel.org" would be nice, too, to get this fixed in
>>>>> 6.4, where this surfaced (reminder: no, a Fixes: tag is not enough to
>>>>> ensure the backport there).
>>>> That's different in the net subsystem. The net (vs. net-next) annotation
>>>> ensures the backport.
>>>
>>> Huh, how does that work? I thought "net" currently means "for 6.5" while
>>> "net-next" implies 6.6?
>>
>> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt

I just learned on social media that's a stale file that doesn't exist
anymore in mainline (I'll talk to Konstantin, maybe he can remove it to
avoid similar problems in the future). That document was converted to
rst and later...

>> See question:
>> I see a network patch and I think it should be backported to stable.
>> Should I request it via "stable@vger.kernel.org" like the references in
>> the kernel's Documentation/process/stable-kernel-rules.rst file say?

...this section was actually removed in dbbe7c962c3 ("docs: networking:
drop special stable handling") [v5.12-rc3]; later that document moved here:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

To quote:

```

1.5.7. Stable tree

While it used to be the case that netdev submissions were not supposed
to carry explicit CC: stable@vger.kernel.org tags that is no longer the
case today. Please follow the standard stable rules in
Documentation/process/stable-kernel-rules.rst, and make sure you include
appropriate Fixes tags!
```

That clears things up. Ciao, Thorsten