Message ID | 20160905132952.27280-4-julian@jusst.de |
---|---|
State | Changes Requested |
Delegated to: | Tom Warren |
Headers | show |
On 09/05/2016 07:29 AM, Julian Scheel wrote: > From: Alban Bedel <alban.bedel@avionic-design.de> > > 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.
On 06.09.16 18:54, Stephen Warren wrote: > On 09/05/2016 07:29 AM, Julian Scheel wrote: >> From: Alban Bedel <alban.bedel@avionic-design.de> >> >> 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.
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 <alban.bedel@avionic-design.de> >>> >>> 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.
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index 448c319..1e07884 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -95,6 +95,10 @@ config TEGRA_DISCONNECT_UDC_ON_BOOT config SYS_MALLOC_F_LEN default 0x1800 +config ENABLE_CORESIGHT + bool "Enable CoreSight" + default y + source "arch/arm/mach-tegra/tegra20/Kconfig" source "arch/arm/mach-tegra/tegra30/Kconfig" source "arch/arm/mach-tegra/tegra114/Kconfig" diff --git a/arch/arm/mach-tegra/cpu.c b/arch/arm/mach-tegra/cpu.c index a3ebb57..23edaf0 100644 --- a/arch/arm/mach-tegra/cpu.c +++ b/arch/arm/mach-tegra/cpu.c @@ -377,6 +377,7 @@ void reset_A9_cpu(int reset) 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 } void halt_avp(void)