Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751518AbdGRQBS (ORCPT ); Tue, 18 Jul 2017 12:01:18 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:35463 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751338AbdGRQBQ (ORCPT ); Tue, 18 Jul 2017 12:01:16 -0400 Subject: Re: [PATCH net-next 11/12] net: dsa: mv88e6xxx: add Energy Detect ops To: David Miller , andrew@lunn.ch Cc: vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com References: <87bmoiuae3.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me> <20170717.134549.2101398770863549187.davem@davemloft.net> <20170717210405.GC11719@lunn.ch> <20170717.141048.999243831453421230.davem@davemloft.net> From: Florian Fainelli Message-ID: Date: Tue, 18 Jul 2017 09:01:01 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170717.141048.999243831453421230.davem@davemloft.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2189 Lines: 57 On 07/17/2017 02:10 PM, David Miller wrote: > From: Andrew Lunn > Date: Mon, 17 Jul 2017 23:04:05 +0200 > >> On Mon, Jul 17, 2017 at 01:45:49PM -0700, David Miller wrote: >>> From: Vivien Didelot >>> Date: Mon, 17 Jul 2017 15:32:52 -0400 >>> >>>> Hi Andrew, >>>> >>>> Andrew Lunn writes: >>>> >>>>> I never liked this. I think it is architecturally wrong for the switch >>>>> to be poking around in the PHY. It should ask the PHY driver. This is >>>>> especially true for external PHYs which might not be a Marvell PHY. >>>> >>>> I share the same concern. However this patch is just isolating the >>>> existing code so that we get rid of the last caps and flags and stop >>>> writing (without reading them first) arbitrary registers. >>>> >>>> Once this portion is moved to the PHY driver, one can remove it from >>>> mv88e6xxx. >>> >>> Seems a reasonable plan of action. >>> >>> Andrew, do you agree? >> >> Hi David >> >> I just fear it will not get fixed, just put into a corner to >> fester. Having to fix it properly before these patches are merged >> provides some incentive. > > If Vivien doesn't make good on his promises to do so, tell me and > I will revert all of these changes. > > Ok? This seems to be completely unfair to Vivien, there is nothing wrong with his patch series per-se other than he was unfortunate enough he highlighted something that needs fixing. This was not a serious enough problem before and it cannot possibly be one now either with just a code move. On a general note, we cannot have whoever was the last one to touch a piece of code that makes us see that this or that said piece of code is less than ideal be selected as the random victim for doing that cleanup, this just does not work. I know this is standard practice in Linux and other open source software (been there before with the USB maintainers), but this creates only one thing: making you want to runaway and scream lalalalala. So let's be pragmatic and maintain a public TODO list for this driver that people can pick items to fix/cleanup/change that have been identified as candidates for patches. -- Florian