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

[Andy Shevchenko]

On Fri, 2016-05-13 at 16:15 +0800, Chuah Kim Tatt wrote:
> From: "Chuah, Kim Tatt" <kim.tatt.chuah@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx>

> Signed-off-by: Chuah, Kim Tatt <kim.tatt.chuah@xxxxxxxxx>
> ---
> Â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@xxxxxxxxxxxxxxx>
Intel Finland Oy