[PATCH v2 0/3] Fix DNV HSUART RX DMA timeout interrupt issue

[PATCH v2 0/3] Fix DNV HSUART RX DMA timeout interrupt issue

[Chuah Kim Tatt]

From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>

These patches fix a DNV HSUART DMA issue with timeout interrupts, where
RX data is stuck in buffer when RX DMA is used and the number of received
bytes is less than 4096.
These patches have been tested on Intel Denverton platform.

Changes from v1:
  - Added patch "serial: 8250_dma: Export serial8250_rx_dma_flush()" to solve
    build error when CONFIG_SERIAL_8250_MID is set to "m".

Chuah, Kim Tatt (3):
  dmaengine: hsu: Export hsu_dma_get_status()
  serial: 8250_dma: Export serial8250_rx_dma_flush()
  serial: 8250_mid: Read RX buffer on RX DMA timeout for DNV

 drivers/dma/hsu/hsu.c              | 90 +++++++++++++++++++++++++++++---------
 drivers/dma/hsu/pci.c              | 11 ++++-
 drivers/tty/serial/8250/8250_dma.c |  1 +
 drivers/tty/serial/8250/8250_mid.c | 24 +++++++---
 include/linux/dma/hsu.h            | 14 ++++--
 5 files changed, 109 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] dmaengine: hsu: Export hsu_dma_get_status()

[Chuah Kim Tatt]

From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>

To allow other code to safely read DMA Channel Status Register (where
the register attribute for Channel Error, Descriptor Time Out &
Descriptor Done fields are read-clear), export hsu_dma_get_status().
hsu_dma_irq() is renamed to hsu_dma_do_irq() and requires Status
Register value to be passed in.

Signed-off-by: Chuah, Kim Tatt <kim.tatt.chuah@intel.com>
---
 drivers/dma/hsu/hsu.c              | 90 +++++++++++++++++++++++++++++---------
 drivers/dma/hsu/pci.c              | 11 ++++-
 drivers/tty/serial/8250/8250_mid.c | 22 +++++++---
 include/linux/dma/hsu.h            | 14 ++++--
 4 files changed, 106 insertions(+), 31 deletions(-)

diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
index f8c5cd5..c5f21ef 100644
--- a/drivers/dma/hsu/hsu.c
+++ b/drivers/dma/hsu/hsu.c
@@ -126,28 +126,33 @@ static void hsu_dma_start_transfer(struct hsu_dma_chan *hsuc)
 	hsu_dma_start_channel(hsuc);
 }
 
-static u32 hsu_dma_chan_get_sr(struct hsu_dma_chan *hsuc)
-{
-	unsigned long flags;
-	u32 sr;
-
-	spin_lock_irqsave(&hsuc->vchan.lock, flags);
-	sr = hsu_chan_readl(hsuc, HSU_CH_SR);
-	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);
-
-	return sr & ~(HSU_CH_SR_DESCE_ANY | HSU_CH_SR_CDESC_ANY);
-}
-
-irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr)
+/*
+ *      hsu_dma_get_status() - get DMA channel status
+ *      @chip: HSUART DMA chip
+ *      @nr: DMA channel number
+ *      @status: pointer for DMA Channel Status Register value
+ *
+ *      Description:
+ *      The function reads and clears the DMA Channel Status Register, checks
+ *      if it was a timeout interrupt and returns a corresponding value.
+ *
+ *      Caller should provide a valid pointer for the DMA Channel Status
+ *      Register value that will be returned in @status.
+ *
+ *      Return:
+ *      1 for DMA timeout status, 0 for other DMA status, or error code for
+ *      invalid parameters or no interrupt pending.
+ */
+int hsu_dma_get_status(struct hsu_dma_chip *chip, unsigned short nr,
+		       u32 *status)
 {
 	struct hsu_dma_chan *hsuc;
-	struct hsu_dma_desc *desc;
 	unsigned long flags;
 	u32 sr;
 
 	/* Sanity check */
 	if (nr >= chip->hsu->nr_channels)
-		return IRQ_NONE;
+		return -EINVAL;
 
 	hsuc = &chip->hsu->chan[nr];
 
@@ -155,22 +160,65 @@ irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr)
 	 * No matter what situation, need read clear the IRQ status
 	 * There is a bug, see Errata 5, HSD 2900918
 	 */
