* [PATCH 1/1] tty: serial: owl: Add support for kernel debugger
@ 2020-05-29 15:50 Cristian Ciocaltea
2020-05-29 15:56 ` Andreas Färber
0 siblings, 1 reply; 3+ messages in thread
From: Cristian Ciocaltea @ 2020-05-29 15:50 UTC (permalink / raw)
To: Andreas Färber, Manivannan Sadhasivam, Greg Kroah-Hartman,
Jiri Slaby
Cc: linux-serial, linux-arm-kernel, linux-kernel, linux-actions
Implement poll_put_char and poll_get_char callbacks in struct uart_ops
that enables OWL UART to be used for KGDB debugging over serial line.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
drivers/tty/serial/owl-uart.c | 45 ++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
index c2fa2f15d50a..26dcc374dec5 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -12,6 +12,7 @@
#include <linux/console.h>
#include <linux/delay.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -20,13 +21,13 @@
#include <linux/tty.h>
#include <linux/tty_flip.h>
-#define OWL_UART_PORT_NUM 7
-#define OWL_UART_DEV_NAME "ttyOWL"
+#define OWL_UART_PORT_NUM 7
+#define OWL_UART_DEV_NAME "ttyOWL"
-#define OWL_UART_CTL 0x000
-#define OWL_UART_RXDAT 0x004
-#define OWL_UART_TXDAT 0x008
-#define OWL_UART_STAT 0x00c
+#define OWL_UART_CTL 0x000
+#define OWL_UART_RXDAT 0x004
+#define OWL_UART_TXDAT 0x008
+#define OWL_UART_STAT 0x00c
#define OWL_UART_CTL_DWLS_MASK GENMASK(1, 0)
#define OWL_UART_CTL_DWLS_5BITS (0x0 << 0)
@@ -461,6 +462,34 @@ static void owl_uart_config_port(struct uart_port *port, int flags)
}
}
+#ifdef CONFIG_CONSOLE_POLL
+
+static int owl_uart_poll_get_char(struct uart_port *port)
+{
+ u32 c = NO_POLL_CHAR;
+
+ if (!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_RFEM))
+ c = owl_uart_read(port, OWL_UART_RXDAT);
+
+ return c;
+}
+
+static void owl_uart_poll_put_char(struct uart_port *port, unsigned char c)
+{
+ /* Wait while TX FIFO is full */
+ while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU)
+ cpu_relax();
+
+ /* Send the character out */
+ owl_uart_write(port, c, OWL_UART_TXDAT);
+
+ /* Wait for transmitter to become empty */
+ while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TRFL_MASK)
+ cpu_relax();
+}
+
+#endif /* CONFIG_CONSOLE_POLL */
+
static const struct uart_ops owl_uart_ops = {
.set_mctrl = owl_uart_set_mctrl,
.get_mctrl = owl_uart_get_mctrl,
@@ -476,6 +505,10 @@ static const struct uart_ops owl_uart_ops = {
.request_port = owl_uart_request_port,
.release_port = owl_uart_release_port,
.verify_port = owl_uart_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_get_char = owl_uart_poll_get_char,
+ .poll_put_char = owl_uart_poll_put_char,
+#endif
};
#ifdef CONFIG_SERIAL_OWL_CONSOLE
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Add support for kernel debugger
2020-05-29 15:50 [PATCH 1/1] tty: serial: owl: Add support for kernel debugger Cristian Ciocaltea
@ 2020-05-29 15:56 ` Andreas Färber
2020-05-29 17:10 ` Cristian Ciocaltea
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Färber @ 2020-05-29 15:56 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Manivannan Sadhasivam, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, linux-arm-kernel, linux-kernel, linux-actions
Hi,
Am 29.05.20 um 17:50 schrieb Cristian Ciocaltea:
> Implement poll_put_char and poll_get_char callbacks in struct uart_ops
> that enables OWL UART to be used for KGDB debugging over serial line.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> ---
> drivers/tty/serial/owl-uart.c | 45 ++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
> index c2fa2f15d50a..26dcc374dec5 100644
> --- a/drivers/tty/serial/owl-uart.c
> +++ b/drivers/tty/serial/owl-uart.c
> @@ -12,6 +12,7 @@
> #include <linux/console.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -20,13 +21,13 @@
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
>
> -#define OWL_UART_PORT_NUM 7
> -#define OWL_UART_DEV_NAME "ttyOWL"
> +#define OWL_UART_PORT_NUM 7
> +#define OWL_UART_DEV_NAME "ttyOWL"
>
> -#define OWL_UART_CTL 0x000
> -#define OWL_UART_RXDAT 0x004
> -#define OWL_UART_TXDAT 0x008
> -#define OWL_UART_STAT 0x00c
> +#define OWL_UART_CTL 0x000
> +#define OWL_UART_RXDAT 0x004
> +#define OWL_UART_TXDAT 0x008
> +#define OWL_UART_STAT 0x00c
Please do not unnecessarily re-indent kernel code. You can do so when
you're actually adding something new.
>
> #define OWL_UART_CTL_DWLS_MASK GENMASK(1, 0)
> #define OWL_UART_CTL_DWLS_5BITS (0x0 << 0)
> @@ -461,6 +462,34 @@ static void owl_uart_config_port(struct uart_port *port, int flags)
> }
> }
>
> +#ifdef CONFIG_CONSOLE_POLL
> +
> +static int owl_uart_poll_get_char(struct uart_port *port)
> +{
> + u32 c = NO_POLL_CHAR;
> +
> + if (!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_RFEM))
> + c = owl_uart_read(port, OWL_UART_RXDAT);
> +
> + return c;
> +}
> +
> +static void owl_uart_poll_put_char(struct uart_port *port, unsigned char c)
> +{
> + /* Wait while TX FIFO is full */
> + while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU)
> + cpu_relax();
> +
> + /* Send the character out */
> + owl_uart_write(port, c, OWL_UART_TXDAT);
> +
> + /* Wait for transmitter to become empty */
> + while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TRFL_MASK)
> + cpu_relax();
> +}
How is this different from earlycon? I dislike that this is being
open-coded. Please try to reuse existing functions for this.
Regards,
Andreas
> +
> +#endif /* CONFIG_CONSOLE_POLL */
> +
> static const struct uart_ops owl_uart_ops = {
> .set_mctrl = owl_uart_set_mctrl,
> .get_mctrl = owl_uart_get_mctrl,
> @@ -476,6 +505,10 @@ static const struct uart_ops owl_uart_ops = {
> .request_port = owl_uart_request_port,
> .release_port = owl_uart_release_port,
> .verify_port = owl_uart_verify_port,
> +#ifdef CONFIG_CONSOLE_POLL
> + .poll_get_char = owl_uart_poll_get_char,
> + .poll_put_char = owl_uart_poll_put_char,
> +#endif
> };
>
> #ifdef CONFIG_SERIAL_OWL_CONSOLE
>
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Add support for kernel debugger
2020-05-29 15:56 ` Andreas Färber
@ 2020-05-29 17:10 ` Cristian Ciocaltea
0 siblings, 0 replies; 3+ messages in thread
From: Cristian Ciocaltea @ 2020-05-29 17:10 UTC (permalink / raw)
To: Andreas Färber
Cc: Manivannan Sadhasivam, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, linux-arm-kernel, linux-kernel, linux-actions
On Fri, May 29, 2020 at 05:56:47PM +0200, Andreas Färber wrote:
> Hi,
>
> Am 29.05.20 um 17:50 schrieb Cristian Ciocaltea:
> > Implement poll_put_char and poll_get_char callbacks in struct uart_ops
> > that enables OWL UART to be used for KGDB debugging over serial line.
> >
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > ---
> > drivers/tty/serial/owl-uart.c | 45 ++++++++++++++++++++++++++++++-----
> > 1 file changed, 39 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
> > index c2fa2f15d50a..26dcc374dec5 100644
> > --- a/drivers/tty/serial/owl-uart.c
> > +++ b/drivers/tty/serial/owl-uart.c
> > @@ -12,6 +12,7 @@
> > #include <linux/console.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > +#include <linux/iopoll.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/platform_device.h>
> > @@ -20,13 +21,13 @@
> > #include <linux/tty.h>
> > #include <linux/tty_flip.h>
> > -#define OWL_UART_PORT_NUM 7
> > -#define OWL_UART_DEV_NAME "ttyOWL"
> > +#define OWL_UART_PORT_NUM 7
> > +#define OWL_UART_DEV_NAME "ttyOWL"
> > -#define OWL_UART_CTL 0x000
> > -#define OWL_UART_RXDAT 0x004
> > -#define OWL_UART_TXDAT 0x008
> > -#define OWL_UART_STAT 0x00c
> > +#define OWL_UART_CTL 0x000
> > +#define OWL_UART_RXDAT 0x004
> > +#define OWL_UART_TXDAT 0x008
> > +#define OWL_UART_STAT 0x00c
>
> Please do not unnecessarily re-indent kernel code. You can do so when you're
> actually adding something new.
>
Hi Andreas,
Thank you for reviewing!
Sure, I will revert unnecessary changes.
>
> > #define OWL_UART_CTL_DWLS_MASK GENMASK(1, 0)
> > #define OWL_UART_CTL_DWLS_5BITS (0x0 << 0)
> > @@ -461,6 +462,34 @@ static void owl_uart_config_port(struct uart_port *port, int flags)
> > }
> > }
> > +#ifdef CONFIG_CONSOLE_POLL
> > +
> > +static int owl_uart_poll_get_char(struct uart_port *port)
> > +{
> > + u32 c = NO_POLL_CHAR;
> > +
> > + if (!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_RFEM))
> > + c = owl_uart_read(port, OWL_UART_RXDAT);
> > +
> > + return c;
> > +}
> > +
> > +static void owl_uart_poll_put_char(struct uart_port *port, unsigned char c)
> > +{
> > + /* Wait while TX FIFO is full */
> > + while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU)
> > + cpu_relax();
> > +
> > + /* Send the character out */
> > + owl_uart_write(port, c, OWL_UART_TXDAT);
> > +
> > + /* Wait for transmitter to become empty */
> > + while (owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TRFL_MASK)
> > + cpu_relax();
> > +}
>
> How is this different from earlycon? I dislike that this is being
> open-coded. Please try to reuse existing functions for this.
>
I actually tried initially to reuse the existing code, but I found
a few (possible) issues:
- owl_uart_port_write() does more things than I think it's really
needed here, i.e. I'm not sure if the locking stuff and the IRQ
setup are required. From what I've noticed, most serial drivers provide
a very simple implementation (and lock free) for the callbacks, but
I couldn't figure out if locking could be required in some
circumstances.
- owl_console_putchar() could be a better alternative, but it depends
on CONFIG_SERIAL_OWL_CONSOLE which might not be enabled if the user
only chooses CONFIG_KGDB_SERIAL_CONSOLE, although this is probably
not a valid scenario.
Kind regards,
Cristi
>
> Regards,
> Andreas
>
> > +
> > +#endif /* CONFIG_CONSOLE_POLL */
> > +
> > static const struct uart_ops owl_uart_ops = {
> > .set_mctrl = owl_uart_set_mctrl,
> > .get_mctrl = owl_uart_get_mctrl,
> > @@ -476,6 +505,10 @@ static const struct uart_ops owl_uart_ops = {
> > .request_port = owl_uart_request_port,
> > .release_port = owl_uart_release_port,
> > .verify_port = owl_uart_verify_port,
> > +#ifdef CONFIG_CONSOLE_POLL
> > + .poll_get_char = owl_uart_poll_get_char,
> > + .poll_put_char = owl_uart_poll_put_char,
> > +#endif
> > };
> > #ifdef CONFIG_SERIAL_OWL_CONSOLE
> >
>
>
> --
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-29 17:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 15:50 [PATCH 1/1] tty: serial: owl: Add support for kernel debugger Cristian Ciocaltea
2020-05-29 15:56 ` Andreas Färber
2020-05-29 17:10 ` Cristian Ciocaltea
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).