[PATCH 1/1] clk: sifive: append missing \n to messages

[PATCH 1/1] clk: sifive: append missing \n to messages

[Heinrich Schuchardt]

If multiple messages are written, line-feeds improve the readability.

Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c b/drivers/clk/analogbits/wrpll-cln28hpc.c
index a3cb109d357..537c696b727 100644
--- a/drivers/clk/analogbits/wrpll-cln28hpc.c
+++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
@@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long post_divr_freq)
 {
 	if (post_divr_freq < MIN_POST_DIVR_FREQ ||
 	    post_divr_freq > MAX_POST_DIVR_FREQ) {
-		WARN(1, "%s: post-divider reference freq out of range: %lu",
+		WARN(1, "%s: post-divider reference freq out of range: %lu\n",
 		     __func__, post_divr_freq);
 		return -ERANGE;
 	}
@@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 target_rate,
 	int range;
 
 	if (c->flags == 0) {
-		WARN(1, "%s called with uninitialized PLL config", __func__);
+		WARN(1, "%s called with uninitialized PLL config\n", __func__);
 		return -EINVAL;
 	}
 
@@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct wrpll_cfg *c,
 	u64 n;
 
 	if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {
-		WARN(1, "external feedback mode not yet supported");
+		WARN(1, "external feedback mode not yet supported\n");
 		return ULONG_MAX;
 	}
 
-- 
2.43.0

Re: [PATCH 1/1] clk: sifive: append missing \n to messages

[Sean Anderson]

On 2/16/24 11:35, Heinrich Schuchardt wrote:
> If multiple messages are written, line-feeds improve the readability.
> 
> Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>   drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c b/drivers/clk/analogbits/wrpll-cln28hpc.c
> index a3cb109d357..537c696b727 100644
> --- a/drivers/clk/analogbits/wrpll-cln28hpc.c
> +++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
> @@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long post_divr_freq)
>   {
>   	if (post_divr_freq < MIN_POST_DIVR_FREQ ||
>   	    post_divr_freq > MAX_POST_DIVR_FREQ) {
> -		WARN(1, "%s: post-divider reference freq out of range: %lu",
> +		WARN(1, "%s: post-divider reference freq out of range: %lu\n",
>   		     __func__, post_divr_freq);
>   		return -ERANGE;
>   	}
> @@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 target_rate,
>   	int range;
>   
>   	if (c->flags == 0) {
> -		WARN(1, "%s called with uninitialized PLL config", __func__);
> +		WARN(1, "%s called with uninitialized PLL config\n", __func__);
>   		return -EINVAL;
>   	}
>   
> @@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct wrpll_cfg *c,
>   	u64 n;
>   
>   	if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {
> -		WARN(1, "external feedback mode not yet supported");
> +		WARN(1, "external feedback mode not yet supported\n");
>   		return ULONG_MAX;
>   	}
>   

Reviewed-by: Sean Anderson <seanga2@gmail.com>

But maybe these should be dev_dbg?

Re: [PATCH 1/1] clk: sifive: append missing \n to messages

[Heinrich Schuchardt]

On 11.04.24 05:13, Sean Anderson wrote:
> On 2/16/24 11:35, Heinrich Schuchardt wrote:
>> If multiple messages are written, line-feeds improve the readability.
>>
>> Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c 
>> b/drivers/clk/analogbits/wrpll-cln28hpc.c
>> index a3cb109d357..537c696b727 100644
>> --- a/drivers/clk/analogbits/wrpll-cln28hpc.c
>> +++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
>> @@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long 
>> post_divr_freq)
>>   {
>>       if (post_divr_freq < MIN_POST_DIVR_FREQ ||
>>           post_divr_freq > MAX_POST_DIVR_FREQ) {
>> -        WARN(1, "%s: post-divider reference freq out of range: %lu",
>> +        WARN(1, "%s: post-divider reference freq out of range: %lu\n",
>>                __func__, post_divr_freq);
>>           return -ERANGE;
>>       }
>> @@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, 
>> u32 target_rate,
>>       int range;
>>       if (c->flags == 0) {
>> -        WARN(1, "%s called with uninitialized PLL config", __func__);
>> +        WARN(1, "%s called with uninitialized PLL config\n", __func__);
>>           return -EINVAL;
>>       }
>> @@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct 
>> wrpll_cfg *c,
>>       u64 n;
>>       if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {
>> -        WARN(1, "external feedback mode not yet supported");
>> +        WARN(1, "external feedback mode not yet supported\n");
>>           return ULONG_MAX;
>>       }
> 
> Reviewed-by: Sean Anderson <seanga2@gmail.com>
> 
> But maybe these should be dev_dbg?

The messages look like indicating misconfiguration. So the user should 
see these.

The kernel code also uses WARN() (and also lacks the line-feeds).

CCing Paul as the Linux maintainer and author.

Best regards

Heinrich

Re: [PATCH 1/1] clk: sifive: append missing \n to messages

[Sean Anderson]

On 4/11/24 04:37, Heinrich Schuchardt wrote:
> On 11.04.24 05:13, Sean Anderson wrote:
>> On 2/16/24 11:35, Heinrich Schuchardt wrote:
>>> If multiple messages are written, line-feeds improve the readability.
>>>
>>> Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> ---
>>>   drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c b/drivers/clk/analogbits/wrpll-cln28hpc.c
>>> index a3cb109d357..537c696b727 100644
>>> --- a/drivers/clk/analogbits/wrpll-cln28hpc.c
>>> +++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
>>> @@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long post_divr_freq)
>>>   {
>>>       if (post_divr_freq < MIN_POST_DIVR_FREQ ||
>>>           post_divr_freq > MAX_POST_DIVR_FREQ) {
>>> -        WARN(1, "%s: post-divider reference freq out of range: %lu",
>>> +        WARN(1, "%s: post-divider reference freq out of range: %lu\n",
>>>                __func__, post_divr_freq);
>>>           return -ERANGE;
>>>       }
>>> @@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 target_rate,
>>>       int range;
>>>       if (c->flags == 0) {
>>> -        WARN(1, "%s called with uninitialized PLL config", __func__);
>>> +        WARN(1, "%s called with uninitialized PLL config\n", __func__);
>>>           return -EINVAL;
>>>       }
>>> @@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct wrpll_cfg *c,
>>>       u64 n;
>>>       if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {
>>> -        WARN(1, "external feedback mode not yet supported");
>>> +        WARN(1, "external feedback mode not yet supported\n");
>>>           return ULONG_MAX;
>>>       }
>>
>> Reviewed-by: Sean Anderson <seanga2@gmail.com>
>>
>> But maybe these should be dev_dbg?
> 
> The messages look like indicating misconfiguration. So the user should see these.

Yeah, but we already return an error. So the user will notice.

> The kernel code also uses WARN() (and also lacks the line-feeds).

In the kernel there are no size restrictions, so it is fine to include messages.

--Sean