PL310 errata workarounds

[Rob Herring]

On Fri, Mar 14, 2014 at 12:57 PM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Fri, Mar 14, 2014 at 11:02:17AM -0500, Rob Herring wrote:
>> The obvious fix is to convert l2x0_flush_line to a function ptr which
>> just further complicates the code.
>
> I currently regard this code as unmaintainable, so right now I'm in the
> process of completely rewriting it, going back to the specs and checking
> what is required.  Sharing code between the L210, L220 and L310 looks
> good from the point of view of keeping the line count down, but actually
> they're quite different and have various different requirements.
>
> Out of those, L220 is the odd one out: all operations are background
> operations there, which means they have to be waited for.  Currently,
> if you have one of these, and you build a multiplatform kernel which
> includes PL310 support, you don't wait for any of these and you probably
> see a lot of data corruption as a result.

Oops. I guess there are not many users as the L220 was primarily used
with the 1176 (and 11MP?) IIRC. I guess one of the primary 1176 based
platforms currently used is RPi, but it appears the BCM2835 doesn't
have an L2 (other than some custom L2 for the GPU).

> PL210 and PL310 are very similar, but PL310 has all the nasty errata
> against it.  Both of these have the nice behaviour that provided you
> stick to the "atomic" operations (physical addressed operations, and
> cache sync) and you don't need to do the errata workaround dances,
> then you don't need the spinlocks... which means less contention on
> a SMP system.

I would definitely like to see removing the spinlock. readl/writel can
be a hotspot because of this if you have them in any critical path.

Really, if you are flushing such large buffers that you need the
optimization, you should fix how the buffers are mapped rather than
optimizing the flush.

>
> Besides, the L2 mess extends outside this file:
>
> static int sirfsoc_pm_enter(suspend_state_t state)
> {
> ...
>                 outer_flush_all();
>                 outer_disable();
>
> So, outer_disable() is defined to disable the L2 cache in a way that
> doesn't result in data loss to the system (which would crash it) so
> why is there that outer_flush_all() call there...  *REMOVED*.

Isn't l2x0_disable racy in SMP? What happens to a write between the
flush and the disable? If it is only usable when running single core,
then we can remove the spinlock there. I don't think any of the custom
disable functions like highbank take the lock.

Rob