Patchwork valgrind: Add support for MIPS architecture

login
register
mail settings
Submitter Vicente Olivert Riera
Date March 28, 2014, 4:46 p.m.
Message ID <1396025209-9495-1-git-send-email-Vincent.Riera@imgtec.com>
Download mbox | patch
Permalink /patch/334799/
State Accepted
Headers show

Comments

Vicente Olivert Riera - March 28, 2014, 4:46 p.m.
Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
---
 package/valgrind/Config.in |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Thomas De Schampheleire - March 28, 2014, 6:16 p.m.
Hi Vicente,

Vicente Olivert Riera <Vincent.Riera@imgtec.com> schreef:
>Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
>Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
>---
> package/valgrind/Config.in |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
>index dacdd86..fd5649b 100644
>--- a/package/valgrind/Config.in
>+++ b/package/valgrind/Config.in
>@@ -1,7 +1,8 @@
> config BR2_PACKAGE_VALGRIND
> 	bool "valgrind"
> 	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
>-		   BR2_cortex_a9 || BR2_powerpc
>+		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
>+		   BR2_mipsel || BR2_mips64 || BR2_mips64el
> 	help
> 	  Tool for debugging and profiling Linux programs.
> 

Did you test this on target?
Do you have any idea why it wasn't enabled for mips before?

Thanks,
Thomas
Vicente Olivert Riera - March 28, 2014, 6:24 p.m.
On 03/28/2014 06:16 PM, Thomas De Schampheleire wrote:
> Hi Vicente,
>
> Vicente Olivert Riera <Vincent.Riera@imgtec.com> schreef:
>> Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
>> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
>> ---
>> package/valgrind/Config.in |    3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
>> index dacdd86..fd5649b 100644
>> --- a/package/valgrind/Config.in
>> +++ b/package/valgrind/Config.in
>> @@ -1,7 +1,8 @@
>> config BR2_PACKAGE_VALGRIND
>> 	bool "valgrind"
>> 	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
>> -		   BR2_cortex_a9 || BR2_powerpc
>> +		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
>> +		   BR2_mipsel || BR2_mips64 || BR2_mips64el
>> 	help
>> 	  Tool for debugging and profiling Linux programs.
>>
>
> Did you test this on target?

Not yet, but it should work.

> Do you have any idea why it wasn't enabled for mips before?

Valgrind added support for MIPS on it's last release, 3.9.0:

http://sourceforge.net/p/valgrind/mailman/valgrind-announce/thread/52738DEF.1070801@acm.org/

But you already knew that. The following message is yours:

"One of the new features of Valgrind 3.9.0 is the addition of MIPS
support. Maybe it would be good to enable Valgrind on these
architectures. I'm adding Vincente and Markos in the Cc list, since
they are our main MIPS contributors. They might probably be interested
in doing this work, after testing on MIPS hardware."

> Thanks,
> Thomas
Thomas De Schampheleire - March 28, 2014, 6:40 p.m.
Vicente Olivert Riera <Vincent.Riera@imgtec.com> schreef:
>On 03/28/2014 06:16 PM, Thomas De Schampheleire wrote:
>> Hi Vicente,
>>
>> Vicente Olivert Riera <Vincent.Riera@imgtec.com> schreef:
>>> Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
>>> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
>>> ---
>>> package/valgrind/Config.in |    3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
>>> index dacdd86..fd5649b 100644
>>> --- a/package/valgrind/Config.in
>>> +++ b/package/valgrind/Config.in
>>> @@ -1,7 +1,8 @@
>>> config BR2_PACKAGE_VALGRIND
>>> 	bool "valgrind"
>>> 	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
>>> -		   BR2_cortex_a9 || BR2_powerpc
>>> +		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
>>> +		   BR2_mipsel || BR2_mips64 || BR2_mips64el
>>> 	help
>>> 	  Tool for debugging and profiling Linux programs.
>>>
>>
>> Did you test this on target?
>
>Not yet, but it should work.
>
>> Do you have any idea why it wasn't enabled for mips before?
>
>Valgrind added support for MIPS on it's last release, 3.9.0:
>
>http://sourceforge.net/p/valgrind/mailman/valgrind-announce/thread/52738DEF.1070801@acm.org/
>
>But you already knew that. The following message is yours:
>
>"One of the new features of Valgrind 3.9.0 is the addition of MIPS
>support. Maybe it would be good to enable Valgrind on these
>architectures. I'm adding Vincente and Markos in the Cc list, since
>they are our main MIPS contributors. They might probably be interested
>in doing this work, after testing on MIPS hardware."

