Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'.
Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/pwm/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 9973c442b455..bca04965bfe6 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -537,7 +537,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
dev_warn(chip->dev,
- "requested disabled, but yielded enabled with duty > 0");
+ "requested disabled, but yielded enabled with duty > 0\n");
/* reapply the state that the driver reported being configured. */
err = chip->ops->apply(chip, pwm, &s1);
--
2.20.1
Hello,
On Sat, Apr 11, 2020 at 05:35:28PM +0200, Christophe JAILLET wrote:
> Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'.
>
> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/pwm/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 9973c442b455..bca04965bfe6 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -537,7 +537,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
>
> if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
> dev_warn(chip->dev,
> - "requested disabled, but yielded enabled with duty > 0");
> + "requested disabled, but yielded enabled with duty > 0\n");
Acked-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Sat, Apr 11, 2020 at 05:35:28PM +0200, Christophe JAILLET wrote:
> Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'.
>
> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/pwm/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 9973c442b455..bca04965bfe6 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -537,7 +537,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
>
> if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
> dev_warn(chip->dev,
> - "requested disabled, but yielded enabled with duty > 0");
> + "requested disabled, but yielded enabled with duty > 0\n");
>
> /* reapply the state that the driver reported being configured. */
> err = chip->ops->apply(chip, pwm, &s1);
I don't think this is strictly necessary any longer since the logging
functions are supposed to add these themselves nowadays. But I like the
consistency of this, so I'll apply this anyway.
Thanks,
Thierry
Huh...
If you look at dev_vprintk_emit(). It looks like if
create_syslog_header() returns a string then vprintk_store() will add
a newline.
I guess that means that dev_printk() can't be used to pr_cont(). And
probably that's deliberate because using pr_cont() after boot is racy
anyway.
regards,
dan carpenter
Le 14/04/2020 ? 15:58, Thierry Reding a ?crit?:
> On Sat, Apr 11, 2020 at 05:35:28PM +0200, Christophe JAILLET wrote:
>> Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'.
>>
>> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
>> Signed-off-by: Christophe JAILLET <[email protected]>
>> ---
>> drivers/pwm/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
>> index 9973c442b455..bca04965bfe6 100644
>> --- a/drivers/pwm/core.c
>> +++ b/drivers/pwm/core.c
>> @@ -537,7 +537,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
>>
>> if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
>> dev_warn(chip->dev,
>> - "requested disabled, but yielded enabled with duty > 0");
>> + "requested disabled, but yielded enabled with duty > 0\n");
>>
>> /* reapply the state that the driver reported being configured. */
>> err = chip->ops->apply(chip, pwm, &s1);
> I don't think this is strictly necessary any longer since the logging
> functions are supposed to add these themselves nowadays. But I like the
> consistency of this, so I'll apply this anyway.
>
> Thanks,
> Thierry
Hi Thierry,
I've sent more or less 10 similar patches against files updated
recently, that is to say against files which are actively maintained.
I've done it to get feedback on the acceptances rate of such proposals.
The goal is not to flood everyone with such patches, but rather to see
if adding a new kind of test to checkpatch.pl makes sense.
Being able to detect early missing trailing '\n' would help maintainers
and patch providers.
You are the 2nd person (I've added Paul Cercueil in copy of my reply)
who reports that he is thinking that it is no more required to add a '\n'.
If you have any pointer about it, it would be much appreciated.
It would mean, that either this additional checkpatch test is useless,
or maybe that it should be reversed and spot *un*needed '\n'.
CJ
On Tue, 2020-04-14 at 21:49 +0300, Dan Carpenter wrote:
> Huh...
>
> If you look at dev_vprintk_emit(). It looks like if
> create_syslog_header() returns a string then vprintk_store() will add
> a newline.
>
> I guess that means that dev_printk() can't be used to pr_cont(). And
> probably that's deliberate because using pr_cont() after boot is racy
> anyway.
Perhaps that's true, it didn't used to be.
There are couple dozen or so KERN_CONT uses after a dev_<level>
without a newline, so those _might_ be broken, but I don't thinks so.
$ git grep -P -A10 '\bdev_\w+\s*\(' | \
grep -P -B10 "KERN_CONT|pr_cont"
On Tue, 2020-04-14 at 20:30 +0200, Christophe JAILLET wrote:
> Being able to detect early missing trailing '\n' would help maintainers
> and patch providers.
>
> You are the 2nd person (I've added Paul Cercueil in copy of my reply)
> who reports that he is thinking that it is no more required to add a '\n'.
The printk subsystem will, for every printk, check
if the last printk has a newline termination and if
it doesn't and the current printk does not start with
KERN_CONT will insert a newline.
The negative to this approach is the last printk,
if it does not have a newline, is buffered and not
emitted until another printk occurs.
There is also the (now small) possibility that
multiple concurrent kernel threads or processes
could interleave printks without a terminating
newline and a different process could emit a
printk that starts with KERN_CONT and the emitted
message could be garbled.
See:
commit 4bcc595ccd80decb4245096e3d1258989c50ed41
Author: Linus Torvalds <[email protected]>
Date: Sat Oct 8 20:32:40 2016 -0700
printk: reinstate KERN_CONT for printing continuation lines