-	sr = hsu_dma_chan_get_sr(hsuc);
+	spin_lock_irqsave(&hsuc->vchan.lock, flags);
+	sr = hsu_chan_readl(hsuc, HSU_CH_SR);
+	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);
+
+	/* Check if any interrupt is pending */
+	sr &= ~(HSU_CH_SR_DESCE_ANY | HSU_CH_SR_CDESC_ANY);
 	if (!sr)
-		return IRQ_NONE;
+		return -EIO;
 
 	/* Timeout IRQ, need wait some time, see Errata 2 */
 	if (sr & HSU_CH_SR_DESCTO_ANY)
 		udelay(2);
 
+	/*
+	 * At this point, at least one of Descriptor Time Out, Channel Error
+	 * or Descriptor Done bits must be set. Clear the Descriptor Time Out
+	 * bits and if sr is still non-zero, it must be channel error or
+	 * descriptor done which are higher priority than timeout and handled
+	 * in hsu_dma_do_irq(). Else, it must be a timeout.
+	 */
 	sr &= ~HSU_CH_SR_DESCTO_ANY;
-	if (!sr)
-		return IRQ_HANDLED;
+
+	*status = sr;
+
+	return sr ? 0 : 1;
+}
+EXPORT_SYMBOL_GPL(hsu_dma_get_status);
+
+/*
+ *      hsu_dma_do_irq() - DMA interrupt handler
+ *      @chip: HSUART DMA chip
+ *      @nr: DMA channel number
+ *      @status: Channel Status Register value
+ *
+ *      Description:
+ *      This function handles Channel Error and Descriptor Done interrupts.
+ *      This function should be called after determining that the DMA interrupt
+ *      is not a normal timeout interrupt, ie. hsu_dma_get_status() returned 0.
+ *
+ *      Return:
+ *      IRQ_NONE for invalid channel number, IRQ_HANDLED otherwise.
+ */
+irqreturn_t hsu_dma_do_irq(struct hsu_dma_chip *chip, unsigned short nr,
+			   u32 status)
+{
+	struct hsu_dma_chan *hsuc;
+	struct hsu_dma_desc *desc;
+	unsigned long flags;
+
+	/* Sanity check */
+	if (nr >= chip->hsu->nr_channels)
+		return IRQ_NONE;
+
+	hsuc = &chip->hsu->chan[nr];
 
 	spin_lock_irqsave(&hsuc->vchan.lock, flags);
 	desc = hsuc->desc;
 	if (desc) {
-		if (sr & HSU_CH_SR_CHE) {
+		if (status & HSU_CH_SR_CHE) {
 			desc->status = DMA_ERROR;
 		} else if (desc->active < desc->nents) {
 			hsu_dma_start_channel(hsuc);
@@ -184,7 +232,7 @@ irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr)
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(hsu_dma_irq);
+EXPORT_SYMBOL_GPL(hsu_dma_do_irq);
 
 static struct hsu_dma_desc *hsu_dma_alloc_desc(unsigned int nents)
 {
diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
index e2db76b..9916058 100644
--- a/drivers/dma/hsu/pci.c
+++ b/drivers/dma/hsu/pci.c
@@ -27,13 +27,20 @@ static irqreturn_t hsu_pci_irq(int irq, void *dev)
 {
 	struct hsu_dma_chip *chip = dev;
 	u32 dmaisr;
+	u32 status;
 	unsigned short i;
 	irqreturn_t ret = IRQ_NONE;
+	int err;
 
 	dmaisr = readl(chip->regs + HSU_PCI_DMAISR);
 	for (i = 0; i < chip->hsu->nr_channels; i++) {
-		if (dmaisr & 0x1)
-			ret |= hsu_dma_irq(chip, i);
+		if (dmaisr & 0x1) {
+			err = hsu_dma_get_status(chip, i, &status);
+			if (err > 0)
+				ret |= IRQ_HANDLED;
+			else if (err == 0)
+				ret |= hsu_dma_do_irq(chip, i, status);
+		}
 		dmaisr >>= 1;
 	}
 
diff --git a/drivers/tty/serial/8250/8250_mid.c b/drivers/tty/serial/8250/8250_mid.c
index 86379a7..b218ff5 100644
--- a/drivers/tty/serial/8250/8250_mid.c
+++ b/drivers/tty/serial/8250/8250_mid.c
@@ -97,12 +97,24 @@ static int dnv_handle_irq(struct uart_port *p)
 {
 	struct mid8250 *mid = p->private_data;
 	unsigned int fisr = serial_port_in(p, INTEL_MID_UART_DNV_FISR);
+	u32 status;
 	int ret = IRQ_NONE;
-
-	if (fisr & BIT(2))
-		ret |= hsu_dma_irq(&mid->dma_chip, 1);
-	if (fisr & BIT(1))
-		ret |= hsu_dma_irq(&mid->dma_chip, 0);
+	int err;
+
+	if (fisr & BIT(2)) {
+		err = hsu_dma_get_status(&mid->dma_chip, 1, &status);
+		if (err > 0)
+			ret |= IRQ_HANDLED;
+		else if (err == 0)
+			ret |= hsu_dma_do_irq(&mid->dma_chip, 1, status);
+	}
+	if (fisr & BIT(1)) {
+		err = hsu_dma_get_status(&mid->dma_chip, 0, &status);
+		if (err > 0)
+			ret |= IRQ_HANDLED;
+		else if (err == 0)
+			ret |= hsu_dma_do_irq(&mid->dma_chip, 0, status);
+	}
 	if (fisr & BIT(0))
 		ret |= serial8250_handle_irq(p, serial_port_in(p, UART_IIR));
 	return ret;
diff --git a/include/linux/dma/hsu.h b/include/linux/dma/hsu.h
index 79df69d..aaff68e 100644
--- a/include/linux/dma/hsu.h
+++ b/include/linux/dma/hsu.h
@@ -39,14 +39,22 @@ struct hsu_dma_chip {
 
 #if IS_ENABLED(CONFIG_HSU_DMA)
 /* Export to the internal users */
-irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip, unsigned short nr);
+int hsu_dma_get_status(struct hsu_dma_chip *chip, unsigned short nr,
+		       u32 *status);
+irqreturn_t hsu_dma_do_irq(struct hsu_dma_chip *chip, unsigned short nr,
+			   u32 status);
 
 /* Export to the platform drivers */
 int hsu_dma_probe(struct hsu_dma_chip *chip);
 int hsu_dma_remove(struct hsu_dma_chip *chip);
 #else
-static inline irqreturn_t hsu_dma_irq(struct hsu_dma_chip *chip,
-				      unsigned short nr)
+static inline int hsu_dma_get_status(struct hsu_dma_chip *chip,
+				     unsigned short nr, u32 *status)
+{
+	return 0;
+}
+static inline irqreturn_t hsu_dma_do_irq(struct hsu_dma_chip *chip,
+					 unsigned short nr, u32 status)
 {
 	return IRQ_NONE;
 }
-- 
1.9.1

[PATCH v2 2/3] serial: 8250_dma: Export serial8250_rx_dma_flush()

[Chuah Kim Tatt]

From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>

Export serial8250_rx_dma_flush() for use by SOC UART drivers.

Signed-off-by: Chuah, Kim Tatt <kim.tatt.chuah@intel.com>
---
 drivers/tty/serial/8250/8250_dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 7f33d1c..3590d01 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -145,6 +145,7 @@ void serial8250_rx_dma_flush(struct uart_8250_port *p)
 		dmaengine_terminate_all(dma->rxchan);
 	}
 }
+EXPORT_SYMBOL_GPL(serial8250_rx_dma_flush);
 
 int serial8250_request_dma(struct uart_8250_port *p)
 {
-- 
1.9.1

[PATCH v2 3/3] serial: 8250_mid: Read RX buffer on RX DMA timeout for DNV

[Chuah Kim Tatt]

From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>

In DNV, when RX DMA is used and number of bytes received is less than
transfer size, only RX DMA timeout interrupt is sent. When this happens,
read the RX buffer.

Signed-off-by: Chuah, Kim Tatt <kim.tatt.chuah@intel.com>
---
 drivers/tty/serial/8250/8250_mid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mid.c b/drivers/tty/serial/8250/8250_mid.c
index b218ff5..339de9c 100644
--- a/drivers/tty/serial/8250/8250_mid.c
+++ b/drivers/tty/serial/8250/8250_mid.c
@@ -96,6 +96,7 @@ static int tng_setup(struct mid8250 *mid, struct uart_port *p)
 static int dnv_handle_irq(struct uart_port *p)
 {
 	struct mid8250 *mid = p->private_data;
+	struct uart_8250_port *up = up_to_u8250p(p);
 	unsigned int fisr = serial_port_in(p, INTEL_MID_UART_DNV_FISR);
 	u32 status;
 	int ret = IRQ_NONE;
@@ -103,9 +104,10 @@ static int dnv_handle_irq(struct uart_port *p)
 
 	if (fisr & BIT(2)) {
 		err = hsu_dma_get_status(&mid->dma_chip, 1, &status);
-		if (err > 0)
+		if (err > 0) {
+			serial8250_rx_dma_flush(up);
 			ret |= IRQ_HANDLED;
-		else if (err == 0)
+		} else if (err == 0)
 			ret |= hsu_dma_do_irq(&mid->dma_chip, 1, status);
 	}
 	if (fisr & BIT(1)) {
-- 
1.9.1

Re: [PATCH v2 0/3] Fix DNV HSUART RX DMA timeout interrupt issue

[Andy Shevchenko]

On Wed, 2016-06-15 at 13:44 +0800, Chuah Kim Tatt wrote:
> From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>
> 
> These patches fix a DNV HSUART DMA issue with timeout interrupts,
> where
> RX data is stuck in buffer when RX DMA is used and the number of
> received
> bytes is less than 4096.
> These patches have been tested on Intel Denverton platform.

For entire series:
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Changes from v1:
>   - Added patch "serial: 8250_dma: Export serial8250_rx_dma_flush()"
> to solve
>     build error when CONFIG_SERIAL_8250_MID is set to "m".
> 
> Chuah, Kim Tatt (3):
>   dmaengine: hsu: Export hsu_dma_get_status()
>   serial: 8250_dma: Export serial8250_rx_dma_flush()
>   serial: 8250_mid: Read RX buffer on RX DMA timeout for DNV
> 
>  drivers/dma/hsu/hsu.c              | 90
> +++++++++++++++++++++++++++++---------
>  drivers/dma/hsu/pci.c              | 11 ++++-
>  drivers/tty/serial/8250/8250_dma.c |  1 +
>  drivers/tty/serial/8250/8250_mid.c | 24 +++++++---
>  include/linux/dma/hsu.h            | 14 ++++--
>  5 files changed, 109 insertions(+), 31 deletions(-)
> 

-- 

Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 0/3] Fix DNV HSUART RX DMA timeout interrupt issue

[Heikki Krogerus]

On Wed, Jun 15, 2016 at 01:44:10PM +0800, Chuah Kim Tatt wrote:
> From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>
> 
> These patches fix a DNV HSUART DMA issue with timeout interrupts, where
> RX data is stuck in buffer when RX DMA is used and the number of received
> bytes is less than 4096.
> These patches have been tested on Intel Denverton platform.

For the series:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> Changes from v1:
>   - Added patch "serial: 8250_dma: Export serial8250_rx_dma_flush()" to solve
>     build error when CONFIG_SERIAL_8250_MID is set to "m".
> 
> Chuah, Kim Tatt (3):
>   dmaengine: hsu: Export hsu_dma_get_status()
>   serial: 8250_dma: Export serial8250_rx_dma_flush()
>   serial: 8250_mid: Read RX buffer on RX DMA timeout for DNV
> 
>  drivers/dma/hsu/hsu.c              | 90 +++++++++++++++++++++++++++++---------
>  drivers/dma/hsu/pci.c              | 11 ++++-
>  drivers/tty/serial/8250/8250_dma.c |  1 +
>  drivers/tty/serial/8250/8250_mid.c | 24 +++++++---
>  include/linux/dma/hsu.h            | 14 ++++--
>  5 files changed, 109 insertions(+), 31 deletions(-)
> 
> -- 
> 1.9.1

Thanks,

-- 
heikki

Re: [PATCH v2 1/3] dmaengine: hsu: Export hsu_dma_get_status()

[Vinod Koul]

On Wed, Jun 15, 2016 at 01:44:11PM +0800, Chuah Kim Tatt wrote:
> From: "Chuah, Kim Tatt" <kim.tatt.chuah@intel.com>
> 
> To allow other code to safely read DMA Channel Status Register (where
> the register attribute for Channel Error, Descriptor Time Out &
> Descriptor Done fields are read-clear), export hsu_dma_get_status().
> hsu_dma_irq() is renamed to hsu_dma_do_irq() and requires Status
> Register value to be passed in.

Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod