Re: [PATCH] console: add new write-only option

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 10/13/20 10:41 AM, Ahmad Fatoum wrote:
> Hello,
> 
> On 10/13/20 9:59 AM, Rouven Czerwinski wrote:
>> Add CONFIG_CONSOLE_WRITEONLY to configure the consoles as write-only,
>> making the bootup effectively non-interactive.
>>
>> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> ---
>>  common/Kconfig          | 11 +++++++++++
>>  common/console.c        |  2 +-
>>  common/console_simple.c |  5 ++++-
>>  common/startup.c        |  5 +++++
>>  4 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 1a5bb53182..abf9b7bd73 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -775,6 +775,17 @@ config CONSOLE_ALLOW_COLOR
>>  	  compile time default for colored console output. After boot it
>>  	  can be controlled using global.allow_color.
>>  
>> +config CONSOLE_WRITEONLY
>> +	prompt "Enable writeonly consoles (non-interactive)"
>> +	depends on CONSOLE_FULL || CONSOLE_SIMPLE
>> +	bool
>> +	help
>> +	  If enabled, all consoles are configured to be write-only, meaning
>> +	  they will not read any input, making the console effectively
>> +	  non-interactive.
>> +	  CAUTION: this will also disable input devices, since they are registered as
>> +	  consoles.
> 
> Did you consider having a serialX.input device parameter instead?

Oh, we already have serialN.active, which can be set selectively
configured to enable/disable input. Would be great if this still
can be used on top of this new Kconfig option to selectively enable.

> That way you can still:
> 
> - Enable some input devices selectively (e.g. reset and power specialkeys) 
> - System still has a recovery mode that can be entered if Linux
>   writes a value to a SoC-internal register (reboot mode)
> - have non-user-facing consoles (remoteproc with co-processor in future)

For those to work, you could do

+active_flags = CONSOLE_STDOUT | CONSOLE_STDERR;
+if (!IS_ENABLED(CONFIG_CONSOLE_WRITEONLY))
+  active_flags |= CONSOLE_STDIN;

 if (activate)
-        console_set_active(newcdev, CONSOLE_STDIN |
-                        CONSOLE_STDOUT | CONSOLE_STDERR);
+        console_set_active(newcdev, flags);


Does this work for you?

> 
> Cheers
> Ahmad
> 
>> +
>>  config PBL_CONSOLE
>>  	depends on PBL_IMAGE
>>  	depends on !CONSOLE_NONE
>> diff --git a/common/console.c b/common/console.c
>> index 3375ecb7e5..fb26886f8c 100644
>> --- a/common/console.c
>> +++ b/common/console.c
>> @@ -93,7 +93,7 @@ int console_set_active(struct console_device *cdev, unsigned flag)
>>  {
>>  	int ret;
>>  
>> -	if (!cdev->getc)
>> +	if (!cdev->getc || IS_ENABLED(CONFIG_CONSOLE_WRITEONLY))
>>  		flag &= ~CONSOLE_STDIN;
>>  	if (!cdev->putc)
>>  		flag &= ~(CONSOLE_STDOUT | CONSOLE_STDERR);
>> diff --git a/common/console_simple.c b/common/console_simple.c
>> index 42224842c5..75257c7b08 100644
>> --- a/common/console_simple.c
>> +++ b/common/console_simple.c
>> @@ -92,7 +92,10 @@ int console_register(struct console_device *newcdev)
>>  		newcdev->setbrg(newcdev, newcdev->baudrate);
>>  	}
>>  
>> -	newcdev->f_active = CONSOLE_STDIN | CONSOLE_STDOUT | CONSOLE_STDERR;
>> +	if (IS_ENABLED(CONFIG_CONSOLE_WRITEONLY))
>> +		newcdev->f_active = CONSOLE_STDOUT | CONSOLE_STDERR;
>> +	else
>> +		newcdev->f_active = CONSOLE_STDIN | CONSOLE_STDOUT | CONSOLE_STDERR;
>>  
>>  	barebox_banner();
>>  
>> diff --git a/common/startup.c b/common/startup.c
>> index ea7ce6b8da..d7bf99df14 100644
>> --- a/common/startup.c
>> +++ b/common/startup.c
>> @@ -253,6 +253,11 @@ enum autoboot_state do_autoboot_countdown(void)
>>  
>>  	menu_exists = stat(MENUFILE, &s) == 0;
>>  
>> +	if (IS_ENABLED(CONFIG_CONSOLE_WRITEONLY)) {
>> +		printf("\nNon-interactive console, booting system\n");
>> +		return autoboot_state = AUTOBOOT_BOOT;
>> +	}
>> +
>>  	if (menu_exists) {
>>  		printf("\nHit m for menu or %s to stop autoboot: ",
>>  		       global_autoboot_abort_keys[global_autoboot_abort_key]);
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox