[PATCH] common: Increase LOGLEVEL back to 6

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] common: Increase LOGLEVEL back to 6

Stefan Roese
Commit 6a3e65dea3e5 ("common: Drop LOGLEVEL to 4") reduced the loglevel
resulting in pr_warn() messages fro e.g. UBI are not shown anymore. With
this change back to 6, messages like this appear again:

nand: attempt to erase a bad/reserved block @6000000

Signed-off-by: Stefan Roese <[hidden email]>
Cc: Tom Rini <[hidden email]>
---
 common/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/Kconfig b/common/Kconfig
index 4d7215a360..678a108508 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -266,7 +266,7 @@ config IDENT_STRING
 
 config LOGLEVEL
  int "loglevel"
- default 4
+ default 6
  range 0 8
  help
   All Messages with a loglevel smaller than the console loglevel will
--
2.18.0

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Tom Rini-4
On Mon, Aug 06, 2018 at 05:13:44PM +0200, Stefan Roese wrote:

> Commit 6a3e65dea3e5 ("common: Drop LOGLEVEL to 4") reduced the loglevel
> resulting in pr_warn() messages fro e.g. UBI are not shown anymore. With
> this change back to 6, messages like this appear again:
>
> nand: attempt to erase a bad/reserved block @6000000
>
> Signed-off-by: Stefan Roese <[hidden email]>
> Cc: Tom Rini <[hidden email]>
> ---
>  common/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 4d7215a360..678a108508 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -266,7 +266,7 @@ config IDENT_STRING
>  
>  config LOGLEVEL
>   int "loglevel"
> - default 4
> + default 6
>   range 0 8
>   help
>    All Messages with a loglevel smaller than the console loglevel will
This is a huge size increase on boards that rely on the default.  And I
bet this causes some boards to no longer link.  That said, if boards
want more logging messages, such as warnings, they can increase it on
their own.  If you feel that whole SoCs would benefit from this, I have
no objection to default 6 if ... being added.  And if the example
message you gave is more than just a warning and should be seen more
often, perhaps it should be a higher level message?  Thanks!

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Stefan Roese
On 06.08.2018 20:52, Tom Rini wrote:

> On Mon, Aug 06, 2018 at 05:13:44PM +0200, Stefan Roese wrote:
>
>> Commit 6a3e65dea3e5 ("common: Drop LOGLEVEL to 4") reduced the loglevel
>> resulting in pr_warn() messages fro e.g. UBI are not shown anymore. With
>> this change back to 6, messages like this appear again:
>>
>> nand: attempt to erase a bad/reserved block @6000000
>>
>> Signed-off-by: Stefan Roese <[hidden email]>
>> Cc: Tom Rini <[hidden email]>
>> ---
>>   common/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 4d7215a360..678a108508 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -266,7 +266,7 @@ config IDENT_STRING
>>  
>>   config LOGLEVEL
>>   int "loglevel"
>> - default 4
>> + default 6
>>   range 0 8
>>   help
>>    All Messages with a loglevel smaller than the console loglevel will
>
> This is a huge size increase on boards that rely on the default.  And I
> bet this causes some boards to no longer link.  That said, if boards
> want more logging messages, such as warnings, they can increase it on
> their own.  If you feel that whole SoCs would benefit from this, I have
> no objection to default 6 if ... being added.  And if the example
> message you gave is more than just a warning and should be seen more
> often, perhaps it should be a higher level message?  Thanks!

Its a "warn" and this should be visible to the user IMHO. One might
discuss, if an "info" or "notice" text should printed, but warnings
seem quite important (at least for my taste). Changing this to even
higher level messages (like "err") is too much. And this would lead
to differences between Linux and U-Boot which we should avoid.

Back to the issue, if you don't want to pull this change for the
default LOGLEVEL, I will change it for this board or platform as you
suggested above. Just let me know.

Thanks,
Stefan
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Tom Rini-4
On Tue, Aug 07, 2018 at 12:18:04PM +0200, Stefan Roese wrote:

