Messages in this thread | | | Date | Wed, 15 Nov 2017 19:53:06 +0900 (KST) | Subject | Re: [PATCH 1/2] r8169: reinstate ALDPS for power saving | From | David Miller <> |
| |
From: Kai-Heng Feng <kai.heng.feng@canonical.com> Date: Wed, 15 Nov 2017 04:00:18 -0500
> Commit ("r8169: enable ALDPS for power saving") caused a regression on > RTL8168evl/8111evl [1], so it got reverted. > > Instead of reverting the whole commit, let's reinstate ALDPS for all > models other than RTL8168evl/8111evl. > > [1] https://lkml.org/lkml/2013/1/5/36 > > Cc: Francois Romieu <romieu@fr.zoreil.com> > Cc: Hayes Wang <hayeswang@realtek.com> > Cc: Jörg Otte <jrg.otte@gmail.com> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Sorry, this isn't going to work.
The problem is that we have no idea what chips this really works for. All we know is that it definitely does not work for one particular chip variant.
The amount of coverage this change is going to get is very small as well, meaning an even greater change of regressions.
Therefore the only acceptable way to handle this is to have a white-list, specific chips that have been explicitly tested and are known to work with this feature, rather than the other way around.
Furthermore, you're not even checking the chip version, you're checking instead whether the firmware is loaded or not. That doesn't seem like a safe way to guard this at all.
| |