Re: [PATCH] riscv/console: Updated the console-config.c file
On Fri, 30 Apr 2021 at 04:44, somesh deshmukh wrote: > > > On Thu, Apr 29, 2021 at 10:59 PM Hesham Almatary < > hesham.almat...@cl.cam.ac.uk> wrote: > >> On Wed, 28 Apr 2021 at 15:23, Somesh Deshmukh >> wrote: >> > >> > - Parsing the sub-node should be available generic not specific to >> Freedom >> > Arty310 board. If we remove the Freedom Arty macro now, it will lose >> > backward compatibility.The proposed change will retain the backward >> > compatibility and also adds the necessary fix for parsing sub-node. >> > >> > - Line 234 and 235 in riscv_console_probe() uses polled handlers for >> ns16550 >> > read and write while, the console_initialize function uses ns16550 >> > interrupt handler. Proposing a fix to make polled handlers consistant >> > through out the console-config.c >> > --- >> > bsps/riscv/riscv/console/console-config.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/bsps/riscv/riscv/console/console-config.c >> b/bsps/riscv/riscv/console/console-config.c >> > index d962a5a418..f203f0be7d 100644 >> > --- a/bsps/riscv/riscv/console/console-config.c >> > +++ b/bsps/riscv/riscv/console/console-config.c >> > @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt) >> > stdout_path = ""; >> >} >> > >> > -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0 >> > +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || >> (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0)) >> >int root; >> >int soc; >> >root = fdt_path_offset(fdt, "/"); >> > @@ -318,7 +318,7 @@ rtems_status_code console_initialize( >> > >> > rtems_termios_device_install( >> >path, >> > - _handler_interrupt, >> > + _handler_polled, >> I would leave the driver interrupt-based as it is. Other than that the >> patch is fine by me. >> >> Thanks Hesham for the review. > One question though, is there any specific reason to use the > ns16550_handler_interrupt? > Interrupt-based drivers are preferred as they don’t unnecessarily waste CPU cycles and power. > > >NULL, > > >>base >> > ); >> > -- >> > 2.25.1 >> > >> > ___ >> > devel mailing list >> > devel@rtems.org >> > http://lists.rtems.org/mailman/listinfo/devel >> > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] riscv/console: Updated the console-config.c file
On Thu, Apr 29, 2021 at 10:59 PM Hesham Almatary < hesham.almat...@cl.cam.ac.uk> wrote: > On Wed, 28 Apr 2021 at 15:23, Somesh Deshmukh > wrote: > > > > - Parsing the sub-node should be available generic not specific to > Freedom > > Arty310 board. If we remove the Freedom Arty macro now, it will lose > > backward compatibility.The proposed change will retain the backward > > compatibility and also adds the necessary fix for parsing sub-node. > > > > - Line 234 and 235 in riscv_console_probe() uses polled handlers for > ns16550 > > read and write while, the console_initialize function uses ns16550 > > interrupt handler. Proposing a fix to make polled handlers consistant > > through out the console-config.c > > --- > > bsps/riscv/riscv/console/console-config.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/bsps/riscv/riscv/console/console-config.c > b/bsps/riscv/riscv/console/console-config.c > > index d962a5a418..f203f0be7d 100644 > > --- a/bsps/riscv/riscv/console/console-config.c > > +++ b/bsps/riscv/riscv/console/console-config.c > > @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt) > > stdout_path = ""; > >} > > > > -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0 > > +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || > (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0)) > >int root; > >int soc; > >root = fdt_path_offset(fdt, "/"); > > @@ -318,7 +318,7 @@ rtems_status_code console_initialize( > > > > rtems_termios_device_install( > >path, > > - _handler_interrupt, > > + _handler_polled, > I would leave the driver interrupt-based as it is. Other than that the > patch is fine by me. > > Thanks Hesham for the review. One question though, is there any specific reason to use the ns16550_handler_interrupt? >NULL, >>base > > ); > > -- > > 2.25.1 > > > > ___ > > devel mailing list > > devel@rtems.org > > http://lists.rtems.org/mailman/listinfo/devel > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] riscv/console: Updated the console-config.c file
On Wed, 28 Apr 2021 at 15:23, Somesh Deshmukh wrote: > > - Parsing the sub-node should be available generic not specific to Freedom > Arty310 board. If we remove the Freedom Arty macro now, it will lose > backward compatibility.The proposed change will retain the backward > compatibility and also adds the necessary fix for parsing sub-node. > > - Line 234 and 235 in riscv_console_probe() uses polled handlers for ns16550 > read and write while, the console_initialize function uses ns16550 > interrupt handler. Proposing a fix to make polled handlers consistant > through out the console-config.c > --- > bsps/riscv/riscv/console/console-config.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/bsps/riscv/riscv/console/console-config.c > b/bsps/riscv/riscv/console/console-config.c > index d962a5a418..f203f0be7d 100644 > --- a/bsps/riscv/riscv/console/console-config.c > +++ b/bsps/riscv/riscv/console/console-config.c > @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt) > stdout_path = ""; >} > > -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0 > +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || > (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0)) >int root; >int soc; >root = fdt_path_offset(fdt, "/"); > @@ -318,7 +318,7 @@ rtems_status_code console_initialize( > > rtems_termios_device_install( >path, > - _handler_interrupt, > + _handler_polled, I would leave the driver interrupt-based as it is. Other than that the patch is fine by me. >NULL, >>base > ); > -- > 2.25.1 > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] riscv/console: Updated the console-config.c file
Hesham, please review. I'm not certain. On Wed, Apr 28, 2021 at 7:23 AM Somesh Deshmukh wrote: > > - Parsing the sub-node should be available generic not specific to Freedom > Arty310 board. If we remove the Freedom Arty macro now, it will lose > backward compatibility.The proposed change will retain the backward > compatibility and also adds the necessary fix for parsing sub-node. > > - Line 234 and 235 in riscv_console_probe() uses polled handlers for ns16550 > read and write while, the console_initialize function uses ns16550 > interrupt handler. Proposing a fix to make polled handlers consistant > through out the console-config.c > --- > bsps/riscv/riscv/console/console-config.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/bsps/riscv/riscv/console/console-config.c > b/bsps/riscv/riscv/console/console-config.c > index d962a5a418..f203f0be7d 100644 > --- a/bsps/riscv/riscv/console/console-config.c > +++ b/bsps/riscv/riscv/console/console-config.c > @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt) > stdout_path = ""; >} > > -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0 > +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || > (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0)) >int root; >int soc; >root = fdt_path_offset(fdt, "/"); > @@ -318,7 +318,7 @@ rtems_status_code console_initialize( > > rtems_termios_device_install( >path, > - _handler_interrupt, > + _handler_polled, >NULL, >>base > ); > -- > 2.25.1 > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] riscv/console: Updated the console-config.c file
- Parsing the sub-node should be available generic not specific to Freedom Arty310 board. If we remove the Freedom Arty macro now, it will lose backward compatibility.The proposed change will retain the backward compatibility and also adds the necessary fix for parsing sub-node. - Line 234 and 235 in riscv_console_probe() uses polled handlers for ns16550 read and write while, the console_initialize function uses ns16550 interrupt handler. Proposing a fix to make polled handlers consistant through out the console-config.c --- bsps/riscv/riscv/console/console-config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bsps/riscv/riscv/console/console-config.c b/bsps/riscv/riscv/console/console-config.c index d962a5a418..f203f0be7d 100644 --- a/bsps/riscv/riscv/console/console-config.c +++ b/bsps/riscv/riscv/console/console-config.c @@ -91,7 +91,7 @@ static int riscv_get_console_node(const void *fdt) stdout_path = ""; } -#if RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0 +#if ((RISCV_ENABLE_FRDME310ARTY_SUPPORT != 0) || (RISCV_CONSOLE_MAX_NS16550_DEVICES > 0)) int root; int soc; root = fdt_path_offset(fdt, "/"); @@ -318,7 +318,7 @@ rtems_status_code console_initialize( rtems_termios_device_install( path, - _handler_interrupt, + _handler_polled, NULL, >base ); -- 2.25.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
