Discussion:
[PATCH 2/9] arm: cache-l2x0: remove duplicate warning
Michał Mirosław
2017-07-20 00:29:23 UTC
Permalink
Warning abound changing of AUX register in l2x0_of_init() is later
repeated in __l2c_init(). Move whole thing over to __l2c_init().

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/mm/cache-l2x0.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 808efbb89b88..ea1e70ff4568 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -819,6 +819,8 @@ static int __init __l2c_init(const struct l2c_init_data *data,
if (old_aux != aux)
pr_warn("L2C: DT/platform modifies aux control register: 0x%08x -> 0x%08x\n",
old_aux, aux);
+ else if (aux_mask != ~0U && aux_val != 0)
+ pr_alert("L2C: platform provided aux values match the hardware, so have no effect. Please remove them.\n");

/* Determine the number of ways */
switch (cache_id & L2X0_CACHE_ID_PART_MASK) {
@@ -1755,7 +1757,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
const struct l2c_init_data *data;
struct device_node *np;
struct resource res;
- u32 cache_id, old_aux;
+ u32 cache_id;
u32 cache_level = 2;
bool nosync = false;

@@ -1778,14 +1780,6 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
of_property_read_bool(np, "arm,io-coherent"))
data = &of_l2c310_coherent_data;

- old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
- if (old_aux != ((old_aux & aux_mask) | aux_val)) {
- pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
- old_aux, (old_aux & aux_mask) | aux_val);
- } else if (aux_mask != ~0U && aux_val != 0) {
- pr_alert("L2C: platform provided aux values match the hardware, so have no effect. Please remove them.\n");
- }
-
/* All L2 caches are unified, so this property should be specified */
if (!of_property_read_bool(np, "cache-unified"))
pr_err("L2C: device tree omits to specify unified cache\n");
--
2.11.0
Michał Mirosław
2017-07-20 00:29:24 UTC
Permalink
Use firmware_ops to provide hook for cache initialization through
Trusted Foundations firmware, as some writes need Secure mode.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/firmware/trusted_foundations.c | 46 ++++++++++++++++++++++++++++++
arch/arm/include/asm/hardware/cache-l2x0.h | 1 +
arch/arm/mm/cache-l2x0.c | 10 ++++++-
3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 3fb1b5a1dce9..81ff71b87438 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -17,11 +17,19 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/of.h>
+#include <asm/io.h>
#include <asm/firmware.h>
+#include <asm/outercache.h>
+#include <asm/hardware/cache-l2x0.h>
#include <asm/trusted_foundations.h>

+#define TF_CACHE_MAINT 0xfffff100
#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200

+#define TF_CACHE_INIT 1
+#define TF_CACHE_FLUSH 2
+#define TF_CACHE_REENABLE 4
+
#define TF_CPU_PM 0xfffffffc
#define TF_CPU_PM_S3 0xffffffe3
#define TF_CPU_PM_S2 0xffffffe6
@@ -63,9 +71,47 @@ static int tf_prepare_idle(void)
return 0;
}

+#ifdef CONFIG_CACHE_L2X0
+static void tf_write_sec(unsigned long val, unsigned reg)
+{
+ unsigned long cur = readl_relaxed(l2x0_base + reg);
+
+ pr_warn("TF: ignoring write_sec[0x%x]: 0x%08lx -> 0x%08lx\n", reg, cur, val);
+}
+
+static void tf_disable_cache(void)
+{
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_FLUSH, l2x0_way_mask);
+}
+
+static void tf_resume_cache(void)
+{
+ unsigned long aux_val = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_REENABLE, aux_val);
+}
+
+static void tf_configure_cache(const struct l2x0_regs *regs)
+{
+ outer_cache.disable = tf_disable_cache;
+ outer_cache.resume = tf_resume_cache;
+}
+
+static int tf_init_cache(void)
+{
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_INIT, 0);
+
+ outer_cache.write_sec = tf_write_sec;
+ outer_cache.configure = tf_configure_cache;
+ return 0;
+}
+#endif /* CONFIG_CACHE_L2X0 */
+
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
.prepare_idle = tf_prepare_idle,
+#ifdef CONFIG_CACHE_L2X0
+ .l2x0_init = tf_init_cache,
+#endif
};

void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 492de655e4f3..665eb0758417 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -194,6 +194,7 @@ struct l2x0_regs {
};

extern void __iomem *l2x0_base;
+extern u32 l2x0_way_mask; /* Bitmask of active ways */
extern struct l2x0_regs l2x0_saved_regs;

#endif /* __ASSEMBLY__ */
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index bbfbc18399f9..f1268e9b35f0 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -30,6 +30,7 @@
#include <asm/cp15.h>
#include <asm/cputype.h>
#include <asm/hardware/cache-l2x0.h>
+#include <asm/firmware.h>
#include "cache-tauros3.h"
#include "cache-aurora-l2.h"

@@ -37,6 +38,7 @@ struct l2c_init_data {
const char *type;
unsigned way_size_0;
unsigned num_lock;
+ void (*init)(void __iomem *, u32 *, u32 *);
void (*of_parse)(const struct device_node *, u32 *, u32 *);
void (*enable)(void __iomem *, unsigned);
void (*fixup)(void __iomem *, u32, struct outer_cache_fns *);
@@ -50,11 +52,11 @@ struct l2c_init_data {

static const struct l2c_init_data *l2x0_data;
static DEFINE_RAW_SPINLOCK(l2x0_lock);
-static u32 l2x0_way_mask; /* Bitmask of active ways */
static u32 l2x0_size;
static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;

void __iomem *l2x0_base;
+u32 l2x0_way_mask; /* Bitmask of active ways */
struct l2x0_regs l2x0_saved_regs;

static bool l2x0_bresp_disable;
@@ -1760,6 +1762,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
u32 cache_id;
u32 cache_level = 2;
bool nosync = false;
+ int err;

np = of_find_matching_node(NULL, l2x0_ids);
if (!np)
@@ -1792,6 +1795,11 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)

nosync = of_property_read_bool(np, "arm,outer-sync-disable");

+ /* Call firmware init */
+ err = call_firmware_op(l2x0_init);
+ if (err && err != -ENOSYS)
+ return err;
+
/* Read back current (default) hardware configuration */
if (data->save)
data->save(l2x0_base);
--
2.11.0
Michał Mirosław
2017-07-20 00:29:23 UTC
Permalink
Allow secure-only erratas to be used in multiarch kernel.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/Kconfig | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a208bfe367b5..a1eff866550b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO

endmenu

+config ARCH_ASSUME_SECURE_PLATFORM
+ bool "Enable ERRATAs using secure-only registers"
+ default !ARCH_MULTIPLATFORM
+ help
+ Allow erratas that need access to secure-only registers.
+
+ Beware: Resulting kernel won't boot on a CPU in non-Secure mode.
+
config ARCH_VIRT
bool "Dummy Virtual Machine"
depends on ARCH_MULTI_V7
@@ -984,7 +992,7 @@ config ARM_ERRATA_430973
config ARM_ERRATA_458693
bool "ARM errata: Processor deadlock when a false hazard is created"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 458693 Cortex-A8 (r2p0)
erratum. For very specific sequences of memory operations, it is
@@ -998,7 +1006,7 @@ config ARM_ERRATA_458693
config ARM_ERRATA_460075
bool "ARM errata: Data written to the L2 cache can be overwritten with stale data"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 460075 Cortex-A8 (r2p0)
erratum. Any asynchronous access to the L2 cache may encounter a
@@ -1011,7 +1019,7 @@ config ARM_ERRATA_460075
config ARM_ERRATA_742230
bool "ARM errata: DMB operation may be faulty"
depends on CPU_V7 && SMP
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 742230 Cortex-A9
(r1p0..r2p2) erratum. Under rare circumstances, a DMB instruction
@@ -1024,7 +1032,7 @@ config ARM_ERRATA_742230
config ARM_ERRATA_742231
bool "ARM errata: Incorrect hazard handling in the SCU may lead to data corruption"
depends on CPU_V7 && SMP
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 742231 Cortex-A9
(r2p0..r2p2) erratum. Under certain conditions, specific to the
@@ -1062,7 +1070,7 @@ config ARM_ERRATA_720789
config ARM_ERRATA_743622
bool "ARM errata: Faulty hazard checking in the Store Buffer may lead to data corruption"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 743622 Cortex-A9
(r2p*) erratum. Under very rare conditions, a faulty
@@ -1076,7 +1084,7 @@ config ARM_ERRATA_743622
config ARM_ERRATA_751472
bool "ARM errata: Interrupted ICIALLUIS may prevent completion of broadcasted operation"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 751472 Cortex-A9 (prior
to r3p0) erratum. An interrupted ICIALLUIS operation may prevent the
--
2.11.0
Michał Mirosław
2017-07-20 00:29:25 UTC
Permalink
Cache enable needs to go via firmware call with TF running.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/mach-tegra/reset-handler.S | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..aae7f5961563 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -78,8 +78,20 @@ ENTRY(tegra_resume)
orr r1, r1, #1
str r1, [r0]
#endif
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ adr r3, __tegra_smc_stack
+ stmia r3, {r4-r12, sp, lr}

-#ifdef CONFIG_CACHE_L2X0
+ mov r0, #3 // local wake
+ mov r3, #0
+ mov r4, #0
+ dsb
+ .arch_extension sec
+ smc #0
+
+ adr r3, __tegra_smc_stack
+ ldmia r3, {r4-r12, sp, pc}
+#elif defined(CONFIG_CACHE_L2X0)
/* L2 cache resume & re-enable */
bl l2c310_early_resume
#endif
@@ -92,6 +104,16 @@ end_ca9_scu_l2_resume:
ENDPROC(tegra_resume)
#endif

+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ .align L1_CACHE_SHIFT
+ .type __tegra_smc_stack, %object
+__tegra_smc_stack:
+ .rept 11
+ .long 0
+ .endr
+ .size __tegra_smc_stack, . - __tegra_smc_stack
+#endif /* CONFIG_TRUSTED_FOUNDATIONS */
+
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_start)
--
2.11.0
Michał Mirosław
2017-07-20 00:29:25 UTC
Permalink
This allows secondary CPUs to boot with Trusted Foundations firmware.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/mach-tegra/reset-handler.S | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index aae7f5961563..f642032a5a08 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -143,6 +143,8 @@ ENTRY(__tegra_cpu_reset_handler)
cpsid aif, 0x13 @ SVC mode, interrupts disabled

tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+#ifdef CONFIG_ARCH_ASSUME_SECURE_PLATFORM
#ifdef CONFIG_ARCH_TEGRA_2x_SOC
t20_check:
cmp r6, #TEGRA20
@@ -172,6 +174,7 @@ t30_errata:
b after_errata
after_t30_check:
#endif
+#endif /* CONFIG_ARCH_ASSUME_SECURE_PLATFORM */
after_errata:
mrc p15, 0, r10, c0, c0, 5 @ MPIDR
and r10, r10, #0x3 @ R10 = CPU number
--
2.11.0
Michał Mirosław
2017-07-20 00:29:25 UTC
Permalink
Fix secondary_data pointer register and document other
occurrences in SMP boot code.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/kernel/head.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 04286fd9e09c..68b13db86bef 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -413,7 +413,7 @@ ENDPROC(secondary_startup)
ENDPROC(secondary_startup_arm)

/*
- * r6 = &secondary_data
+ * r7 = &secondary_data
*/
ENTRY(__secondary_switched)
ldr sp, [r7, #12] @ get secondary_data.stack
@@ -443,6 +443,7 @@ __secondary_data:
* r2 = atags or dtb pointer
* r4 = TTBR pointer (low word)
* r5 = TTBR pointer (high word if LPAE)
+ * r7 = secondary_data pointer (SMP)
* r9 = processor ID
* r13 = *virtual* address to jump to upon completion
*/
@@ -480,6 +481,7 @@ ENDPROC(__enable_mmu)
* r0 = cp#15 control register
* r1 = machine ID
* r2 = atags or dtb pointer
+ * r7 = secondary_data pointer (SMP)
* r9 = processor ID
* r13 = *virtual* address to jump to upon completion
*
--
2.11.0
Michał Mirosław
2017-07-20 00:29:26 UTC
Permalink
This removes unnecessary lock causing following BUG splat:

BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
no locks held by swapper/0/0.
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
[<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
[<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
[<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
[<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
[<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
[<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
[<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
[<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
[<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
[<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
[<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
drivers/soc/tegra/pmc.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..3e6ec9fdba41 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
if (!tegra_powergate_is_valid(id))
return -EINVAL;

- mutex_lock(&pmc->powergates_lock);
status = tegra_powergate_state(id);
- mutex_unlock(&pmc->powergates_lock);

return status;
}
--
2.11.0
Jon Hunter
2017-07-20 12:45:12 UTC
Permalink
Post by Michał Mirosław
BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
no locks held by swapper/0/0.
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
[<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
[<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
[<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
[<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
[<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
[<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
[<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
[<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
[<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
[<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
[<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
---
drivers/soc/tegra/pmc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..3e6ec9fdba41 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
if (!tegra_powergate_is_valid(id))
return -EINVAL;
- mutex_lock(&pmc->powergates_lock);
status = tegra_powergate_state(id);
- mutex_unlock(&pmc->powergates_lock);
return status;
}
Thanks for the fix. However, I would prefer that we fix this the following
way ...

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index a2d163f759b4..546d2b121c39 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)

cpu_rst_status = readl(clk_base +
TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
- cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
+ cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
+ tegra_pmc_cpu_is_powered(2) ||
+ tegra_pmc_cpu_is_powered(3);

if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
return false;
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..f61371ea3fe0 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
if (id < 0)
return false;

- return tegra_powergate_is_powered(id);
+ return tegra_powergate_state(id);
}


Could you try the above and make sure that this works?

I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
be used by any driver. Or maybe we could even get rid of
tegra_powergate_is_powered() if we don't really need it.

Cheers
Jon
--
nvpublic
Michał Mirosław
2017-07-20 16:28:52 UTC
Permalink
Post by Jon Hunter
Post by Michał Mirosław
BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
no locks held by swapper/0/0.
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
[<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
[<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
[<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
[<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
[<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
[<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
[<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
[<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
[<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
[<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
[<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
---
drivers/soc/tegra/pmc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..3e6ec9fdba41 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
if (!tegra_powergate_is_valid(id))
return -EINVAL;
- mutex_lock(&pmc->powergates_lock);
status = tegra_powergate_state(id);
- mutex_unlock(&pmc->powergates_lock);
return status;
}
Thanks for the fix. However, I would prefer that we fix this the following
way ...
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index a2d163f759b4..546d2b121c39 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)
cpu_rst_status = readl(clk_base +
TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
- cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
+ cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
+ tegra_pmc_cpu_is_powered(2) ||
+ tegra_pmc_cpu_is_powered(3);
if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
return false;
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..f61371ea3fe0 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
if (id < 0)
return false;
- return tegra_powergate_is_powered(id);
+ return tegra_powergate_state(id);
}
Could you try the above and make sure that this works?
Something ate TABs in your patch, but yes, it works.
Post by Jon Hunter
I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
be used by any driver. Or maybe we could even get rid of
tegra_powergate_is_powered() if we don't really need it.
This mutex does not protect anything here, though - there is only one register
read inside the locked section, and after unlock other CPU might change
it before the read value gets returned.

Code size-wise it would be better to remove tegra_pmc_cpu_is_powered()
instead.

Best Regards,
Michał Mirosław
Jon Hunter
2017-07-21 08:15:31 UTC
Permalink
Post by Michał Mirosław
Post by Jon Hunter
Post by Michał Mirosław
BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
no locks held by swapper/0/0.
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
[<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
[<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
[<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
[<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
[<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
[<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
[<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
[<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
[<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
[<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
[<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
---
drivers/soc/tegra/pmc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..3e6ec9fdba41 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
if (!tegra_powergate_is_valid(id))
return -EINVAL;
- mutex_lock(&pmc->powergates_lock);
status = tegra_powergate_state(id);
- mutex_unlock(&pmc->powergates_lock);
return status;
}
Thanks for the fix. However, I would prefer that we fix this the following
way ...
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index a2d163f759b4..546d2b121c39 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)
cpu_rst_status = readl(clk_base +
TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
- cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
+ cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
+ tegra_pmc_cpu_is_powered(2) ||
+ tegra_pmc_cpu_is_powered(3);
if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
return false;
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..f61371ea3fe0 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
if (id < 0)
return false;
- return tegra_powergate_is_powered(id);
+ return tegra_powergate_state(id);
}
Could you try the above and make sure that this works?
Something ate TABs in your patch, but yes, it works.
Post by Jon Hunter
I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
be used by any driver. Or maybe we could even get rid of
tegra_powergate_is_powered() if we don't really need it.
This mutex does not protect anything here, though - there is only one register
read inside the locked section, and after unlock other CPU might change
it before the read value gets returned.
Code size-wise it would be better to remove tegra_pmc_cpu_is_powered()
instead.
Maybe, but tegra_pmc_cpu_is_powered() is needed by
tegra30_boot_secondary().

Jon
--
nvpublic
Michał Mirosław
2017-07-20 00:29:23 UTC
Permalink
Share l2x0_base between cache-l2x0 and pmu drivers. They are duplicates.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/include/asm/hardware/cache-l2x0.h | 5 +++--
arch/arm/mm/cache-l2x0-pmu.c | 8 ++------
arch/arm/mm/cache-l2x0.c | 4 ++--
3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 736292b42fca..492de655e4f3 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -167,11 +167,11 @@ static inline int l2x0_of_init(u32 aux_val, u32 aux_mask)
#endif

#ifdef CONFIG_CACHE_L2X0_PMU
-void l2x0_pmu_register(void __iomem *base, u32 part);
+void l2x0_pmu_register(u32 part);
void l2x0_pmu_suspend(void);
void l2x0_pmu_resume(void);
#else
-static inline void l2x0_pmu_register(void __iomem *base, u32 part) {}
+static inline void l2x0_pmu_register(u32 part) {}
static inline void l2x0_pmu_suspend(void) {}
static inline void l2x0_pmu_resume(void) {}
#endif
@@ -193,6 +193,7 @@ struct l2x0_regs {
unsigned long aux2_ctrl;
};

+extern void __iomem *l2x0_base;
extern struct l2x0_regs l2x0_saved_regs;

#endif /* __ASSEMBLY__ */
diff --git a/arch/arm/mm/cache-l2x0-pmu.c b/arch/arm/mm/cache-l2x0-pmu.c
index 0a1e2280141f..1f36801d337f 100644
--- a/arch/arm/mm/cache-l2x0-pmu.c
+++ b/arch/arm/mm/cache-l2x0-pmu.c
@@ -29,7 +29,6 @@

#define PMU_NR_COUNTERS 2

-static void __iomem *l2x0_base;
static struct pmu *l2x0_pmu;
static cpumask_t pmu_cpu;

@@ -491,7 +490,7 @@ void l2x0_pmu_resume(void)
l2x0_pmu_enable(l2x0_pmu);
}

-void __init l2x0_pmu_register(void __iomem *base, u32 part)
+void __init l2x0_pmu_register(u32 part)
{
/*
* Determine whether we support the PMU, and choose the name for sysfs.
@@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
* supported by this driver.
*
* We must defer registering the PMU until the perf subsystem is up and
- * running, so just stash the name and base, and leave that to another
- * initcall.
+ * running, so just stash the name, and leave that to another initcall.
*/
switch (part & L2X0_CACHE_ID_PART_MASK) {
case L2X0_CACHE_ID_PART_L220:
@@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
default:
return;
}
-
- l2x0_base = base;
}

static __init int l2x0_pmu_init(void)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index ea1e70ff4568..bbfbc18399f9 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -48,13 +48,13 @@ struct l2c_init_data {

#define CACHE_LINE_SIZE 32

-static void __iomem *l2x0_base;
static const struct l2c_init_data *l2x0_data;
static DEFINE_RAW_SPINLOCK(l2x0_lock);
static u32 l2x0_way_mask; /* Bitmask of active ways */
static u32 l2x0_size;
static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;

+void __iomem *l2x0_base;
struct l2x0_regs l2x0_saved_regs;

static bool l2x0_bresp_disable;
@@ -900,7 +900,7 @@ static int __init __l2c_init(const struct l2c_init_data *data,
pr_info("%s: CACHE_ID 0x%08x, AUX_CTRL 0x%08x\n",
data->type, cache_id, aux);

- l2x0_pmu_register(l2x0_base, cache_id);
+ l2x0_pmu_register(cache_id);

return 0;
}
--
2.11.0
Michał Mirosław
2017-07-20 00:29:24 UTC
Permalink
Announce Trusted Foundations version for debugging and documentation.

Signed-off-by: Michał Mirosław <mirq-***@rere.qmqm.pl>
---
arch/arm/firmware/trusted_foundations.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 81ff71b87438..105655802dfd 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -30,6 +30,8 @@
#define TF_CACHE_FLUSH 2
#define TF_CACHE_REENABLE 4

+#define TF_GET_PROTOCOL_VERSION 0xfffffffb
+
#define TF_CPU_PM 0xfffffffc
#define TF_CPU_PM_S3 0xffffffe3
#define TF_CPU_PM_S2 0xffffffe6
@@ -39,7 +41,7 @@

static unsigned long cpu_boot_addr;

-static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
+static u64 __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
{
asm volatile(
".arch_extension sec\n\t"
@@ -54,6 +56,7 @@ static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
:
: "r" (type), "r" (arg1), "r" (arg2)
: "memory");
+ return 0; /* silence gcc warning */
}

static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
@@ -106,6 +109,11 @@ static int tf_init_cache(void)
}
#endif /* CONFIG_CACHE_L2X0 */

+static u32 tf_get_protocol_version(void)
+{
+ return tf_generic_smc(TF_GET_PROTOCOL_VERSION, 0, 0) >> 48;
+}
+
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
.prepare_idle = tf_prepare_idle,
@@ -116,6 +124,12 @@ static const struct firmware_ops trusted_foundations_ops = {

void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
{
+ unsigned int protocol = tf_get_protocol_version();
+
+ pr_info("TF: version %u.%u, protocol %u.%u\n",
+ pd->version_major, pd->version_minor,
+ protocol >> 8, protocol & 0xFF);
+
/*
* we are not using version information for now since currently
* supported SMCs are compatible with all TF releases
@@ -141,5 +155,6 @@ void of_register_trusted_foundations(void)
&pdata.version_minor);
if (err != 0)
panic("Trusted Foundation: missing version-minor property\n");
+
register_trusted_foundations(&pdata);
}
--
2.11.0
Mikko Perttunen
2017-07-20 07:48:31 UTC
Permalink
Here is a suprisingly small set of patches that enable Asus TF300T tablet
to boot and have all cores available. TF300T is one of a consumer devices
based on NVidia's Cardhu reference tablet.
This is very cool! How did you debug it, or did it work on the first
try? :) I have a TF201 myself, probably quite similar to this, might be
worth a try.
This series is an arch-dependent part. TF parts were extracted from ASUS's
GPL code dump. The rest being driver code - a work in progress - is
https://rere.qmqm.pl/git/linux
This URL doesn't work for me at least in the browser, but
https://rere.qmqm.pl/git/?p=linux;a=summary works.

Regarding the USB device mode - yes, there are several drivers for
roughly the same hardware. It would be good to get both host and device
mode with the same driver so that we could have OTG support.

Cheers,
Mikko
Best Regards,
Michał Mirosław
---
ARM: enable secure platform-only erratas
arm: cache-l2x0: remove duplicate warning
arm: cache-l2x0: share l2x0_base
ARM: trusted_foundations: enable L2x0 cache via firmware_ops
ARM: trusted_foundations: announce firmware version
ARM: init: update secondary_data register documentation
ARM: tegra: enable cache via TF
ARM: tegra: avoid touching Secure registers in reset handler
ARM: tegra: fix sleeping while atomic in CPU idle
arch/arm/Kconfig | 20 +++++++---
arch/arm/firmware/trusted_foundations.c | 63 +++++++++++++++++++++++++++++-
arch/arm/include/asm/hardware/cache-l2x0.h | 6 ++-
arch/arm/kernel/head.S | 4 +-
arch/arm/mach-tegra/reset-handler.S | 27 ++++++++++++-
arch/arm/mm/cache-l2x0-pmu.c | 8 +---
arch/arm/mm/cache-l2x0.c | 26 ++++++------
drivers/soc/tegra/pmc.c | 2 -
8 files changed, 125 insertions(+), 31 deletions(-)
Michał Mirosław
2017-07-20 15:07:30 UTC
Permalink
Here is a suprisingly small set of patches that enable Asus TF300T tablet
to boot and have all cores available. TF300T is one of a consumer devices
based on NVidia's Cardhu reference tablet.
This is very cool! How did you debug it, or did it work on the first try? :)
I have a TF201 myself, probably quite similar to this, might be worth a try.
Unfortunatelly it worked only after about 300 compile-upload-boot cycles. ;-)
I hacked an early console using bootloader framebuffer. The code for it is
a spaghetti monster, but if you would like to try it, I can add a patch.
This series is an arch-dependent part. TF parts were extracted from ASUS's
GPL code dump. The rest being driver code - a work in progress - is
https://rere.qmqm.pl/git/linux
This URL doesn't work for me at least in the browser, but
https://rere.qmqm.pl/git/?p=linux;a=summary works.
[...]

Ah, I should have been more precise: the url is for git clone (read-only).

Best Regards,
Michał Mirosław
Loading...