Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752111AbdLBPpL (ORCPT ); Sat, 2 Dec 2017 10:45:11 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:59542 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751779AbdLBPpK (ORCPT ); Sat, 2 Dec 2017 10:45:10 -0500 Date: Sat, 2 Dec 2017 15:00:31 +0000 From: Greg KH To: Marcus Wolf Cc: dan.carpenter@oracle.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: pi433: rf69.c: Introduced define DEBUG_FUNC_ENTRY Message-ID: <20171202150031.GD1242@kroah.com> References: <1512215150-8401-1-git-send-email-linux@wolf-entwicklungen.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1512215150-8401-1-git-send-email-linux@wolf-entwicklungen.de> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1668 Lines: 49 On Sat, Dec 02, 2017 at 01:45:50PM +0200, Marcus Wolf wrote: > Since dev_dbg already depends on define DEBUG, there was no sense, to enclose > dev_dbg lines with #ifdef DEBUG. > Instead of removing #ifdef DEBUG, I introduced define DEBUG_FUNC_ENTRY. So now it is > possible to switch of the function entry debug lines even while debug is switched on. Ick ick ick. No, these lines should just all be deleted. Use ftrace if you want to see what functions are being called, that's what it is there for. Don't create driver-specific defines and functionality for when we already have that functionality for the whole of the kernel. That's really redundant and messy. > Signed-off-by: Marcus Wolf > --- > drivers/staging/pi433/rf69.c | 118 +++++++++++++++++++++--------------------- > 1 file changed, 58 insertions(+), 60 deletions(-) > > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c > index 12c9df9..0df084e 100644 > --- a/drivers/staging/pi433/rf69.c > +++ b/drivers/staging/pi433/rf69.c > @@ -15,8 +15,10 @@ > * GNU General Public License for more details. > */ > > -/* enable prosa debug info */ > +/* generic enable/disable dev_dbg */ > #undef DEBUG > +/* enable print function entry */ > +#undef DEBUG_FUNC_ENTRY > /* enable print of values on reg access */ > #undef DEBUG_VALUES > /* enable print of values on fifo access */ > @@ -40,7 +42,7 @@ > > int rf69_set_mode(struct spi_device *spi, enum mode mode) > { > - #ifdef DEBUG > + #ifdef DEBUG_FUNC_ENTRY > dev_dbg(&spi->dev, "set: mode"); > #endif So this whole #ifdef dev_dbg #endif should all be removed. thanks, greg k-h