From: Doug Anderson <dianders@chromium.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Roja Rani Yarubandi <rojay@codeaurora.org>,
Mark Brown <broonie@kernel.org>, Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
linux-spi <linux-spi@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Akash Asthana <akashast@codeaurora.org>,
msavaliy@qti.qualcomm.com
Subject: Re: [PATCH] spi: spi-geni-qcom: Fix NULL pointer access in geni_spi_isr
Date: Thu, 10 Dec 2020 17:51:53 -0800 [thread overview]
Message-ID: <CAD=FV=WuyuF-PL2PMnLjWCyWGzOqn8beTVP3ZXWvfLdLhPh8=A@mail.gmail.com> (raw)
In-Reply-To: <160765077856.1580929.643282739071441296@swboyd.mtv.corp.google.com>
Hi,
On Thu, Dec 10, 2020 at 5:39 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Doug Anderson (2020-12-10 17:30:17)
> > On Thu, Dec 10, 2020 at 5:21 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Yeah and so if it comes way later because it timed out then what's the
> > > point of calling synchronize_irq() again? To make the completion
> > > variable set when it won't be tested again until it is reinitialized?
> >
> > Presumably the idea is to try to recover to a somewhat usable state
> > again? We're not rebooting the machine so, even though this transfer
> > failed, we will undoubtedly do another transfer later. If that
> > "abort" interrupt comes way later while we're setting up the next
> > transfer we'll really confuse ourselves.
>
> The interrupt handler just sets a completion variable. What does that
> confuse?
The interrupt handler sees a "DONE" interrupt. If we've made it far
enough into setting up the next transfer that "cur_xfer" has been set
then it might do more, no?
> > I guess you could go the route of adding a synchronize_irq() at the
> > start of the next transfer, but I'd rather add the overhead in the
> > exceptional case (the timeout) than the normal case. In the normal
> > case we don't need to worry about random IRQs from the past transfer
> > suddenly showing up.
> >
>
> How does adding synchronize_irq() at the end guarantee that the abort is
> cleared out of the hardware though? It seems to assume that the abort is
> pending at the GIC when it could still be running through the hardware
> and not executed yet. It seems like a synchronize_irq() for that is
> wishful thinking that the irq is merely pending even though it timed
> out and possibly never ran. Maybe it's stuck in a write buffer in the
> CPU?
I guess I'm asserting that if a full second passed (because we timed
out) and after that full second no interrupts are pending then the
interrupt will never come. That seems a reasonable assumption to me.
It seems hard to believe it'd be stuck in a write buffer for a full
second?
-Doug
next prev parent reply other threads:[~2020-12-11 1:53 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-03 7:44 [PATCH] spi: spi-geni-qcom: Fix NULL pointer access in geni_spi_isr Roja Rani Yarubandi
2020-12-03 16:40 ` Doug Anderson
2020-12-10 3:17 ` Stephen Boyd
2020-12-10 17:14 ` Doug Anderson
2020-12-10 22:57 ` Stephen Boyd
2020-12-10 23:07 ` Doug Anderson
2020-12-10 23:32 ` Stephen Boyd
2020-12-10 23:50 ` Doug Anderson
2020-12-11 0:50 ` Stephen Boyd
2020-12-11 1:04 ` Doug Anderson
2020-12-11 1:21 ` Stephen Boyd
2020-12-11 1:30 ` Doug Anderson
2020-12-11 1:39 ` Stephen Boyd
2020-12-11 1:51 ` Doug Anderson [this message]
2020-12-12 1:32 ` Stephen Boyd
2020-12-15 0:31 ` Doug Anderson
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='CAD=FV=WuyuF-PL2PMnLjWCyWGzOqn8beTVP3ZXWvfLdLhPh8=A@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=agross@kernel.org \
--cc=akashast@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=broonie@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=msavaliy@qti.qualcomm.com \
--cc=rojay@codeaurora.org \
--cc=swboyd@chromium.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).