Message-ID: <a8b7492c-3724-fd14-9923-473ec3fe221f@i2se.com>
Date:   Mon, 28 Dec 2020 18:36:05 +0100
From:   Stefan Wahren <stefan.wahren@...e.com>
To:     Ivan Sistik <sistik@...olutions.sk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-serial@...r.kernel.org, sbranden@...adcom.com,
        linux-kernel@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
        suravee.suthikulpanit@....com, jslaby@...e.com,
        linux-arm-kernel@...ts.infradead.org, wahrenst@....net
Subject: Re: [PATCH] tty: serial: amba-pl011: added RS485 support v2

Hi Ivan,

please place the patch version between [ ... ] in the subject.

Am 28.12.20 um 17:41 schrieb Ivan Sistik:
> AMBA PL011 do not have hardware support for RS485. This implementation is
> for drive enable signal (DE), which switch direction of RS485 driver chip.
> This signal si drived by RTS pin. Correct multiplexor settings have to be
> provided to Device Tree. Usually it is 'ctsrts', which is used for enabling
> of HW flow control, too.
>
> DE signal is switched by starting transmition from serial core and data
> transfer is initiated by first hrtimer if there is delay before send
> enabled.
>
> There is missing FIFO empty interrupt in PL011. It is replaced by second
> hrtimer which is started if there are no more data in port transmit buffer.
> Notice that port transmit buffer is not the same as HW TX FIFO. Time of
> this timmer is set to char send time and it is running until fifo is empty.
> This kind of implementation cause that there can be unwanted delay of one
> timer tick before DE signal is switched. This is used to prevent data loss
> during transmit. Second timer can start first if there is delay after send
> enabled.
>
> Signed-off-by: Ivan Sistik <sistik@...olutions.sk>
> ---
>
> Notes:
>     This patch is ported and corrected version of my previous patch which can
>     be reviewed here:
>     https://lore.kernel.org/lkml/20200106235203.27256-1-sistik@3ksolutions.sk/
>     
>     I have been waiting for some time to see if Lukas Wunner <lukas@...ner.de>
>     will create patch with his own solution.
>     
>     Now I am successfully running my imeplementation for almost one year in
>     production environment. There are no problems with it.
Can you please tell which exact platform / config template has been tested?
>  I have made
>     corrections to patch according to notes from Greg Kroah-Hartman
>     <gregkh@...uxfoundation.org>.
>
>  arch/arm/configs/bcm2835_defconfig |   1 +
>  drivers/tty/serial/Kconfig         |  11 +
>  drivers/tty/serial/amba-pl011.c    | 474 ++++++++++++++++++++++++++++-
>  3 files changed, 483 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
> index 519ff58e6..c2f630937 100644
> --- a/arch/arm/configs/bcm2835_defconfig
> +++ b/arch/arm/configs/bcm2835_defconfig
> @@ -86,6 +86,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
>  CONFIG_SERIAL_8250_BCM2835AUX=y
>  CONFIG_SERIAL_AMBA_PL011=y
>  CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
> +CONFIG_SERIAL_AMBA_PL011_SOFT_RS485=y
>  CONFIG_SERIAL_DEV_BUS=y
>  CONFIG_TTY_PRINTK=y
>  CONFIG_I2C_CHARDEV=y
This change is fine, but must be a separate patch. In this case, you can
move all your notes and patch changelog in the cover letter.
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index a9751a83d..c33461511 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -75,6 +75,17 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>  
> +config SERIAL_AMBA_PL011_SOFT_RS485
> +	bool "RS485 software direction switching for ARM AMBA PL011 serial"
> +	depends on SERIAL_AMBA_PL011=y
> +	help
> +	  Enable RS485 software direction switching of driver enable (RTS pin)
> +	  for ARM AMBA PL011 serial. AMBA PL011 does not have HW support for
> +	  RS485. This driver use 2 hrtimers. One is used for rs485 delays.
> +	  Secon one is used for polling of TX FIFO. There is not TX FIFO
Secon +d?
> +	  empty interrupt in PL011. Secondary timer is started by empty
> +	  transmit buffer.
> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 16720c97a..f45b9042b 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -46,6 +46,7 @@
>  #include <linux/sizes.h>
>  #include <linux/io.h>
>  #include <linux/acpi.h>
> +#include <linux/math64.h>
>  
>  #include "amba-pl011.h"
>  
> @@ -60,6 +61,18 @@
>  #define UART_DR_ERROR		(UART011_DR_OE|UART011_DR_BE|UART011_DR_PE|UART011_DR_FE)
>  #define UART_DUMMY_DR_RX	(1 << 16)
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
Please try to reduce the amout of these ifdefs. It makes the code harder
to maintain.
> +/*
> + * Enum with current status
> + */
> +enum rs485_status {
> +	rs485_receiving,
> +	rs485_delay_before_send,
> +	rs485_sending,
> +	rs485_delay_after_send
> +};
> +#endif
> +
>  static u16 pl011_std_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_FR] = UART01x_FR,
> @@ -270,6 +283,16 @@ struct uart_amba_port {
>  	unsigned int		old_cr;		/* state during shutdown */
>  	unsigned int		fixed_baud;	/* vendor-set fixed baud rate */
>  	char			type[12];
> +
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	enum rs485_status	rs485_current_status; /* status used for RTS */
> +	enum rs485_status	rs485_next_status; /* this status after tick */
> +	struct hrtimer		rs485_delay_timer;
> +	struct hrtimer		rs485_tx_empty_poll_timer;
> +	unsigned long		send_char_time;	/* send char (nanoseconds) */
> +	bool			rs485_last_char_sending;
> +#endif
> +
>  #ifdef CONFIG_DMA_ENGINE
>  	/* DMA stuff */
>  	bool			using_tx_dma;
> @@ -280,6 +303,25 @@ struct uart_amba_port {
>  #endif
>  };
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +
> +static void pl011_rs485_start_rts_delay(struct uart_amba_port *uap);
> +
> +#define RS485_SET_RTS_SIGNAL(pUAP, value)		\
> +	do {						\
> +		unsigned int rts_temp_cr;		\
> +		rts_temp_cr = pl011_read(pUAP, REG_CR);	\
> +		if (!(value))				\
> +			rts_temp_cr |= UART011_CR_RTS;	\
> +		else					\
> +			rts_temp_cr &= ~UART011_CR_RTS;	\
> +		pl011_write(rts_temp_cr, pUAP, REG_CR);	\
> +	} while (0)
Please make this a function
> +
> +#define RS485_TX_FIFO_EMPTY(pUAP)			\
> +	(pl011_read(pUAP, REG_FR) & UART011_FR_TXFE)
> +#endif
> +
>  static unsigned int pl011_reg_to_offset(const struct uart_amba_port *uap,
>  	unsigned int reg)
>  {
> @@ -1301,6 +1343,11 @@ static void pl011_stop_tx(struct uart_port *port)
>  	uap->im &= ~UART011_TXIM;
>  	pl011_write(uap->im, uap, REG_IMSC);
>  	pl011_dma_tx_stop(uap);
> +
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	if (uap->port.rs485.flags & SER_RS485_ENABLED)
> +		pl011_rs485_start_rts_delay(uap);
> +#endif
>  }
>  
>  static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq);
> @@ -1319,8 +1366,123 @@ static void pl011_start_tx(struct uart_port *port)
>  	struct uart_amba_port *uap =
>  	    container_of(port, struct uart_amba_port, port);
>  
> -	if (!pl011_dma_tx_start(uap))
> -		pl011_start_tx_pio(uap);
> +#define START_PL011_TX()				\
> +	do {						\
> +		if (!pl011_dma_tx_start(uap))		\
> +			pl011_start_tx_pio(uap);	\
> +	} while (0)
> +
> +#ifndef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	START_PL011_TX();
> +#else
> +
> +#define CANCEL_RS485_TIMERS()						 \
> +	do {								 \
> +		hrtimer_try_to_cancel(&(uap->rs485_delay_timer));	 \
> +		hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));\
> +	} while (0)
> +
> +	if (uap->port.rs485.flags & SER_RS485_ENABLED) {
> +		ktime_t ktime;
> +
> +		switch (uap->rs485_current_status) {
> +		case rs485_delay_after_send:
> +			/* stop old delay timer */
> +			CANCEL_RS485_TIMERS();
> +
> +			/* check if timer expired */
> +			if (uap->rs485_current_status
> +					!= rs485_delay_after_send) {
> +				/* Timer expired and RTS is in wrong state.*/
> +				uap->rs485_current_status
> +					= rs485_delay_before_send;
> +				uap->rs485_next_status = rs485_sending;
> +
> +				/* Set RTS */
Please avoid obvious comments
> +				RS485_SET_RTS_SIGNAL(uap,
> +					uap->port.rs485.flags
> +						& SER_RS485_RTS_ON_SEND);
> +
> +				/* Start timer */
> +				ktime = ktime_set(0,
> +					  uap->port.rs485
> +						.delay_rts_before_send
> +					  * 1000000L);
> +
> +				hrtimer_start(
> +					&(uap->rs485_delay_timer),
> +					ktime,
> +					HRTIMER_MODE_REL);
> +				return;
> +			}
> +
> +			/* timer was stopped and driver can continue sending */
> +			uap->rs485_current_status = rs485_sending;
> +			uap->rs485_next_status = rs485_sending;
> +
> +			/* driver is already in sending state */
> +			START_PL011_TX();
> +			break;
> +
> +
> +		case rs485_sending:
> +			/* stop old timer. There can be running timer	*/
> +			/* which is checking TX FIFO empty flag		*/
> +			CANCEL_RS485_TIMERS();
> +
> +			/* driver is already in sending state */
> +			START_PL011_TX();
> +			break;
> +
> +		case rs485_receiving:
> +		default:
> +			/* stop old timer. There can be running timer	*/
> +			/* which is checking TX FIFO empty flag		*/
> +			CANCEL_RS485_TIMERS();
> +
> +			/* Set RTS */
> +			RS485_SET_RTS_SIGNAL(uap,
> +				     uap->port.rs485.flags
> +					     & SER_RS485_RTS_ON_SEND);
> +
> +			if (uap->port.rs485.delay_rts_before_send == 0) {
> +				/* Change state */
> +				uap->rs485_current_status
> +					= rs485_sending;
> +				uap->rs485_next_status
> +					= rs485_sending;
> +
> +				/* driver is in sending state */
> +				START_PL011_TX();
> +				break;
> +			}
> +
> +			/* Change state */
> +			uap->rs485_current_status
> +				= rs485_delay_before_send;
> +			uap->rs485_next_status = rs485_sending;
> +
> +			/* Start timer */
> +			ktime = ktime_set(0,
> +				  uap->port.rs485.delay_rts_before_send
> +				  * 1000000L);
> +			hrtimer_start(&(uap->rs485_delay_timer),
> +				ktime,
> +				HRTIMER_MODE_REL);
> +			break;
> +
> +		case rs485_delay_before_send:
> +			/* do nothing because delay timer should be running */
> +			break;
> +		}
> +	} else {
> +		START_PL011_TX();
> +	}
> +#undef CANCEL_RS485_TIMERS

