diff mbox series

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

Message ID 82ea9e63-d8c8-0b86-cd88-913cc249fa9a@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] r8169: fix ASPM-related problem for chip version 42 and 43 | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1341 this patch: 1341
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1364 this patch: 1364
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1364 this patch: 1364
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 13 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Heiner Kallweit July 13, 2023, 7:46 p.m. UTC
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(+)

Comments

Thorsten Leemhuis July 14, 2023, 3:31 a.m. UTC | #1
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
Heiner Kallweit July 14, 2023, 5:34 a.m. UTC | #2
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
Thorsten Leemhuis July 14, 2023, 6:30 a.m. UTC | #3
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
Heiner Kallweit July 14, 2023, 6:34 a.m. UTC | #4
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
Thorsten Leemhuis July 14, 2023, 6:58 a.m. UTC | #5
[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
Thorsten Leemhuis July 14, 2023, 8:16 a.m. UTC | #6
[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
diff mbox series

Patch

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);