mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] console: add new write-only option
@ 2020-10-13  7:59 Rouven Czerwinski
  2020-10-13  8:41 ` Ahmad Fatoum
  0 siblings, 1 reply; 3+ messages in thread
From: Rouven Czerwinski @ 2020-10-13  7:59 UTC (permalink / raw)
  To: barebox; +Cc: Rouven Czerwinski

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.
+
 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]);
-- 
2.28.0


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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] console: add new write-only option
  2020-10-13  7:59 [PATCH] console: add new write-only option Rouven Czerwinski
@ 2020-10-13  8:41 ` Ahmad Fatoum
  2020-10-13  9:17   ` Ahmad Fatoum
  0 siblings, 1 reply; 3+ messages in thread
From: Ahmad Fatoum @ 2020-10-13  8:41 UTC (permalink / raw)
  To: Rouven Czerwinski, barebox

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[1] device parameter instead?
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)

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] console: add new write-only option
  2020-10-13  8:41 ` Ahmad Fatoum
@ 2020-10-13  9:17   ` Ahmad Fatoum
  0 siblings, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2020-10-13  9:17 UTC (permalink / raw)
  To: Rouven Czerwinski, barebox

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-13  9:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13  7:59 [PATCH] console: add new write-only option Rouven Czerwinski
2020-10-13  8:41 ` Ahmad Fatoum
2020-10-13  9:17   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox