public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ARM: cache-l2x0: clean up aurora cache handling
Date: Wed, 19 Nov 2014 16:06:31 +0100	[thread overview]
Message-ID: <4475865.jdxZMh688N@wuerfel> (raw)
In-Reply-To: <20141119145015.GG4042@n2100.arm.linux.org.uk>

On Wednesday 19 November 2014 14:50:15 Russell King - ARM Linux wrote:
> On Wed, Nov 19, 2014 at 03:40:12PM +0100, Arnd Bergmann wrote:
> > The aurora cache controller is the only remaining user of a couple
> > of functions in this file and are completely unused when that is
> > disabled, leading to build warnings:
> > 
> > arch/arm/mm/cache-l2x0.c:167:13: warning: 'l2x0_cache_sync' defined but not used [-Wunused-function]
> > arch/arm/mm/cache-l2x0.c:184:13: warning: 'l2x0_flush_all' defined but not used [-Wunused-function]
> > arch/arm/mm/cache-l2x0.c:194:13: warning: 'l2x0_disable' defined but not used [-Wunused-function]
> > 
> > With the knowledge that the code is now aurora-specific, we can
> > simplify it noticeably:
> > 
> > - The pl310 errata workarounds are not needed on aurora and can be removed
> > - The cache_wait() macro is never needed since this is not an l210/l220
> 
> How do we know?  You claim it's "not a L210/L220" but how do we actually
> know that we don't have to wait for the cache operation to complete?
>
> When this is built without L310 support, Aurora will wait for the cache
> operations to complete, but when built with L310 support, it won't.

I'll update the description to say:

- As confirmed by Thomas Petazzoni from the data sheet, the cache_wait()
  macro is never needed.

based on the previous email exchange. Thanks for pointing out that the
description doesn't reflect what we had discussed already.

> > - aurora_pa_range can keep the spinlock while syncing the cache
> 
> If the operations complete synchronously (in other words, the L2 cache
> doesn't accept another transaction until the cache operation has completed)
> the spinlocks should not be necessary.
> 
> They're necessary for some of the ARM caches which allow cache operations
> to run asynchronously (hence the wait stuff) but if you don't have the
> wait stuff as you imply above, they can't be asynchronous.

I think the spinlock is still needed, it now only protects this sequence:

                raw_spin_lock_irqsave(&l2x0_lock, flags);
                writel_relaxed(start, base + AURORA_RANGE_BASE_ADDR_REG);
                writel_relaxed(range_end - CACHE_LINE_SIZE, base + offset);
                writel_relaxed(0, base + AURORA_SYNC_REG);
                raw_spin_unlock_irqrestore(&l2x0_lock, flags);

without the lock, two concurrent processes might start writing into
AURORA_RANGE_BASE_ADDR_REG before the first one writes the operation
register.

	Arnd

  reply	other threads:[~2014-11-19 15:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19 14:40 [PATCH 1/2] ARM: cache-l2x0: clean up aurora cache handling Arnd Bergmann
2014-11-19 14:41 ` [PATCH 2/2] ARM: cache-l2x0: optimize aurora range operations Arnd Bergmann
2014-11-19 14:50 ` [PATCH 1/2] ARM: cache-l2x0: clean up aurora cache handling Russell King - ARM Linux
2014-11-19 15:06   ` Arnd Bergmann [this message]
2014-11-19 15:10     ` Arnd Bergmann
  -- strict thread matches above, loose matches on Subject: below --
2014-09-08 14:53 [PATCH] ARM: avoid l2x0 build warning for CONFIG_OF=n Arnd Bergmann
2014-09-08 15:39 ` Russell King - ARM Linux
2014-09-08 20:36   ` Arnd Bergmann
2014-09-08 20:42     ` [PATCH 1/2] ARM: cache-l2x0: clean up aurora cache handling Arnd Bergmann
2014-09-11  8:54       ` Thomas Petazzoni
2014-09-11  9:50         ` Arnd Bergmann
2014-09-11 10:07           ` Thomas Petazzoni
2014-09-11 10:16         ` Russell King - ARM Linux
2014-09-11 10:31           ` Thomas Petazzoni
2014-09-11 10:08       ` Thomas Petazzoni

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=4475865.jdxZMh688N@wuerfel \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.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