On Wed, 2017-03-15 at 14:13 +0000, Dan O'Donovan wrote:
> Currently, Auto Flow Control is not working correctly on the Atom
> X5-Z8350 "Cherry Trail" SoC, because an "RTS override" feature is
> enabled in a vendor-specific register in the LPSS UART. The symptom
> is that RTS is not de-asserted as it should be when RTS/CTS flow
> control is enabled and the RX FIFO fills up.
>
> This appears to be introduced by commit 1f47a77c4e49 ("ACPI / LPSS:
> not using UART RTS override with Auto Flow Control").
>
> To _disable_ the RTS override, bit 3 needs to be _set_ in the
> "GENERAL" register at offset 808h. The power-on default is 0. The
> aforementioned commit appears to have assumed the inverse of this.
Thanks for spotting this.
Documentation I have says the same.
Nevertheless I would like to have Heikki's Ack before going with this
one.
Moreover, some tests on Baytrail and Cherrytrail platforms should be
performed. Do you have a test case step-by-step to reproduce the issue?
It might also explain the following (would be nice to test with this
revert reverted):
commit f967fc8f165fadb72166f2bd4785094b3ca21307
Author: Frederic Danis <[email protected]>
Date: Fri Oct 9 17:14:56 2015 +0200
Revert "serial: 8250_dma: don't bother DMA with small transfers"
See my comments regarding to code below.
>
> Signed-off-by: Dan O'Donovan <[email protected]>
> ---
> drivers/acpi/acpi_lpss.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index 8ea836c..4b3f2d5 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -45,7 +45,7 @@ ACPI_MODULE_NAME("acpi_lpss");
> #define LPSS_RESETS_RESET_APB BIT(1)
> #define LPSS_GENERAL 0x08
> #define LPSS_GENERAL_LTR_MODE_SW BIT(2)
> -#define LPSS_GENERAL_UART_RTS_OVRD BIT(3)
> +#define LPSS_GENERAL_UART_RTS_NO_OVRD BIT(3)
Better to use closer to documentation, i.e.
_RTS_DIS_OVRD
> #define LPSS_SW_LTR 0x10
> #define LPSS_AUTO_LTR 0x14
> #define LPSS_LTR_SNOOP_REQ BIT(15)
> @@ -123,10 +123,10 @@ static void lpss_uart_setup(struct
> lpss_private_data *pdata)
> writel(val | LPSS_TX_INT_MASK, pdata->mmio_base + offset);
>
> val = readl(pdata->mmio_base + LPSS_UART_CPR);
> - if (!(val & LPSS_UART_CPR_AFCE)) {
> + if (val & LPSS_UART_CPR_AFCE) {
Perhaps we need to clear it otherwise to be sure
something like
offset = pdata->dev_desc->prv_offset + LPSS_GENERAL;
val = readl(pdata->mmio_base + offset);
if (..._AFCE)
val |= LPSS_GENERAL_UART_RTS_DIS_OVRD;
else
val &= ~LPSS_GENERAL_UART_RTS_DIS_OVRD;
writel(val, pdata->mmio_base +
offset);
> offset = pdata->dev_desc->prv_offset + LPSS_GENERAL;
> val = readl(pdata->mmio_base + offset);
> - val |= LPSS_GENERAL_UART_RTS_OVRD;
> + val |= LPSS_GENERAL_UART_RTS_NO_OVRD;
> writel(val, pdata->mmio_base + offset);
> }
> }
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