> On 06.08.2018 20:52, Tom Rini wrote:
> >On Mon, Aug 06, 2018 at 05:13:44PM +0200, Stefan Roese wrote:
> >
> >>Commit 6a3e65dea3e5 ("common: Drop LOGLEVEL to 4") reduced the loglevel
> >>resulting in pr_warn() messages fro e.g. UBI are not shown anymore. With
> >>this change back to 6, messages like this appear again:
> >>
> >>nand: attempt to erase a bad/reserved block @6000000
> >>
> >>Signed-off-by: Stefan Roese <[hidden email]>
> >>Cc: Tom Rini <[hidden email]>
> >>---
> >>  common/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/common/Kconfig b/common/Kconfig
> >>index 4d7215a360..678a108508 100644
> >>--- a/common/Kconfig
> >>+++ b/common/Kconfig
> >>@@ -266,7 +266,7 @@ config IDENT_STRING
> >>  config LOGLEVEL
> >>   int "loglevel"
> >>- default 4
> >>+ default 6
> >>   range 0 8
> >>   help
> >>    All Messages with a loglevel smaller than the console loglevel will
> >
> >This is a huge size increase on boards that rely on the default.  And I
> >bet this causes some boards to no longer link.  That said, if boards
> >want more logging messages, such as warnings, they can increase it on
> >their own.  If you feel that whole SoCs would benefit from this, I have
> >no objection to default 6 if ... being added.  And if the example
> >message you gave is more than just a warning and should be seen more
> >often, perhaps it should be a higher level message?  Thanks!
>
> Its a "warn" and this should be visible to the user IMHO. One might
> discuss, if an "info" or "notice" text should printed, but warnings
> seem quite important (at least for my taste). Changing this to even
> higher level messages (like "err") is too much. And this would lead
> to differences between Linux and U-Boot which we should avoid.
>
> Back to the issue, if you don't want to pull this change for the
> default LOGLEVEL, I will change it for this board or platform as you
> suggested above. Just let me know.
Please bump it per board/SoC, thanks!

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Wolfgang Denk
Dear Tom,

In message <20180807111435.GD29229@bill-the-cat> you wrote:
>
> > Its a "warn" and this should be visible to the user IMHO. One might
> > discuss, if an "info" or "notice" text should printed, but warnings
> > seem quite important (at least for my taste). Changing this to even
...
> Please bump it per board/SoC, thanks!

So we stop printing warnings now, just to satisfy code size
limitations on a few boards?

This is a totally wrong approach!  If code size is a problem, these
boards should disable unneeded or at least non-essential features,
but you don;t want to ignore warnings, especially if these cause a
command to not perform the expected operation.

Please reconsider!

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [hidden email]
A positive attitude may not solve all  your  problems,  but  it  will
annoy enough people to make it worth the effort.      - Herm Albright
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Tom Rini-4
On Tue, Aug 07, 2018 at 01:37:34PM +0200, Wolfgang Denk wrote:

> Dear Tom,
>
> In message <20180807111435.GD29229@bill-the-cat> you wrote:
> >
> > > Its a "warn" and this should be visible to the user IMHO. One might
> > > discuss, if an "info" or "notice" text should printed, but warnings
> > > seem quite important (at least for my taste). Changing this to even
> ...
> > Please bump it per board/SoC, thanks!
>
> So we stop printing warnings now, just to satisfy code size
> limitations on a few boards?
>
> This is a totally wrong approach!  If code size is a problem, these
> boards should disable unneeded or at least non-essential features,
> but you don;t want to ignore warnings, especially if these cause a
> command to not perform the expected operation.
Yes, by default "warnings", which means pr_warn, along with pr_notice
and pr_info get optimized away.  In no case should a warning be printed
in a case where we aren't otherwise recovering and working correctly.  I
think there's a strong case here that the root problem here is that the
prints in question ought to be pr_err and that should be corrected in
the kernel too.

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] common: Increase LOGLEVEL back to 6

Tom Rini-4
On Tue, Aug 07, 2018 at 09:17:34AM -0400, Tom Rini wrote:

> On Tue, Aug 07, 2018 at 01:37:34PM +0200, Wolfgang Denk wrote:
> > Dear Tom,
> >
> > In message <20180807111435.GD29229@bill-the-cat> you wrote:
> > >
> > > > Its a "warn" and this should be visible to the user IMHO. One might
> > > > discuss, if an "info" or "notice" text should printed, but warnings
> > > > seem quite important (at least for my taste). Changing this to even
> > ...
> > > Please bump it per board/SoC, thanks!
> >
> > So we stop printing warnings now, just to satisfy code size
> > limitations on a few boards?
> >
> > This is a totally wrong approach!  If code size is a problem, these
> > boards should disable unneeded or at least non-essential features,
> > but you don;t want to ignore warnings, especially if these cause a
> > command to not perform the expected operation.
>
> Yes, by default "warnings", which means pr_warn, along with pr_notice
> and pr_info get optimized away.  In no case should a warning be printed
> in a case where we aren't otherwise recovering and working correctly.  I
> think there's a strong case here that the root problem here is that the
> prints in question ought to be pr_err and that should be corrected in
> the kernel too.
That said, I am not strictly against increasing the default, slightly,
to 5 (which means 4 and lower included).  The troublesome boards already
set things to 3 in their configs and we can make SPL_LOGLEVEL be untied
from LOGLEVEL.  But I want someone to show me the before/after size
deltas on that too.

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment