From: Arnd Bergmann <arnd@arndb.de>
To: "Shenhar, Talel" <talel@amazon.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
David Miller <davem@davemloft.net>,
gregkh <gregkh@linuxfoundation.org>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
Thomas Gleixner <tglx@linutronix.de>,
Patrick Venture <venture@google.com>,
Linus Walleij <linus.walleij@linaro.org>,
Olof Johansson <olof@lixom.net>,
Maxime Ripard <mripard@kernel.org>,
Santosh Shilimkar <ssantosh@kernel.org>,
paul.kocialkowski@bootlin.com, mjourdan@baylibre.com,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, DTML <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
David Woodhouse <dwmw@amazon.co.uk>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
hhhawa@amazon.com, ronenk@amazon.com, jonnyc@amazon.com,
hanochu@amazon.com, barakw@amazon.com
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 17:16:56 +0200 [thread overview]
Message-ID: <CAK8P3a1fbK-qoK+K1ZsWsU3rkxxZgZGaK8ywFAcM4va1GRn_FQ@mail.gmail.com> (raw)
In-Reply-To: <8f7840c3-a682-04a5-18bf-ac7a723725b0@amazon.com>
On Mon, Sep 9, 2019 at 4:11 PM Shenhar, Talel <talel@amazon.com> wrote:
> On 9/9/2019 4:41 PM, Arnd Bergmann wrote:
>
> In current implementation of v1, I am not doing any read barrier, Hence,
> using the non-relaxed will add unneeded memory barrier.
>
> I have no strong objection moving to the non-relaxed version and have an
> unneeded memory barrier, as this path is not "hot" one.
Ok, then please add it.
> Beside of avoiding the unneeded memory barrier, I would be happy to keep
> common behavior for our drivers:
>
> e.g.
>
> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-al-fic.c#L49
>
>
> So what do you think we should go with? relaxed or non-relaxed?
The al_fic_set_trigger() function is clearly a slow-path and should use the
non-relaxed functions. In case of al_fic_irq_handler(), the extra barrier
might introduce a measurable overhead, but at the same time I'm
not sure if that one is correct without the barrier:
If you have an MSI-type interrupt for notifying a device driver of
a DMA completion, there might not be any other barrier between
the arrival of the MSI message and the CPU accessing the data.
Depending on how strict the hardware implements MSI and how
the IRQ is chained, this could lead to data corruption.
If the interrupt is only used for level or edge triggered interrupts,
this is ok since you already need another register read in
the driver before it can safely access a DMA buffer.
In either case, if you can prove that it's safe to use the relaxed
version here and you think that it may help, it would be good to
add a comment explaining the reasoning.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: "Shenhar, Talel" <talel@amazon.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
David Miller <davem@davemloft.net>,
gregkh <gregkh@linuxfoundation.org>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
Thomas Gleixner <tglx@linutronix.de>,
Patrick Venture <venture@google.com>,
Linus Walleij <linus.walleij@linaro.org>,
Olof Johansson <olof@lixom.net>,
Maxime Ripard <mripard@kernel.org>,
Santosh Shilimkar <ssantosh@kernel.org>,
paul.kocialkowski@bootlin.com, mjourdan@baylibre.com,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, DTML <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>David
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 17:16:56 +0200 [thread overview]
Message-ID: <CAK8P3a1fbK-qoK+K1ZsWsU3rkxxZgZGaK8ywFAcM4va1GRn_FQ@mail.gmail.com> (raw)
In-Reply-To: <8f7840c3-a682-04a5-18bf-ac7a723725b0@amazon.com>
On Mon, Sep 9, 2019 at 4:11 PM Shenhar, Talel <talel@amazon.com> wrote:
> On 9/9/2019 4:41 PM, Arnd Bergmann wrote:
>
> In current implementation of v1, I am not doing any read barrier, Hence,
> using the non-relaxed will add unneeded memory barrier.
>
> I have no strong objection moving to the non-relaxed version and have an
> unneeded memory barrier, as this path is not "hot" one.
Ok, then please add it.
> Beside of avoiding the unneeded memory barrier, I would be happy to keep
> common behavior for our drivers:
>
> e.g.
>
> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-al-fic.c#L49
>
>
> So what do you think we should go with? relaxed or non-relaxed?
The al_fic_set_trigger() function is clearly a slow-path and should use the
non-relaxed functions. In case of al_fic_irq_handler(), the extra barrier
might introduce a measurable overhead, but at the same time I'm
not sure if that one is correct without the barrier:
If you have an MSI-type interrupt for notifying a device driver of
a DMA completion, there might not be any other barrier between
the arrival of the MSI message and the CPU accessing the data.
Depending on how strict the hardware implements MSI and how
the IRQ is chained, this could lead to data corruption.
If the interrupt is only used for level or edge triggered interrupts,
this is ok since you already need another register read in
the driver before it can safely access a DMA buffer.
In either case, if you can prove that it's safe to use the relaxed
version here and you think that it may help, it would be good to
add a comment explaining the reasoning.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: "Shenhar, Talel" <talel@amazon.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
mjourdan@baylibre.com, Catalin Marinas <catalin.marinas@arm.com>,
Linus Walleij <linus.walleij@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
jonnyc@amazon.com,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
ronenk@amazon.com, Will Deacon <will@kernel.org>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
DTML <devicetree@vger.kernel.org>,
Maxime Ripard <mripard@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Santosh Shilimkar <ssantosh@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
hanochu@amazon.com,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
barakw@amazon.com, hhhawa@amazon.com,
gregkh <gregkh@linuxfoundation.org>,
paul.kocialkowski@bootlin.com,
Patrick Venture <venture@google.com>,
Olof Johansson <olof@lixom.net>,
David Miller <davem@davemloft.net>,
David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 17:16:56 +0200 [thread overview]
Message-ID: <CAK8P3a1fbK-qoK+K1ZsWsU3rkxxZgZGaK8ywFAcM4va1GRn_FQ@mail.gmail.com> (raw)
In-Reply-To: <8f7840c3-a682-04a5-18bf-ac7a723725b0@amazon.com>
On Mon, Sep 9, 2019 at 4:11 PM Shenhar, Talel <talel@amazon.com> wrote:
> On 9/9/2019 4:41 PM, Arnd Bergmann wrote:
>
> In current implementation of v1, I am not doing any read barrier, Hence,
> using the non-relaxed will add unneeded memory barrier.
>
> I have no strong objection moving to the non-relaxed version and have an
> unneeded memory barrier, as this path is not "hot" one.
Ok, then please add it.
> Beside of avoiding the unneeded memory barrier, I would be happy to keep
> common behavior for our drivers:
>
> e.g.
>
> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-al-fic.c#L49
>
>
> So what do you think we should go with? relaxed or non-relaxed?
The al_fic_set_trigger() function is clearly a slow-path and should use the
non-relaxed functions. In case of al_fic_irq_handler(), the extra barrier
might introduce a measurable overhead, but at the same time I'm
not sure if that one is correct without the barrier:
If you have an MSI-type interrupt for notifying a device driver of
a DMA completion, there might not be any other barrier between
the arrival of the MSI message and the CPU accessing the data.
Depending on how strict the hardware implements MSI and how
the IRQ is chained, this could lead to data corruption.
If the interrupt is only used for level or edge triggered interrupts,
this is ok since you already need another register read in
the driver before it can safely access a DMA buffer.
In either case, if you can prove that it's safe to use the relaxed
version here and you think that it may help, it would be good to
add a comment explaining the reasoning.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-09-09 15:17 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-09 9:10 [PATCH 0/3] Amazon's Annapurna Labs POS Driver Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` [PATCH 1/3] dt-bindings: soc: al-pos: Amazon's Annapurna Labs POS Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:44 ` Arnd Bergmann
2019-09-09 9:44 ` Arnd Bergmann
2019-09-09 9:44 ` Arnd Bergmann
2019-09-09 11:12 ` Shenhar, Talel
2019-09-09 11:12 ` Shenhar, Talel
2019-09-09 11:12 ` Shenhar, Talel
2019-09-09 13:41 ` Arnd Bergmann
2019-09-09 13:41 ` Arnd Bergmann
2019-09-09 13:41 ` Arnd Bergmann
2019-09-09 14:11 ` Shenhar, Talel
2019-09-09 14:11 ` Shenhar, Talel
2019-09-09 14:11 ` Shenhar, Talel
2019-09-09 15:16 ` Arnd Bergmann [this message]
2019-09-09 15:16 ` Arnd Bergmann
2019-09-09 15:16 ` Arnd Bergmann
2019-09-10 6:21 ` Shenhar, Talel
2019-09-10 6:21 ` Shenhar, Talel
2019-09-10 6:21 ` Shenhar, Talel
2019-09-09 11:51 ` kbuild test robot
2019-09-09 11:51 ` kbuild test robot
2019-09-09 11:51 ` kbuild test robot
2019-09-09 9:10 ` [PATCH 3/3] arm64: alpine: select AL_POS Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:10 ` Talel Shenhar
2019-09-09 9:40 ` Arnd Bergmann
2019-09-09 9:40 ` Arnd Bergmann
2019-09-09 9:40 ` Arnd Bergmann
2019-09-09 10:16 ` Shenhar, Talel
2019-09-09 10:16 ` Shenhar, Talel
2019-09-09 10:16 ` Shenhar, Talel
2019-09-09 13:45 ` Arnd Bergmann
2019-09-09 13:45 ` Arnd Bergmann
2019-09-09 13:45 ` Arnd Bergmann
2019-09-09 13:58 ` Shenhar, Talel
2019-09-09 13:58 ` Shenhar, Talel
2019-09-09 13:58 ` Shenhar, Talel
2019-09-09 15:08 ` Arnd Bergmann
2019-09-09 15:08 ` Arnd Bergmann
2019-09-09 15:08 ` Arnd Bergmann
2019-09-10 6:17 ` Shenhar, Talel
2019-09-10 6:17 ` Shenhar, Talel
2019-09-10 6:17 ` Shenhar, Talel
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=CAK8P3a1fbK-qoK+K1ZsWsU3rkxxZgZGaK8ywFAcM4va1GRn_FQ@mail.gmail.com \
--to=arnd@arndb.de \
--cc=barakw@amazon.com \
--cc=benh@kernel.crashing.org \
--cc=catalin.marinas@arm.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=dwmw@amazon.co.uk \
--cc=gregkh@linuxfoundation.org \
--cc=hanochu@amazon.com \
--cc=hhhawa@amazon.com \
--cc=jonnyc@amazon.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mchehab+samsung@kernel.org \
--cc=mjourdan@baylibre.com \
--cc=mripard@kernel.org \
--cc=nicolas.ferre@microchip.com \
--cc=olof@lixom.net \
--cc=paul.kocialkowski@bootlin.com \
--cc=robh+dt@kernel.org \
--cc=ronenk@amazon.com \
--cc=ssantosh@kernel.org \
--cc=talel@amazon.com \
--cc=tglx@linutronix.de \
--cc=venture@google.com \
--cc=will@kernel.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.