[PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

While debugging why my 8-port XR17V35X card does not work
with the in-kernel serial driver, I found that some chip
specific wakeup code is necessary to get it working.

Specificly the EFR_ECB bit needs to be set in the
chip-specific UART_XR_EFR register, so that writing to a
couple of other configuration bits is enabled.

Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Michael Welling <mwelling@ieee.org>
Cc: Joe Schultz <jschultz@xes-inc.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/tty/serial/8250/8250_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 4506e40..de34db3 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2176,6 +2176,18 @@ int serial8250_do_startup(struct uart_port *port)
 	 */
 	enable_rsa(up);
 #endif
+
+	/*
+	 * Do this right before clearing the FIFOs to prevent
+	 * interrupt storm.
+	 */
+	if (port->type == PORT_XR17V35X) {
+		/* Wake up and initialize UART */
+		serial_port_out(port, UART_XR_EFR, UART_EFR_ECB);
+		serial_port_out(port, UART_IER, 0);
+		serial_port_out(port, UART_LCR, 0);
+	}
+
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
-- 
1.8.4.5

Re: [PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Michael Welling]

On Fri, Jun 19, 2015 at 05:12:17PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> While debugging why my 8-port XR17V35X card does not work
> with the in-kernel serial driver, I found that some chip
> specific wakeup code is necessary to get it working.
> 
> Specificly the EFR_ECB bit needs to be set in the
> chip-specific UART_XR_EFR register, so that writing to a
> couple of other configuration bits is enabled.
> 
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Cc: Michael Welling <mwelling@ieee.org>
> Cc: Joe Schultz <jschultz@xes-inc.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/tty/serial/8250/8250_core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 4506e40..de34db3 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -2176,6 +2176,18 @@ int serial8250_do_startup(struct uart_port *port)
>  	 */
>  	enable_rsa(up);
>  #endif
> +
> +	/*
> +	 * Do this right before clearing the FIFOs to prevent
> +	 * interrupt storm.
> +	 */
> +	if (port->type == PORT_XR17V35X) {
> +		/* Wake up and initialize UART */
> +		serial_port_out(port, UART_XR_EFR, UART_EFR_ECB);
> +		serial_port_out(port, UART_IER, 0);
> +		serial_port_out(port, UART_LCR, 0);
> +	}
> +

Looks good to me. The datasheet indicates that bit 4 of the EFR needs to be set to access
to IER [7:5], ISR [5:4], FCR[5:4], MCR[7:5], MSR.

>  	/*
>  	 * Clear the FIFO buffers and disable them.
>  	 * (they will be reenabled in set_termios())
> -- 
> 1.8.4.5
> 

Re: [PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Joerg Roedel]

On Fri, Jun 19, 2015 at 12:09:10PM -0500, Michael Welling wrote:
> 
> Looks good to me. The datasheet indicates that bit 4 of the EFR needs to be set to access
> to IER [7:5], ISR [5:4], FCR[5:4], MCR[7:5], MSR.

Thanks! I updated the patch a little bit and wrote a better changelog
entry, mentioning the above bits. Will send a new version after the
merge window.


	Joerg

[PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The XR17V35X UART needs the ECB bit set in its XR_EFR
register to enable access to IER [7:5], ISR [5:4], FCR[5:4],
MCR[7:5], and MSR [7:0].

Also reset the IER register to mask interrupts after access
to all bits of this register has been enabled.

This makes my 8-port XR17V35X working with the in-kernel
serial driver.

Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Michael Welling <mwelling@ieee.org>
Cc: Joe Schultz <jschultz@xes-inc.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/tty/serial/8250/8250_core.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 37fff12..4124a91 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2164,6 +2164,21 @@ int serial8250_do_startup(struct uart_port *port)
 	 */
 	enable_rsa(up);
 #endif
+
+	if (port->type == PORT_XR17V35X) {
+		/*
+		 * First enable access to IER [7:5], ISR [5:4], FCR [5:4],
+		 * MCR [7:5] and MSR [7:0]
+		 */
+		serial_port_out(port, UART_XR_EFR, UART_EFR_ECB);
+
+		/*
+		 * Make sure all interrups are masked until initialization is
+		 * complete and the FIFOs are cleared
+		 */
+		serial_port_out(port, UART_IER, 0);
+	}
+
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
-- 
1.9.1

Re: [PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Michael Welling]

On Thu, Jul 16, 2015 at 10:29:13AM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> The XR17V35X UART needs the ECB bit set in its XR_EFR
> register to enable access to IER [7:5], ISR [5:4], FCR[5:4],
> MCR[7:5], and MSR [7:0].
> 
> Also reset the IER register to mask interrupts after access
> to all bits of this register has been enabled.
> 
> This makes my 8-port XR17V35X working with the in-kernel
> serial driver.
> 
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Cc: Michael Welling <mwelling@ieee.org>
> Cc: Joe Schultz <jschultz@xes-inc.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/tty/serial/8250/8250_core.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 37fff12..4124a91 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -2164,6 +2164,21 @@ int serial8250_do_startup(struct uart_port *port)
>  	 */
>  	enable_rsa(up);
>  #endif
> +
> +	if (port->type == PORT_XR17V35X) {
> +		/*
> +		 * First enable access to IER [7:5], ISR [5:4], FCR [5:4],
> +		 * MCR [7:5] and MSR [7:0]
> +		 */
> +		serial_port_out(port, UART_XR_EFR, UART_EFR_ECB);
> +
> +		/*
> +		 * Make sure all interrups are masked until initialization is
> +		 * complete and the FIFOs are cleared
> +		 */
> +		serial_port_out(port, UART_IER, 0);
> +	}
> +

Reviewed-by: Michael Welling <mwelling@ieee.org>

>  	/*
>  	 * Clear the FIFO buffers and disable them.
>  	 * (they will be reenabled in set_termios())
> -- 
> 1.9.1
> 

Re: [PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Peter Hurley]

On 07/16/2015 04:29 AM, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> The XR17V35X UART needs the ECB bit set in its XR_EFR
> register to enable access to IER [7:5], ISR [5:4], FCR[5:4],
> MCR[7:5], and MSR [7:0].
> 
> Also reset the IER register to mask interrupts after access
> to all bits of this register has been enabled.
> 
> This makes my 8-port XR17V35X working with the in-kernel
> serial driver.

Reviewed-by: Peter Hurley <peter@hurleysoftware.com>

Also, would you please find out what bits are set in the IER
that this fixes and let me know? I don't have this hardware.

Regards,
Peter Hurley

> Cc: Peter Hurley <peter@hurleysoftware.com>
> Cc: Michael Welling <mwelling@ieee.org>
> Cc: Joe Schultz <jschultz@xes-inc.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/tty/serial/8250/8250_core.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 37fff12..4124a91 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -2164,6 +2164,21 @@ int serial8250_do_startup(struct uart_port *port)
>  	 */
>  	enable_rsa(up);
>  #endif
> +
> +	if (port->type == PORT_XR17V35X) {
> +		/*
> +		 * First enable access to IER [7:5], ISR [5:4], FCR [5:4],
> +		 * MCR [7:5] and MSR [7:0]
> +		 */
> +		serial_port_out(port, UART_XR_EFR, UART_EFR_ECB);
> +
> +		/*
> +		 * Make sure all interrups are masked until initialization is
> +		 * complete and the FIFOs are cleared
> +		 */
> +		serial_port_out(port, UART_IER, 0);
> +	}
> +
>  	/*
>  	 * Clear the FIFO buffers and disable them.
>  	 * (they will be reenabled in set_termios())
> 

Re: [PATCH] serial: 8250: Do XR17V35X specific wakeup in serial8250_do_startup

[Joerg Roedel]

Hi Peter,

On Fri, Jul 17, 2015 at 09:34:31AM -0400, Peter Hurley wrote:
> On 07/16/2015 04:29 AM, Joerg Roedel wrote:
> > From: Joerg Roedel <jroedel@suse.de>
> > 
> > The XR17V35X UART needs the ECB bit set in its XR_EFR
> > register to enable access to IER [7:5], ISR [5:4], FCR[5:4],
> > MCR[7:5], and MSR [7:0].
> > 
> > Also reset the IER register to mask interrupts after access
> > to all bits of this register has been enabled.
> > 
> > This makes my 8-port XR17V35X working with the in-kernel
> > serial driver.
> 
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>

Thanks for your review.

> Also, would you please find out what bits are set in the IER
> that this fixes and let me know? I don't have this hardware.

Unfortunatly I put the card back into a production machine and can't
easily check this anymore. But I'll let you know when I update the
kernel on the box next time.


	Joerg