public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
@ 2017-04-18 14:25 Ravi Babu
  2017-04-18 14:44 ` Lukasz Majewski
  0 siblings, 1 reply; 6+ messages in thread
From: Ravi Babu @ 2017-04-18 14:25 UTC (permalink / raw)
  To: u-boot

This patch fixes the compilation error
common/cli_hush.c:3349: undefined reference to 'realloc_simple'

The dfu uses run_command(), it is part of cli_hush.c
but defining CONFIG_HUSH_PARSER for spl-dfu causes
this compilation error.

Signed-off-by: Ravi Babu <ravibabu@ti.com>
---
 common/cli.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cli.c b/common/cli.c
index a433ef2..5e0869b 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 int run_command(const char *cmd, int flag)
 {
-#ifndef CONFIG_HUSH_PARSER
+#if defined(CONFIG_SPL_DFU_SUPPORT) || !defined(CONFIG_HUSH_PARSER)
 	/*
 	 * cli_run_command can return 0 or 1 for success, so clean up
 	 * its result.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
  2017-04-18 14:25 Ravi Babu
@ 2017-04-18 14:44 ` Lukasz Majewski
  2017-04-19  7:19   ` B, Ravi
  0 siblings, 1 reply; 6+ messages in thread
From: Lukasz Majewski @ 2017-04-18 14:44 UTC (permalink / raw)
  To: u-boot

Hi Ravi,

> This patch fixes the compilation error
> common/cli_hush.c:3349: undefined reference to 'realloc_simple'
> 
> The dfu uses run_command(), it is part of cli_hush.c
> but defining CONFIG_HUSH_PARSER for spl-dfu causes
> this compilation error.
> 
> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> ---
>  common/cli.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/cli.c b/common/cli.c
> index a433ef2..5e0869b 100644
> --- a/common/cli.c
> +++ b/common/cli.c
> @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  int run_command(const char *cmd, int flag)
>  {
> -#ifndef CONFIG_HUSH_PARSER
> +#if defined(CONFIG_SPL_DFU_SUPPORT) || !defined(CONFIG_HUSH_PARSER)

I must admit that this seems odd to me....since we should avoided adding
(SPL DFU) dependency to common parser code.

Maybe some kconfig tweaks would help?

>  	/*
>  	 * cli_run_command can return 0 or 1 for success, so clean up
>  	 * its result.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
  2017-04-18 14:44 ` Lukasz Majewski
@ 2017-04-19  7:19   ` B, Ravi
  0 siblings, 0 replies; 6+ messages in thread
From: B, Ravi @ 2017-04-19  7:19 UTC (permalink / raw)
  To: u-boot

Hi Lukasz

>> diff --git a/common/cli.c b/common/cli.c index a433ef2..5e0869b 100644
>> --- a/common/cli.c
>> +++ b/common/cli.c
>> @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   */
>>  int run_command(const char *cmd, int flag)  { -#ifndef 
>> CONFIG_HUSH_PARSER
>> +#if defined(CONFIG_SPL_DFU_SUPPORT) || !defined(CONFIG_HUSH_PARSER)

>I must admit that this seems odd to me....since we should avoided adding
> (SPL DFU) dependency to common parser code.

Yes, I agree, or use CONFIG_SPL_BUILD.

>Maybe some kconfig tweaks would help?

You Mean, unselect CONFIG_HUSH_PARSER when SPL_DFU_SUPPORT enabled?

Regards
Ravi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
       [not found] <1492589976-11595-1-git-send-email-ravibabu@ti.com>
@ 2017-04-19 10:53 ` Lukasz Majewski
  2017-04-19 11:00   ` B, Ravi
  2017-04-20 12:49   ` B, Ravi
  0 siblings, 2 replies; 6+ messages in thread
From: Lukasz Majewski @ 2017-04-19 10:53 UTC (permalink / raw)
  To: u-boot

Hi Ravi,

Please add u-boot mailing list to CC, when sending patches.

