All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Gross <andy.gross@linaro.org>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Abhishek Sahu <absahu@codeaurora.org>,
	agross@codeaurora.org, david.brown@linaro.org,
	gregkh@linuxfoundation.org, jslaby@suse.com,
	linux-soc@vger.kernel.org, linux-serial@vger.kernel.org,
	sricharan@codeaurora.org, architt@codeaurora.org,
	linux-arm-msm@vger.kernel.org, ntelkar@codeaurora.org,
	galak@codeaurora.org, linux-kernel@vger.kernel.org,
	dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Charanya <charanya@codeaurora.org>
Subject: Re: [PATCH] tty: serial: msm: Disable restoring Rx interrupts for DMA Mode
Date: Thu, 12 May 2016 00:02:45 -0500	[thread overview]
Message-ID: <20160512050245.GD8453@hector.attlocal.net> (raw)
In-Reply-To: <20160512014126.GK3492@codeaurora.org>

On Wed, May 11, 2016 at 06:41:26PM -0700, Stephen Boyd wrote:
> On 05/10, Abhishek Sahu wrote:
> > From: Charanya <charanya@codeaurora.org>
> 
> Was it intentional to only have one name here?
> 
> > 
> > The Data loss was happening with current QCOM MSM serial driver during
> > large file transfer due to simultaneous enabling of both UART and
> > DMA interrupt. When UART operates in DMA mode, RXLEV (Rx FIFO over
> > watermark) or RXSTALE (stale interrupts) should not be enabled,
> > since these conditions will be handled by DMA controller itself.
> > If these interrupts are enabled then normal UART ISR will read some
> > bytes of data from Rx Buffer and DMA controller will not receive
> > these bytes of data, which will cause data loss.
> > 
> > Now this patch removed the code for enabling of RXLEV and RXSTALE
> > interrupt in DMA Rx completion routine.
> 
> I'm lost, we keep both these irqs masked (well only if uartdm
> version is 1.4 or greater) pretty much the entire time we're
> using DMA for RX. msm_start_rx_dma() will mask them and then when
> the callback completes we'll unmask them (the part that's deleted
> in this patch), but then we'll go back and remask them almost
> immediately because we call msm_start_rx_dma() from the dma
> completion handler.
> 
> Can you clearly describe how this is actually fixing any
> problems? What's the sequence of events that happens to cause
> corruption?
> 
> This does raise the question though why we ever mask/unmask these
> interrupts if we're always going to keep them masked while doing
> DMA RX. Presumably if we can use DMA to RX, we can always use it
> and set things up properly at startup time instead of later on.

Thats probably the right thing to do.  We shouldn't be masking/unmasking
the unused IRQs to begin with.

WARNING: multiple messages have this Message-ID (diff)
From: andy.gross@linaro.org (Andy Gross)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] tty: serial: msm: Disable restoring Rx interrupts for DMA Mode
Date: Thu, 12 May 2016 00:02:45 -0500	[thread overview]
Message-ID: <20160512050245.GD8453@hector.attlocal.net> (raw)
In-Reply-To: <20160512014126.GK3492@codeaurora.org>

On Wed, May 11, 2016 at 06:41:26PM -0700, Stephen Boyd wrote:
> On 05/10, Abhishek Sahu wrote:
> > From: Charanya <charanya@codeaurora.org>
> 
> Was it intentional to only have one name here?
> 
> > 
> > The Data loss was happening with current QCOM MSM serial driver during
> > large file transfer due to simultaneous enabling of both UART and
> > DMA interrupt. When UART operates in DMA mode, RXLEV (Rx FIFO over
> > watermark) or RXSTALE (stale interrupts) should not be enabled,
> > since these conditions will be handled by DMA controller itself.
> > If these interrupts are enabled then normal UART ISR will read some
> > bytes of data from Rx Buffer and DMA controller will not receive
> > these bytes of data, which will cause data loss.
> > 
> > Now this patch removed the code for enabling of RXLEV and RXSTALE
> > interrupt in DMA Rx completion routine.
> 
> I'm lost, we keep both these irqs masked (well only if uartdm
> version is 1.4 or greater) pretty much the entire time we're
> using DMA for RX. msm_start_rx_dma() will mask them and then when
> the callback completes we'll unmask them (the part that's deleted
> in this patch), but then we'll go back and remask them almost
> immediately because we call msm_start_rx_dma() from the dma
> completion handler.
> 
> Can you clearly describe how this is actually fixing any
> problems? What's the sequence of events that happens to cause
> corruption?
> 
> This does raise the question though why we ever mask/unmask these
> interrupts if we're always going to keep them masked while doing
> DMA RX. Presumably if we can use DMA to RX, we can always use it
> and set things up properly at startup time instead of later on.

Thats probably the right thing to do.  We shouldn't be masking/unmasking
the unused IRQs to begin with.

  reply	other threads:[~2016-05-12  5:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-10 16:09 [PATCH] tty: serial: msm: Disable restoring Rx interrupts for DMA Mode Abhishek Sahu
2016-05-10 16:09 ` Abhishek Sahu
2016-05-12  1:41 ` Stephen Boyd
2016-05-12  1:41   ` Stephen Boyd
2016-05-12  5:02   ` Andy Gross [this message]
2016-05-12  5:02     ` Andy Gross
2016-05-13  9:18     ` charanya
2016-05-13  9:18       ` charanya at codeaurora.org
2016-05-25 22:46       ` Stephen Boyd
2016-05-25 22:46         ` Stephen Boyd
2016-06-02  9:07         ` charanya
2016-06-02  9:07           ` charanya at codeaurora.org
2016-06-02 18:37           ` Stephen Boyd
2016-06-02 18:37             ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160512050245.GD8453@hector.attlocal.net \
    --to=andy.gross@linaro.org \
    --cc=absahu@codeaurora.org \
    --cc=agross@codeaurora.org \
    --cc=architt@codeaurora.org \
    --cc=charanya@codeaurora.org \
    --cc=david.brown@linaro.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-soc@vger.kernel.org \
    --cc=ntelkar@codeaurora.org \
    --cc=sboyd@codeaurora.org \
    --cc=sricharan@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.