2017-11-16 01:02:44

by Francois Romieu

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

David Miller <[email protected]> :
[...]
> The amount of coverage this change is going to get is very small as
> well, meaning an even greater chance of regressions.

Yes.

> 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.

Actually the chip specific xyz_hw_phy_config methods call the relevant
aldps enabling helper _but_ the 8168evl dedicated xyz_hw_phy_config
doesn't. The firmware loaded check is just a distraction for the
busy reviewer.

--
Ueimor


From 1584129180826269477@xxx Wed Nov 15 10:55:13 +0000 2017
X-GM-THRID: 1584123135258654096
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread