Patchwork [v2] r8169: remove "PHY reset until link up" log spam

login
register
mail settings
Submitter Peter Wu
Date July 23, 2013, 12:38 p.m.
Message ID <1765554.SGJMeyexPP@al>
Download mbox | patch
Permalink /patch/261068/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Peter Wu - July 23, 2013, 12:38 p.m.
This message was added in commit a7154cb8 (June 2004, [PATCH] r8169:
link handling and phy reset rework) and is printed every ten seconds
when no cable is connected. (Before that commit, "Reset RTL8169s PHY"
would be printed instead.)

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---

On Tuesday 23 July 2013 16:22:57 Sergei Shtylyov wrote:
> On 23-07-2013 13:55, Peter Wu wrote:
> > This message was added in commit a7154cb8 (June 2004) and is printed
> 
>     Please also specify that commit's summary line in parens.
Done, while at it I have also noted that an other spam message would be
logged.
---
 drivers/net/ethernet/realtek/r8169.c | 2 --
 1 file changed, 2 deletions(-)
fran├žois romieu - July 23, 2013, 8:23 p.m.
Peter Wu <lekensteyn@gmail.com> :
> This message was added in commit a7154cb8 (June 2004, [PATCH] r8169:
> link handling and phy reset rework) and is printed every ten seconds
> when no cable is connected.
+                            and runtime power management is disabled.

It isn't completely academic: Fedora has it enabled.

You have the implicit ack of David, so please keep the facts right.
Peter Wu - July 23, 2013, 8:48 p.m.
On Tuesday 23 July 2013 22:23:12 Francois Romieu wrote:
> Peter Wu <lekensteyn@gmail.com> :
> > This message was added in commit a7154cb8 (June 2004, [PATCH] r8169:
> > link handling and phy reset rework) and is printed every ten seconds
> > when no cable is connected.
> 
> +                            and runtime power management is disabled.
> It isn't completely academic: Fedora has it enabled.
I have not really considered runtime PM before, aren't the ethtool operations 
actually broken with runtime PM enabled? Or does the ethtool core take care of 
waking devices (which I doubt)? I don't see any attempt to wake devices in the 
r8169 ethtool operations.

> You have the implicit ack of David, so please keep the facts right.
I am not trying to hide details, before I send the third revision of third 
commit message, what other details may I have overseen? If you want to, you 
can also create a patch with an appropriate commit message, I don't mind.

Regards,
Peter
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
fran├žois romieu - July 23, 2013, 9:50 p.m.
Peter Wu <lekensteyn@gmail.com> :
[...]
> I have not really considered runtime PM before, aren't the ethtool
> operations actually broken with runtime PM enabled ? Or does the ethtool
> core take care of waking devices (which I doubt) ? I don't see any attempt
> to wake devices in the r8169 ethtool operations.

ethtool core bails out.

See dev_ethtool, netif_device_present, rtl8169_net_suspend and
netif_device_detach.

[...]
> > You have the implicit ack of David, so please keep the facts right.
> I am not trying to hide details, before I send the third revision of third 
> commit message, what other details may I have overseen ?

Use netif_dbg instead of removing the message ?

> If you want to, you can also create a patch with an appropriate commit
> message, I don't mind.

[...]
- when no cable is connected.
+ when no cable is connected and runtime power management is disabled.

That's it. Your patch, your credit.

Patch

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 880015c..63f04af 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -3689,8 +3689,6 @@  static void rtl_phy_work(struct rtl8169_private *tp)
 	if (tp->link_ok(ioaddr))
 		return;
 
-	netif_warn(tp, link, tp->dev, "PHY reset until link up\n");
-
 	tp->phy_reset_enable(tp);
 
 out_mod_timer: