Messages in this thread | | | From | Simo Koskinen <> | Date | Mon, 26 Jun 2017 11:12:54 +0200 | Subject | Re: [PATCH] Staging : rts5208 : checkpatch.pl fixes |
| |
On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches <joe@perches.com> 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?
On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches <joe@perches.com> 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 >> >> 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. > >> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c > [] >> @@ -910,8 +910,12 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir) >> int retval; >> bool ddr_rx = false; >> >> - dev_dbg(rtsx_dev(chip), "sd_change_phase (sample_point = %d, tune_dir = %d)\n", >> - sample_point, tune_dir); >> + dev_dbg(rtsx_dev(chip), "%s (%s = %d, %s = %d)\n", >> + "sd_change_phase", >> + "sample_point", >> + sample_point, >> + "tune_dir", >> + tune_dir); > > etc. >
| |