> This patch fixes the compilation error
> common/cli_hush.c:3349: undefined reference to 'realloc_simple'
> 
> The dfu does not depend on HUSH_PARSER.
> The dfu, fastboot uses run_command(), it is
> part of cli.c and use cli_simple_run_command().
> 
> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> ---
>  common/Makefile | 1 -
>  common/cli.c    | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/common/Makefile b/common/Makefile
> index bcd2486..dc5cb1d 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -90,7 +90,6 @@ endif # !CONFIG_SPL_BUILD
>  
>  ifdef CONFIG_SPL_BUILD
>  obj-$(CONFIG_SPL_DFU_SUPPORT) += dfu.o
> -obj-$(CONFIG_SPL_DFU_SUPPORT) += cli_hush.o
>  obj-$(CONFIG_SPL_HASH_SUPPORT) += hash.o
>  obj-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
>  obj-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o
> diff --git a/common/cli.c b/common/cli.c
> index a433ef2..3dd4c2b 100644
> --- a/common/cli.c
> +++ b/common/cli.c
> @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  int run_command(const char *cmd, int flag)
>  {
> -#ifndef CONFIG_HUSH_PARSER
> +#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_HUSH_PARSER)
>  	/*
>  	 * cli_run_command can return 0 or 1 for success, so clean up
>  	 * its result.

So for the initial support of the DFU in SPL you did not needed the
HUSH parser.

What is the motivation to enable it now? Enabling the HUSH in SPL
should make the binary even bigger - hence it is counterintuitive. 


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
  2017-04-19 10:53 ` [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu Lukasz Majewski
@ 2017-04-19 11:00   ` B, Ravi
  2017-04-20 12:49   ` B, Ravi
  1 sibling, 0 replies; 6+ messages in thread
From: B, Ravi @ 2017-04-19 11:00 UTC (permalink / raw)
  To: u-boot

Hi Lukasz

>> 
>> diff --git a/common/Makefile b/common/Makefile index bcd2486..dc5cb1d 
>> 100644
>> --- a/common/Makefile
>> +++ b/common/Makefile
>> @@ -90,7 +90,6 @@ endif # !CONFIG_SPL_BUILD
>>  
>>  ifdef CONFIG_SPL_BUILD
>>  obj-$(CONFIG_SPL_DFU_SUPPORT) += dfu.o
>> -obj-$(CONFIG_SPL_DFU_SUPPORT) += cli_hush.o
>>  obj-$(CONFIG_SPL_HASH_SUPPORT) += hash.o
>>  obj-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
>>  obj-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o diff --git 
>> a/common/cli.c b/common/cli.c index a433ef2..3dd4c2b 100644
>> --- a/common/cli.c
>> +++ b/common/cli.c
>> @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   */
>>  int run_command(const char *cmd, int flag)  { -#ifndef 
>> CONFIG_HUSH_PARSER
>> +#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_HUSH_PARSER)
>>  	/*
>>  	 * cli_run_command can return 0 or 1 for success, so clean up
>>  	 * its result.

>So for the initial support of the DFU in SPL you did not needed the HUSH parser.

>What is the motivation to enable it now? Enabling the HUSH in SPL should make the binary even bigger - hence it is counterintuitive. 

The CONFIG_HUSH_PARSER is enabled in dra7xx_evm_defconfig by commit: adad96e: configs: Re-sync HUSH options
Hence CONFIG_HUSH_PARSER included for both SPL and u-boot as well. 
The cli.c is common for both SPL & u-boot.

Regards
Ravi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu
  2017-04-19 10:53 ` [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu Lukasz Majewski
  2017-04-19 11:00   ` B, Ravi
@ 2017-04-20 12:49   ` B, Ravi
  1 sibling, 0 replies; 6+ messages in thread
From: B, Ravi @ 2017-04-20 12:49 UTC (permalink / raw)
  To: u-boot

Hi Lukasz

>>> a/common/cli.c b/common/cli.c index a433ef2..3dd4c2b 100644
>>> --- a/common/cli.c
>>> +++ b/common/cli.c
>>> @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>   */
>>>  int run_command(const char *cmd, int flag)  { -#ifndef 
>>> CONFIG_HUSH_PARSER
>>> +#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_HUSH_PARSER)
>>>  	/*
>>>  	 * cli_run_command can return 0 or 1 for success, so clean up
>>>  	 * its result.

>>So for the initial support of the DFU in SPL you did not needed the HUSH parser.

>>What is the motivation to enable it now? Enabling the HUSH in SPL should make the binary even bigger - hence it is counterintuitive. 

>The CONFIG_HUSH_PARSER is enabled in dra7xx_evm_defconfig by commit: adad96e: configs: Re-sync HUSH options Hence CONFIG_HUSH_PARSER included for both SPL and u-boot as well. 
>The cli.c is common for both SPL & u-boot.

Since by default HUSH_PARSER is defined for all *_evm_defconfigs, which affect for both SPL/U-Boot.
There is no way to select simple_cli() in run_command(). I feel this would be simple solution to use CONFIG_SPL_BUILD.
Even any Kconfig tweaks also would endup in similar way.

Let me know any alternate option.

Another point: 
The SPL_DFU_SUPPORT is depends on SPL_RAM_SUPPORT. I will send separate patch to add the dependency in Kconfig selection that SPL_DFU_SUPPORT is selected only if SPL_RAM_SUPPORT is selected.

Thanks & Regards
Ravi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-20 12:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1492589976-11595-1-git-send-email-ravibabu@ti.com>
2017-04-19 10:53 ` [U-Boot] [PATCH] spl: dfu: compilation fixes for spl-dfu Lukasz Majewski
2017-04-19 11:00   ` B, Ravi
2017-04-20 12:49   ` B, Ravi
2017-04-18 14:25 Ravi Babu
2017-04-18 14:44 ` Lukasz Majewski
2017-04-19  7:19   ` B, Ravi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox