diff mbox

[RFC,v2,2/5] ARM: mm: rename jump labels in v7_flush_dcache_all function

Message ID 1347986135-17979-3-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lorenzo Pieralisi Sept. 18, 2012, 4:35 p.m. UTC
This patch renames jump labels in v7_flush_dcache_all in order to define
a specific flush cache levels entry point.

TODO: factor out the level flushing loop if considered worthwhile and
      define the input registers requirements.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/mm/cache-v7.S | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Nicolas Pitre Sept. 18, 2012, 6:13 p.m. UTC | #1
On Tue, 18 Sep 2012, Lorenzo Pieralisi wrote:

> This patch renames jump labels in v7_flush_dcache_all in order to define
> a specific flush cache levels entry point.
> 
> TODO: factor out the level flushing loop if considered worthwhile and
>       define the input registers requirements.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Acked-by: Nicolas Pitre <nico@linaro.org>

> ---
>  arch/arm/mm/cache-v7.S | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index d1fa2f6..140b294 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -48,7 +48,7 @@ ENTRY(v7_flush_dcache_louis)
>  	mov	r3, r3, lsr #20			@ r3 = LoUIS * 2
>  	moveq	pc, lr				@ return if level == 0
>  	mov	r10, #0				@ r10 (starting level) = 0
> -	b	loop1				@ start flushing cache levels
> +	b	flush_levels			@ start flushing cache levels
>  ENDPROC(v7_flush_dcache_louis)
>  
>  /*
> @@ -67,7 +67,7 @@ ENTRY(v7_flush_dcache_all)
>  	mov	r3, r3, lsr #23			@ left align loc bit field
>  	beq	finished			@ if loc is 0, then no need to clean
>  	mov	r10, #0				@ start clean at cache level 0
> -loop1:
> +flush_levels:
>  	add	r2, r10, r10, lsr #1		@ work out 3x current cache level
>  	mov	r1, r0, lsr r2			@ extract cache type bits from clidr
>  	and	r1, r1, #7			@ mask of the bits for current cache only
> @@ -89,9 +89,9 @@ loop1:
>  	clz	r5, r4				@ find bit position of way size increment
>  	ldr	r7, =0x7fff
>  	ands	r7, r7, r1, lsr #13		@ extract max number of the index size
> -loop2:
> +loop1:
>  	mov	r9, r4				@ create working copy of max way size
> -loop3:
> +loop2:
>   ARM(	orr	r11, r10, r9, lsl r5	)	@ factor way and cache number into r11
>   THUMB(	lsl	r6, r9, r5		)
>   THUMB(	orr	r11, r10, r6		)	@ factor way and cache number into r11
> @@ -100,13 +100,13 @@ loop3:
>   THUMB(	orr	r11, r11, r6		)	@ factor index number into r11
>  	mcr	p15, 0, r11, c7, c14, 2		@ clean & invalidate by set/way
>  	subs	r9, r9, #1			@ decrement the way
> -	bge	loop3
> -	subs	r7, r7, #1			@ decrement the index
>  	bge	loop2
> +	subs	r7, r7, #1			@ decrement the index
> +	bge	loop1
>  skip:
>  	add	r10, r10, #2			@ increment cache number
>  	cmp	r3, r10
> -	bgt	loop1
> +	bgt	flush_levels
>  finished:
>  	mov	r10, #0				@ swith back to cache level 0
>  	mcr	p15, 2, r10, c0, c0, 0		@ select current cache level in cssr
> -- 
> 1.7.12
> 
>
tip-bot for Dave Martin Sept. 19, 2012, 1:51 p.m. UTC | #2
On Tue, Sep 18, 2012 at 05:35:32PM +0100, Lorenzo Pieralisi wrote:
> This patch renames jump labels in v7_flush_dcache_all in order to define
> a specific flush cache levels entry point.
> 
> TODO: factor out the level flushing loop if considered worthwhile and
>       define the input registers requirements.

In the context of this series, this patch seems to do nothing at all (?)

Maybe it would make sense to defer this patch until you post something
that uses it.

Given that I have a fair expectation that you will build something useful
on top of this in the near future, I don't have a strong feeling about
it, though.

Cheers
---Dave

> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  arch/arm/mm/cache-v7.S | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index d1fa2f6..140b294 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -48,7 +48,7 @@ ENTRY(v7_flush_dcache_louis)
>  	mov	r3, r3, lsr #20			@ r3 = LoUIS * 2
>  	moveq	pc, lr				@ return if level == 0
>  	mov	r10, #0				@ r10 (starting level) = 0
> -	b	loop1				@ start flushing cache levels
> +	b	flush_levels			@ start flushing cache levels
>  ENDPROC(v7_flush_dcache_louis)
>  
>  /*
> @@ -67,7 +67,7 @@ ENTRY(v7_flush_dcache_all)
>  	mov	r3, r3, lsr #23			@ left align loc bit field
>  	beq	finished			@ if loc is 0, then no need to clean
>  	mov	r10, #0				@ start clean at cache level 0
> -loop1:
> +flush_levels:
>  	add	r2, r10, r10, lsr #1		@ work out 3x current cache level
>  	mov	r1, r0, lsr r2			@ extract cache type bits from clidr
>  	and	r1, r1, #7			@ mask of the bits for current cache only
> @@ -89,9 +89,9 @@ loop1:
>  	clz	r5, r4				@ find bit position of way size increment
>  	ldr	r7, =0x7fff
>  	ands	r7, r7, r1, lsr #13		@ extract max number of the index size
> -loop2:
> +loop1:
>  	mov	r9, r4				@ create working copy of max way size
> -loop3:
> +loop2:
>   ARM(	orr	r11, r10, r9, lsl r5	)	@ factor way and cache number into r11
>   THUMB(	lsl	r6, r9, r5		)
>   THUMB(	orr	r11, r10, r6		)	@ factor way and cache number into r11
> @@ -100,13 +100,13 @@ loop3:
>   THUMB(	orr	r11, r11, r6		)	@ factor index number into r11
>  	mcr	p15, 0, r11, c7, c14, 2		@ clean & invalidate by set/way
>  	subs	r9, r9, #1			@ decrement the way
> -	bge	loop3
> -	subs	r7, r7, #1			@ decrement the index
>  	bge	loop2
> +	subs	r7, r7, #1			@ decrement the index
> +	bge	loop1
>  skip:
>  	add	r10, r10, #2			@ increment cache number
>  	cmp	r3, r10
> -	bgt	loop1
> +	bgt	flush_levels
>  finished:
>  	mov	r10, #0				@ swith back to cache level 0
>  	mcr	p15, 2, r10, c0, c0, 0		@ select current cache level in cssr
> -- 
> 1.7.12
> 
>
Lorenzo Pieralisi Sept. 20, 2012, 10:32 a.m. UTC | #3
On Wed, Sep 19, 2012 at 02:51:56PM +0100, Dave Martin wrote:
> On Tue, Sep 18, 2012 at 05:35:32PM +0100, Lorenzo Pieralisi wrote:
> > This patch renames jump labels in v7_flush_dcache_all in order to define
> > a specific flush cache levels entry point.
> > 
> > TODO: factor out the level flushing loop if considered worthwhile and
> >       define the input registers requirements.
> 
> In the context of this series, this patch seems to do nothing at all (?)

Agreed, it is just replacing some labels. I thought that something like:

b flush_levels

is clearer than:

b loop1

If I manage to factor out the cache level flushing loop I think things
are even better, I just did not want to change v7_flush_dcache_all, I would
avoid doing that, unless, as I mentioned, it is considered worthwhile.

> Maybe it would make sense to defer this patch until you post something
> that uses it.

v7_flush_dcache_louis uses it, I have no problem in deferring it though.

Lorenzo
tip-bot for Dave Martin Sept. 20, 2012, 11:01 a.m. UTC | #4
On Thu, Sep 20, 2012 at 11:32:12AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Sep 19, 2012 at 02:51:56PM +0100, Dave Martin wrote:
> > On Tue, Sep 18, 2012 at 05:35:32PM +0100, Lorenzo Pieralisi wrote:
> > > This patch renames jump labels in v7_flush_dcache_all in order to define
> > > a specific flush cache levels entry point.
> > > 
> > > TODO: factor out the level flushing loop if considered worthwhile and
> > >       define the input registers requirements.
> > 
> > In the context of this series, this patch seems to do nothing at all (?)
> 
> Agreed, it is just replacing some labels. I thought that something like:
> 
> b flush_levels
> 
> is clearer than:
> 
> b loop1
> 
> If I manage to factor out the cache level flushing loop I think things
> are even better, I just did not want to change v7_flush_dcache_all, I would
> avoid doing that, unless, as I mentioned, it is considered worthwhile.
> 
> > Maybe it would make sense to defer this patch until you post something
> > that uses it.
> 
> v7_flush_dcache_louis uses it, I have no problem in deferring it though.

I don't think it's necessary to defer it -- I just wanted to understand
whether there was some context here I wasn't aware of.

Cheers
---Dave
diff mbox

Patch

diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index d1fa2f6..140b294 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -48,7 +48,7 @@  ENTRY(v7_flush_dcache_louis)
 	mov	r3, r3, lsr #20			@ r3 = LoUIS * 2
 	moveq	pc, lr				@ return if level == 0
 	mov	r10, #0				@ r10 (starting level) = 0
-	b	loop1				@ start flushing cache levels
+	b	flush_levels			@ start flushing cache levels
 ENDPROC(v7_flush_dcache_louis)
 
 /*
@@ -67,7 +67,7 @@  ENTRY(v7_flush_dcache_all)
 	mov	r3, r3, lsr #23			@ left align loc bit field
 	beq	finished			@ if loc is 0, then no need to clean
 	mov	r10, #0				@ start clean at cache level 0
-loop1:
+flush_levels:
 	add	r2, r10, r10, lsr #1		@ work out 3x current cache level
 	mov	r1, r0, lsr r2			@ extract cache type bits from clidr
 	and	r1, r1, #7			@ mask of the bits for current cache only
@@ -89,9 +89,9 @@  loop1:
 	clz	r5, r4				@ find bit position of way size increment
 	ldr	r7, =0x7fff
 	ands	r7, r7, r1, lsr #13		@ extract max number of the index size
-loop2:
+loop1:
 	mov	r9, r4				@ create working copy of max way size
-loop3:
+loop2:
  ARM(	orr	r11, r10, r9, lsl r5	)	@ factor way and cache number into r11
  THUMB(	lsl	r6, r9, r5		)
  THUMB(	orr	r11, r10, r6		)	@ factor way and cache number into r11
@@ -100,13 +100,13 @@  loop3:
  THUMB(	orr	r11, r11, r6		)	@ factor index number into r11
 	mcr	p15, 0, r11, c7, c14, 2		@ clean & invalidate by set/way
 	subs	r9, r9, #1			@ decrement the way
-	bge	loop3
-	subs	r7, r7, #1			@ decrement the index
 	bge	loop2
+	subs	r7, r7, #1			@ decrement the index
+	bge	loop1
 skip:
 	add	r10, r10, #2			@ increment cache number
 	cmp	r3, r10
-	bgt	loop1
+	bgt	flush_levels
 finished:
 	mov	r10, #0				@ swith back to cache level 0
 	mcr	p15, 2, r10, c0, c0, 0		@ select current cache level in cssr