public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: Yann Sionneau <ysionneau@kalrayinc.com>
To: Wolfram Sang <wsa@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jan Bottorff <janb@os.amperecomputing.com>,
	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 14:38:22 +0200	[thread overview]
Message-ID: <da400d3e-a357-1ae8-cb92-728cc4974b67@kalrayinc.com> (raw)
In-Reply-To: <ZQl1zwVkx9n2MPvr@shikoro>

Hi,

On 9/19/23 12:19, Wolfram Sang wrote:
>> I also agree that a wmb() in the i2c driver is not the more elegant fix.
>> For similar reasons, we hid barriers in the write*() macros, drivers
>> need to stay architecture-agnostic as much as possible.
> Exactly my thinking. I wanted to read this patch discussion later this
> week. But from glimpsing at it so far, I already wondered why there
> isn't a memory barrier in the final accessor to the register.

The regmap accessors used by the designware driver end up calling 
writel_relaxed() and readl_relaxed() : 
https://elixir.bootlin.com/linux/v6.6-rc2/source/drivers/i2c/busses/i2c-designware-common.c#L71

Those usually end up just being volatile accesses, making some kind of 
compiler barrier preventing the memory mapped register accesses to be 
moved/removed/merged.

I kind of think it's OK. It starts not being OK when you want some 
ordering between those and some memory accesses in DDR like the problem 
discussed here.

In those cases I would say the smp_* barriers are what we are supposed 
to use, isn't it?

Regards,

-- 

Yann






  reply	other threads:[~2023-09-19 12:38 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 [this message]
2023-09-19 14:51                   ` Catalin Marinas
2023-09-19 14:55                     ` Wolfram Sang
2023-09-19 18:54                     ` Jan Bottorff
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=da400d3e-a357-1ae8-cb92-728cc4974b67@kalrayinc.com \
    --to=ysionneau@kalrayinc.com \
    --cc=andi.shyti@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=catalin.marinas@arm.com \
    --cc=fancer.lancer@gmail.com \
    --cc=janb@os.amperecomputing.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 \
    /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