[PATCH 3/4] bsps/riscv: UART - Comment code that queries UART interrupts
sebastian.huber at embedded-brains.de
Wed Apr 3 06:39:29 UTC 2019
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 <chrisj at 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);
>>>> - 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
>> 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? :)
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.huber at embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
More information about the devel