diff mbox

[U-Boot,V2,2/4] ARM: 926ejs: use debug() for misaligned addresses

Message ID 1333383529-16064-1-git-send-email-sbabic@denx.de
State Accepted
Commit c8d9ceaf061c5ea5c17ee9e95dbff563e41904c0
Headers show

Commit Message

Stefano Babic April 2, 2012, 4:18 p.m. UTC
Misaligned warnings are useful to debug faulty drivers.
A misaligned warning is printed also when the driver
is correct - use debug() instead of printf().

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Albert Aribaud <albert.u.boot@aribaud.net>
CC: Mike Frysinger <vapier@gentoo.org>
CC: Marek Vasut <marex@denx.de>
---

Changes since V1:

This substitutes [PATCH 2/4] net: round up before calling flush_cache
- change warning in cache.c instead of fixing cmd_net.c
	(M. Frysinger, M. Vasut)

 arch/arm/cpu/arm926ejs/cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Marek Vasut April 2, 2012, 4:29 p.m. UTC | #1
Dear Stefano Babic,

> Misaligned warnings are useful to debug faulty drivers.
> A misaligned warning is printed also when the driver
> is correct - use debug() instead of printf().
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Albert Aribaud <albert.u.boot@aribaud.net>
> CC: Mike Frysinger <vapier@gentoo.org>
> CC: Marek Vasut <marex@denx.de>

Acked-by: Marek Vasut <marex@denx.de>

> ---
> 
> Changes since V1:
> 
> This substitutes [PATCH 2/4] net: round up before calling flush_cache
> - change warning in cache.c instead of fixing cmd_net.c
> 	(M. Frysinger, M. Vasut)
> 
>  arch/arm/cpu/arm926ejs/cache.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/cache.c
> b/arch/arm/cpu/arm926ejs/cache.c index 5b23e3a..4430578 100644
> --- a/arch/arm/cpu/arm926ejs/cache.c
> +++ b/arch/arm/cpu/arm926ejs/cache.c
> @@ -55,7 +55,7 @@ static int check_cache_range(unsigned long start,
> unsigned long stop) ok = 0;
> 
>  	if (!ok)
> -		printf("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
> +		debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
>  			start, stop);
> 
>  	return ok;

Best regards,
Marek Vasut
Mike Frysinger April 2, 2012, 6:23 p.m. UTC | #2
On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
> Misaligned warnings are useful to debug faulty drivers.
> A misaligned warning is printed also when the driver
> is correct - use debug() instead of printf().

unfortunately, this turns the failure into a silent one.  if i read
the code correctly, you still return an error in this code path which
means things don't actually get flushed/invalidated.

to the original concept, i have no problem with cache funcs all
warning on misalignment via debug() so developers can quickly see if
things need to be reviewed.
-mike
Marek Vasut April 2, 2012, 6:42 p.m. UTC | #3
Dear Mike Frysinger,

> On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
> > Misaligned warnings are useful to debug faulty drivers.
> > A misaligned warning is printed also when the driver
> > is correct - use debug() instead of printf().
> 
> unfortunately, this turns the failure into a silent one.  if i read
> the code correctly, you still return an error in this code path which
> means things don't actually get flushed/invalidated.

You certainly do return an error, yes. And you don't do the op ... but then, 
what's the whole point of this check?

You can just ignore the return value in the flush() op, but I still don't like 
how this is ignored.

> to the original concept, i have no problem with cache funcs all
> warning on misalignment via debug() so developers can quickly see if
> things need to be reviewed.
> -mike

Best regards,
Marek Vasut
Mike Frysinger April 2, 2012, 7:07 p.m. UTC | #4
On Mon, Apr 2, 2012 at 14:42, Marek Vasut wrote:
>> On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
>> > Misaligned warnings are useful to debug faulty drivers.
>> > A misaligned warning is printed also when the driver
>> > is correct - use debug() instead of printf().
>>
>> unfortunately, this turns the failure into a silent one.  if i read
>> the code correctly, you still return an error in this code path which
>> means things don't actually get flushed/invalidated.
>
> You certainly do return an error, yes. And you don't do the op ...
>
> You can just ignore the return value in the flush() op, but I still don't like
> how this is ignored.

this is all internal to this soc's cache code.  the common API
provides no support for returning an "error" because there is no such
thing with these funcs.  you do the operation on the region requested
regardless of implicit side-effects.

> but then, what's the whole point of this check?

my point all along :p
-mike
diff mbox

Patch

diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c
index 5b23e3a..4430578 100644
--- a/arch/arm/cpu/arm926ejs/cache.c
+++ b/arch/arm/cpu/arm926ejs/cache.c
@@ -55,7 +55,7 @@  static int check_cache_range(unsigned long start, unsigned long stop)
 		ok = 0;
 
 	if (!ok)
-		printf("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
+		debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
 			start, stop);
 
 	return ok;