[PATCH v2 0/3] Handle UART without interrupt on TEMT using em485

[PATCH v2 0/3] Handle UART without interrupt on TEMT using em485

[Eric Tremblay]

Thanks everyone for the comments. I apply most of the comments on version 1
but there is still a pending point with the Jiri comment about the safety of:
struct tty_struct *tty = p->port.state->port.tty;
I thought about adding a check with tty_port_initialized() before accessing
the pointer, but I saw some other places where that same pointer is accessed
without further protection, at least from what I see. 

Changes from v1 to v2:
- Use UART_CAP_NOTEMT instead of UART_CAP_TEMT
- Use some predefined macro to reduce magicness
- Reset active_timer in temt timer handler
- add uart_get_byte_size
- set UART_CAP_NOTEMT in uart_config for PORT_16550A_FSL64
- Improve commit messages
- Improve grammar and spelling
- Add Giulio and Heiko SoB to reflect previous work

Eric Tremblay (3):
  serial: 8250: Handle UART without interrupt on TEMT using em485
  serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64
  serial: 8250: add compatible for fsl,16550-FIFO64

 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_of.c   |  2 +
 drivers/tty/serial/8250/8250_port.c | 68 ++++++++++++++++++++++++++++-
 drivers/tty/serial/serial_core.c    | 29 ++++++++----
 include/linux/serial_8250.h         |  2 +
 include/linux/serial_core.h         |  2 +
 6 files changed, 94 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[Eric Tremblay]

The patch introduce the UART_CAP_NOTEMT capability. The capability
indicate that the UART doesn't have an interrupt available on TEMT.

In the case where the device does not support it, we calculate the
maximum time it could take for the transmitter to empty the
shift register. When we get in the situation where we get the
THRE interrupt, we check if the TEMT bit is set. If it's not, we start
the a timer and recall __stop_tx() after the delay.

The transmit sequence is a bit modified when the capability is set. The
new timer is used between the last interrupt(THRE) and a potential
stop_tx timer.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
[moved to use added UART_CAP_TEMT]
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
[moved to use added UART_CAP_NOTEMT, improve timeout]
Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>
---
 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_port.c | 66 ++++++++++++++++++++++++++++-
 drivers/tty/serial/serial_core.c    | 29 +++++++++----
 include/linux/serial_8250.h         |  2 +
 include/linux/serial_core.h         |  2 +
 5 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 52bb21205bb6..6bb6b7321cfc 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -82,6 +82,7 @@ struct serial8250_config {
 #define UART_CAP_MINI	(1 << 17)	/* Mini UART on BCM283X family lacks:
 					 * STOP PARITY EPAR SPAR WLEN5 WLEN6
 					 */
+#define UART_CAP_NOTEMT	(1 << 18)	/* UART without interrupt on TEMT available*/
 
 #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
 #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index b0af13074cd3..00c2cb64e8a7 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -558,8 +558,21 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
 	}
 }
 
+static inline void serial8250_em485_update_temt_delay(struct uart_8250_port *p,
+			unsigned int cflag, unsigned int baud)
+{
+	unsigned int bits;
+
+	if (!p->em485)
+		return;
+
+	bits = uart_get_byte_size(cflag);
+	p->em485->no_temt_delay = bits * NSEC_PER_SEC / baud;
+}
+
 static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t);
 static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t);
+static enum hrtimer_restart serial8250_em485_handle_no_temt(struct hrtimer *t);
 
 void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
 {
@@ -618,6 +631,16 @@ static int serial8250_em485_init(struct uart_8250_port *p)
 		     HRTIMER_MODE_REL);
 	hrtimer_init(&p->em485->start_tx_timer, CLOCK_MONOTONIC,
 		     HRTIMER_MODE_REL);
