s3c24xx : TIOCM_CTS handling problem

s3c24xx : TIOCM_CTS handling problem

[Matthieu CASTET]

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

Hi,

there is a problem in samsung soc serial driver with the report of
TIOCM_CTS in get_mctrl.

The uart layer check the status of the TIOCM_CTS in the open or
set_termios function and cache the result in the tty->hw_stopped flag.

Then when sending data, in the function uart_start, it check
tty->hw_stopped flag before calling the start_tx uart driver callback.


For the samsung uart driver it means :
- if the other side is not connected or in a reset state, then CTS
signal is not activated
- we open and configure the uart. get_mctrl don't report TIOCM_CTS flag.
uart layer set tty->hw_stopped flag.
- we connect or start the other side. CTS signal become activated.
- we send some characters on the uart. This call uart_write which fill
uart tx queue and call uart_start. uart_start doesn't call driver
start_tx because tty->hw_stopped flag is set.
- because driver start_tx is never call, the data is never send on the uart.

>From what I see, working driver managing TIOCM_CTS flag (amba-pl011,
8250), call uart_handle_cts_change when the uart driver detect a CTS
signal change (via a interrupt).

Because the samsung controller doesn't generate an interrupt when CTS
signal change, we can't call uart_handle_cts_change.

And easy fix could be to always report TIOCM_CTS.

I believe this bug is tricky and can affect other drivers.
Can this be documented in Documentation/serial/driver like the attached
patch ?


Matthieu

[-- Attachment #2: uart_doc --]
[-- Type: text/plain, Size: 663 bytes --]

diff --git a/Documentation/serial/driver b/Documentation/serial/driver
index 77ba0af..8395d5a 100644
--- a/Documentation/serial/driver
+++ b/Documentation/serial/driver
@@ -109,6 +109,10 @@ hardware.
 	the port does not support CTS, DCD or DSR, the driver should
 	indicate that the signal is permanently active.  If RI is
 	not available, the signal should not be indicated as active.
+	If the port does not support automatic detection of change of
+	CTS or DCD (and report via uart_handle_cts_change or 
+	uart_handle_dcd_change) the driver should indicate that the
+	signal is permanently active.
 
 	Locking: port->lock taken.
 	Interrupts: locally disabled.

Re: better Documentation for TIOCM_CTS handling

[Matthieu CASTET]

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

Hi,

can we add in the documentation that if the driver want to manage CTS or
DCD, it should be able to report the change with uart_handle_xx_change
function ?

Something like the attached patch.


Thanks


Matthieu

[-- Attachment #2: uart_doc --]
[-- Type: text/plain, Size: 663 bytes --]

diff --git a/Documentation/serial/driver b/Documentation/serial/driver
index 77ba0af..8395d5a 100644
--- a/Documentation/serial/driver
+++ b/Documentation/serial/driver
@@ -109,6 +109,10 @@ hardware.
 	the port does not support CTS, DCD or DSR, the driver should
 	indicate that the signal is permanently active.  If RI is
 	not available, the signal should not be indicated as active.
+	If the port does not support automatic detection of change of
+	CTS or DCD (and report via uart_handle_cts_change or 
+	uart_handle_dcd_change) the driver should indicate that the
+	signal is permanently active.
 
 	Locking: port->lock taken.
 	Interrupts: locally disabled.

Re: s3c24xx : TIOCM_CTS handling problem

[Matthieu CASTET]

Matthieu CASTET a écrit :
> Hi,
> 
> there is a problem in samsung soc serial driver with the report of
> TIOCM_CTS in get_mctrl.
> 
> The uart layer check the status of the TIOCM_CTS in the open or
> set_termios function and cache the result in the tty->hw_stopped flag.
> 
> Then when sending data, in the function uart_start, it check
> tty->hw_stopped flag before calling the start_tx uart driver callback.
> 
> 
> For the samsung uart driver it means :
> - if the other side is not connected or in a reset state, then CTS
> signal is not activated
> - we open and configure the uart. get_mctrl don't report TIOCM_CTS flag.
> uart layer set tty->hw_stopped flag.
> - we connect or start the other side. CTS signal become activated.
> - we send some characters on the uart. This call uart_write which fill
> uart tx queue and call uart_start. uart_start doesn't call driver
> start_tx because tty->hw_stopped flag is set.
> - because driver start_tx is never call, the data is never send on the uart.
> 
>>From what I see, working driver managing TIOCM_CTS flag (amba-pl011,
> 8250), call uart_handle_cts_change when the uart driver detect a CTS
> signal change (via a interrupt).
> 
> Because the samsung controller doesn't generate an interrupt when CTS
> signal change, we can't call uart_handle_cts_change.
> 
> And easy fix could be to always report TIOCM_CTS.
> 
Ping ?

The bug is still present on git tree and I have got no replies.


Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: s3c24xx : TIOCM_CTS handling problem

[Alan Cox]

> > And easy fix could be to always report TIOCM_CTS.
> > 
> Ping ?
> 
> The bug is still present on git tree and I have got no replies.

Send patches instead then ;)

If the chip can sense the CTS line then you could check on a timer event
but it might be cleaner to add the functionality needed to allow a driver
to say "always call my start_tx" method.

Re: s3c24xx : TIOCM_CTS handling problem

[Matthieu CASTET]

Hi Alan,

Alan Cox a écrit :
>>> And easy fix could be to always report TIOCM_CTS.
>>>
>> Ping ?
>>
>> The bug is still present on git tree and I have got no replies.
> 
> Send patches instead then ;)
> 
> If the chip can sense the CTS line then you could check on a timer event
May be it could be usefull to do something generic that call driver
get_mctrl instead of doing it in the driver (like it is done in
bfin_5xx, sa1100, ...)

> but it might be cleaner to add the functionality needed to allow a driver
> to say "always call my start_tx" method.
Or to say "recheck get_mctrl before my start_tx).
But have you got an idea how the driver could ask that to the serial core ?
flags in struct uart_port seems full.


Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html