Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1665461 > unrolled thread

[PATCH 00/18] spi: qup: Fixes and add support for >64k transfers

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/18] spi: qup: Fixes and add support for >64k transfers Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-14 08:00 +0200
    [PATCH 15/18] spi: qup: refactor spi_qup_prep_sg Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-14 08:00 +0200
    [PATCH 01/18] spi: qup: Enable chip select support Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-14 08:00 +0200
      Re: [PATCH 01/18] spi: qup: Enable chip select support Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-06-14 11:50 +0200
    [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-14 08:00 +0200
      Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling Sricharan R <sricharan@codeaurora.org> - 2017-06-14 09:30 +0200
        Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling Andy Gross <andy.gross@linaro.org> - 2017-06-14 22:10 +0200
          Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling Sricharan R <sricharan@codeaurora.org> - 2017-06-15 08:00 +0200

#1665461 — [PATCH 00/18] spi: qup: Fixes and add support for >64k transfers

v1:
	This series fixes some existing issues in the code for both
	interrupt and dma mode. Patches 1 - 11 are the fixes.
	Random failures/timeout are observed without these fixes.
	Also, the current driver does not support block transfers > 64K
	and the driver quietly fails. Patches 12 - 18 add support for this
	in both interrupt and dma mode.

	The entire series has been tested on ipq4019 with
	SPI-NOR flash for block sizes > 64k.

Varadarajan Narayanan (18):
  spi: qup: Enable chip select support
  spi: qup: Setup DMA mode correctly
  spi: qup: Add completion timeout for dma mode
  spi: qup: Add completion timeout for fifo/block mode
  spi: qup: Place the QUP in run mode before DMA transactions
  spi: qup: Fix error handling in spi_qup_prep_sg
  spi: qup: Fix transaction done signaling
  spi: qup: Handle v1 dma completion differently
  spi: qup: Do block sized read/write in block mode
  spi: qup: Fix DMA mode interrupt handling
  spi: qup: properly detect extra interrupts
  spi: qup: refactor spi_qup_io_config into two functions
  spi: qup: call io_config in mode specific function
  spi: qup: allow block mode to generate multiple transactions
  spi: qup: refactor spi_qup_prep_sg
  spi: qup: allow multiple DMA transactions per spi xfer
  spi: qup: Ensure done detection
  spi: qup: support for qup v1 dma

 .../devicetree/bindings/spi/qcom,spi-qup.txt       |   6 +
 drivers/spi/spi-qup.c                              | 639 +++++++++++++++------
 2 files changed, 462 insertions(+), 183 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [next] | [standalone]

#1665462 — [PATCH 15/18] spi: qup: refactor spi_qup_prep_sg

Take specific sgl and nent to be prepared.  This is in
preparation for splitting DMA into multiple transacations, this
contains no code changes just refactoring.

Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 02d4e10..b65a6a4 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -382,26 +382,19 @@ static void spi_qup_write(struct spi_qup *controller)
 	} while (remainder);
 }
 
-static int spi_qup_prep_sg(struct spi_master *master, struct spi_transfer *xfer,
-			   enum dma_transfer_direction dir,
+static int spi_qup_prep_sg(struct spi_master *master, struct scatterlist *sgl,
+			   unsigned int nents, enum dma_transfer_direction dir,
 			   dma_async_tx_callback callback, void *data)
 {
 	unsigned long flags = DMA_PREP_INTERRUPT | DMA_PREP_FENCE;
 	struct dma_async_tx_descriptor *desc;
-	struct scatterlist *sgl;
 	struct dma_chan *chan;
 	dma_cookie_t cookie;
-	unsigned int nents;
 
-	if (dir == DMA_MEM_TO_DEV) {
+	if (dir == DMA_MEM_TO_DEV)
 		chan = master->dma_tx;
-		nents = xfer->tx_sg.nents;
-		sgl = xfer->tx_sg.sgl;
-	} else {
+	else
 		chan = master->dma_rx;
-		nents = xfer->rx_sg.nents;
-		sgl = xfer->rx_sg.sgl;
-	}
 
 	desc = dmaengine_prep_slave_sg(chan, sgl, nents, dir, flags);
 	if (IS_ERR_OR_NULL(desc))
@@ -445,8 +438,9 @@ static int spi_qup_do_dma(struct spi_device *spi, struct spi_transfer *xfer,
 	}
 
 	if (xfer->rx_buf) {
-		ret = spi_qup_prep_sg(master, xfer, DMA_DEV_TO_MEM,
-					done, &qup->rxc);
+		ret = spi_qup_prep_sg(master, xfer->rx_sg.sgl,
+				      xfer->rx_sg.nents, DMA_DEV_TO_MEM,
+				      done, &qup->rxc);
 		if (ret)
 			return ret;
 
@@ -454,8 +448,9 @@ static int spi_qup_do_dma(struct spi_device *spi, struct spi_transfer *xfer,
 	}
 
 	if (xfer->tx_buf) {
-		ret = spi_qup_prep_sg(master, xfer, DMA_MEM_TO_DEV,
-					done, &qup->txc);
+		ret = spi_qup_prep_sg(master, xfer->tx_sg.sgl,
+				      xfer->tx_sg.nents, DMA_MEM_TO_DEV,
+				      done, &qup->txc);
 		if (ret)
 			return ret;
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1665463 — [PATCH 01/18] spi: qup: Enable chip select support

Enable chip select support for QUP versions later than v1

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 1bfa889..c0d4def 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -750,6 +750,24 @@ static int spi_qup_init_dma(struct spi_master *master, resource_size_t base)
 	return ret;
 }
 
+static void spi_qup_set_cs(struct spi_device *spi, bool val)
+{
+	struct spi_qup *controller;
+	u32 spi_ioc;
+	u32 spi_ioc_orig;
+
+	controller = spi_master_get_devdata(spi->master);
+	spi_ioc = readl_relaxed(controller->base + SPI_IO_CONTROL);
+	spi_ioc_orig = spi_ioc;
+	if (!val)
+		spi_ioc |= SPI_IO_C_FORCE_CS;
+	else
+		spi_ioc &= ~SPI_IO_C_FORCE_CS;
+
+	if (spi_ioc != spi_ioc_orig)
+		writel_relaxed(spi_ioc, controller->base + SPI_IO_CONTROL);
+}
+
 static int spi_qup_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -846,6 +864,9 @@ static int spi_qup_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(dev->of_node, "qcom,spi-qup-v1.1.1"))
 		controller->qup_v1 = 1;
 
+	if (!controller->qup_v1)
+		master->set_cs = spi_qup_set_cs;
+
 	spin_lock_init(&controller->lock);
 	init_completion(&controller->done);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1665649 — Re: [PATCH 01/18] spi: qup: Enable chip select support

Hi,

On 06/14/2017 08:52 AM, Varadarajan Narayanan wrote:
> Enable chip select support for QUP versions later than v1

Could you be more descriptive here because in the git history of the
driver there is a commit "4a8573abe965115bc5b064401fd669b74e985258 spi:
qup: Remove chip select function" which removes chip select
functionality. So this patch leads to a situation where we use gpio cs
for qup_v1 and not for next versions!?

> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  drivers/spi/spi-qup.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

<snip>

-- 
regards,
Stan

[toc] | [prev] | [next] | [standalone]

#1665464 — [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling

This is needed for v1, where the i/o completion is not
handled in the dma driver.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 872de28..bd53e82 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -510,9 +510,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 
 	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
 	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
-	writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
 
 	if (!xfer) {
+		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
 		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
 				    qup_err, spi_err, opflags);
 		return IRQ_HANDLED;
@@ -540,7 +540,15 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 		error = -EIO;
 	}
 
-	if (!spi_qup_is_dma_xfer(controller->mode)) {
+	if (spi_qup_is_dma_xfer(controller->mode)) {
+		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
+		if (opflags & QUP_OP_IN_SERVICE_FLAG &&
+		    opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
+			complete(&controller->rxc);
+		if (opflags & QUP_OP_OUT_SERVICE_FLAG &&
+		    opflags & QUP_OP_MAX_OUTPUT_DONE_FLAG)
+			complete(&controller->txc);
+	} else {
 		if (opflags & QUP_OP_IN_SERVICE_FLAG)
 			spi_qup_read(controller, xfer);
 
@@ -553,6 +561,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 	controller->xfer = xfer;
 	spin_unlock_irqrestore(&controller->lock, flags);
 
+	/* re-read opflags as flags may have changed due to actions above */
+	opflags = readl_relaxed(controller->base + QUP_OPERATIONAL);
+
 	if ((controller->rx_bytes == xfer->len &&
 		(opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) ||  error)
 		complete(&controller->done);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1665503 — Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling

Hi Varada,

On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote:
> This is needed for v1, where the i/o completion is not
> handled in the dma driver.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  drivers/spi/spi-qup.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> index 872de28..bd53e82 100644
> --- a/drivers/spi/spi-qup.c
> +++ b/drivers/spi/spi-qup.c
> @@ -510,9 +510,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
>  
>  	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
>  	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
> -	writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
>  
>  	if (!xfer) {
> +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);

 This does look correct to remove acknowledging the QUP in normal case and
  do it conditionally only when xfer = NULL.

>  		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
>  				    qup_err, spi_err, opflags);
>  		return IRQ_HANDLED;
> @@ -540,7 +540,15 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
>  		error = -EIO;
>  	}
>  
> -	if (!spi_qup_is_dma_xfer(controller->mode)) {
> +	if (spi_qup_is_dma_xfer(controller->mode)) {
> +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> +		if (opflags & QUP_OP_IN_SERVICE_FLAG &&
> +		    opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
> +			complete(&controller->rxc);
> +		if (opflags & QUP_OP_OUT_SERVICE_FLAG &&
> +		    opflags & QUP_OP_MAX_OUTPUT_DONE_FLAG)
> +			complete(&controller->txc);
> +	} else {

 Is this because in patch #8 that we do not populate the dma callback
 for v1. If that is done, this should not be required at all, as the
 complete would be signalled from the dma callback.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1666187 — Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling

On Wed, Jun 14, 2017 at 12:51:11PM +0530, Sricharan R wrote:
> Hi Varada,
> 
> On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote:
> > This is needed for v1, where the i/o completion is not
> > handled in the dma driver.
> > 
> > Signed-off-by: Andy Gross <andy.gross@linaro.org>
> > Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> > ---
> >  drivers/spi/spi-qup.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> > index 872de28..bd53e82 100644
> > --- a/drivers/spi/spi-qup.c
> > +++ b/drivers/spi/spi-qup.c
> > @@ -510,9 +510,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
> >  
> >  	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
> >  	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
> > -	writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> >  
> >  	if (!xfer) {
> > +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> 
>  This does look correct to remove acknowledging the QUP in normal case and
>   do it conditionally only when xfer = NULL.

This is to probably mask the issue of getting erroneous/spurious IRQs.

> 
> >  		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
> >  				    qup_err, spi_err, opflags);
> >  		return IRQ_HANDLED;
> > @@ -540,7 +540,15 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
> >  		error = -EIO;
> >  	}
> >  
> > -	if (!spi_qup_is_dma_xfer(controller->mode)) {
> > +	if (spi_qup_is_dma_xfer(controller->mode)) {
> > +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> > +		if (opflags & QUP_OP_IN_SERVICE_FLAG &&
> > +		    opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
> > +			complete(&controller->rxc);
> > +		if (opflags & QUP_OP_OUT_SERVICE_FLAG &&
> > +		    opflags & QUP_OP_MAX_OUTPUT_DONE_FLAG)
> > +			complete(&controller->txc);
> > +	} else {
> 
>  Is this because in patch #8 that we do not populate the dma callback
>  for v1. If that is done, this should not be required at all, as the
>  complete would be signalled from the dma callback.

I believe that is true.  There shouldn't be any IRQs for DMA enabled
transactions (at least BAM-dma).

[toc] | [prev] | [next] | [standalone]

#1666478 — Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling

Hi Andy,

On 6/15/2017 1:36 AM, Andy Gross wrote:
> On Wed, Jun 14, 2017 at 12:51:11PM +0530, Sricharan R wrote:
>> Hi Varada,
>>
>> On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote:
>>> This is needed for v1, where the i/o completion is not
>>> handled in the dma driver.
>>>
>>> Signed-off-by: Andy Gross <andy.gross@linaro.org>
>>> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
>>> ---
>>>  drivers/spi/spi-qup.c | 15 +++++++++++++--
>>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
>>> index 872de28..bd53e82 100644
>>> --- a/drivers/spi/spi-qup.c
>>> +++ b/drivers/spi/spi-qup.c
>>> @@ -510,9 +510,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
>>>  
>>>  	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
>>>  	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
>>> -	writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
>>>  
>>>  	if (!xfer) {
>>> +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
>>
>>  This does look correct to remove acknowledging the QUP in normal case and
>>   do it conditionally only when xfer = NULL.
> 
> This is to probably mask the issue of getting erroneous/spurious IRQs.
> 

 hmm, now the QUP_OPERATIONAL is not written to acknowledge the interrupts in
 normal case seems to be wrong.

>>
>>>  		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
>>>  				    qup_err, spi_err, opflags);
>>>  		return IRQ_HANDLED;
>>> @@ -540,7 +540,15 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
>>>  		error = -EIO;
>>>  	}
>>>  
>>> -	if (!spi_qup_is_dma_xfer(controller->mode)) {
>>> +	if (spi_qup_is_dma_xfer(controller->mode)) {
>>> +		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
>>> +		if (opflags & QUP_OP_IN_SERVICE_FLAG &&
>>> +		    opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
>>> +			complete(&controller->rxc);
>>> +		if (opflags & QUP_OP_OUT_SERVICE_FLAG &&
>>> +		    opflags & QUP_OP_MAX_OUTPUT_DONE_FLAG)
>>> +			complete(&controller->txc);
>>> +	} else {
>>
>>  Is this because in patch #8 that we do not populate the dma callback
>>  for v1. If that is done, this should not be required at all, as the
>>  complete would be signalled from the dma callback.
> 
> I believe that is true.  There shouldn't be any IRQs for DMA enabled
> transactions (at least BAM-dma).

 yeah, the above hunk looks like is ADM specific, not sure why ADM cannot
 work with dma callbacks.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web