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

From: Francois Romieu
Date: Wed Nov 15 2017 - 18:34:03 EST


David Miller <davem@xxxxxxxxxxxxx> :
[...]
> 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