| Message ID | ff344a31-4cb8-1e49-7b5e-3a729125444b@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] r8169: fix powering up RTL8168h | expand |
On Mon, May 7, 2018 at 7:11 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote: > Since commit a92a08499b1f "r8169: improve runtime pm in general and > suspend unused ports" interfaces w/o link are runtime-suspended after > 10s. On systems where drivers take longer to load this can lead to the > situation that the interface is runtime-suspended already when it's > initially brought up. > This shouldn't be a problem because rtl_open() resumes MAC/PHY. > However with at least one chip version the interface doesn't properly > come up, as reported here: > https://bugzilla.kernel.org/show_bug.cgi?id=199549 > > The vendor driver uses a delay to give certain chip versions some > time to resume before starting the PHY configuration. So let's do > the same. I don't know which chip versions may be affected, > therefore apply this delay always. > > This patch was reported to fix the issue for RTL8168h. > I was able to reproduce the issue on an Asus H310I-Plus which also > uses a RTL8168h. Also in my case the patch fixed the issue. > > Reported-by: Slava Kardakov <ojab@ojab.ru> > Tested-by: Slava Kardakov <ojab@ojab.ru> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Maybe also Cc: stable@vger.kernel.org # v4.16+ ? > --- > This patch will not apply to net-next as it conflicts with other > changes which have been done in the meantime. So I'll send a > separate patch for net-next. > --- > drivers/net/ethernet/realtek/r8169.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 604ae783..c7aac1fc 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -4981,6 +4981,9 @@ static void rtl_pll_power_down(struct rtl8169_private *tp) > static void rtl_pll_power_up(struct rtl8169_private *tp) > { > rtl_generic_op(tp, tp->pll_power_ops.up); > + > + /* give MAC/PHY some time to resume */ > + msleep(20); > } > > static void rtl_init_pll_power_ops(struct rtl8169_private *tp) > -- > 2.17.0 >
On Mon, May 07, 2018 at 07:20:53PM +0000, ojab // wrote: > On Mon, May 7, 2018 at 7:11 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote: > > Since commit a92a08499b1f "r8169: improve runtime pm in general and > > suspend unused ports" interfaces w/o link are runtime-suspended after > > 10s. On systems where drivers take longer to load this can lead to the > > situation that the interface is runtime-suspended already when it's > > initially brought up. > > This shouldn't be a problem because rtl_open() resumes MAC/PHY. > > However with at least one chip version the interface doesn't properly > > come up, as reported here: > > https://bugzilla.kernel.org/show_bug.cgi?id=199549 > > > > The vendor driver uses a delay to give certain chip versions some > > time to resume before starting the PHY configuration. So let's do > > the same. I don't know which chip versions may be affected, > > therefore apply this delay always. > > > > This patch was reported to fix the issue for RTL8168h. > > I was able to reproduce the issue on an Asus H310I-Plus which also > > uses a RTL8168h. Also in my case the patch fixed the issue. > > > > Reported-by: Slava Kardakov <ojab@ojab.ru> > > Tested-by: Slava Kardakov <ojab@ojab.ru> > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > Maybe also > Cc: stable@vger.kernel.org # v4.16+ No need. Heiner correctly marked this for net, not net-next, so David with do what is needed for it to goto stable. Andrew
From: Heiner Kallweit <hkallweit1@gmail.com> Date: Mon, 7 May 2018 21:11:21 +0200 > Since commit a92a08499b1f "r8169: improve runtime pm in general and > suspend unused ports" interfaces w/o link are runtime-suspended after > 10s. On systems where drivers take longer to load this can lead to the > situation that the interface is runtime-suspended already when it's > initially brought up. > This shouldn't be a problem because rtl_open() resumes MAC/PHY. > However with at least one chip version the interface doesn't properly > come up, as reported here: > https://bugzilla.kernel.org/show_bug.cgi?id=199549 > > The vendor driver uses a delay to give certain chip versions some > time to resume before starting the PHY configuration. So let's do > the same. I don't know which chip versions may be affected, > therefore apply this delay always. > > This patch was reported to fix the issue for RTL8168h. > I was able to reproduce the issue on an Asus H310I-Plus which also > uses a RTL8168h. Also in my case the patch fixed the issue. > > Reported-by: Slava Kardakov <ojab@ojab.ru> > Tested-by: Slava Kardakov <ojab@ojab.ru> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied and queued up for -stable. > This patch will not apply to net-next as it conflicts with other > changes which have been done in the meantime. So I'll send a > separate patch for net-next. That's fine, I'll deal with it when I next merge net into net-next. Sending another copy of this patch for net-next is not the way to deal with this. Just make me aware of the impending complict and I will resolve it when I see it. Thanks.
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 604ae783..c7aac1fc 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -4981,6 +4981,9 @@ static void rtl_pll_power_down(struct rtl8169_private *tp) static void rtl_pll_power_up(struct rtl8169_private *tp) { rtl_generic_op(tp, tp->pll_power_ops.up); + + /* give MAC/PHY some time to resume */ + msleep(20); } static void rtl_init_pll_power_ops(struct rtl8169_private *tp)