From: gregory.clement@free-electrons•com (Gregory CLEMENT)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V3 3/6] arm: cache-l2x0: add support for Aurora L2 cache ctrl
Date: Thu, 20 Sep 2012 09:26:12 +0200 [thread overview]
Message-ID: <505AC514.2050805@free-electrons.com> (raw)
In-Reply-To: <20120915204257.GL12245@n2100.arm.linux.org.uk>
On 09/15/2012 10:42 PM, Russell King - ARM Linux wrote:> On Wed, Sep 05, 2012 at 03:44:34PM +0200, Gregory CLEMENT wrote:
>> @@ -275,6 +281,112 @@ static void l2x0_flush_range(unsigned long start, unsigned long end)
>> cache_sync();
>> raw_spin_unlock_irqrestore(&l2x0_lock, flags);
>> }
>> +/*
>
> Where's the blank line?
I will fix it
>
>> + * Note that the end addresses passed to Linux primitives are
>> + * noninclusive, while the hardware cache range operations use
>> + * inclusive start and end addresses.
>> + */
>> +static unsigned long calc_range_end(unsigned long start, unsigned long end)
>> +{
>> + if (!IS_ALIGNED(start, CACHE_LINE_SIZE)) {
>> + pr_warn("%s: start address not align on a cache line size\n",
>> + __func__);
>> + start &= ~(CACHE_LINE_SIZE-1);
>> + };
>
> No semicolon here. But why is this check even here?
>
I will remove it, see below.
>> +
>> + if (!IS_ALIGNED(end, CACHE_LINE_SIZE)) {
>> + pr_warn("%s: end address not align on a cache line size\n",
>> + __func__);
>> + end = (PAGE_ALIGN(end));
>> + }
>
> And this one - and why when it fails do you align to a page not a cache
> line?
I guess it was a bad copy/paste. it should be
end = ALIGN(end, CACHE_LINE_SIZE);
But I will remove it too.
>
>> +static void aurora_inv_range(unsigned long start, unsigned long end)
>> +{
>> + /*
>> + * round start and end adresses up to cache line size
>> + */
>> + start &= ~(CACHE_LINE_SIZE - 1);
>> + end = ALIGN(end, CACHE_LINE_SIZE);
>> +
>> + /*
>> + * Invalidate all full cache lines between 'start' and 'end'.
>> + */
>> + while (start < end) {
>> + unsigned long range_end = calc_range_end(start, end);
>
> And note that you (above) guarantee that the start/end addresses are
> cache line aligned. It only goes wrong if your calc_range_end()
> fails - but isn't that a matter of internal proving that your code is
> correct, rather than lumbering all kernels with such checking?
>
This part of the code was almost the same than the one in
cache-feroceon-l2.c. In first the versions there was BUG_ON() to test
if start and end were aligned on a cache line. Will Deacon proposed to
fix the addresses instead of rising an oops. But in the end, we can
just remove it, right.
I am going to submit an updated series which I hope will meet your
expectation.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2012-09-20 7:26 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-05 13:44 [PATCH V3] Add support for Aurora L2 Cache Controller Gregory CLEMENT
2012-09-05 13:44 ` [PATCH V3 1/6] arm: cache-l2x0: make outer_cache_fns a field of l2x0_of_data Gregory CLEMENT
2012-09-09 19:27 ` Jason Cooper
2012-09-15 20:35 ` Russell King - ARM Linux
2012-09-20 6:40 ` Gregory CLEMENT
2012-09-05 13:44 ` [PATCH V3 2/6] arm: cache-l2x0: add an optional register to save/restore Gregory CLEMENT
2012-09-09 19:28 ` Jason Cooper
2012-09-05 13:44 ` [PATCH V3 3/6] arm: cache-l2x0: add support for Aurora L2 cache ctrl Gregory CLEMENT
2012-09-06 11:11 ` Will Deacon
2012-09-06 11:49 ` Gregory CLEMENT
2012-09-06 13:02 ` Will Deacon
2012-09-09 19:33 ` Jason Cooper
2012-09-15 20:42 ` Russell King - ARM Linux
2012-09-20 7:26 ` Gregory CLEMENT [this message]
2012-09-05 13:44 ` [PATCH V3 4/6] arm: mvebu: add L2 cache support Gregory CLEMENT
2012-09-09 19:35 ` Jason Cooper
2012-09-05 13:44 ` [PATCH V3 5/6] arm: mvebu: add Aurora L2 Cache Controller to the DT Gregory CLEMENT
2012-09-09 19:36 ` Jason Cooper
2012-09-05 13:44 ` [PATCH V3 6/6] arm: l2x0: add aurora related properties to OF binding Gregory CLEMENT
2012-09-09 19:37 ` Jason Cooper
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=505AC514.2050805@free-electrons.com \
--to=gregory.clement@free-electrons$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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