All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thierry Reding" <thierry.reding@gmail.com>
To: "Marc Dietrich" <marvin24@gmx.de>,
	"Ben Dooks" <ben.dooks@codethink.co.uk>
Cc: <linux-staging@lists.linux.dev>, <linux-tegra@vger.kernel.org>,
	<gregkh@linuxfoundation.org>,
	"Thierry Reding" <treding@nvidia.com>
Subject: Re: [PATCH] staging: nvec: make i2c controller register writes robust
Date: Thu, 30 May 2024 17:12:58 +0200	[thread overview]
Message-ID: <D1N2RF28D1Q9.9BPMZE1VNFIU@gmail.com> (raw)
In-Reply-To: <79c10e8e-bf3e-7eca-a0cd-e177a270a517@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On Wed May 1, 2024 at 9:03 PM CEST, Marc Dietrich wrote:
> Hi Ben,
>
> On Mon, 22 Apr 2024, Ben Dooks wrote:
>
> > On 21/04/2024 11:46, Marc Dietrich wrote:
> >> The i2c controller needs to read back the data written to its registers.
> >> This way we can avoid the long delay in the interrupt handler.
> >>
> >> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> >> ---
> >>   drivers/staging/nvec/nvec.c | 41 ++++++++++++++++++++++---------------
> >>   1 file changed, 24 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> >> index 45df190c2f94..214839f51048 100644
> >> --- a/drivers/staging/nvec/nvec.c
> >> +++ b/drivers/staging/nvec/nvec.c
> >> @@ -570,6 +570,22 @@ static void nvec_tx_set(struct nvec_chip *nvec)
> >>   		(uint)nvec->tx->size, nvec->tx->data[1]);
> >>   }
> >>
> >> +/**
> >> + * i2c_writel - safely write to an I2C client controller register
> >> + * @val: value to be written
> >> + * @reg: register to write to
> >> + *
> >> + * A write to an I2C controller register needs to be read back to make
> >> sure
> >> + * that the value has arrived.
> >> + */
> >> +static void i2c_writel(u32 val, void *reg)
> >> +{
> >> +	writel_relaxed(val, reg);
> >> +
> >> +	/* read back register to make sure that register writes completed */
> >> +	readl_relaxed(reg);
> >> +}
> >
> > I thought the default behaviour of writel() should be to force writes
> > out of any CPU buffers. Are there any bus isuses here causing the code
> > to be necessary (and if so, why is there another buffer breaking the
> > writel behaviour?)
>
> if fear that's a question only NVIDIA can answer.

Ben,

in case you didn't see the discussion on v2 of this patch, it's here:

	https://lore.kernel.org/all/D1N2OIXAF6QQ.3TCYLBU42CJ3U@gmail.com/

in a nutshell: there's indeed another buffer here that causes these
writes to be queued and the read-back flushes that queue.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-05-30 15:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-21 10:46 [PATCH] staging: nvec: make i2c controller register writes robust Marc Dietrich
2024-04-21 11:13 ` Greg KH
2024-04-21 18:44   ` Marc Dietrich
2024-04-22 11:29 ` Ben Dooks
2024-05-01 19:03   ` Marc Dietrich
2024-05-30 15:12     ` Thierry Reding [this message]
2024-05-20  8:11 ` Marc Dietrich
2024-05-20  8:52   ` Greg KH
2024-05-20  9:03   ` Greg KH

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=D1N2RF28D1Q9.9BPMZE1VNFIU@gmail.com \
    --to=thierry.reding@gmail.com \
    --cc=ben.dooks@codethink.co.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-tegra@vger.kernel.org \
    --cc=marvin24@gmx.de \
    --cc=treding@nvidia.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 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.