public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Bottorff <janb@os.amperecomputing.com>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Yann Sionneau <ysionneau@kalrayinc.com>
Cc: Wolfram Sang <wsa@kernel.org>,
	Serge Semin <fancer.lancer@gmail.com>,
	Yann Sionneau <yann@sionneau.net>, Will Deacon <will@kernel.org>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Jan Dabros <jsd@semihalf.com>, Andi Shyti <andi.shyti@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] i2c: designware: Fix corrupted memory seen in the ISR
Date: Tue, 19 Sep 2023 11:54:10 -0700	[thread overview]
Message-ID: <cde7e2fc-2e13-4b82-98b3-3d3a52c4c185@os.amperecomputing.com> (raw)
In-Reply-To: <ZQm1UyZ0g7KxRW3a@arm.com>

On 9/19/2023 7:51 AM, Catalin Marinas wrote:
> 
> While smp_* is ok, it really depends on what the regmap_write() does. Is
> it a write to a shared peripheral (if not, you may need a DSB)? Does the
> regmap_write() caller know this? That's why I think having the barrier
> in dw_reg_write() is better.
> 
> If you do want to stick to a fix in i2c_dw_xfer_init(), you could go for
> dma_wmb(). While this is not strictly DMA, it's sharing data with
> another coherent agent (a different CPU in this instance). The smp_wmb()
> is more about communication via memory not involving I/O. But this still
> assumes that the caller knows regmap_write() ends up with an I/O
> write*() (potentially relaxed).

If we wanted maximum correctness wouldn't we need something like 
writel_triggers_interrupt/regmap_write_triggers_interrupt or maybe 
preinterrupt_wmb?

The ARM docs do have a specific example case where the device write 
triggers an interrupt, and that example specifically says a DSB barrier 
is needed.

If I look at the ARM GIC IPI send function gic_ipi_send_mask in 
https://elixir.bootlin.com/linux/v6.6-rc2/source/drivers/irqchip/irq-gic-v3.c#L1354 
is says:

         /*
	 * Ensure that stores to Normal memory are visible to the
	 * other CPUs before issuing the IPI.
	 */
	dsb(ishst);

I would think the IPI send code is very carefully tuned for performance, 
and would not use a barrier any stronger than required.

I believe dma_wmb maps to DMB on ARM64.

- Jan







  parent reply	other threads:[~2023-09-19 18:54 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-13 23:29 [PATCH v2] i2c: designware: Fix corrupted memory seen in the ISR Jan Bottorff
2023-09-14 18:46 ` Andy Shevchenko
2023-09-14 18:47   ` Andy Shevchenko
2023-09-14 20:52     ` Jan Bottorff
2023-09-15 12:44 ` Jarkko Nikula
2023-09-15 15:21 ` Serge Semin
2023-09-16  1:47   ` Jan Bottorff
2023-09-17  0:01     ` Serge Semin
2023-09-17 20:08       ` Yann Sionneau
2023-09-18 23:14         ` Serge Semin
2023-09-19  3:45           ` Jan Bottorff
2023-09-19  9:55             ` Catalin Marinas
2023-09-19 10:19               ` Wolfram Sang
2023-09-19 12:38                 ` Yann Sionneau
2023-09-19 14:51                   ` Catalin Marinas
2023-09-19 14:55                     ` Wolfram Sang
2023-09-19 18:54                     ` Jan Bottorff [this message]
2023-09-19 21:05                       ` Serge Semin
2023-09-20  9:08                         ` Wolfram Sang
2023-09-20 13:27                           ` Yann Sionneau
2023-09-20 19:14                             ` Jan Bottorff
2023-09-25 12:54                               ` Serge Semin
2023-09-25 19:39                                 ` Jan Bottorff
2023-09-27 19:38                                   ` Wolfram Sang
2023-09-29  8:48                                     ` Jarkko Nikula
2023-10-26 11:18                                       ` Wolfram Sang
2023-10-31  0:12                                         ` Jan Bottorff
2023-10-31  5:51                                           ` Wolfram Sang
2023-10-31  8:44                                           ` Yann Sionneau
2023-10-31 12:10                                             ` Jarkko Nikula
2023-10-31 13:06                                               ` Serge Semin
2023-11-01 16:51                                                 ` Jan Bottorff
2023-09-20 11:03                         ` Catalin Marinas
2023-09-20 10:44                       ` Catalin Marinas
2023-09-20 11:05                         ` Catalin Marinas

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=cde7e2fc-2e13-4b82-98b3-3d3a52c4c185@os.amperecomputing.com \
    --to=janb@os.amperecomputing.com \
    --cc=andi.shyti@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=catalin.marinas@arm.com \
    --cc=fancer.lancer@gmail.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=jsd@semihalf.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=p.zabel@pengutronix.de \
    --cc=will@kernel.org \
    --cc=wsa@kernel.org \
    --cc=yann@sionneau.net \
    --cc=ysionneau@kalrayinc.com \
    /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