[PATCH] bsps: Add BSP_FLUSH_KERNEL_CHAR_OUTPUT option
Sebastian Huber
sebastian.huber at embedded-brains.de
Tue Mar 19 06:40:55 UTC 2024
On 19.03.24 03:16, Chris Johns wrote:
> On 19/3/2024 3:49 am, Sebastian Huber wrote:
>> Make the kernel I/O output character device flushing configurable. The
>> bsp_reset() function should reset the unit and do nothing else.
>> ---
>> bsps/aarch64/xilinx-zynqmp/console/console.c | 3 ++-
>> bsps/aarch64/xilinx-zynqmp/include/bsp.h | 2 --
>> .../console/console-config.c | 3 ++-
>> bsps/arm/xilinx-zynqmp-rpu/include/bsp.h | 2 --
>> bsps/arm/xilinx-zynqmp-rpu/start/bspreset.c | 3 ---
>> .../xilinx-zynqmp/console/console-config.c | 2 +-
>> bsps/arm/xilinx-zynqmp/include/bsp.h | 2 --
>> bsps/arm/xilinx-zynqmp/start/bspreset.c | 4 +---
>> bsps/include/bsp/bootcard.h | 5 +++++
>> bsps/shared/start/bspfatal-default.c | 4 ++++
>> spec/build/bsps/bspopts.yml | 2 ++
>> spec/build/bsps/optflushkerncharout.yml | 20 +++++++++++++++++++
>> 12 files changed, 37 insertions(+), 15 deletions(-)
>> create mode 100644 spec/build/bsps/optflushkerncharout.yml
>>
>> diff --git a/bsps/aarch64/xilinx-zynqmp/console/console.c b/bsps/aarch64/xilinx-zynqmp/console/console.c
>> index 9ce0b1da63..d1b2850952 100644
>> --- a/bsps/aarch64/xilinx-zynqmp/console/console.c
>> +++ b/bsps/aarch64/xilinx-zynqmp/console/console.c
>> @@ -41,6 +41,7 @@
>> #include <rtems/termiostypes.h>
>>
>> #include <bsp/aarch64-mmu.h>
>> +#include <bsp/bootcard.h>
>> #include <bsp/fdt.h>
>> #include <bsp/irq.h>
>>
>> @@ -234,7 +235,7 @@ rtems_status_code console_initialize(
>> return RTEMS_SUCCESSFUL;
>> }
>>
>> -void zynqmp_debug_console_flush(void)
>> +void bsp_flush_kernel_char_output(void)
>> {
>> zynq_uart_reset_tx_flush(&zynqmp_uart_instances[BSP_CONSOLE_MINOR]);
>> }
>> diff --git a/bsps/aarch64/xilinx-zynqmp/include/bsp.h b/bsps/aarch64/xilinx-zynqmp/include/bsp.h
>> index 0ccca8b196..d36abde415 100644
>> --- a/bsps/aarch64/xilinx-zynqmp/include/bsp.h
>> +++ b/bsps/aarch64/xilinx-zynqmp/include/bsp.h
>> @@ -86,8 +86,6 @@ BSP_START_TEXT_SECTION void zynqmp_setup_mmu_and_cache(void);
>> */
>> BSP_START_TEXT_SECTION void zynqmp_setup_secondary_cpu_mmu_and_cache( void );
>>
>> -void zynqmp_debug_console_flush(void);
>> -
>> uint32_t zynqmp_clock_i2c0(void);
>>
>> uint32_t zynqmp_clock_i2c1(void);
>> diff --git a/bsps/arm/xilinx-zynqmp-rpu/console/console-config.c b/bsps/arm/xilinx-zynqmp-rpu/console/console-config.c
>> index f52e008f2b..196b2dec58 100644
>> --- a/bsps/arm/xilinx-zynqmp-rpu/console/console-config.c
>> +++ b/bsps/arm/xilinx-zynqmp-rpu/console/console-config.c
>> @@ -35,6 +35,7 @@
>> #include <rtems/sysinit.h>
>> #include <rtems/termiostypes.h>
>>
>> +#include <bsp/bootcard.h>
>> #include <bsp/irq.h>
>> #include <dev/serial/zynq-uart.h>
>>
>> @@ -79,7 +80,7 @@ rtems_status_code console_initialize(
>> return RTEMS_SUCCESSFUL;
>> }
>>
>> -void zynqmp_debug_console_flush(void)
>> +void bsp_flush_kernel_char_output(void)
>> {
>> zynq_uart_reset_tx_flush(&zynqmp_uart_instances[BSP_CONSOLE_MINOR]);
>> }
>> diff --git a/bsps/arm/xilinx-zynqmp-rpu/include/bsp.h b/bsps/arm/xilinx-zynqmp-rpu/include/bsp.h
>> index e386bd4b26..060147967c 100644
>> --- a/bsps/arm/xilinx-zynqmp-rpu/include/bsp.h
>> +++ b/bsps/arm/xilinx-zynqmp-rpu/include/bsp.h
>> @@ -83,8 +83,6 @@ extern "C" {
>> */
>> BSP_START_TEXT_SECTION void zynqmp_setup_mpu_and_cache(void);
>>
>> -void zynqmp_debug_console_flush(void);
>> -
>> #ifdef __cplusplus
>> }
>> #endif /* __cplusplus */
>> diff --git a/bsps/arm/xilinx-zynqmp-rpu/start/bspreset.c b/bsps/arm/xilinx-zynqmp-rpu/start/bspreset.c
>> index eecb4da838..a894f55f6e 100644
>> --- a/bsps/arm/xilinx-zynqmp-rpu/start/bspreset.c
>> +++ b/bsps/arm/xilinx-zynqmp-rpu/start/bspreset.c
>> @@ -30,13 +30,10 @@
>> * POSSIBILITY OF SUCH DAMAGE.
>> */
>>
>> -#include <bsp.h>
>> #include <bsp/bootcard.h>
>>
>> void bsp_reset(void)
>> {
>> - zynqmp_debug_console_flush();
>
> Why remove the call here?
>
>> -
>> while (true) {
>> /* Wait */
>> }
This BSP is rather new. Flushing the UART followed by an infinite loop
doesn't really make sense. I did a full test run for this BSP and there
are some more things to fix.
>> diff --git a/bsps/arm/xilinx-zynqmp/console/console-config.c b/bsps/arm/xilinx-zynqmp/console/console-config.c
>> index eadd7f11a2..360d193ba2 100644
>> --- a/bsps/arm/xilinx-zynqmp/console/console-config.c
>> +++ b/bsps/arm/xilinx-zynqmp/console/console-config.c
>> @@ -81,7 +81,7 @@ rtems_status_code console_initialize(
>> return RTEMS_SUCCESSFUL;
>> }
>>
>> -void zynqmp_debug_console_flush(void)
>> +void bsp_flush_kernel_char_output(void)
>> {
>> zynq_uart_reset_tx_flush(&zynqmp_uart_instances[BSP_CONSOLE_MINOR]);
>> }
>> diff --git a/bsps/arm/xilinx-zynqmp/include/bsp.h b/bsps/arm/xilinx-zynqmp/include/bsp.h
>> index a08a5feee9..2785e4c2e0 100644
>> --- a/bsps/arm/xilinx-zynqmp/include/bsp.h
>> +++ b/bsps/arm/xilinx-zynqmp/include/bsp.h
>> @@ -79,8 +79,6 @@ extern "C" {
>> */
>> BSP_START_TEXT_SECTION void zynqmp_setup_mmu_and_cache(void);
>>
>> -void zynqmp_debug_console_flush(void);
>> -
>> #ifdef __cplusplus
>> }
>> #endif /* __cplusplus */
>> diff --git a/bsps/arm/xilinx-zynqmp/start/bspreset.c b/bsps/arm/xilinx-zynqmp/start/bspreset.c
>> index b43b19b05f..ed2f4da83a 100644
>> --- a/bsps/arm/xilinx-zynqmp/start/bspreset.c
>> +++ b/bsps/arm/xilinx-zynqmp/start/bspreset.c
>> @@ -30,12 +30,10 @@
>> * POSSIBILITY OF SUCH DAMAGE.
>> */
>>
>> -#include <bsp.h>
>> +#include <bsp/bootcard.h>
>>
>> void bsp_reset(void)
>> {
>> - zynqmp_debug_console_flush();
>> -
>
> And here?
This job moved to bsp_fatal_extension() and is now configurable.
>
>> while (true) {
>> /* Wait */
>> }
>> diff --git a/bsps/include/bsp/bootcard.h b/bsps/include/bsp/bootcard.h
>> index 5f339d65f8..dfdc3ae7e0 100644
>> --- a/bsps/include/bsp/bootcard.h
>> +++ b/bsps/include/bsp/bootcard.h
>> @@ -94,6 +94,11 @@ struct Per_CPU_Control;
>> */
>> void bsp_start_on_secondary_processor(struct Per_CPU_Control *cpu_self);
>>
>> +/**
>> + * @brief Flushes the kernel I/O output character device provided by the BSP.
>> + */
>> +void bsp_flush_kernel_char_output(void);
>> +
>> /** @} */
>>
>> #ifdef __cplusplus
>> diff --git a/bsps/shared/start/bspfatal-default.c b/bsps/shared/start/bspfatal-default.c
>> index 557a0960fa..de3f414407 100644
>> --- a/bsps/shared/start/bspfatal-default.c
>> +++ b/bsps/shared/start/bspfatal-default.c
>> @@ -184,6 +184,10 @@ void bsp_fatal_extension(
>> printk("\n");
>> #endif
>>
>> + #if BSP_FLUSH_KERNEL_CHAR_OUTPUT
>> + bsp_flush_kernel_char_output();
>> + #endif
>> +
>> /*
>> * Check both conditions -- if you want to ask for reboot, then
>> * you must have meant to reset the board.
>> diff --git a/spec/build/bsps/bspopts.yml b/spec/build/bsps/bspopts.yml
>> index 8128a56330..bdab4b72f3 100644
>> --- a/spec/build/bsps/bspopts.yml
>> +++ b/spec/build/bsps/bspopts.yml
>> @@ -29,6 +29,8 @@ links:
>> uid: optincludes
>> - role: build-dependency
>> uid: optcflags
>> +- role: build-dependency
>> + uid: optflushkerncharout
>> - role: build-dependency
>> uid: optlinkflags
>> - role: build-dependency
>> diff --git a/spec/build/bsps/optflushkerncharout.yml b/spec/build/bsps/optflushkerncharout.yml
>> new file mode 100644
>> index 0000000000..c250229dd9
>> --- /dev/null
>> +++ b/spec/build/bsps/optflushkerncharout.yml
>> @@ -0,0 +1,20 @@
>> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
>> +actions:
>> +- get-integer: null
>> +- define: null
>> +build-type: option
>> +copyrights:
>> +- Copyright (C) 2024 embedded brains GmbH & Co. KG
>> +default:
>> +- enabled-by: bsps/arm/xilinx-zynq
>> + value: 1
>> +- enabled-by: true
>> + value: 0
>> +description: |
>> + If defined to a non-zero value, flush the kernel I/O output character device
>> + provided by the BSP when the application exits.
>> +enabled-by: true
>> +format: '{}'
>> +links: []
>> +name: BSP_FLUSH_KERNEL_CHAR_OUTPUT
>> +type: build
>
> Is the flush enabled by default?
It should be only enabled for the BSPs which use this feature right now.
These are the arm/xilinx-zynq and arm/xilinx-zynqmp BSPs. The
arm/xilinx-zynqmp is missing in the patch, I will fix this in v2.
>
> Why would you want to disable it?
The arm/xilinx-zynq and arm/xilinx-zynqmp BSPs are the only ones doing
an UART flush in bsp_reset(). The bsp_reset() function should reset the
system and do nothing more. Doing additional things like flushing an
UART device may not make sense for all applications. Some applications
may not use the UART device, so it may not be initialized and powered
off. Some applications may use it with an application-specific protocol
which doesn't like the additional four '\r' during reset. Doing a UART
flush take soem time and some applications may prefer a fast reset time.
The bsp_reset() is the wrong place to do add specific cleanup functions.
--
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
More information about the devel
mailing list