+
+	if (p->capabilities & UART_CAP_NOTEMT) {
+		struct tty_struct *tty = p->port.state->port.tty;
+
+		serial8250_em485_update_temt_delay(p, tty->termios.c_cflag,
+						   tty_get_baud_rate(tty));
+		hrtimer_init(&p->em485->no_temt_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		p->em485->no_temt_timer.function = &serial8250_em485_handle_no_temt;
+	}
+
 	p->em485->stop_tx_timer.function = &serial8250_em485_handle_stop_tx;
 	p->em485->start_tx_timer.function = &serial8250_em485_handle_start_tx;
 	p->em485->port = p;
@@ -649,6 +672,7 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
 
 	hrtimer_cancel(&p->em485->start_tx_timer);
 	hrtimer_cancel(&p->em485->stop_tx_timer);
+	hrtimer_cancel(&p->em485->no_temt_timer);
 
 	kfree(p->em485);
 	p->em485 = NULL;
@@ -1494,6 +1518,11 @@ static void start_hrtimer_ms(struct hrtimer *hrt, unsigned long msec)
 	hrtimer_start(hrt, t, HRTIMER_MODE_REL);
 }
 
+static void start_hrtimer_ns(struct hrtimer *hrt, unsigned long nsec)
+{
+	hrtimer_start(hrt, ns_to_ktime(nsec), HRTIMER_MODE_REL);
+}
+
 static void __stop_tx_rs485(struct uart_8250_port *p)
 {
 	struct uart_8250_em485 *em485 = p->em485;
@@ -1531,8 +1560,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
 		 * shift register are empty. It is for device driver to enable
 		 * interrupt on TEMT.
 		 */
-		if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
+		if ((lsr & BOTH_EMPTY) != BOTH_EMPTY) {
+			if (!(p->capabilities & UART_CAP_NOTEMT))
+				return;
+
+			/*
+			 * On devices with no TEMT interrupt available, start
+			 * a timer for a byte time. The timer will recall
+			 * __stop_tx().
+			 */
+			em485->active_timer = &em485->no_temt_timer;
+			start_hrtimer_ns(&em485->no_temt_timer, em485->no_temt_delay);
 			return;
+		}
 
 		__stop_tx_rs485(p);
 	}
@@ -1631,6 +1671,27 @@ static inline void start_tx_rs485(struct uart_port *port)
 	__start_tx(port);
 }
 
