[U-Boot,v3,3/3] config: Remove Blackfin CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

As Mike Frysinger writes:
  We didn't enable silent=1 by default in any of the Blackfin boards
  Just made the functionality available to people if they wanted to
  test things out with it when prototyping on dev boards.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3:
- Added part 3 of the patch to turn off the deprecated config option
for Blackfin, where it was confirmed that it's not needed.

 include/configs/bfin_adi_common.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

On Tuesday 10 January 2012 17:26:22 Doug Anderson wrote:
> As Mike Frysinger writes:
>   We didn't enable silent=1 by default in any of the Blackfin boards
>   Just made the functionality available to people if they wanted to
>   test things out with it when prototyping on dev boards.
> 
> --- a/include/configs/bfin_adi_common.h
> +++ b/include/configs/bfin_adi_common.h
> @@ -108,7 +108,6 @@
>  #define CONFIG_LOADS_ECHO	1
>  #define CONFIG_JTAG_CONSOLE
>  #define CONFIG_SILENT_CONSOLE
> -#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
>  #ifndef CONFIG_BAUDRATE
>  # define CONFIG_BAUDRATE	57600
>  #endif

not sure why you don't just squash it into 1/3 and never bother defining it ?
-mike

Mike,

On Tue, Jan 10, 2012 at 3:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> not sure why you don't just squash it into 1/3 and never bother defining it ?

I was attempting to keep changes isolated as much as possible, but
everyone seems to like to do this differently and I'm still getting a
feel for what you guys prefer.  It sounds like Wolfgang would actually
prefer all three of these squashed into a single patch, which I'll do
if there's a v4.


...I'm still awaiting Wolfgang's suggestion for which of the following
he'd like me to submit instead of this series:

1. A stripped down version of the change to use malloc with the caveat
that it's up to the user not to overrun any hardcoded limits in the
kernel (I'm pretty sure that the kernel already has code to
truncate--at least on most platforms).

2. A version of the change to add a platform-specific
COMMAND_LINE_SIZE to u-boot (keeping in sync with the kernel) with a
fix in u-boot to never buffer overrun (changing behavior to truncate).

3. Something else (?)


Thanks!

-Doug

On Tuesday 10 January 2012 18:38:07 Doug Anderson wrote:
> 3. Something else (?)

getenv returns a writable buffer, so i wonder if things couldn't be avoided 
easily all around.  after all, you're just deleting bytes here, not adding new 
ones.
-mike

On Tue, Jan 10, 2012 at 3:44 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 18:38:07 Doug Anderson wrote:
> getenv returns a writable buffer, so i wonder if things couldn't be avoided
> easily all around.  after all, you're just deleting bytes here, not adding new
> ones.

Actually, that's not quite true.  The current code will add "console="
in the case that it finds no console argument.

On Tuesday 10 January 2012 18:48:12 Doug Anderson wrote:
> On Tue, Jan 10, 2012 at 3:44 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Tuesday 10 January 2012 18:38:07 Doug Anderson wrote:
> > getenv returns a writable buffer, so i wonder if things couldn't be
> > avoided easily all around.  after all, you're just deleting bytes here,
> > not adding new ones.
> 
> Actually, that's not quite true.  The current code will add "console="
> in the case that it finds no console argument.

true ... we could still relegate the strdup/free to that step ...
-mike

Dear Doug,

In message <CAD=FV=Ui2Zww2bGT+QSjTg=K0v9BGGSAxShrTsXOQyPbA9XRSg@mail.gmail.com> you wrote:
> 
> ...I'm still awaiting Wolfgang's suggestion for which of the following
> he'd like me to submit instead of this series:
> 
> 1. A stripped down version of the change to use malloc with the caveat
> that it's up to the user not to overrun any hardcoded limits in the
> kernel (I'm pretty sure that the kernel already has code to
> truncate--at least on most platforms).
> 
> 2. A version of the change to add a platform-specific
> COMMAND_LINE_SIZE to u-boot (keeping in sync with the kernel) with a
> fix in u-boot to never buffer overrun (changing behavior to truncate).

I have to admit that I have no clear opinion here yet.

The existing code is from a time when all architectures had a pretty
low limit on the command line size - IIRC even PPC had only 256 bytes
by then, hard coded.

I think we have two options:

1) Try not to exceed any limits imposed by the Linux kernel.
   Advantage: we can catch situations where the user overflows the
           limits and print an error message (IIRC this needs to be
           added), so the user gets aware of the pronlem.
   Disadvantage: we need to add architecture specific definitions for
           the command line size, and keep these in sync with any
	   related changes to the Linux kernel.  We know in advance
	   that this will not work really well.

2) We just make sure not to overwrite array bounds in U-Boot, and
   allow passing arbitrary sized buffers to the Linux kernel.
   Advantage: the code in U-Boot can be simple and clean
   Disadvantage: the resulting behaviour is not exactly user-friendly,
           as a silent truncation in the Linux kernel is probably hard
	   to spot.

Hm...

I agree that the old code needs fixing.  I think it would be nice if
we could adapt U-Boot behaviour, but I fear that actually it cannot
work at all, as we don't know which Linux kernel version the user will
use, and what their limits might be.

So in the end 2) is probably the most sensible approach here.

Best regards,

Wolfgang Denk

Dear Wolfgang,

On Wed, Jan 11, 2012 at 12:11 AM, Wolfgang Denk <wd@denx.de> wrote:
> I have to admit that I have no clear opinion here yet.
>
> The existing code is from a time when all architectures had a pretty
> low limit on the command line size - IIRC even PPC had only 256 bytes
> by then, hard coded.
>
> I think we have two options:
>
> 1) Try not to exceed any limits imposed by the Linux kernel.
>   Advantage: we can catch situations where the user overflows the
>           limits and print an error message (IIRC this needs to be
>           added), so the user gets aware of the pronlem.
>   Disadvantage: we need to add architecture specific definitions for
>           the command line size, and keep these in sync with any
>           related changes to the Linux kernel.  We know in advance
>           that this will not work really well.
>
> 2) We just make sure not to overwrite array bounds in U-Boot, and
>   allow passing arbitrary sized buffers to the Linux kernel.
>   Advantage: the code in U-Boot can be simple and clean
>   Disadvantage: the resulting behaviour is not exactly user-friendly,
>           as a silent truncation in the Linux kernel is probably hard
>           to spot.
>
> Hm...
>
> I agree that the old code needs fixing.  I think it would be nice if
> we could adapt U-Boot behaviour, but I fear that actually it cannot
> work at all, as we don't know which Linux kernel version the user will
> use, and what their limits might be.
>
> So in the end 2) is probably the most sensible approach here.

Thank you for all of your time on this issue.  I have just sent a
patch implementing 2) with as little extra cruft as possible (I hope).
 Let me know if there is anything else you need here.  Thanks!

-Doug