diff mbox

[U-Boot,V2,1/2] imx: spl: enable CONFIG_SPL_WATCHDOG_SUPPORT

Message ID 1451910448-28192-1-git-send-email-peng.fan@nxp.com
State Accepted
Commit ed3fb1fb22fb9e0e786c58c2d80cd030b89b9b7d
Delegated to: Stefano Babic
Headers show

Commit Message

Peng Fan Jan. 4, 2016, 12:27 p.m. UTC
This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
which breaks SPL build when DEBUG macro defined.

"
arch/arm/lib/built-in.o: In function `do_reset':
~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
"
Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tim Harvey <tharvey@gateworks.com>
---
 include/configs/imx6_spl.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Eric Nelson Jan. 4, 2016, 12:31 p.m. UTC | #1
Hi Peng,

On 01/04/2016 05:27 AM, Peng Fan wrote:
> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
> which breaks SPL build when DEBUG macro defined.
> 
> "
> arch/arm/lib/built-in.o: In function `do_reset':
> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
> "
> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> ---
>  include/configs/imx6_spl.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> index 43ce7fe..68d3fd7 100644
> --- a/include/configs/imx6_spl.h
> +++ b/include/configs/imx6_spl.h
> @@ -34,6 +34,7 @@
>  #define CONFIG_SPL_SERIAL_SUPPORT
>  #define CONFIG_SPL_I2C_SUPPORT
>  #define CONFIG_SPL_GPIO_SUPPORT
> +#define CONFIG_SPL_WATCHDOG_SUPPORT
>  

I've been carrying a local patch for the same, so

Acked-by: Eric Nelson <eric@nelint.com>
Marek Vasut Jan. 4, 2016, 12:38 p.m. UTC | #2
On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
> which breaks SPL build when DEBUG macro defined.
> 
> "
> arch/arm/lib/built-in.o: In function `do_reset':
> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
> "
> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.

Why don't you implement dummy reset_cpu() {} instead ?

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> ---
>  include/configs/imx6_spl.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> index 43ce7fe..68d3fd7 100644
> --- a/include/configs/imx6_spl.h
> +++ b/include/configs/imx6_spl.h
> @@ -34,6 +34,7 @@
>  #define CONFIG_SPL_SERIAL_SUPPORT
>  #define CONFIG_SPL_I2C_SUPPORT
>  #define CONFIG_SPL_GPIO_SUPPORT
> +#define CONFIG_SPL_WATCHDOG_SUPPORT
> 
>  /* NAND support */
>  #if defined(CONFIG_SPL_NAND_SUPPORT)

Best regards,
Marek Vasut
Peng Fan Jan. 4, 2016, 12:40 p.m. UTC | #3
Hi Marek,
On Mon, Jan 04, 2016 at 01:38:23PM +0100, Marek Vasut wrote:
>On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
>> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
>> which breaks SPL build when DEBUG macro defined.
>> 
>> "
>> arch/arm/lib/built-in.o: In function `do_reset':
>> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
>> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
>> "
>> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
>
>Why don't you implement dummy reset_cpu() {} instead ?

Do you mean this, https://patchwork.ozlabs.org/patch/562232/?

Regards,
Peng

>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  include/configs/imx6_spl.h | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
>> index 43ce7fe..68d3fd7 100644
>> --- a/include/configs/imx6_spl.h
>> +++ b/include/configs/imx6_spl.h
>> @@ -34,6 +34,7 @@
>>  #define CONFIG_SPL_SERIAL_SUPPORT
>>  #define CONFIG_SPL_I2C_SUPPORT
>>  #define CONFIG_SPL_GPIO_SUPPORT
>> +#define CONFIG_SPL_WATCHDOG_SUPPORT
>> 
>>  /* NAND support */
>>  #if defined(CONFIG_SPL_NAND_SUPPORT)
>
>Best regards,
>Marek Vasut
Marek Vasut Jan. 4, 2016, 12:45 p.m. UTC | #4
On Monday, January 04, 2016 at 01:40:10 PM, Peng Fan wrote:
> Hi Marek,
> 
> On Mon, Jan 04, 2016 at 01:38:23PM +0100, Marek Vasut wrote:
> >On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
> >> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
> >> which breaks SPL build when DEBUG macro defined.
> >> 
> >> "
> >> arch/arm/lib/built-in.o: In function `do_reset':
> >> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
> >> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
> >> "
> >> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
> >
> >Why don't you implement dummy reset_cpu() {} instead ?
> 
> Do you mean this, https://patchwork.ozlabs.org/patch/562232/?

Yes, I'd prefer that, since I do not want to see watchdog support becoming
mandatory part of the SPL build. Watchdog support should be optional.

Best regards,
Marek Vasut
Peng Fan Jan. 4, 2016, 12:49 p.m. UTC | #5
On Mon, Jan 04, 2016 at 01:45:49PM +0100, Marek Vasut wrote:
>On Monday, January 04, 2016 at 01:40:10 PM, Peng Fan wrote:
>> Hi Marek,
>> 
>> On Mon, Jan 04, 2016 at 01:38:23PM +0100, Marek Vasut wrote:
>> >On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
>> >> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
>> >> which breaks SPL build when DEBUG macro defined.
>> >> 
>> >> "
>> >> arch/arm/lib/built-in.o: In function `do_reset':
>> >> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
>> >> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
>> >> "
>> >> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
>> >
>> >Why don't you implement dummy reset_cpu() {} instead ?
>> 
>> Do you mean this, https://patchwork.ozlabs.org/patch/562232/?
>
>Yes, I'd prefer that, since I do not want to see watchdog support becoming
>mandatory part of the SPL build. Watchdog support should be optional.

Wait for Stefano's comments on this -:)

Regards,
Peng.

>
>Best regards,
>Marek Vasut
Stefano Babic Jan. 4, 2016, 12:57 p.m. UTC | #6
Hi Marek, Peng,

On 04/01/2016 13:45, Marek Vasut wrote:
> On Monday, January 04, 2016 at 01:40:10 PM, Peng Fan wrote:
>> Hi Marek,
>>
>> On Mon, Jan 04, 2016 at 01:38:23PM +0100, Marek Vasut wrote:
>>> On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
>>>> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
>>>> which breaks SPL build when DEBUG macro defined.
>>>>
>>>> "
>>>> arch/arm/lib/built-in.o: In function `do_reset':
>>>> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
>>>> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
>>>> "
>>>> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
>>>
>>> Why don't you implement dummy reset_cpu() {} instead ?
>>
>> Do you mean this, https://patchwork.ozlabs.org/patch/562232/?
> 
> Yes, I'd prefer that, since I do not want to see watchdog support becoming
> mandatory part of the SPL build. Watchdog support should be optional.

Yes, enabling watchdog should be not be mandatory. Anyway, this happens
only if CONFIG_IMX_WATCHDOG is set, else CONFIG_SPL_WATCHDOG_SUPPORT
lets only to link reset_cpu() to the build. This is also what happens
for all MX6 boards that do not enable the watchdog but need reset_cpu().

Adding another dummy function looks nasty to me, and Fabio sent some
time ago patches to clean up this and drop empty reset_cpu() inside
board files. We should not create this mess again.

Best regards,
Stefano Babic
Marek Vasut Jan. 4, 2016, 1:23 p.m. UTC | #7
On Monday, January 04, 2016 at 01:57:59 PM, Stefano Babic wrote:
> Hi Marek, Peng,
> 
> On 04/01/2016 13:45, Marek Vasut wrote:
> > On Monday, January 04, 2016 at 01:40:10 PM, Peng Fan wrote:
> >> Hi Marek,
> >> 
> >> On Mon, Jan 04, 2016 at 01:38:23PM +0100, Marek Vasut wrote:
> >>> On Monday, January 04, 2016 at 01:27:27 PM, Peng Fan wrote:
> >>>> This commit 4bdcbe60a142b08eefccb0e326a37ba81d3389e8 removes reset_cpu
> >>>> which breaks SPL build when DEBUG macro defined.
> >>>> 
> >>>> "
> >>>> arch/arm/lib/built-in.o: In function `do_reset':
> >>>> ~/uboot/arch/arm/lib/reset.c:45: undefined reference to `reset_cpu'
> >>>> scripts/Makefile.spl:244: recipe for target 'spl/u-boot-spl' failed
> >>>> "
> >>>> Enable CONFIG_SPL_WATCHDOG_SUPPORT to fix this issue.
> >>> 
> >>> Why don't you implement dummy reset_cpu() {} instead ?
> >> 
> >> Do you mean this, https://patchwork.ozlabs.org/patch/562232/?
> > 
> > Yes, I'd prefer that, since I do not want to see watchdog support
> > becoming mandatory part of the SPL build. Watchdog support should be
> > optional.
> 
> Yes, enabling watchdog should be not be mandatory. Anyway, this happens
> only if CONFIG_IMX_WATCHDOG is set, else CONFIG_SPL_WATCHDOG_SUPPORT
> lets only to link reset_cpu() to the build.

This is completely counter-intuitive and stupid. I suspect this behavior
is required on iMX because iMXes reset through watchdog, yes ? In that
case, add a comment explaining why we always select watchdog in SPL on
i.MXes.

> This is also what happens
> for all MX6 boards that do not enable the watchdog but need reset_cpu().
> 
> Adding another dummy function looks nasty to me, and Fabio sent some
> time ago patches to clean up this and drop empty reset_cpu() inside
> board files. We should not create this mess again.

Fine

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
index 43ce7fe..68d3fd7 100644
--- a/include/configs/imx6_spl.h
+++ b/include/configs/imx6_spl.h
@@ -34,6 +34,7 @@ 
 #define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_I2C_SUPPORT
 #define CONFIG_SPL_GPIO_SUPPORT
+#define CONFIG_SPL_WATCHDOG_SUPPORT
 
 /* NAND support */
 #if defined(CONFIG_SPL_NAND_SUPPORT)