linux-kernel - Re: [PATCH 1/1] tty: serial: owl: Add support for kernel debugger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <16ff435f-9172-e01d-dfe6-7aa8575c4bd6@suse.de>
Date:   Fri, 29 May 2020 17:56:47 +0200
From:   Andreas Färber <afaerber@...e.de>
To:     Cristian Ciocaltea <cristian.ciocaltea@...il.com>
Cc:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-actions@...ts.infradead.org
Subject: Re: [PATCH 1/1] tty: serial: owl: Add support for kernel debugger

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@...il.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)