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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, 2016-05-13 at 16:15 +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: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 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;
>  }

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