On Mon, Jun 26, 2017 at 11:12 AM, Simo Koskinen <koskis...@gmail.com> wrote:
> On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches <j...@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?
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
