All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/4] Add ctrlc_ignore environment variable to ignore Ctrl-C
Date: Thu, 12 Jun 2014 01:36:32 -0400	[thread overview]
Message-ID: <CAPnjgZ3p20664FOc9vPaV=z3dEdv7T=mmLpwbSy8NjiFUP2A8w@mail.gmail.com> (raw)
In-Reply-To: <20140612050334.792DB380601@gemini.denx.de>

Hi Wolfgang,

On 12 June 2014 01:03, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ1HZ5Eh8b15sCgYKrmCqkbT5UBcZWPf6zDvqrebzS2N=A@mail.gmail.com> you wrote:
>>
>> > Hm... ignoring it would mean there is no way to interrupt long running
>> > commands.  I'm not sure if this is actually an improvement.
>> > Eventually we should try to define the wanted behaviour first.
>> > My initial feelingis that ^C should terminate a running command nd
>> > return us to the shell, but not terminate U-Boot.  Outside of sandbox,
>> > the only regular way to terminate U-Boot is the "reboot" command.
>> > Maybe we should do the same in sandbox?
>>
>> It is very convenient to terminate U-Boot with Ctrl-C - it makes it
>> work like a 'normal' program, and you can still terminate a
>> long-running command - it just quits U-Boot just like any other
>> command-line utility. When quickly running tests it is helpful. Also
>> it is less confusing I think for people who don't know how to exit.
>
> But that's the point: U-Boot with it's CLI is NOT "a 'normal' program".
> It's an interactive tool like a shell or an editor.  When you run a
> shell (say, bash) as CLI then you also expect that ^C will only
> terminate the currently running command, and not exit the shell.

I agree that is more correct, although I must admit the current way is
much easier to use IMO. And I run a lot of sandbox tests. But as they
say, patches welcome :-) I suppose one solution would be to introduce
a configuration file for sandbox, as with patman.

>
>> You can use '-t raw' to get the behaviour you want. Is that good enough?
>
> This should be the default, I think.
>
>> U-Boot sandbox does not yet support 'reboot', but 'reset' does quit U-Boot.
>
> Ah, yes.  Typo.  I meant "reset", of course.

OK, then this is fine.

>
>> >> I'm not sure if you recall the serial driver buffer patch I sent for
>> >
>> > I'm afraid I don't.
>>
>> Actually I think I was thinking of Scott Wood's patch:
>>
>> http://patchwork.ozlabs.org/patch/90066/
>
> Ah, this one.  Well, frankly, I don't lioke that for a number of
> reasons:
>
> - We have a ton of different UART drivers.Any such implementation
>   should be general enough to be usable on more than one type, ideally
>   completely hardware independent.

Yes, it could go at the console level.

> - This buffering of data in this patch is intended to solve a specific
>   problem that could be avoided by more clever test scripts.  Instead
>   of just dumping an endless stream of characters to the serial
>   console independent of what U-Boot is doing, one should always wit
>   for the next CLI prompt before sending the next command.  Tools like
>   "expect" can do this easily.

Agreed it could be done that way, but it is so much easier if U-Boot
can behave in a simple way with input. We may end up with more
complicated test scripts although I would prefer that we focus on unit
tests a bit more.

> - We have to decide what we want.  Either we define the serial input
>   system of U-Boot as intentionally simple, allowing it to work with
>   minimal resources (like very, very early after reset, long before
>   relocation to RAM, i. e. without much space on the stack, without
>   writable data segment, without .bss).  Or we want a feature-rich,
>   powerful input system with maximum data throuhput, buffering, type
>   ahead, line disciplines, etc.  The current implementation is clearly
>   following the former design ideas, and I think this is OK so.  The
>   second method is indeed more powerful, but quickly caklls for
>   additional complexity to implement properly - say, interrupt support
>   for the UART drivers, which means we enter a whole new leel of
>   complexity.  The current implementation is clearly following the
>   former design ideas, and I think this is OK so.  The second method
>   is indeed more powerful, but quickly caklls for additional
>   complexity to implement properly - say, interrupt support for the
>   UART drivers, which means we enter a whole new leel of complexity.

Fair enough, I don't disagree with this at all. The use case for
buffering is when you have the LCD running and it is quite slow to
print characters. You then can't type quickly at the keyboard -
U-Boot just gets behind. It happens on snow and seaboard when the
caches are off.

>
>> Yes I wanted to avoid that also. I guess we are left with signal
>> handling as the solution. But for now I might just disable Ctrl-C for
>> sandbox unless the 'raw' terminal is used. That will allow the tests
>> to work correctly at least.
>
> As mentioned, I think the default behaviour should be different.

Understood.

Regards,
Simon

  reply	other threads:[~2014-06-12  5:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-05 18:27 [U-Boot] [PATCH 0/4] Improve/fix tests for commands and trace Simon Glass
2014-06-05 18:27 ` [U-Boot] [PATCH 1/4] Reactivate the tracing feature Simon Glass
2014-06-10  4:47   ` Masahiro Yamada
2014-06-12  3:42     ` Simon Glass
2014-06-12  3:50       ` Simon Glass
2014-06-13  2:56         ` Masahiro Yamada
2014-06-14 20:34           ` Simon Glass
2014-06-13  2:56       ` Masahiro Yamada
2014-06-05 18:27 ` [U-Boot] [PATCH 2/4] Add ctrlc_ignore environment variable to ignore Ctrl-C Simon Glass
2014-06-05 21:43   ` Wolfgang Denk
2014-06-06 20:01     ` Simon Glass
2014-06-06 22:10       ` Wolfgang Denk
2014-06-08  4:01         ` Simon Glass
2014-06-10  5:08           ` Wolfgang Denk
2014-06-12  4:35             ` Simon Glass
2014-06-12  5:03               ` Wolfgang Denk
2014-06-12  5:36                 ` Simon Glass [this message]
2014-06-12  6:22                   ` Wolfgang Denk
2014-06-05 18:27 ` [U-Boot] [PATCH 3/4] test: Remove tabs from trace test Simon Glass
2014-06-05 18:27 ` [U-Boot] [PATCH 4/4] test: Add a test for command repeat Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPnjgZ3p20664FOc9vPaV=z3dEdv7T=mmLpwbSy8NjiFUP2A8w@mail.gmail.com' \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.