+static enum hrtimer_restart serial8250_em485_handle_no_temt(struct hrtimer *t)
+{
+	struct uart_8250_em485 *em485;
+	struct uart_8250_port *p;
+	unsigned long flags;
+
+	em485 = container_of(t, struct uart_8250_em485, no_temt_timer);
+	p = em485->port;
+
+	serial8250_rpm_get(p);
+	spin_lock_irqsave(&p->port.lock, flags);
+	if (em485->active_timer == &em485->no_temt_timer) {
+		__stop_tx(p);
+		em485->active_timer = NULL;
+	}
+
+	spin_unlock_irqrestore(&p->port.lock, flags);
+	serial8250_rpm_put(p);
+	return HRTIMER_NORESTART;
+}
+
 static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t)
 {
 	struct uart_8250_em485 *em485;
@@ -2792,6 +2853,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	serial8250_set_divisor(port, baud, quot, frac);
 
+	if (up->capabilities & UART_CAP_NOTEMT)
+		serial8250_em485_update_temt_delay(up, termios->c_cflag, baud);
+
 	/*
 	 * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
 	 * is written without DLAB set, this mode will be disabled.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 828f9ad1be49..8aab9384e6b4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -322,17 +322,12 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 }
 
 /**
- *	uart_update_timeout - update per-port FIFO timeout.
- *	@port:  uart_port structure describing the port
+ *	uart_get_byte_size
  *	@cflag: termios cflag value
- *	@baud:  speed of the port
  *
- *	Set the port FIFO timeout value.  The @cflag value should
- *	reflect the actual hardware settings.
+ * Get the size of a byte in bits.
  */
-void
-uart_update_timeout(struct uart_port *port, unsigned int cflag,
-		    unsigned int baud)
+unsigned int uart_get_byte_size(unsigned int cflag)
 {
 	unsigned int bits;
 
@@ -357,6 +352,24 @@ uart_update_timeout(struct uart_port *port, unsigned int cflag,
 	if (cflag & PARENB)
 		bits++;
 
+	return bits;
+}
+
+/**
+ *	uart_update_timeout - update per-port FIFO timeout.
+ *	@port:  uart_port structure describing the port
+ *	@cflag: termios cflag value
+ *	@baud:  speed of the port
+ *
+ *	Set the port FIFO timeout value.  The @cflag value should
+ *	reflect the actual hardware settings.
+ */
+void
+uart_update_timeout(struct uart_port *port, unsigned int cflag,
+		    unsigned int baud)
+{
+	unsigned int bits = uart_get_byte_size(cflag);
+
 	/*
 	 * The total number of bits to be transmitted in the fifo.
 	 */
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 9e655055112d..ec8682e8c19e 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -79,7 +79,9 @@ struct uart_8250_ops {
 struct uart_8250_em485 {
 	struct hrtimer		start_tx_timer; /* "rs485 start tx" timer */
 	struct hrtimer		stop_tx_timer;  /* "rs485 stop tx" timer */
+	struct hrtimer		no_temt_timer;  /* "rs485 no TEMT interrupt" timer */
 	struct hrtimer		*active_timer;  /* pointer to active timer */
+	unsigned long		no_temt_delay;  /* Delay for no_temt_timer */
 	struct uart_8250_port	*port;          /* for hrtimer callbacks */
 	unsigned int		tx_stopped:1;	/* tx is currently stopped */
 };
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index e1b684e33841..ecc0bbf5135e 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -332,6 +332,8 @@ unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termios
 				unsigned int max);
 unsigned int uart_get_divisor(struct uart_port *port, unsigned int baud);
 
+unsigned int uart_get_byte_size(unsigned int cflag);
+
 /* Base timer interval for polling */
 static inline int uart_poll_timeout(struct uart_port *port)
 {
-- 
2.17.1

[PATCH v2 2/3] serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64

[Eric Tremblay]

This port doesn't have an interrupt on TEMT available when using
the FIFO mode.

Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>
---
 drivers/tty/serial/8250/8250_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 00c2cb64e8a7..dd904f736c61 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -262,7 +262,7 @@ static const struct serial8250_config uart_config[] = {
 		.tx_loadsz	= 63,
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 |
 				  UART_FCR7_64BYTE,
-		.flags		= UART_CAP_FIFO,
+		.flags		= UART_CAP_FIFO | UART_CAP_NOTEMT,
 	},
 	[PORT_RT2880] = {
 		.name		= "Palmchip BK-3103",
-- 
2.17.1

[PATCH v2 3/3] serial: 8250: add compatible for fsl,16550-FIFO64

[Eric Tremblay]

Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>
---
 drivers/tty/serial/8250/8250_of.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 65e9045dafe6..4efc62c0b25c 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -313,6 +313,8 @@ static const struct of_device_id of_platform_serial_table[] = {
 		.data = (void *)PORT_ALTR_16550_F64, },
 	{ .compatible = "altr,16550-FIFO128",
 		.data = (void *)PORT_ALTR_16550_F128, },
+	{ .compatible = "fsl,16550-FIFO64",
+		.data = (void *)PORT_16550A_FSL64, },
 	{ .compatible = "mediatek,mtk-btif",
 		.data = (void *)PORT_MTK_BTIF, },
 	{ .compatible = "mrvl,mmp-uart",
-- 
2.17.1

Re: [PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]

Hi Eric,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on usb/usb-testing v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Eric-Tremblay/Handle-UART-without-interrupt-on-TEMT-using-em485/20210205-002255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: ia64-randconfig-r032-20210204 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/070c956e2d260c56b13f43b7d092b4a20664248c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Tremblay/Handle-UART-without-interrupt-on-TEMT-using-em485/20210205-002255
        git checkout 070c956e2d260c56b13f43b7d092b4a20664248c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "uart_get_byte_size" [drivers/tty/serial/8250/8250_base.ko] undefined!

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30617 bytes --]

Re: [PATCH v2 0/3] Handle UART without interrupt on TEMT using em485

[Andy Shevchenko]

On Thu, Feb 04, 2021 at 11:11:55AM -0500, Eric Tremblay wrote:
> Thanks everyone for the comments. I apply most of the comments on version 1
> but there is still a pending point with the Jiri comment about the safety of:
> struct tty_struct *tty = p->port.state->port.tty;
> I thought about adding a check with tty_port_initialized() before accessing
> the pointer, but I saw some other places where that same pointer is accessed
> without further protection, at least from what I see.

Thanks for the update. Unfortunately I'm a bit busy with other prioritized
stuff, but I will review this next week.

> Changes from v1 to v2:
> - Use UART_CAP_NOTEMT instead of UART_CAP_TEMT
> - Use some predefined macro to reduce magicness
> - Reset active_timer in temt timer handler
> - add uart_get_byte_size
> - set UART_CAP_NOTEMT in uart_config for PORT_16550A_FSL64
> - Improve commit messages
> - Improve grammar and spelling
> - Add Giulio and Heiko SoB to reflect previous work
> 
> Eric Tremblay (3):
>   serial: 8250: Handle UART without interrupt on TEMT using em485
>   serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64
>   serial: 8250: add compatible for fsl,16550-FIFO64
> 
>  drivers/tty/serial/8250/8250.h      |  1 +
>  drivers/tty/serial/8250/8250_of.c   |  2 +
>  drivers/tty/serial/8250/8250_port.c | 68 ++++++++++++++++++++++++++++-
>  drivers/tty/serial/serial_core.c    | 29 ++++++++----
>  include/linux/serial_8250.h         |  2 +
>  include/linux/serial_core.h         |  2 +
>  6 files changed, 94 insertions(+), 10 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] Handle UART without interrupt on TEMT using em485

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

Hello,

On Fri, Feb 05, 2021 at 04:20:00PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 04, 2021 at 11:11:55AM -0500, Eric Tremblay wrote:
> > Thanks everyone for the comments. I apply most of the comments on version 1
> > but there is still a pending point with the Jiri comment about the safety of:
> > struct tty_struct *tty = p->port.state->port.tty;
> > I thought about adding a check with tty_port_initialized() before accessing
> > the pointer, but I saw some other places where that same pointer is accessed
> > without further protection, at least from what I see.
> 
> Thanks for the update. Unfortunately I'm a bit busy with other prioritized
> stuff, but I will review this next week.

I assume this fell through the cracks as "next week" is already over ...?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/3] Handle UART without interrupt on TEMT using em485

[Andy Shevchenko]

On Wed, Sep 29, 2021 at 10:07:47PM +0200, Uwe Kleine-König wrote:
> On Fri, Feb 05, 2021 at 04:20:00PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 04, 2021 at 11:11:55AM -0500, Eric Tremblay wrote:
> > > Thanks everyone for the comments. I apply most of the comments on version 1
> > > but there is still a pending point with the Jiri comment about the safety of:
> > > struct tty_struct *tty = p->port.state->port.tty;
> > > I thought about adding a check with tty_port_initialized() before accessing
> > > the pointer, but I saw some other places where that same pointer is accessed
> > > without further protection, at least from what I see.
> > 
> > Thanks for the update. Unfortunately I'm a bit busy with other prioritized
> > stuff, but I will review this next week.
> 
> I assume this fell through the cracks as "next week" is already over ...?

Indeed. Anyways, there is a kbuildbot message against patch 1 as I see and
needs to be addressed. I'm going to look into the code right now.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[Andy Shevchenko]

On Thu, Feb 04, 2021 at 11:11:56AM -0500, Eric Tremblay wrote:
> The patch introduce the UART_CAP_NOTEMT capability. The capability

s/The patch//

> indicate that the UART doesn't have an interrupt available on TEMT.

indicates

> In the case where the device does not support it, we calculate the
> maximum time it could take for the transmitter to empty the
> shift register. When we get in the situation where we get the
> THRE interrupt, we check if the TEMT bit is set. If it's not, we start
> the a timer and recall __stop_tx() after the delay.
> 
> The transmit sequence is a bit modified when the capability is set. The
> new timer is used between the last interrupt(THRE) and a potential
> stop_tx timer.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> [moved to use added UART_CAP_TEMT]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> [moved to use added UART_CAP_NOTEMT, improve timeout]
> Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>

Does it need Co-developed-by tag(s)?

...

> +#define UART_CAP_NOTEMT	(1 << 18)	/* UART without interrupt on TEMT available*/

Missed space in the comment .

...

> +static inline void serial8250_em485_update_temt_delay(struct uart_8250_port *p,
> +			unsigned int cflag, unsigned int baud)
> +{
> +	unsigned int bits;
> +
> +	if (!p->em485)
> +		return;
> +
> +	bits = uart_get_byte_size(cflag);

Should be rebased. We have tty_get_frame_size() for a while.

> +	p->em485->no_temt_delay = bits * NSEC_PER_SEC / baud;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] serial: 8250: Add UART_CAP_NOTEMT on PORT_16550A_FSL64

[Andy Shevchenko]

On Thu, Feb 04, 2021 at 11:11:57AM -0500, Eric Tremblay wrote:
> This port doesn't have an interrupt on TEMT available when using

s/This port/Port on Freescale ... (or what is that?)/

> the FIFO mode.

With amended commit message
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>
> ---
>  drivers/tty/serial/8250/8250_port.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 00c2cb64e8a7..dd904f736c61 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -262,7 +262,7 @@ static const struct serial8250_config uart_config[] = {
>  		.tx_loadsz	= 63,
>  		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 |
>  				  UART_FCR7_64BYTE,
> -		.flags		= UART_CAP_FIFO,
> +		.flags		= UART_CAP_FIFO | UART_CAP_NOTEMT,
>  	},
>  	[PORT_RT2880] = {
>  		.name		= "Palmchip BK-3103",
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] serial: 8250: add compatible for fsl,16550-FIFO64

[Andy Shevchenko]

On Thu, Feb 04, 2021 at 11:11:58AM -0500, Eric Tremblay wrote:

Needs a commit message.

> Signed-off-by: Eric Tremblay <etremblay@distech-controls.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

Hello,

On Fri, Feb 05, 2021 at 05:36:44AM +0800, kernel test robot wrote:
> Hi Eric,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on tty/tty-testing]
> [also build test ERROR on usb/usb-testing v5.11-rc6 next-20210125]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Eric-Tremblay/Handle-UART-without-interrupt-on-TEMT-using-em485/20210205-002255
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
> config: ia64-randconfig-r032-20210204 (attached as .config)
> compiler: ia64-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/070c956e2d260c56b13f43b7d092b4a20664248c
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Tremblay/Handle-UART-without-interrupt-on-TEMT-using-em485/20210205-002255
>         git checkout 070c956e2d260c56b13f43b7d092b4a20664248c
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> ERROR: modpost: "uart_get_byte_size" [drivers/tty/serial/8250/8250_base.ko] undefined!

FTR: This is a missing EXPORT_SYMBOL_GPL for uart_get_byte_size().

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[Andy Shevchenko]

On Fri, Oct 01, 2021 at 02:30:33PM +0200, Uwe Kleine-König wrote:
> On Fri, Feb 05, 2021 at 05:36:44AM +0800, kernel test robot wrote:

> > >> ERROR: modpost: "uart_get_byte_size" [drivers/tty/serial/8250/8250_base.ko] undefined!
> 
> FTR: This is a missing EXPORT_SYMBOL_GPL for uart_get_byte_size().

It seems we don't need that function anymore since we have similar one already.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] serial: 8250: Handle UART without interrupt on TEMT using em485

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

Hello,

[dropped christoph.muellner@theobroma-systems.com from Cc: as the
address doesn't seem to work any more]

On Thu, Feb 04, 2021 at 11:11:56AM -0500, Eric Tremblay wrote:
> The patch introduce the UART_CAP_NOTEMT capability. The capability
> indicate that the UART doesn't have an interrupt available on TEMT.
> 
> In the case where the device does not support it, we calculate the
> maximum time it could take for the transmitter to empty the
> shift register. When we get in the situation where we get the
> THRE interrupt, we check if the TEMT bit is set. If it's not, we start
> the a timer and recall __stop_tx() after the delay.
> 
> The transmit sequence is a bit modified when the capability is set. The
> new timer is used between the last interrupt(THRE) and a potential
> stop_tx timer.

I wonder if the required change can be simplified by just increasing the
stop_tx_timer timeout as there is nothing that has to happen when the
shifter is empty apart from starting the stop_tx timer.

This would require a good documentation because the semantic of the stop
timer would change.

@Eric: Do you plan to address the feedback comments here? I'd volunteer
as a tester if yes. Otherwise there might be some incentive to work on
this series myself.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]