Re: [PATCH 1/2] r8169: reinstate ALDPS for power saving

[David Miller]

From: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
> Cc: Hayes Wang <hayeswang@xxxxxxxxxxx>
> Cc: Jörg Otte <jrg.otte@xxxxxxxxx>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>

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.