[PATCH 2/5] ARM: Add Broadcom Brahma-B15 readahead cache support
Russell King - ARM Linux
linux at arm.linux.org.uk
Mon Mar 16 17:10:56 PDT 2015
On Mon, Mar 16, 2015 at 02:20:53PM -0700, Florian Fainelli wrote:
> On 16/03/15 14:02, Russell King - ARM Linux wrote:
> > On Fri, Mar 06, 2015 at 04:54:50PM -0800, Florian Fainelli wrote:
> >> This patch adds support for the Broadcom Brahma-B15 CPU readahead cache
> >> controller. This cache controller sits between the L2 and the memory bus
> >> and its purpose is to provide a friendler burst size towards the DDR
> >> interface than the native cache line size.
> >> The readahead cache is mostly transparent, except for
> >> flush_kern_cache_all, flush_kern_cache_louis and flush_icache_all, which
> >> is precisely what we are overriding here.
> >> The readahead cache only intercepts reads, not writes, as such, some
> >> data can remain stale in any of its buffers, such that we need to flush
> >> it, which is an operation that needs to happen in a particular order:
> >> - disable the readahead cache
> >> - flush it
> >> - call the appropriate cache-v7.S function
> >> - re-enable
> >> This patch tries to minimize the impact to the cache-v7.S file by only
> >> providing a stub in case CONFIG_CACHE_B15_RAC is enabled (default for
> >> ARCH_BRCMSTB since it is the current user).
> >> Signed-off-by: Alamy Liu <alamyliu at broadcom.com>
> >> Signed-off-by: Florian Fainelli <f.fainelli at gmail.com>
> >> ---
> >> +/* Bitmask to enable instruction and data prefetching with a 256-bytes stride */
> >> +#define RAC_DATA_INST_EN_MASK (1 << RACPREFINST_SHIFT | \
> >> + RACENPREF_MASK << RACENINST_SHIFT | \
> >> + 1 << RACPREFDATA_SHIFT | \
> >> + RACENPREF_MASK << RACENDATA_SHIFT)
> >> +
> >> +#define RAC_ENABLED (1 << 0)
> > BIT(0) ?
> > However, you don't use RAC_ENABLED as a bitmask, but a bit index, so
> > shouldn't this be zero?
> In subsequent patches we have a need for distinguishing RAC_ENABLED from
> RAC_SUSPENDED, so that's the primary reason for using it as a bitmask
> (could make that clear somewhere).
However, test_bit() etc take a bit _number_ not a bit _mask_. So:
Passing in 1 << 0 will test bit 1 rather than bit 0.
Passing in 1 << 1 will test bit 2 rather than bit 1.
Passing in 1 << 2 will test bit 4 rather than bit 2.
Passing in 1 << 3 will test bit 8 rather than bit 3.
This is not what you wanted. Either use a mask directly, or use
test_bit() with a bit number etc. Don't try and do both together. :)
> > What happens when the system goes down (eg, for kexec?) Does the RAC
> > need to be disabled for that?
> Per boot convention, I would say so, yes, since this is another level of
> instruction and data cache, we should turn it off. Can we register some
> sort of notifier specifically for kexec?
The code at present doesn't expect there to be platform specific caches,
so that probably isn't catered for yet. I mentioned the point to raise
the issue that there's an oversight here.
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
More information about the linux-arm-kernel