From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751465AbdFZJ35 (ORCPT ); Mon, 26 Jun 2017 05:29:57 -0400 Received: from mail-ua0-f193.google.com ([209.85.217.193]:36233 "EHLO mail-ua0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751353AbdFZJ3v (ORCPT ); Mon, 26 Jun 2017 05:29:51 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170623135548.GA21856@elsys-VirtualBox> <1498233591.24295.16.camel@perches.com> From: Frans Klaver Date: Mon, 26 Jun 2017 11:29:49 +0200 Message-ID: Subject: Re: [PATCH] Staging : rts5208 : checkpatch.pl fixes To: Simo Koskinen Cc: Joe Perches , Greg KH , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 26, 2017 at 11:12 AM, Simo Koskinen wrote: > On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches wrote: >> On Fri, 2017-06-23 at 15:55 +0200, Simo Koskinen wrote: >>> Fixed some issues reported by checkpatch.pl script. >> [] >>> diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c >>> index b8177f5..ceef5fc 100644 >>> --- a/drivers/staging/rts5208/rtsx.c >>> +++ b/drivers/staging/rts5208/rtsx.c >>> @@ -1009,7 +1009,7 @@ static void rtsx_remove(struct pci_dev *pci) >>> { >>> struct rtsx_dev *dev = pci_get_drvdata(pci); >>> >>> - dev_info(&pci->dev, "rtsx_remove() called\n"); >>> + dev_info(&pci->dev, "%s called\n", "rtsx_remove()"); >> >> This would be better as dev_dbg > True, I can change that... > > >>> >>> quiesce_and_remove_host(dev); >>> release_everything(dev); >>> diff --git a/drivers/staging/rts5208/rtsx_chip.c b/drivers/staging/rts5208/rtsx_chip.c >>> index 7f4107b..892b97a 100644 >>> --- a/drivers/staging/rts5208/rtsx_chip.c >>> +++ b/drivers/staging/rts5208/rtsx_chip.c >>> @@ -616,8 +616,10 @@ int rtsx_reset_chip(struct rtsx_chip *chip) >>> else >>> retval = rtsx_pre_handle_sdio_new(chip); >>> >>> - dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (rtsx_reset_chip)\n", >>> - (unsigned int)(chip->need_reset)); >>> + dev_dbg(rtsx_dev(chip), "%s = 0x%x (%s)\n", >>> + "chip->need_reset", >>> + (unsigned int)(chip->need_reset), >>> + "rtsx_reset_chip"); >> >> This and other changes that take part of the format >> and convert them to '"%s", substrings' are not good. >> checkpatch doesn't emit a warning for long formats. > The reason for changes were the following warning when run the > checkpatch.pl script: > > WARNING: Prefer using '"%s...", __func__' to using 'rtsx_reset_chip', > this function's name, in a string > #619: FILE: rtsx_chip.c:619: > + dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (rtsx_reset_chip)\n", > > So it's not a good idea to fix these warnings? The warning suggests you change this to dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (%s)\n", (unsigned int)(chip->need_reset), __func__); It doesn't mention the chip->need_reset part. The reason is simply that when the function name changes, this doesn't have to be updated. That same reasoning doesn't hold up for when something changes in "chip->need_reset". Frans