Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752070AbdLFMs6 (ORCPT ); Wed, 6 Dec 2017 07:48:58 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:46682 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751416AbdLFMs4 (ORCPT ); Wed, 6 Dec 2017 07:48:56 -0500 Date: Wed, 6 Dec 2017 15:47:31 +0300 From: Dan Carpenter To: Marcus Wolf Cc: Simon =?iso-8859-1?Q?Sandstr=F6m?= , gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux@Wolf-Entwicklungen.de, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode Message-ID: <20171206100312.zt3grfx2zlsz52kz@mwanda> References: <20171205220849.5486-1-simon@nikanor.nu> <20171205220849.5486-9-simon@nikanor.nu> <2b04702d-65f0-ab53-d28f-e20e79b6c1c1@smarthome-wolf.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2b04702d-65f0-ab53-d28f-e20e79b6c1c1@smarthome-wolf.de> User-Agent: NeoMutt/20170609 (1.8.3) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1288 Lines: 33 On Wed, Dec 06, 2017 at 11:11:27AM +0200, Marcus Wolf wrote: > > Since the rule for kernel development seems to be, not to care about future, > most probably you patch is fine, anyway. > Yeah. Deleting code if there is no user is required to move this code out of staging... I've worked in staging for a long time and I've seen patches from thousands of people. Simon really seems to understand kernel style and that's less common than one might think. It's a valuable skill. If you would just leave him to work then this driver would get fixed... You're making it very difficult because you're complaining about the work which *needs* to be done. It's discouraging for people sending patches. This is very frustrating... :( On the naming, I think having a function which is named "enable" but actually disables a feature is a non-starter. What about we do either one of these: pi433_enable_(spi); pi433_disable_(spi); Or: pi433_set_(spi, SETTING); It's still a standard and we'll just decide on a case by case basis what to use. This is a tiny driver and it's really easy to grep for the feature name to find the functions you want. Plus all the config is done in one location so it's really not that hard. regards, dan carpenter