Re: Re: [PATCH V4] serial/uart/8250: Add tunable RX interrupt trigger I/F of FIFO buffers
From: Yoshihiro YUNOMAE
Date: Thu Mar 20 2014 - 04:13:38 EST
Hi Stephen,
Thank you for your reply.
(2014/03/20 4:51), Stephen Warren wrote:
On 03/19/2014 04:15 AM, Yoshihiro YUNOMAE wrote:
Add tunable RX interrupt trigger I/F of FIFO buffers.
Serial devices are used as not only message communication devices but control
or sending communication devices. For the latter uses, normally small data
will be exchanged, so user applications want to receive data unit as soon as
possible for real-time tendency. If we have a sensor which sends a 1 byte data
each time and must control a device based on the sensor feedback, the RX
interrupt should be triggered for each data.
According to HW specification of serial UART devices, RX interrupt trigger
can be changed, but the trigger is hard-coded. For example, RX interrupt trigger
in 16550A can be set to 1, 4, 8, or 14 bytes for HW, but current driver sets
the trigger to only 8bytes.
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 69932b7..fc17fb2 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -531,11 +531,10 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
{
- unsigned char fcr;
-
serial8250_clear_fifos(p);
- fcr = uart_config[p->port.type].fcr;
- serial_out(p, UART_FCR, fcr);
+ if (!p->fcr)
+ p->fcr = uart_config[p->port.type].fcr;
Rather than sprinkling this initialization all over the place, can't the
fcr value be set up one time, when the port is first created?
It's nice idea. If we can store default value of fcr to up->fcr when
the port is first created, we don't need to check whether p->fcr exists
or not.
Maybe by adding the operation after register_dev_spec_attr_grp()
in my patch, we can avoid the check.
+static int convert_fcr2val(struct uart_8250_port *up, unsigned char fcr)
+{
+ unsigned char trig_raw = fcr & UART_FCR_TRIGGER_MASK;
+
+ switch (up->port.type) {
+ case PORT_16550A:
+ if (trig_raw == UART_FCR_R_TRIG_00)
+ return 1;
+ else if (trig_raw == UART_FCR_R_TRIG_01)
+ return 4;
+ else if (trig_raw == UART_FCR_R_TRIG_10)
+ return 8;
+ else if (trig_raw == UART_FCR_R_TRIG_11)
+ return 14;
+ break;
+ }
+ return -EOPNOTSUPP;
+}
Rather than implementing this translation inside this one function
(well, and convert_val2rxtrig() too), perhaps it would make sense to put
the values into uart_config[] and just look them up here. That would
better isolate the type-specific data into one place.
Sure. I'll apply following implementation:
[PORT_16550A] = {
.name = "16550A",
.fifo_size = 16,
.tx_loadsz = 16,
.fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+ .rx_trig_byte = {1, 4, 8, 14},
.flags = UART_CAP_FIFO,
};
The comment right before UART_FCR_R_TRIG_00 in
include/uapi/linux/serial_reg.h might help when filling in any extra
fields in uart_config[].
OK, I'll add following rx_trig_byte to PORT_16650V2, PORT_16654,
PORT_16750, and PORT_TEGRA in uart_config[].
/*
* Note: The FIFO trigger levels are chip specific:
* RX:76 = 00 01 10 11 TX:54 = 00 01 10 11
* PC16550D: 1 4 8 14 xx xx xx xx
* TI16C550A: 1 4 8 14 xx xx xx xx
* TI16C550C: 1 4 8 14 xx xx xx xx
* ST16C550: 1 4 8 14 xx xx xx xx
* ST16C650: 8 16 24 28 16 8 24 30 PORT_16650V2
* NS16C552: 1 4 8 14 xx xx xx xx
* ST16C654: 8 16 56 60 8 16 32 56 PORT_16654
* TI16C750: 1 16 32 56 xx xx xx xx PORT_16750
* TI16C752: 8 16 56 60 8 16 32 56
* Tegra: 1 4 8 14 16 8 4 1 PORT_TEGRA
*/
Thanks,
Yoshihiro YUNOMAE
--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/