<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 29, 2021 at 10:59 PM Hesham Almatary <<a href="mailto:hesham.almatary@cl.cam.ac.uk">hesham.almatary@cl.cam.ac.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, 28 Apr 2021 at 15:23, Somesh Deshmukh<br>
<<a href="mailto:someshdeshmukh07@gmail.com" target="_blank">someshdeshmukh07@gmail.com</a>> wrote:<br>
><br>
> - Parsing the sub-node should be available generic not specific to Freedom<br>
>   Arty310 board. If we remove the Freedom Arty macro now, it will lose<br>
>   backward compatibility.The proposed change will retain the backward<br>
>   compatibility and also adds the necessary fix for parsing sub-node.<br>
><br>
> - Line 234 and 235 in riscv_console_probe() uses polled handlers for ns16550<br>
>   read and write while, the console_initialize function uses ns16550<br>
>   interrupt handler. Proposing a fix to make polled handlers consistant<br>
>   through out the console-config.c<br>
> ---<br>
>  bsps/riscv/riscv/console/console-config.c | 4 ++--<br>
>  1 file changed, 2 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/bsps/riscv/riscv/console/console-config.c b/bsps/riscv/riscv/console/console-config.c<br>
> index d962a5a418..f203f0be7d 100644<br>
> --- a/bsps/riscv/riscv/console/console-config.c<br>
> +++ b/bsps/riscv/riscv/console/console-config.c<br>
> @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt)<br>
>      stdout_path = "";<br>
>    }<br>
><br>
> -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0<br>
> +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0))<br>
>    int root;<br>
>    int soc;<br>
>    root = fdt_path_offset(fdt, "/");<br>
> @@ -318,7 +318,7 @@ rtems_status_code console_initialize(<br>
><br>
>      rtems_termios_device_install(<br>
>        path,<br>
> -      &ns16550_handler_interrupt,<br>
> +      &ns16550_handler_polled,<br>
I would leave the driver interrupt-based as it is. Other than that the<br>
patch is fine by me.<br>
<br></blockquote><div><span style="color:rgb(102,0,0)"><span style="background-color:rgb(255,255,255)">Thanks Hesham for the review. <br></span></span></div><div><span style="background-color:rgb(116,27,71)"><span style="color:rgb(76,17,48)"><span style="background-color:rgb(243,243,243)"><span style="color:rgb(102,0,0)"><span style="background-color:rgb(255,255,255)">One question though, is there any specific reason to use the ns16550_handler_interrupt?</span></span><span style=""></span></span></span></span></div><div><span style="background-color:rgb(255,255,255)"><span style=""></span></span><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>        NULL, </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>        &ctx->base<br>
>      );<br>
> --<br>
> 2.25.1<br>
><br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>