Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 16 Nov 2017 00:33:43 +0100
From:   Francois Romieu <romieu@fr.zoreil.com>
To:     David Miller <davem@davemloft.net>
Cc:     kai.heng.feng@canonical.com, nic_swsd@realtek.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        hayeswang@realtek.com, jrg.otte@gmail.com
Subject: Re: [PATCH 1/2] r8169: reinstate ALDPS for power saving
Message-ID: <20171115233343.GA28812@electric-eye.fr.zoreil.com>
References: <20171115090019.23972-1-kai.heng.feng@canonical.com>
 <20171115.195306.1005850318419905294.davem@davemloft.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171115.195306.1005850318419905294.davem@davemloft.net>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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