On 08/06/2015 07:25 AM, Peter Hurley wrote:
> On 08/06/2015 02:14 AM, Tim Kryger wrote:
>> On Wed, Jul 29, 2015 at 3:23 AM, Qipeng Zha <qipeng.zha@xxxxxxxxx> wrote:
>>> According to DesignWare 8250 spec, if auto flow control
>>> mode is enabled, a change in CTS does not cause an interrupt,
>>> so sw-assisted CTS flow control mode will not work properly.
>>>
>>> There reported an GPS firmware download failure issue, and we
>>> verified the root cause is, the default sw-assisted CTS flow
>>> control mode can not work properly since no interrupt when got
>>> CTS signal.
>>>
>>> This patch is to enable auto CTS mode by defaut if CRTSCTS
>>> is enable for DesignWare 8250 controller.
>>>
>>> Signed-off-by: Huiquan Zhong <huiquan.zhong@xxxxxxxxx>
>>> Signed-off-by: Qipeng Zha <qipeng.zha@xxxxxxxxx>
>>>
>>> ---
>>> change in V3:
>>> update author name format;
>>> change in V2:
>>> update ACPI enumerated device driver accordingly;
>>> ---
>>> drivers/tty/serial/8250/8250_dw.c | 5 +++++
>>> drivers/tty/serial/8250/8250_pci.c | 4 ++++
>>> 2 files changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>>> index 6ae5b85..f78afeb 100644
>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>> @@ -258,6 +258,11 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
>>>
>>> if (!ret)
>>> p->uartclk = rate;
>>> +
>>> + p->status &= ~UPSTAT_AUTOCTS;
>>> + if (termios->c_cflag & CRTSCTS)
>>> + p->status |= UPSTAT_AUTOCTS;
>>> +
>>> out:
>>> serial8250_do_set_termios(p, termios, old);
>>> }
>>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>>> index 892eb32..5aeabe4 100644
>>> --- a/drivers/tty/serial/8250/8250_pci.c
>>> +++ b/drivers/tty/serial/8250/8250_pci.c
>>> @@ -1438,6 +1438,10 @@ byt_set_termios(struct uart_port *p, struct ktermios *termios,
>>> reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE;
>>> writel(reg, p->membase + BYT_PRV_CLK);
>>>
>>> + p->status &= ~UPSTAT_AUTOCTS;
>>> + if (termios->c_cflag & CRTSCTS)
>>> + p->status |= UPSTAT_AUTOCTS;
>>> +
>>> serial8250_do_set_termios(p, termios, old);
>>> }
>>>
>>> --
>>
>> This isn't right.
>>
>> The Designware UART IP may be configured without auto flow control support.
>>
>> Also, 8250s indicate if they can do automatic flow control with UART_CAP_AFE.
>>
>> This capability is unconditionally set for Bay Trail in byt_serial_setup().
>>
>> However, dw8250_setup_port() reads out the component parameter register to
>> see how the IP block was configured and only sets the capability when it is
>> supported. I assume most, but not all, do.
>>
>> Some other 8250 based devices also support this capability. What about them?
>>
>> Why not set UPSTAT_AUTOCTS in serial8250_do_set_termios() at the same time
>> as UART_MCR_AFE is set to actually turn on the automatic flow control?
>
> Because it's unclear if those other 8250 based devices which declare UART_CAP_AFE
> generate interrupts for DCTS or not.
>
> If UART_CAP_AFE is set and interrupts are generated for DCTS, then there is no
> requirement to set UPSTAT_AUTOCTS because the software state will accurately
> reflect the hardware state. When I added UPSTAT_AUTOCTS for devices which do
> not generate interrupts for DCTS, I preserved the _existing driver behavior_
> for UART_CAP_AFE devices.
>
> If the DW 8250 does not generate interrupts for DCTS when UART_MCR_AFE is set,
> then UPSTAT_AUTOCTS is required.
> That's as simple as checking the capability bit in dw8250_set_termios().
In retrospect, that statement is a little vague. I meant like this:
--- >% ---
Subject: [PATCH] serial: 8250_dw: Only indicate autoCTS if UART_CAP_AFE
Not all DesignWare 8250 IPs may have been configured to support
UART_CAP_AFE. dw8250_setup_port() reads out the component parameter
register to see how the IP block was configured and only sets the
capability when it is supported.
Test UART_CAP_AFE capability before indicating autoCTS status.
Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
drivers/tty/serial/8250/8250_dw.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 8d04ea7..e0bc5f1 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -238,6 +238,7 @@ dw8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
struct ktermios *old)
{
+ struct uart_8250_port *up = up_to_u8250p(p);
unsigned int baud = tty_termios_baud_rate(termios);
struct dw8250_data *d = p->private_data;
unsigned int rate;
@@ -258,9 +259,11 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
if (!ret)
p->uartclk = rate;
- p->status &= ~UPSTAT_AUTOCTS;
- if (termios->c_cflag & CRTSCTS)
- p->status |= UPSTAT_AUTOCTS;
+ if (up->capabilities & UART_CAP_AFE) {
+ p->status &= ~UPSTAT_AUTOCTS;
+ if (termios->c_cflag & CRTSCTS)
+ p->status |= UPSTAT_AUTOCTS;
+ }
out:
serial8250_do_set_termios(p, termios, old);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
