| Message ID | 6ee88060c129715980592a1ae33c93923916a14b.1590766726.git.cristian.ciocaltea@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] tty: serial: owl: Add support for kernel debugger | expand |
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 >
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)
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
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(-)