[U-Boot] The ctrlc() does not work when used from post_hotkeys_pressed()
Hello,
Working on the POST for mpc834x based board I encountered the following problem: The ctrlc() routine does not work when used from post_hotkeys_pressed(). The value of ctrlc_disabled variable defined as static in the console.c file is lost after code relocation.
By adding the ctrlc_disabled to global data gd structure the problem was solved.
Here is the code changes:
common/console.c | 7 +++---- include/asm-ppc/global_data.h | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/console.c b/common/console.c index dc0d13b..101c308 100644 --- a/common/console.c +++ b/common/console.c @@ -400,11 +400,10 @@ void vprintf(const char *fmt, va_list args) }
/* test if ctrl-c was pressed */ -static int ctrlc_disabled = 0; /* see disable_ctrl() */ static int ctrlc_was_pressed = 0; int ctrlc(void) { - if (!ctrlc_disabled && gd->have_console) { + if (!gd->ctrlc_disabled && gd->have_console) { if (tstc()) { switch (getc()) { case 0x03: /* ^C - Control C */ @@ -423,9 +422,9 @@ int ctrlc(void) */ int disable_ctrlc(int disable) { - int prev = ctrlc_disabled; /* save previous state */ + int prev = gd->ctrlc_disabled; /* save previous state */
- ctrlc_disabled = disable; + gd->ctrlc_disabled = disable; return prev; }
diff --git a/include/asm-ppc/global_data.h b/include/asm-ppc/global_data.h index 55e7e20..7b333ef 100644 --- a/include/asm-ppc/global_data.h +++ b/include/asm-ppc/global_data.h @@ -132,7 +132,8 @@ typedef struct global_data { #endif unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ - unsigned long have_console; /* serial_init() was called */ + unsigned char have_console; /* serial_init() was called */ + unsigned char ctrlc_disabled; /* Is the <ctrl C> enabled? */ #if defined(CONFIG_SYS_ALLOC_DPRAM) || defined(CONFIG_CPM2) unsigned int dp_alloc_base; unsigned int dp_alloc_top;
My questions are: 1) I tested this change for ppc branch only. There are a number of boards belonging to other CPU architectures that also use ctrlc() call from within the post_hotkeys_pressed(). However, I am not sure they use POST at all. I went through the list and found only one such board with CONFIG_POST defined. Does anybody has objection that this change will be expanded to other CPU architectures also?
2) I wondered why the have_console variable which gets true/false values only allocates 4 bytes in memory. So I changed it to 1 byte. Thus, adding ctrlc_disabled of one byte size also did not change the global_data structure size. Any thoughts? Should it be delivered as separate patch?
Dear Michael Zaidman,
please STOP posting bease64 encoded stuff!
Please send plain text only.
In message 660c0f821002160043g73b85a1bn3b372c9f051a2ce3@mail.gmail.com you wrote:
Working on the POST for mpc834x based board I encountered  the following problem: The ctrlc() routine does not work when used from post_hotkeys_pressed().
Hm... is this not to be expected?
As mentioned before, the POST code is supposed to be NOT interactive. In such an envrionment, handling of ^C makes no sense.
Also, lot of it is running before relocation to RAM, where features like switchable devices or interactive input is not available yet.
The value of ctrlc_disabled variable defined as static in the console.c file is lost after code relocation.
Actually you cannot modify it before relocation.
By adding the ctrlc_disabled to global data gd structure the problem was solved.
It makes no sense to add all sort of random data to gd. gd should be kept strictly minimal.
My questions are:
- I tested this change for ppc branch only. There are a number of boards
belonging to other CPU architectures that also use ctrlc() call from within the post_hotkeys_pressed(). However, I am not sure they use POST at all. I went through the list and found only one such board with CONFIG_POST defined. Does anybody has objection that this change will be expanded to other CPU architectures also?
I object against this change in general. It violates the whole philosophy of POST, which is supposed to be automatic tests.
Best regards,
Wolfgang Denk
Dear Wolfgang,
Please send plain text only.
Sorry for inconvenience but it is what I am actually trying to do from within my gmail. Very strange...
wrote:
Working on the POST for mpc834x based board I encountered the following problem: The ctrlc() routine does not work when used from post_hotkeys_pressed().
Hm... is this not to be expected?
As mentioned before, the POST code is supposed to be NOT interactive.
Then how we supposed to switch the POST_SLOWTEST mode?
Is the proprietary HW button scanning implemented in post_hotkeys_pressed() of some boards contradicting with "NOT interactive" claim?
In such an envrionment, handling of ^C makes no sense.
What about those boards that call ctrlc() from post_hotkeys_pressed()?
Also, lot of it is running before relocation to RAM, where features like switchable devices or interactive input is not available yet.
Ok, at least in the ppc branch the interactive input is available before the first POST ROM test is executed.
The value of ctrlc_disabled variable defined as static in the console.c file is lost after code relocation.
Actually you cannot modify it before relocation.
Correct, so I added it to the global data structure in such a way that its size was not affected.
By adding the ctrlc_disabled to global data gd structure the problem was solved.
It makes no sense to add all sort of random data to gd. gd should be kept strictly minimal.
Agree, the structure should be as small as possible. Please read the question #2 of my original posting where I addressed this issue.
Best regards,
Wolfgang Denk
Thanks, Michael
Dear Michael Zaidman,
In message 660c0f821002160653w6efd755du2d4e8143be53e99c@mail.gmail.com you wrote:
As mentioned before, the POST code is supposed to be NOT interactive.
Then how we supposed to switch the POST_SLOWTEST mode?
Is the proprietary HW button scanning implemented in post_hotkeys_pressed() of some boards contradicting with "NOT interactive" claim?
No, not at all. This feature performs a one-time scanning of some inputs at power on (or at reset). This can be buttons that a user presses (and holds pressed) when power cycling / resetting the device, or DIP switches, or similar.
We only record the state to switch on a specific boot mode.
We do not need any interactivity during the POST itself.
The same could be implemented for example by setting a specific envrionment variable, or similar.
In such an envrionment, handling of ^C makes no sense.
What about those boards that call ctrlc() from post_hotkeys_pressed()?
I see this in board/xes/xpedite1000/xpedite1000.c and board/sandburst/common/sb_common.c only, and I have no idea what it is supposed to mean or how it is supposed to work. You have to ask this the implementers of that specific code. I haven't seen it before.
Best regards,
Wolfgang Denk
Dear Wolfgang,
On Tue, Feb 16, 2010 at 5:24 PM, Wolfgang Denk wd@denx.de wrote:
As mentioned before, the POST code is supposed to be NOT interactive.
Then how we supposed to switch the POST_SLOWTEST mode?
Is the proprietary HW button scanning implemented in post_hotkeys_pressed() of some boards contradicting with "NOT interactive" claim?
No, not at all. This feature performs a one-time scanning of some inputs at power on (or at reset). This can be buttons that a user presses (and holds pressed) when power cycling / resetting the device, or DIP switches, or similar.
We only record the state to switch on a specific boot mode.
Sorry, but I still do not understand why it can not be ctrlc() also. It can be scanned only once in the post_hotkeys_pressed() in the same manner any other mentioned by you above input is scanned.
In my board I implemented both methods: HW botton and ctrlc() and both are working OK. Just think, the ^C add us great possibility to switch the board into SLOW POST mode without touching it physically. It can be done even remotely via support's department VPN connection to the host PC serially connected to the board as in the case of my company product. The ^C can be send by ATE (Automatic Test Equipment) station in fully automatic way and so on...
We do not need any interactivity during the POST itself.
I am not speaking about interaction during POST itself also. All I am trying to say is that we already have the console input available prior the post is running. Why do not utilize this advantage?
Regards, Michael
Dear Michael Zaidman,
In message 660c0f821002160835h3f231337i67d073e646ffe72a@mail.gmail.com you wrote:
Sorry, but I still do not understand why it can not be ctrlc() also. It can be scanned only once in the post_hotkeys_pressed() in the same manner any other mentioned by you above input is scanned.
Simply because ctrlc() assumes to have a woeking console device, and devices support depends on code which is not available before relocation to RAM.
In my board I implemented both methods: HW botton and ctrlc() and both are working OK. Just think, the ^C add us great possibility
You reported that ctrlc() was _not_ working - which is to be expected, as I already explained to you.
I am not speaking about interaction during POST itself also. All I am trying to say is that we already have the console input available prior the post is running. Why do not utilize this advantage?
You have someting that seems to work for you, but this does not mean that it works on other boards, on other processors or on other architectures.
I don't intend to dicuss this in more and more iterations. You know my opinion.
Best regards,
Wolfgang Denk
participants (2)
-
Michael Zaidman -
Wolfgang Denk