Re: [U-Boot] [PATCHv5 3/4] ARM: tegra124: Add an option to disable CoreSight

2016-09-06 Thread Stephen Warren

On 09/06/2016 12:06 PM, Julian Scheel wrote:

On 06.09.16 18:54, Stephen Warren wrote:

On 09/05/2016 07:29 AM, Julian Scheel wrote:

From: Alban Bedel 

When running on a SoC with a secure bootloader CoreSight isn't
allowed, so add an option to disable the CoreSight init.



diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig



+config ENABLE_CORESIGHT
+bool "Enable CoreSight"
+default y


Why "default y"? This changes behaviour on all our open development
boards. "default y" should be dropped, and the option should be added to
the defconfig for your board, or selected by the board's Kconfig option.


It's default yes, because the current codepath has it enabled. This
patch actually makes it possible to disable CoreSight by setting this
option to "n". Previously the code in clock_enable_coresight was run
unconditionally.


Oh right, I read the config option backwards. It's fine as-is then.


diff --git a/arch/arm/mach-tegra/cpu.c b/arch/arm/mach-tegra/cpu.c



 void clock_enable_coresight(int enable)
 {
+#if defined(CONFIG_ENABLE_CORESIGHT)
 u32 rst, src = 2;

 debug("%s entry\n", __func__);
@@ -402,6 +403,7 @@ void clock_enable_coresight(int enable)
 writel(rst, CSITE_CPU_DBG3_LAR);
 }
 }
+#endif
 }


It might be better to ifdef out the call-site. Otherwise, someone
reading that code won't have any idea that the function actually does
nothing.


Ok, I can change that.


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCHv5 3/4] ARM: tegra124: Add an option to disable CoreSight

2016-09-06 Thread Julian Scheel

On 06.09.16 18:54, Stephen Warren wrote:

On 09/05/2016 07:29 AM, Julian Scheel wrote:

From: Alban Bedel 

When running on a SoC with a secure bootloader CoreSight isn't
allowed, so add an option to disable the CoreSight init.



diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig



+config ENABLE_CORESIGHT
+bool "Enable CoreSight"
+default y


Why "default y"? This changes behaviour on all our open development
boards. "default y" should be dropped, and the option should be added to
the defconfig for your board, or selected by the board's Kconfig option.


It's default yes, because the current codepath has it enabled. This 
patch actually makes it possible to disable CoreSight by setting this 
option to "n". Previously the code in clock_enable_coresight was run 
unconditionally.



diff --git a/arch/arm/mach-tegra/cpu.c b/arch/arm/mach-tegra/cpu.c



 void clock_enable_coresight(int enable)
 {
+#if defined(CONFIG_ENABLE_CORESIGHT)
 u32 rst, src = 2;

 debug("%s entry\n", __func__);
@@ -402,6 +403,7 @@ void clock_enable_coresight(int enable)
 writel(rst, CSITE_CPU_DBG3_LAR);
 }
 }
+#endif
 }


It might be better to ifdef out the call-site. Otherwise, someone
reading that code won't have any idea that the function actually does
nothing.


Ok, I can change that.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCHv5 3/4] ARM: tegra124: Add an option to disable CoreSight

2016-09-06 Thread Stephen Warren

On 09/05/2016 07:29 AM, Julian Scheel wrote:

From: Alban Bedel 

When running on a SoC with a secure bootloader CoreSight isn't
allowed, so add an option to disable the CoreSight init.



diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig



+config ENABLE_CORESIGHT
+   bool "Enable CoreSight"
+   default y


Why "default y"? This changes behaviour on all our open development 
boards. "default y" should be dropped, and the option should be added to 
the defconfig for your board, or selected by the board's Kconfig option.



diff --git a/arch/arm/mach-tegra/cpu.c b/arch/arm/mach-tegra/cpu.c



 void clock_enable_coresight(int enable)
 {
+#if defined(CONFIG_ENABLE_CORESIGHT)
u32 rst, src = 2;

debug("%s entry\n", __func__);
@@ -402,6 +403,7 @@ void clock_enable_coresight(int enable)
writel(rst, CSITE_CPU_DBG3_LAR);
}
}
+#endif
 }


It might be better to ifdef out the call-site. Otherwise, someone 
reading that code won't have any idea that the function actually does 
nothing.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot