[PATCH 0/9] Bringing Asus TF300T support to mainline

[PATCH 0/9] Bringing Asus TF300T support to mainline

[Michał Mirosław]

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 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
available in branch tf300t at:

        https://rere.qmqm.pl/git/linux

Best Regards,
Michał Mirosław

---

Michał Mirosław (9):
  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(-)

-- 
2.11.0

[PATCH 1/9] ARM: enable secure platform-only erratas

[Michał Mirosław]

Allow secure-only erratas to be used in multiarch kernel.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 1/9] ARM: enable secure platform-only erratas

[Dmitry Osipenko]

On 20.07.2017 03:29, Michał Mirosław wrote:
> Allow secure-only erratas to be used in multiarch kernel.
> 
> Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---
>  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

What about ARCH_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
> 

Re: [PATCH 1/9] ARM: enable secure platform-only erratas

[Dmitry Osipenko]

On 20.07.2017 03:29, Michał Mirosław wrote:
> Allow secure-only erratas to be used in multiarch kernel.
> 
> Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---
>  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

I think default should always be "Yes" and this option shouldn't affect
multiplatform kernels. So a multiplatform kernel wouldn't be an option for your
device.

> +	help
> +	  Allow erratas that need access to secure-only registers.
> +
> +	  Beware: Resulting kernel won't boot on a CPU in non-Secure mode.
> +

Probably better to say here something like that: Disable this option to avoid
CPU erratas that require access to secure-only registers. This might be needed
in a case of running under a secure monitor.

>  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
> 

Re: [PATCH 1/9] ARM: enable secure platform-only erratas

[Russell King - ARM Linux]

On Thu, Oct 05, 2017 at 09:16:12PM +0300, Dmitry Osipenko wrote:
> On 20.07.2017 03:29, Michał Mirosław wrote:
> > Allow secure-only erratas to be used in multiarch kernel.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> > ---
> >  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
> 
> I think default should always be "Yes" and this option shouldn't affect
> multiplatform kernels. So a multiplatform kernel wouldn't be an option
> for your device.

No, that changes the current behaviour.

Current behaviour is for the secure-only errata to be disabled when the
multi-platform option is enabled, because multi-platform kernels have to
be able to run in the non-secure world.  Defaulting this option to "yes"
means that these errata become visible.

I have to wonder why you need this though - your patches seem to be
targetting a platform that runs in non-secure world, and enabling these
errata would make the kernel non-bootable on your platform.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 1/9] ARM: enable secure platform-only erratas

[Dmitry Osipenko]

On 20.12.2017 02:28, Russell King - ARM Linux wrote:
> On Thu, Oct 05, 2017 at 09:16:12PM +0300, Dmitry Osipenko wrote:
>> On 20.07.2017 03:29, Michał Mirosław wrote:
>>> Allow secure-only erratas to be used in multiarch kernel.
>>>
>>> Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
>>> ---
>>>  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
>>
>> I think default should always be "Yes" and this option shouldn't affect
>> multiplatform kernels. So a multiplatform kernel wouldn't be an option
>> for your device.
> 
> No, that changes the current behaviour.
> 
> Current behaviour is for the secure-only errata to be disabled when the
> multi-platform option is enabled, because multi-platform kernels have to
> be able to run in the non-secure world.  Defaulting this option to "yes"
> means that these errata become visible.

Indeed, I got it inverted.

> I have to wonder why you need this though - your patches seem to be
> targetting a platform that runs in non-secure world, and enabling these
> errata would make the kernel non-bootable on your platform.

Perhaps because Michał made the Tegra's CPU reset handler hardcoded to either
secure or to non-secure case based on the kernels configuration. I've showed how
we can get rid of that inflexibility in [0], maybe Michał could pick up the idea
in the next iteration of the patches.

[0] https://marc.info/?l=linux-tegra&m=151371042522835

[PATCH 2/9] arm: cache-l2x0: remove duplicate warning

[Michał Mirosław]

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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

[PATCH 3/9] arm: cache-l2x0: share l2x0_base

[Michał Mirosław]

Share l2x0_base between cache-l2x0 and pmu drivers.  They are duplicates.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 3/9] arm: cache-l2x0: share l2x0_base

[Mark Rutland]

On Thu, Jul 20, 2017 at 02:29:23AM +0200, Michał Mirosław wrote:
> Share l2x0_base between cache-l2x0 and pmu drivers.  They are duplicates.

This is not quite true, and this change introduces bugs.

The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
it recognises:

> -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;

... note that we may return *before* setting l2x0_base.

This (deliberately) prevents us from erroneously registering a PMU for
l2x0 variants for which we don't know how to drive any IMPLEMENTATION
DEFINED PMU functionality.

This cahnge breaks that, and will result in l2x0_pmu_init() trying to
register a PMU with a NULL name.

So NAK to this change.

Thanks,
Mark.

Re: [PATCH 3/9] arm: cache-l2x0: share l2x0_base

[Russell King - ARM Linux]

On Thu, Oct 05, 2017 at 05:48:32PM +0100, Mark Rutland wrote:
> On Thu, Jul 20, 2017 at 02:29:23AM +0200, Michał Mirosław wrote:
> > Share l2x0_base between cache-l2x0 and pmu drivers.  They are duplicates.
> 
> This is not quite true, and this change introduces bugs.
> 
> The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
> it recognises:
> 
> > -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;
> 
> ... note that we may return *before* setting l2x0_base.
> 
> This (deliberately) prevents us from erroneously registering a PMU for
> l2x0 variants for which we don't know how to drive any IMPLEMENTATION
> DEFINED PMU functionality.
> 
> This cahnge breaks that, and will result in l2x0_pmu_init() trying to
> register a PMU with a NULL name.
> 
> So NAK to this change.

Also I don't want the virtual base address of the L2 cache exposed to
the world, that's an invitation for people to do what they did years
ago, and throw code left right and centre into the tree that accesses
the L2 cache directly from all sorts of places - the result was very
much a mess that took a big patch series to sort out.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops

[Michał Mirosław]

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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops

[Dmitry Osipenko]

On 20.07.2017 03:29, Michał Mirosław wrote:
> 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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---
>  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 */

What about to make this comment a bit more explanatory? Add couple words about
what firmware initializes, maybe it enables access from CPU.. of course if you
have any clue.

> +	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);
> 

Re: [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops

[Dmitry Osipenko]

On 20.07.2017 03:29, Michał Mirosław wrote:
> 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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---

I've tested this patch on my T30, which I ported to current linux-next, and it
works excellent. Michał, are you going to re-spin this patch?

[PATCH 5/9] ARM: trusted_foundations: announce firmware version

[Michał Mirosław]

Announce Trusted Foundations version for debugging and documentation.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

[PATCH 6/9] ARM: init: update secondary_data register documentation

[Michał Mirosław]

Fix secondary_data pointer register and document other
occurrences in SMP boot code.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 6/9] ARM: init: update secondary_data register documentation

[Russell King - ARM Linux]

On Thu, Jul 20, 2017 at 02:29:25AM +0200, Michał Mirosław wrote:
> Fix secondary_data pointer register and document other
> occurrences in SMP boot code.
> 
> Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>

It's probably worth referecing commit 34d926266500 ("[ARM] Fix SMP booting")
since we have it in our history for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler

[Michał Mirosław]

This allows secondary CPUs to boot with Trusted Foundations firmware.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

[PATCH 7/9] ARM: tegra: enable cache via TF

[Michał Mirosław]

Cache enable needs to go via firmware call with TF running.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 7/9] ARM: tegra: enable cache via TF

[Dmitry Osipenko]

On 20.07.2017 03:29, Michał Mirosław wrote:
> Cache enable needs to go via firmware call with TF running.
> 
> Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---

Perhaps we can unify secure and non-secure modes. The code below works on both
secure-t30 and nonsecure-t20, all CPU's booted and working fine.

The "ARM: enable secure platform-only erratas" patch isn't needed in this case.


diff --git a/arch/arm/mach-tegra/reset-handler.S
b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..9a92bbf8b5b0 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -80,6 +80,28 @@ ENTRY(tegra_resume)
 #endif

 #ifdef CONFIG_CACHE_L2X0
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+	adr	r3, __tegra_cpu_reset_handler_data
+	ldr	r0, [r3, #RESET_DATA(TF_PRESENT)]
+	cmp	r0, #0
+	beq	ca9_scu_l2_resume
+
+	adr	r3, __tegra_smc_stack
+	stmia	r3, {r4-r12, sp, lr}
+
+	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}
+
+	b	end_ca9_scu_l2_resume
+ca9_scu_l2_resume:
+#endif
 	/* L2 cache resume & re-enable */
 	bl	l2c310_early_resume
 #endif
@@ -92,6 +114,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)

@@ -121,6 +153,12 @@ ENTRY(__tegra_cpu_reset_handler)
 	cpsid	aif, 0x13			@ SVC mode, interrupts disabled

 	tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+	adr	r5, __tegra_cpu_reset_handler_data
+	ldr	r0, [r5, #RESET_DATA(TF_PRESENT)]
+	cmp	r0, #0
+	bne	after_errata
+
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 t20_check:
 	cmp	r6, #TEGRA20
@@ -285,6 +323,10 @@ __tegra_cpu_reset_handler_data:
 	.equ	__tegra20_cpu1_resettable_status_offset, \
 					. - __tegra_cpu_reset_handler_start
 	.byte	0
+	.align	4
+	.globl	__tegra_tf_present
+	.equ	__tegra_tf_present, . - __tegra_cpu_reset_handler_start
+	.long	0
 	.align L1_CACHE_SHIFT

 ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index dc558892753c..9b6558a69308 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -18,6 +18,7 @@
 #include <linux/cpumask.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/of.h>

 #include <soc/tegra/fuse.h>

@@ -89,6 +90,15 @@ static void __init tegra_cpu_reset_handler_enable(void)

 void __init tegra_cpu_reset_handler_init(void)
 {
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations");
+	if (np) {
+		__tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] = true;
+		of_node_put(np);
+	}
+#endif

 #ifdef CONFIG_SMP
 	__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 9c479c7925b8..0d9ddc022ece 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -25,7 +25,9 @@
 #define TEGRA_RESET_STARTUP_SECONDARY	3
 #define TEGRA_RESET_STARTUP_LP2		4
 #define TEGRA_RESET_STARTUP_LP1		5
-#define TEGRA_RESET_DATA_SIZE		6
+#define TEGRA_RESET_RESETTABLE_STATUS	6
+#define TEGRA_RESET_TF_PRESENT		7
+#define TEGRA_RESET_DATA_SIZE		8

 #ifndef __ASSEMBLY__

Re: [PATCH 7/9] ARM: tegra: enable cache via TF

[Russell King - ARM Linux]

On Thu, Jul 20, 2017 at 02:29:25AM +0200, Michał Mirosław wrote:
> 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}

You don't want to jump to the 'lr' value previously stacked here.  I
also wonder whether you need all this stacking, or whether you're
just doing it because you don't know whether its necessary.  From
what I can see, the only register that this code cares about is r8,
although it would be wise to place a comment in the code if the smc
call corrupts the other registers.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle

[Michał Mirosław]

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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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

Re: [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle

[Jon Hunter]

On 20/07/17 01:29, Michał Mirosław wrote:
> 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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> ---
>  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

Re: [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle

[Michał Mirosław]

On Thu, Jul 20, 2017 at 01:45:12PM +0100, Jon Hunter wrote:
> 
> On 20/07/17 01:29, Michał Mirosław wrote:
> > 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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> > ---
> >  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.

> 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

Re: [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle

[Jon Hunter]

On 20/07/17 17:28, Michał Mirosław wrote:
> On Thu, Jul 20, 2017 at 01:45:12PM +0100, Jon Hunter wrote:
>>
>> On 20/07/17 01:29, Michał Mirosław wrote:
>>> 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-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
>>> ---
>>>  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.
> 
>> 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

Re: [PATCH 0/9] Bringing Asus TF300T support to mainline

[Mikko Perttunen]

On 20.07.2017 03:29, Michał Mirosław wrote:
> 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
> available in branch tf300t at:
>
>         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
>
> ---
>
> Michał Mirosław (9):
>   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(-)
>

Re: [PATCH 0/9] Bringing Asus TF300T support to mainline

[Michał Mirosław]

On Thu, Jul 20, 2017 at 10:48:31AM +0300, Mikko Perttunen wrote:
> On 20.07.2017 03:29, Michał Mirosław wrote:
> > 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
> > available in branch tf300t at:
> > 
> >         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

Re: [PATCH 0/9] Bringing Asus TF300T support to mainline

[Michał Mirosław]

On Thu, Jul 20, 2017 at 02:29:22AM +0200, Michał Mirosław wrote:
> 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 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
> available in branch tf300t at:
> 
>         https://rere.qmqm.pl/git/linux

Just a quick note, that I've rebased the branch on v4.14-rc3 and added
some more patches on top. This includes updated defconfig and devicetree
with working eMMC and an attempt at making TF300T's dock hot-pluggable.

Best Regards,
Michał Mirosław

Re: [PATCH 0/9] Bringing Asus TF300T support to mainline

[Dmitry Osipenko]

On 05.10.2017 00:25, Michał Mirosław wrote:
> On Thu, Jul 20, 2017 at 02:29:22AM +0200, Michał Mirosław wrote:
>> 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 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
>> available in branch tf300t at:
>>
>>         https://rere.qmqm.pl/git/linux
> 
> Just a quick note, that I've rebased the branch on v4.14-rc3 and added
> some more patches on top. This includes updated defconfig and devicetree
> with working eMMC and an attempt at making TF300T's dock hot-pluggable.
> 

Just a small note.. I think you should develop on top of linux-next, since your
patches would be applied to linux-next first.

Re: [PATCH 0/9] Bringing Asus TF300T support to mainline

[Dmitry Osipenko]

On 05.10.2017 18:52, Dmitry Osipenko wrote:
> On 05.10.2017 00:25, Michał Mirosław wrote:
>> On Thu, Jul 20, 2017 at 02:29:22AM +0200, Michał Mirosław wrote:
>>> 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 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
>>> available in branch tf300t at:
>>>
>>>         https://rere.qmqm.pl/git/linux
>>
>> Just a quick note, that I've rebased the branch on v4.14-rc3 and added
>> some more patches on top. This includes updated defconfig and devicetree
>> with working eMMC and an attempt at making TF300T's dock hot-pluggable.
>>
> 
> Just a small note.. I think you should develop on top of linux-next, since your
> patches would be applied to linux-next first.
> 

Also I think you should split the series into logically separated patchsets.
Like each of "cleanup" patches should be standalone, arm: / l2x0: / soc: patches
factored out into individual patchsets.

[PATCH 0/9] Bringing Asus TF300T support to mainline

[Michał Mirosław]

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 series is an arch-dependent part. The rest being driver code - a work
in progress - is available in branch tf300t at:

	https://rere.qmqm.pl/git/linux

Best Regards,
Michał Mirosław

---
Michał Mirosław (9):
  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(-)

-- 
2.11.0

[PATCH 1/9] ARM: enable secure platform-only erratas

[Michał Mirosław]

Allow secure-only erratas to be used in multiarch kernel.

Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
---
 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