Please make this also a function

> +
> +#endif
> +
> +#undef START_PL011_TX
>  }
>  
>  static void pl011_stop_rx(struct uart_port *port)
> @@ -1476,6 +1638,169 @@ static void check_apply_cts_event_workaround(struct uart_amba_port *uap)
>  	dummy_read = pl011_read(uap, REG_ICR);
>  }
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +
> +/*
> + * Change state according to pending delay
> + * Locking: port is locked in this function
> + */
> +static enum hrtimer_restart
> +pl011_rs485_tx_poll_timer(struct hrtimer *timer)
> +{
> +	unsigned long flags;
> +	ktime_t ktime;
> +
> +	struct uart_amba_port *uap =
> +		container_of(timer, struct uart_amba_port,
> +			     rs485_tx_empty_poll_timer);
> +
> +	spin_lock_irqsave(&uap->port.lock, flags);
> +
> +	if (!(uart_circ_empty(&uap->port.state->xmit))) {
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
> +	}
> +
> +	if (!RS485_TX_FIFO_EMPTY(uap) || !uap->rs485_last_char_sending) {
> +		/*
> +		 *  FIFO is empty but there is last char in transmit shift
> +		 * register so we need one more tick
> +		 */
> +		uap->rs485_last_char_sending = RS485_TX_FIFO_EMPTY(uap);
> +
> +		hrtimer_forward_now(timer, ktime_set(0, uap->send_char_time));
> +
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_RESTART;
> +	}
> +
> +	/* Check if delay after send is set*/
> +	if (uap->port.rs485.delay_rts_after_send == 0) {
> +		/* Change state */
> +		uap->rs485_current_status = rs485_receiving;
> +		uap->rs485_next_status = rs485_receiving;
> +
> +		/* if there is no delay after send change RTS value*/
> +		RS485_SET_RTS_SIGNAL(uap,
> +			     uap->port.rs485.flags
> +				     & SER_RS485_RTS_AFTER_SEND);
> +
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
> +	}
> +
> +	/* Change state */
> +	uap->rs485_current_status = rs485_delay_after_send;
> +	uap->rs485_next_status = rs485_receiving;
> +
> +	/* RTS will be set in timer handler */
> +
> +	/* Start delay timer */
> +	ktime = ktime_set(0, (uap->port.rs485.delay_rts_after_send
> +			* 1000000L));
> +	hrtimer_start(&(uap->rs485_delay_timer), ktime, HRTIMER_MODE_REL);
> +
> +	spin_unlock_irqrestore(&uap->port.lock, flags);
> +	return HRTIMER_NORESTART;
> +}
> +
> +/*
> + * Change state according to pending delay
> + * Locking: port is locked in this function
> + */
> +static enum hrtimer_restart
> +pl011_rs485_timer(struct hrtimer *timer)
> +{
> +	unsigned long flags;
> +
> +	struct uart_amba_port *uap =
> +		container_of(timer, struct uart_amba_port, rs485_delay_timer);
> +
> +	spin_lock_irqsave(&uap->port.lock, flags);
> +
> +	if (uap->rs485_current_status == uap->rs485_next_status) {
> +		/* timer was canceled or handled */
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
How about moving the two statements at the end of the function, use a
goto here ...
> +	}
> +
> +	switch (uap->rs485_current_status) {
> +	case rs485_delay_before_send:
> +		uap->rs485_current_status = rs485_sending;
> +		uap->rs485_next_status = rs485_sending;
> +		if (!pl011_dma_tx_start(uap))
> +			pl011_start_tx_pio(uap);
> +
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
> +
> +	case rs485_delay_after_send:
> +		uap->rs485_current_status = rs485_receiving;
> +		uap->rs485_next_status = rs485_receiving;
> +		RS485_SET_RTS_SIGNAL(uap,
> +			     uap->port.rs485.flags
> +				     & SER_RS485_RTS_AFTER_SEND);
> +
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
> +
> +	default:
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +		return HRTIMER_NORESTART;
> +	}
and simplify all switch cases here.
> +}
> +
> +/*
> + * Evaluate transmit buffer status and start delay to off
> + * Locking: called with port lock held and IRQs disabled
> + */
> +static void pl011_rs485_start_rts_delay(struct uart_amba_port *uap)
> +{
> +	ktime_t ktime;
> +
> +	if (uap->rs485_current_status == rs485_receiving)
> +		return;
> +
> +	/* if there is timeout in progress cancel it and start new */
> +	hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
> +	hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
> +
> +
> +	if (!RS485_TX_FIFO_EMPTY(uap)
> +			|| uap->port.rs485.delay_rts_after_send == 0) {
> +		/*
> +		 * Schedule validation timer if there is data in TX FIFO
> +		 * because there is not TX FIFO empty interrupt
> +		 */
> +
> +		uap->rs485_current_status = rs485_sending;
> +		uap->rs485_next_status = rs485_sending;
> +
> +		uap->rs485_last_char_sending = false;
> +
> +		ktime = ktime_set(0, uap->send_char_time);
> +		hrtimer_start(&(uap->rs485_tx_empty_poll_timer),
> +			ktime,
> +			HRTIMER_MODE_REL);
> +		return;
> +	}
> +
> +	/* Change state */
> +	uap->rs485_current_status = rs485_delay_after_send;
> +	uap->rs485_next_status = rs485_receiving;
> +
> +	/* RTS will be set in timer handler */
> +
> +	/* Start timer */
> +	ktime = ktime_set(0, (uap->port.rs485.delay_rts_after_send
> +			* 1000000L));
> +
> +	hrtimer_start(&(uap->rs485_delay_timer),
> +		ktime,
> +		HRTIMER_MODE_REL);
> +}
> +#endif
> +
>  static irqreturn_t pl011_int(int irq, void *dev_id)
>  {
>  	struct uart_amba_port *uap = dev_id;
> @@ -1499,9 +1824,11 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
>  				else
>  					pl011_rx_chars(uap);
>  			}
> +
>  			if (status & (UART011_DSRMIS|UART011_DCDMIS|
>  				      UART011_CTSMIS|UART011_RIMIS))
>  				pl011_modem_status(uap);
> +
>  			if (status & UART011_TXIS)
>  				pl011_tx_chars(uap, true);
>  
> @@ -1618,6 +1945,11 @@ static void pl011_quiesce_irqs(struct uart_port *port)
>  	 */
>  	pl011_write(pl011_read(uap, REG_IMSC) & ~UART011_TXIM, uap,
>  		    REG_IMSC);
> +
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	if (uap->port.rs485.flags & SER_RS485_ENABLED)
> +		pl011_rs485_start_rts_delay(uap);
> +#endif
>  }
>  
>  static int pl011_get_poll_char(struct uart_port *port)
> @@ -1690,6 +2022,27 @@ static int pl011_hwinit(struct uart_port *port)
>  		if (plat->init)
>  			plat->init();
>  	}
> +
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	/*
> +	 * Initialize timers used for RS485
> +	 */
> +	hrtimer_init(&(uap->rs485_delay_timer),
> +		CLOCK_MONOTONIC,
> +		HRTIMER_MODE_REL);
> +
> +	uap->rs485_delay_timer.function = &pl011_rs485_timer;
> +
> +	hrtimer_init(&(uap->rs485_tx_empty_poll_timer),
> +		CLOCK_MONOTONIC,
> +		HRTIMER_MODE_REL);
> +
> +	uap->rs485_tx_empty_poll_timer.function = &pl011_rs485_tx_poll_timer;
> +
> +	uap->rs485_current_status = rs485_receiving;
> +	RS485_SET_RTS_SIGNAL(uap, false);
> +#endif
> +
>  	return 0;
>  }
>  
> @@ -1873,6 +2226,16 @@ static void pl011_shutdown(struct uart_port *port)
>  	struct uart_amba_port *uap =
>  		container_of(port, struct uart_amba_port, port);
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	if (uap->port.rs485.flags & SER_RS485_ENABLED) {
> +		hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
> +		hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
> +
> +		uap->rs485_current_status = rs485_receiving;
> +		RS485_SET_RTS_SIGNAL(uap, true);
> +	}
> +#endif
> +
>  	pl011_disable_interrupts(uap);
>  
>  	pl011_dma_shutdown(uap);
> @@ -1955,6 +2318,24 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
>  	unsigned long flags;
>  	unsigned int baud, quot, clkdiv;
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	unsigned int transfer_bit_count;
> +	unsigned long char_transfer_time;
> +
> +	/*
> +	 * Calculate bit count which will be send
> +	 * by UART. It is used for calculation of
> +	 * time required to start timer until TX FIFO (HW) is empty
> +	 * There is not interrupt for FIFO empty in PL011.
> +	 * There is only FIFO empty flag in REG_FR.
> +	 */
> +	transfer_bit_count = 0;
> +
> +#define	ADD_DATA_BITS(bits)	(transfer_bit_count += bits)
> +#else
> +#define	ADD_DATA_BITS(bits)
> +#endif
> +
>  	if (uap->vendor->oversampling)
>  		clkdiv = 8;
>  	else
> @@ -1981,29 +2362,53 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
>  	switch (termios->c_cflag & CSIZE) {
>  	case CS5:
>  		lcr_h = UART01x_LCRH_WLEN_5;
> +		ADD_DATA_BITS(7);
>  		break;
>  	case CS6:
>  		lcr_h = UART01x_LCRH_WLEN_6;
> +		ADD_DATA_BITS(8);
>  		break;
>  	case CS7:
>  		lcr_h = UART01x_LCRH_WLEN_7;
> +		ADD_DATA_BITS(9);
>  		break;
>  	default: // CS8
>  		lcr_h = UART01x_LCRH_WLEN_8;
> +		ADD_DATA_BITS(10);
>  		break;
>  	}
> -	if (termios->c_cflag & CSTOPB)
> +
> +	if (termios->c_cflag & CSTOPB) {
>  		lcr_h |= UART01x_LCRH_STP2;
> +		ADD_DATA_BITS(1);
> +	}
> +
>  	if (termios->c_cflag & PARENB) {
>  		lcr_h |= UART01x_LCRH_PEN;
> +		ADD_DATA_BITS(1);
> +
>  		if (!(termios->c_cflag & PARODD))
>  			lcr_h |= UART01x_LCRH_EPS;
> +
>  		if (termios->c_cflag & CMSPAR)
>  			lcr_h |= UART011_LCRH_SPS;
>  	}
> +
> +#undef ADD_DATA_BITS
> +
>  	if (uap->fifosize > 1)
>  		lcr_h |= UART01x_LCRH_FEN;
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	/* Calculate time required to send one char (nanoseconds) */
> +	char_transfer_time =
> +		(unsigned long) div_u64(
> +				mul_u32_u32(
> +					(u32)transfer_bit_count,
> +					(u32)NSEC_PER_SEC),
> +				(u32)baud);
> +#endif
> +
>  	spin_lock_irqsave(&port->lock, flags);
>  
>  	/*
> @@ -2020,6 +2425,11 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
>  	old_cr = pl011_read(uap, REG_CR);
>  	pl011_write(0, uap, REG_CR);
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	/* Update send_char_time in locked context */
> +	uap->send_char_time = char_transfer_time;
> +#endif
> +
>  	if (termios->c_cflag & CRTSCTS) {
>  		if (old_cr & UART011_CR_RTS)
>  			old_cr |= UART011_CR_RTSEN;
> @@ -2091,6 +2501,7 @@ static const char *pl011_type(struct uart_port *port)
>  {
>  	struct uart_amba_port *uap =
>  	    container_of(port, struct uart_amba_port, port);
> +
Please avoid unrelated whitespace changes
>  	return uap->port.type == PORT_AMBA ? uap->type : NULL;
>  }
>  
> @@ -2122,6 +2533,47 @@ static void pl011_config_port(struct uart_port *port, int flags)
>  	}
>  }
>  
> +/*
> + * Configure RS485
> + * Locking: called with port lock held and IRQs disabled
> + */
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +static int pl011_config_rs485(struct uart_port *port,
> +			      struct serial_rs485 *rs485)
> +{
> +	bool was_disabled;
> +	struct uart_amba_port *uap =
> +			container_of(port, struct uart_amba_port, port);
> +
> +	was_disabled = !(port->rs485.flags & SER_RS485_ENABLED);
> +
> +	port->rs485.flags = rs485->flags;
> +	port->rs485.delay_rts_after_send = rs485->delay_rts_after_send;
> +	port->rs485.delay_rts_before_send = rs485->delay_rts_before_send;
> +
> +	if (port->rs485.flags & SER_RS485_ENABLED) {
> +		unsigned int cr;
> +
> +		hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
> +		hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
> +
> +		/* If RS485 is enabled, disable auto RTS */
> +		cr = pl011_read(uap, REG_CR);
> +		cr &= ~UART011_CR_RTSEN;
> +		pl011_write(cr, uap, REG_CR);
> +
> +		uap->rs485_current_status = rs485_receiving;
> +		RS485_SET_RTS_SIGNAL(uap,
> +			     port->rs485.flags
> +				     & SER_RS485_RTS_AFTER_SEND);
> +	} else {
> +		RS485_SET_RTS_SIGNAL(uap, true);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * verify the new serial_struct (for TIOCSSERIAL).
>   */
> @@ -2647,6 +3099,12 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>  	uap->port.irq = dev->irq[0];
>  	uap->port.ops = &amba_pl011_pops;
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +	uap->port.rs485_config = &pl011_config_rs485;
> +	uap->port.rs485.flags = 0;	/* RS485 is not enabled by default */
> +	dev_info(&dev->dev, "Software switching for RS485 enabled\n");
> +#endif
> +
>  	snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
>  
>  	ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr);
> @@ -2819,10 +3277,15 @@ static struct amba_driver pl011_driver = {
>  
>  static int __init pl011_init(void)
>  {
> +#ifndef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
>  	printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
> +#else
> +	printk(KERN_INFO "Serial: AMBA PL011 UART driver with soft RS485 support\n");
> +#endif

I think one log message about RS485 support is enough, so please drop
the new dev_info above. An alternative solution for this ifdef could be
the usage of IS_ENABLED()

Thanks
Stefan

>  
>  	if (platform_driver_register(&arm_sbsa_uart_platform_driver))
>  		pr_warn("could not register SBSA UART platform driver\n");
> +
>  	return amba_driver_register(&pl011_driver);
>  }
>  
> @@ -2832,6 +3295,11 @@ static void __exit pl011_exit(void)
>  	amba_driver_unregister(&pl011_driver);
>  }
>  
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +#undef RS485_SET_RTS_SIGNAL
> +#undef RS485_TX_FIFO_EMPTY
> +#endif
> +
>  /*
>   * While this can be a module, if builtin it's most likely the console
>   * So let's leave module_exit but move module_init to an earlier place

Powered by blists - more mailing lists