From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752884AbcEMLSb (ORCPT ); Fri, 13 May 2016 07:18:31 -0400 Received: from mga14.intel.com ([192.55.52.115]:40555 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751741AbcEMLS3 (ORCPT ); Fri, 13 May 2016 07:18:29 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,614,1455004800"; d="scan'208";a="965797334" Message-ID: <1463138378.17131.328.camel@linux.intel.com> Subject: Re: [PATCH 1/2] dmaengine: hsu: Export hsu_dma_get_status() From: Andy Shevchenko To: Chuah Kim Tatt , gregkh@linuxfoundation.org, vinod.koul@intel.com Cc: heikki.krogerus@linux.intel.com, mika.westerberg@linux.intel.com, linux-kernel@vger.kernel.org, jui.nee.tan@intel.com Date: Fri, 13 May 2016 14:19:38 +0300 In-Reply-To: <1463127320-13943-2-git-send-email-kim.tatt.chuah@intel.com> References: <1463127320-13943-1-git-send-email-kim.tatt.chuah@intel.com> <1463127320-13943-2-git-send-email-kim.tatt.chuah@intel.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2016-05-13 at 16:15 +0800, Chuah Kim Tatt wrote: > From: "Chuah, Kim Tatt" > > 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 > Signed-off-by: Chuah, Kim Tatt > --- >  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 Intel Finland Oy