From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1D15C433E0 for ; Thu, 11 Feb 2021 14:29:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 504D664D8F for ; Thu, 11 Feb 2021 14:29:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232360AbhBKO2Q (ORCPT ); Thu, 11 Feb 2021 09:28:16 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:34938 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232218AbhBKOUL (ORCPT ); Thu, 11 Feb 2021 09:20:11 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lACoV-005aey-Jy; Thu, 11 Feb 2021 15:19:23 +0100 Date: Thu, 11 Feb 2021 15:19:23 +0100 From: Andrew Lunn To: Stefan Chulski Cc: David Miller , "netdev@vger.kernel.org" , "thomas.petazzoni@bootlin.com" , Nadav Haklai , Yan Markman , "linux-kernel@vger.kernel.org" , "kuba@kernel.org" , "linux@armlinux.org.uk" , "mw@semihalf.com" , "rmk+kernel@armlinux.org.uk" , "atenart@kernel.org" , "devicetree@vger.kernel.org" , "robh+dt@kernel.org" , "sebastian.hesselbarth@gmail.com" , "gregory.clement@bootlin.com" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [EXT] Re: [PATCH v12 net-next 12/15] net: mvpp2: add BM protection underrun feature support Message-ID: References: <1612950500-9682-1-git-send-email-stefanc@marvell.com> <1612950500-9682-13-git-send-email-stefanc@marvell.com> <20210210.152924.767175240247395907.davem@davemloft.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 11, 2021 at 08:22:19AM +0000, Stefan Chulski wrote: > > > > > ---------------------------------------------------------------------- > > From: > > Date: Wed, 10 Feb 2021 11:48:17 +0200 > > > > > > > > +static int bm_underrun_protect = 1; > > > + > > > +module_param(bm_underrun_protect, int, 0444); > > > +MODULE_PARM_DESC(bm_underrun_protect, "Set BM underrun protect > > > +feature (0-1), def=1"); > > > > No new module parameters, please. > > Ok, I would remove new module parameters. > By the way why new module parameters forbitten? Historically, module parameters are a bad interface for configuration. Vendors have stuffed all sorts of random junk into module parameters. There is little documentation. Different drivers can have similar looking module parameters which do different things. Or different module parameters, which actually do the same thing. But maybe with slightly different parameters. We get a much better overall result if you stop and think for a while. How can this be made a generic configuration knob which multiple vendors could use? And then add it to ethtool. Extend the ethtool -h text and the man page. Maybe even hack some other vendors driver to make use of it. Or we have also found out, that pushing back on parameters like this, the developers goes back and looks at the code, and sometimes figures out a way to automatically do the right thing, removing the configuration knob, and just making it all simpler for the user to use. Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A524C433E0 for ; Thu, 11 Feb 2021 14:20:56 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D709164DE2 for ; Thu, 11 Feb 2021 14:20:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D709164DE2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lunn.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Q6EgiGckJeuCmiH3vfwOKQ2RlNjenB7S2SBtYEW3Tjw=; b=OyIpX69jE3DcTPDDlnr+9dvGr MMJHEh88+80UfANN2bY9MTt3WDw2Vmd9QjHmOh5yN0brXBgbGX6jHwftCr5MsetvoC1c/PKfATHzG jvua9PCbC6A7ahYrjjyFYZjSwOsLEe62kCeQZ0YIPHRlO/txYCVurhXNAvtMMsYspyHPfpdtZUUTW nr3HmSQtz+pHhG8HlgWLLfpUxdxVgMteoQCvARnYtcYSXfbzNgpA508AV1jrDJshZ8HfVG78pmp3I teEFnSWlQ7K7KBP6cecP9I51UrIb2Zwp5by3Eymyqdh+zS2YY6RMBi9oV3f671ZhifNn8TksVvlC4 En1Tj+7Qw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lACoo-0000rz-Rq; Thu, 11 Feb 2021 14:19:42 +0000 Received: from vps0.lunn.ch ([185.16.172.187]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lACol-0000r0-2o for linux-arm-kernel@lists.infradead.org; Thu, 11 Feb 2021 14:19:40 +0000 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lACoV-005aey-Jy; Thu, 11 Feb 2021 15:19:23 +0100 Date: Thu, 11 Feb 2021 15:19:23 +0100 From: Andrew Lunn To: Stefan Chulski Subject: Re: [EXT] Re: [PATCH v12 net-next 12/15] net: mvpp2: add BM protection underrun feature support Message-ID: References: <1612950500-9682-1-git-send-email-stefanc@marvell.com> <1612950500-9682-13-git-send-email-stefanc@marvell.com> <20210210.152924.767175240247395907.davem@davemloft.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210211_091939_179661_E87B26BF X-CRM114-Status: GOOD ( 12.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , Yan Markman , "gregory.clement@bootlin.com" , "netdev@vger.kernel.org" , "atenart@kernel.org" , "linux-kernel@vger.kernel.org" , "linux@armlinux.org.uk" , Nadav Haklai , "rmk+kernel@armlinux.org.uk" , "robh+dt@kernel.org" , "thomas.petazzoni@bootlin.com" , "kuba@kernel.org" , "mw@semihalf.com" , David Miller , "linux-arm-kernel@lists.infradead.org" , "sebastian.hesselbarth@gmail.com" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Feb 11, 2021 at 08:22:19AM +0000, Stefan Chulski wrote: > > > > > ---------------------------------------------------------------------- > > From: > > Date: Wed, 10 Feb 2021 11:48:17 +0200 > > > > > > > > +static int bm_underrun_protect = 1; > > > + > > > +module_param(bm_underrun_protect, int, 0444); > > > +MODULE_PARM_DESC(bm_underrun_protect, "Set BM underrun protect > > > +feature (0-1), def=1"); > > > > No new module parameters, please. > > Ok, I would remove new module parameters. > By the way why new module parameters forbitten? Historically, module parameters are a bad interface for configuration. Vendors have stuffed all sorts of random junk into module parameters. There is little documentation. Different drivers can have similar looking module parameters which do different things. Or different module parameters, which actually do the same thing. But maybe with slightly different parameters. We get a much better overall result if you stop and think for a while. How can this be made a generic configuration knob which multiple vendors could use? And then add it to ethtool. Extend the ethtool -h text and the man page. Maybe even hack some other vendors driver to make use of it. Or we have also found out, that pushing back on parameters like this, the developers goes back and looks at the code, and sometimes figures out a way to automatically do the right thing, removing the configuration knob, and just making it all simpler for the user to use. Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel