On 03/04/2019 00:07, Chris Johns wrote:
On 2/4/19 10:08 pm, Hesham Almatary wrote:
On Sun, 31 Mar 2019 at 22:50, Chris Johns <chr...@rtems.org> wrote:
On 1/4/19 1:33 am, Hesham Almatary wrote:
* Different RISC-V DTBs use different names for UART interrupts such as
"interrupts" and "interrupts-extended", so it is not portable.
* polling functions are currently used, so there is no need to query interrupts.
---
bsps/riscv/riscv/console/console-config.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/bsps/riscv/riscv/console/console-config.c
b/bsps/riscv/riscv/console/console-config.c
index 04d0b28361..81e46f304f 100644
--- a/bsps/riscv/riscv/console/console-config.c
+++ b/bsps/riscv/riscv/console/console-config.c
@@ -183,13 +183,20 @@ static void riscv_console_probe(void)
ctx->clock = fdt32_to_cpu(val[0]);
- val = (fdt32_t *) fdt_getprop(fdt, node, "interrupts", &len);
+ /* XXX Different RISC-V DTBs use different property names for interrupt
+ sources. If an interrupt-driven driver is needed, uncomment, replace the
+ "interrupts-extended" string below with your target's DTB name for UART
+ interrupts, and use interrupt-driven UART functions instead of
+ ns16550_polled_* functions */
+
+ /* val = (fdt32_t *) fdt_getprop(fdt, node, "interrupts-extended", &len);
Asking a user to edit a BSP complicates a user's ability to configuration manage
and track RTEMS so I prefer we find and use other solutions. For example you
could replace the commented call with a weak function of:
The comment is not really for the user, but rather for RISC-V BSP
developers. I came across this issue when I was trying to get this BSP
(which is supposed to be a generic riscv one, but it only accounts for
QEMU's DTB format) to work on another platform.
And what is this BSP developer to do when they see this and need it?
What if I need the commented code and I post a change that uncomments this code
and comments the other call. I suspect the patch would be voted down.
The driver uses interrupts, so the patch as is is not all right from my
point of view, see my comment in another e-mail to this patch.
val = rtems_riscv_get_console_interrupts(fdt, node, &len);
which returns by default "interrupts". A user can then provide a non-weak
version of function and override the default behaviour and return
"interrupts-extended"
That makes things slightly complicated for the user I think. What
about having a BSP option in the riscv configure.ac that specifies the
DTB UART interrupt node name? For example, similar to how we currently
pass -DRISCV_ENABLE_HTIF_SUPPORT to build RISC-V BSP for Spike, we can
add another BSP_FDT_UART_INTERRUPT option (which defaults to
"interrupts"), but allow the user to override it at configure time.
Sure, that is fine ... but can we have some User Manual documentation of the
options this BSP has if that is OK? :)
https://docs.rtems.org/branches/master/user/bsps/bsps-riscv.html#build-configuration-options
The device tree support itself should not use BSP options.
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
