lkml.org 
[lkml]   [2014]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Re: [PATCH V4] serial/uart/8250: Add tunable RX interrupt trigger I/F of FIFO buffers
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@hitachi.com




\
 
 \ /
  Last update: 2014-03-20 10:01    [W:0.368 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site