I think you are confusing the two Thomas'es, but that's ok :-).

Thanks for your explanation...

Best regards,
Thomas
Thomas Petazzoni - March 29, 2014, 9:23 a.m.
Dear Vicente Olivert Riera,

On Fri, 28 Mar 2014 16:46:49 +0000, Vicente Olivert Riera wrote:
> Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
> ---
>  package/valgrind/Config.in |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)

Applied, thanks.

Thomas
Thomas Petazzoni - March 29, 2014, 3:47 p.m.
Dear Vicente Olivert Riera,

On Fri, 28 Mar 2014 16:46:49 +0000, Vicente Olivert Riera wrote:
> Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
> ---
>  package/valgrind/Config.in |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
> index dacdd86..fd5649b 100644
> --- a/package/valgrind/Config.in
> +++ b/package/valgrind/Config.in
> @@ -1,7 +1,8 @@
>  config BR2_PACKAGE_VALGRIND
>  	bool "valgrind"
>  	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
> -		   BR2_cortex_a9 || BR2_powerpc
> +		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
> +		   BR2_mipsel || BR2_mips64 || BR2_mips64el
>  	help
>  	  Tool for debugging and profiling Linux programs.

This breaks the build on MIPS64:

  http://autobuild.buildroot.org/results/213/21352bcbe1b309fef0f996c275cdfcda08619d96/build-end.log

Thomas
Vicente Olivert Riera - March 31, 2014, 8:58 a.m.
On 03/29/2014 03:47 PM, Thomas Petazzoni wrote:
> Dear Vicente Olivert Riera,
>
> On Fri, 28 Mar 2014 16:46:49 +0000, Vicente Olivert Riera wrote:
>> Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
>> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
>> ---
>>   package/valgrind/Config.in |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
>> index dacdd86..fd5649b 100644
>> --- a/package/valgrind/Config.in
>> +++ b/package/valgrind/Config.in
>> @@ -1,7 +1,8 @@
>>   config BR2_PACKAGE_VALGRIND
>>   	bool "valgrind"
>>   	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
>> -		   BR2_cortex_a9 || BR2_powerpc
>> +		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
>> +		   BR2_mipsel || BR2_mips64 || BR2_mips64el
>>   	help
>>   	  Tool for debugging and profiling Linux programs.
>
> This breaks the build on MIPS64:
>
>    http://autobuild.buildroot.org/results/213/21352bcbe1b309fef0f996c275cdfcda08619d96/build-end.log
>
> Thomas
>

Yeah, I sent a patch to upstream few weeks ago, and it was applied. I 
will send it to BR today.

Thanks for reminding me this problem.

Patch

diff --git a/package/valgrind/Config.in b/package/valgrind/Config.in
index dacdd86..fd5649b 100644
--- a/package/valgrind/Config.in
+++ b/package/valgrind/Config.in
@@ -1,7 +1,8 @@ 
 config BR2_PACKAGE_VALGRIND
 	bool "valgrind"
 	depends on BR2_i386 || BR2_x86_64 || BR2_cortex_a8 || \
-		   BR2_cortex_a9 || BR2_powerpc
+		   BR2_cortex_a9 || BR2_powerpc || BR2_mips || \
+		   BR2_mipsel || BR2_mips64 || BR2_mips64el
 	help
 	  Tool for debugging and profiling Linux programs.