* [PATCH v4 1/4] dmaengine: pl330: remove pdata based initialization
From: Krzysztof Kozlowski @ 2017-01-13 13:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484292400-29730-2-git-send-email-m.szyprowski@samsung.com>
On Fri, Jan 13, 2017 at 08:26:37AM +0100, Marek Szyprowski wrote:
> This driver is now used only on platforms which support device tree, so
> it is safe to remove legacy platform data based initialization code.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> arch/arm/plat-samsung/devs.c | 1 -
For plat-samsung:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 02/15] ARM: davinci: Don't rely on public mmc header to include leds.h
From: Sekhar Nori @ 2017-01-13 13:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484313256-25993-3-git-send-email-ulf.hansson@linaro.org>
On Friday 13 January 2017 06:44 PM, Ulf Hansson wrote:
> Some of the davinci boards, da850-evm, dm644x-evm and neuros-osd2 depends
> on leds.h. Explicitly include it instead of relying on the public mmc
> header host.h.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: <linux-arm-kernel@lists.infradead.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
From: Marc Zyngier @ 2017-01-13 13:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113123612.GA31994@cbox>
[+ Suzuki, who wrote the whole cpus_have_const_cap thing]
On 13/01/17 12:36, Christoffer Dall wrote:
> On Fri, Jan 13, 2017 at 11:31:32AM +0000, Marc Zyngier wrote:
>> From: Jintack Lim <jintack@cs.columbia.edu>
>>
>> Current KVM world switch code is unintentionally setting wrong bits to
>> CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
>> timer. Bit positions of CNTHCTL_EL2 are changing depending on
>> HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
>> not set, but they are 11th and 10th bits respectively when E2H is set.
>>
>> In fact, on VHE we only need to set those bits once, not for every world
>> switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
>> 1, which makes those bits have no effect for the host kernel execution.
>> So we just set those bits once for guests, and that's it.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm/include/asm/virt.h | 5 +++++
>> arch/arm/kvm/arm.c | 3 +++
>> arch/arm64/include/asm/virt.h | 9 +++++++++
>> include/kvm/arm_arch_timer.h | 1 +
>> virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
>> virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
>> 6 files changed, 62 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
>> index a2e75b8..6dae195 100644
>> --- a/arch/arm/include/asm/virt.h
>> +++ b/arch/arm/include/asm/virt.h
>> @@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
>> return false;
>> }
>>
>> +static inline bool has_vhe(void)
>> +{
>> + return false;
>> +}
>> +
>> /* The section containing the hypervisor idmap text */
>> extern char __hyp_idmap_text_start[];
>> extern char __hyp_idmap_text_end[];
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 1167678..9d74464 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
>> __cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
>> __cpu_init_stage2();
>>
>> + if (is_kernel_in_hyp_mode())
>> + kvm_timer_init_vhe();
>> +
>> kvm_arm_init_debug();
>> }
>>
>> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
>> index fea1073..439f6b5 100644
>> --- a/arch/arm64/include/asm/virt.h
>> +++ b/arch/arm64/include/asm/virt.h
>> @@ -47,6 +47,7 @@
>> #include <asm/ptrace.h>
>> #include <asm/sections.h>
>> #include <asm/sysreg.h>
>> +#include <asm/cpufeature.h>
>>
>> /*
>> * __boot_cpu_mode records what mode CPUs were booted in.
>> @@ -80,6 +81,14 @@ static inline bool is_kernel_in_hyp_mode(void)
>> return read_sysreg(CurrentEL) == CurrentEL_EL2;
>> }
>>
>> +static inline bool has_vhe(void)
>> +{
>> + if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>
> I was experimenting with using has_vhe for some of the optimization code
> I was writing, and I saw a hyp crash as a result. That made me wonder
> if this is really safe in Hyp mode?
>
> Specifically, there is no guarantee that this will actually be inlined
> in the caller, right? At least that's what I can gather from trying to
> understand the semantics of the inline keyword in the GCC manual.
Indeed, there is no strict guarantee that this is enforced. We should
probably have __always_inline instead. But having checked the generated
code for __timer_restore_state, the function is definitely inlined
(gcc 6.2). Happy to queue an extra patch changing that.
> Further, are we guaranteed that the static branch gets compiled into
> something that doesn't actually look at cpu_hwcap_keys, which is not
> mapped in hyp mode?
Here's the disassembly:
ffff000008ad01d0 <__timer_restore_state>:
ffff000008ad01d0: f9400001 ldr x1, [x0]
ffff000008ad01d4: 9240bc21 and x1, x1, #0xffffffffffff
ffff000008ad01d8: d503201f nop
ffff000008ad01dc: d503201f nop
ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
ffff000008ad01e4: 927ef842 and x2, x2, #0xfffffffffffffffd
ffff000008ad01e8: b2400042 orr x2, x2, #0x1
ffff000008ad01ec: d51ce102 msr cnthctl_el2, x2
ffff000008ad01f0: d2834002 mov x2, #0x1a00 // #6656
ffff000008ad01f4: 8b020000 add x0, x0, x2
ffff000008ad01f8: 91038002 add x2, x0, #0xe0
ffff000008ad01fc: 39425443 ldrb w3, [x2,#149]
ffff000008ad0200: 34000103 cbz w3, ffff000008ad0220 <__timer_restore_state+0x50>
ffff000008ad0204: f945a821 ldr x1, [x1,#2896]
ffff000008ad0208: d51ce061 msr cntvoff_el2, x1
ffff000008ad020c: f9400441 ldr x1, [x2,#8]
ffff000008ad0210: d51be341 msr cntv_cval_el0, x1
ffff000008ad0214: d5033fdf isb
ffff000008ad0218: b940e000 ldr w0, [x0,#224]
ffff000008ad021c: d51be320 msr cntv_ctl_el0, x0
ffff000008ad0220: d65f03c0 ret
The static branch resolves as such when VHE is enabled (taken from
a running model):
ffff000008ad01d0 <__timer_restore_state>:
ffff000008ad01d0: f9400001 ldr x1, [x0]
ffff000008ad01d4: 9240bc21 nop
ffff000008ad01d8: d503201f nop
ffff000008ad01dc: d503201f b ffff000008ad01f0
ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
[...]
That's using a toolchain that supports the "asm goto" feature that is used
to implement static branches (and that's guaranteed not to generate any
memory access other than the code patching itself).
Now, with a toolchain that doesn't support this, such as gcc 4.8:
ffff000008aa5168 <__timer_restore_state>:
ffff000008aa5168: f9400001 ldr x1, [x0]
ffff000008aa516c: 9240bc21 and x1, x1, #0xffffffffffff
ffff000008aa5170: d503201f nop
ffff000008aa5174: f00038a2 adrp x2, ffff0000091bc000 <reset_devices>
ffff000008aa5178: 9113e042 add x2, x2, #0x4f8
ffff000008aa517c: b9402c42 ldr w2, [x2,#44]
ffff000008aa5180: 6b1f005f cmp w2, wzr
ffff000008aa5184: 540000ac b.gt ffff000008aa5198 <__timer_restore_state+0x30>
ffff000008aa5188: d53ce102 mrs x2, cnthctl_el2
ffff000008aa518c: 927ef842 and x2, x2, #0xfffffffffffffffd
ffff000008aa5190: b2400042 orr x2, x2, #0x1
ffff000008aa5194: d51ce102 msr cnthctl_el2, x2
ffff000008aa5198: 91400402 add x2, x0, #0x1, lsl #12
ffff000008aa519c: 396dd443 ldrb w3, [x2,#2933]
ffff000008aa51a0: 34000103 cbz w3, ffff000008aa51c0 <__timer_restore_state+0x58>
ffff000008aa51a4: f945a821 ldr x1, [x1,#2896]
ffff000008aa51a8: d51ce061 msr cntvoff_el2, x1
ffff000008aa51ac: f9457441 ldr x1, [x2,#2792]
ffff000008aa51b0: d51be341 msr cntv_cval_el0, x1
ffff000008aa51b4: d5033fdf isb
ffff000008aa51b8: b95ae000 ldr w0, [x0,#6880]
ffff000008aa51bc: d51be320 msr cntv_ctl_el0, x0
ffff000008aa51c0: d65f03c0 ret
This is now controlled by some date located at FFFF0000091BC524:
maz at approximate:~/Work/arm-platforms$ aarch64-linux-gnu-objdump -h vmlinux
vmlinux: file format elf64-littleaarch64
Sections:
Idx Name Size VMA LMA File off Algn
[...]
23 .bss 000da348 ffff0000091b8000 ffff0000091b8000 01147a00 2**12
ALLOC
That's the BSS, which we do map in HYP (fairly recent).
But maybe we should have have some stronger guarantees that we'll
always get things inlined, and that the "const" side is enforced:
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index b4989df..4710469 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
}
/* System capability check for constant caps */
-static inline bool cpus_have_const_cap(int num)
+static __always_inline bool cpus_have_const_cap(int num)
{
- if (num >= ARM64_NCAPS)
- return false;
+ BUILD_BUG_ON(!__builtin_constant_p(num));
+ BUILD_BUG_ON(num >= ARM64_NCAPS);
+
return static_branch_unlikely(&cpu_hwcap_keys[num]);
}
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 439f6b5..1257701 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -81,7 +81,7 @@ static inline bool is_kernel_in_hyp_mode(void)
return read_sysreg(CurrentEL) == CurrentEL_EL2;
}
-static inline bool has_vhe(void)
+static __always_inline bool has_vhe(void)
{
if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
return true;
But that's probably another patch or two. Thoughts?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related
* [RFC PATCH 09/10] drivers/perf: Add support for ARMv8.2 Statistical Profiling Extension
From: Will Deacon @ 2017-01-13 13:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111150239.724c8117e997fa31cfa4d602@arm.com>
On Wed, Jan 11, 2017 at 03:02:39PM -0600, Kim Phillips wrote:
> On Wed, 11 Jan 2017 12:37:15 +0000
> Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jan 10, 2017 at 04:04:19PM -0600, Kim Phillips wrote:
> > > Also, curiously, arm_spe_pmu doesn't appear in 'perf list' (even when
> > > SPE h/w is present).
> >
> > Weird, it would be nice to understand why that is. The sysfs plumbing should
> > all be there, so I'd expect to see something. On my laptop, for example,
> > intel_pt appears as:
> >
> > intel_pt// [Kernel PMU event]
> >
> > and strace show perf doing the following:
> >
> > stat("/sys/bus/event_source/devices/intel_pt/format", {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
> > open("/sys/bus/event_source/devices/intel_pt/format", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 82
> > open("/sys/bus/event_source/devices/intel_pt/format/psb_period", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/noretcomp", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/tsc", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/cyc_thresh", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/mtc_period", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/cyc", O_RDONLY) = 83
> > open("/sys/bus/event_source/devices/intel_pt/format/mtc", O_RDONLY) = 83
> > stat("/sys/bus/event_source/devices/intel_pt/events", 0x7ffe54eebb40) = -1 ENOENT (No such file or directory)
> > stat("/sys/bus/event_source/devices/intel_pt/type", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> > open("/sys/bus/event_source/devices/intel_pt/type", O_RDONLY) = 82
> > stat("/sys/bus/event_source/devices/intel_pt/cpumask", 0x7ffe54eedd60) = -1 ENOENT (No such file or directory)
> > stat("/sys/bus/event_source/devices/intel_pt/cpus", 0x7ffe54eedd60) = -1 ENOENT (No such file or directory)
> > stat("/sys/bus/event_source/devices/intel_pt/caps/mtc", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> > open("/sys/bus/event_source/devices/intel_pt/caps/mtc", O_RDONLY) = 82
> > stat("/sys/bus/event_source/devices/intel_pt/caps/psb_cyc", {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> > open("/sys/bus/event_source/devices/intel_pt/caps/psb_cyc", O_RDONLY) = 82
> >
> > What do you see for SPE?
>
> 2154 newfstatat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format", {st_mode=S_IFDIR|0755, st_size=0, ...}, 0) = 0
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 58
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/branch_filter", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/ts_enable", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/pa_enable", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/event_filter", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/load_filter", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/jitter", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/store_filter", O_RDONLY) = 59
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/format/min_latency", O_RDONLY) = 59
> 2154 newfstatat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/events", 0xffffcd6bb078, 0) = -1 ENOENT (No such file or directory)
> 2154 newfstatat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/type", {st_mode=S_IFREG|0444, st_size=4096, ...}, 0) = 0
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/type", O_RDONLY) = 58
> 2154 newfstatat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/cpumask", {st_mode=S_IFREG|0444, st_size=4096, ...}, 0) = 0
> 2154 openat(AT_FDCWD, "/sys/bus/event_source/devices/arm_spe_pmu_0/cpumask", O_RDONLY) = 58
>
> they're identical up until /.../cpumask's stat, which exists on the
> ARM SPE run (as opposed to the Intel run).
>From a quick look at the perf tool code, it looks like you need to ensure
that pmu->selectable is set to true for SPE, since it doesn't advertise
any events (the intel pt and coresight code does this already).
Will
^ permalink raw reply
* [PATCH v3 03/15] ARM: davinci: Don't rely on public mmc header to include interrupt.h
From: Sekhar Nori @ 2017-01-13 13:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484313256-25993-4-git-send-email-ulf.hansson@linaro.org>
On Friday 13 January 2017 06:44 PM, Ulf Hansson wrote:
> The davinci board omapl138-hawk, depends on interrupt.h. Explicitly include
> it instead of relying on the public mmc header host.h.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: <linux-arm-kernel@lists.infradead.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Additionally checked that 02/15 and 03/15 do not clash with anything I
have queued so far. And built test my tree + these two patches with
davinci_all_defconfig.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
From: Christoffer Dall @ 2017-01-13 13:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d496020c-a097-ffd5-adb7-7edb7c1a214e@arm.com>
On Fri, Jan 13, 2017 at 01:30:29PM +0000, Marc Zyngier wrote:
> [+ Suzuki, who wrote the whole cpus_have_const_cap thing]
>
> On 13/01/17 12:36, Christoffer Dall wrote:
> > On Fri, Jan 13, 2017 at 11:31:32AM +0000, Marc Zyngier wrote:
> >> From: Jintack Lim <jintack@cs.columbia.edu>
> >>
> >> Current KVM world switch code is unintentionally setting wrong bits to
> >> CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
> >> timer. Bit positions of CNTHCTL_EL2 are changing depending on
> >> HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
> >> not set, but they are 11th and 10th bits respectively when E2H is set.
> >>
> >> In fact, on VHE we only need to set those bits once, not for every world
> >> switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
> >> 1, which makes those bits have no effect for the host kernel execution.
> >> So we just set those bits once for guests, and that's it.
> >>
> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >> arch/arm/include/asm/virt.h | 5 +++++
> >> arch/arm/kvm/arm.c | 3 +++
> >> arch/arm64/include/asm/virt.h | 9 +++++++++
> >> include/kvm/arm_arch_timer.h | 1 +
> >> virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
> >> virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
> >> 6 files changed, 62 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
> >> index a2e75b8..6dae195 100644
> >> --- a/arch/arm/include/asm/virt.h
> >> +++ b/arch/arm/include/asm/virt.h
> >> @@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
> >> return false;
> >> }
> >>
> >> +static inline bool has_vhe(void)
> >> +{
> >> + return false;
> >> +}
> >> +
> >> /* The section containing the hypervisor idmap text */
> >> extern char __hyp_idmap_text_start[];
> >> extern char __hyp_idmap_text_end[];
> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> >> index 1167678..9d74464 100644
> >> --- a/arch/arm/kvm/arm.c
> >> +++ b/arch/arm/kvm/arm.c
> >> @@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
> >> __cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
> >> __cpu_init_stage2();
> >>
> >> + if (is_kernel_in_hyp_mode())
> >> + kvm_timer_init_vhe();
> >> +
> >> kvm_arm_init_debug();
> >> }
> >>
> >> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> >> index fea1073..439f6b5 100644
> >> --- a/arch/arm64/include/asm/virt.h
> >> +++ b/arch/arm64/include/asm/virt.h
> >> @@ -47,6 +47,7 @@
> >> #include <asm/ptrace.h>
> >> #include <asm/sections.h>
> >> #include <asm/sysreg.h>
> >> +#include <asm/cpufeature.h>
> >>
> >> /*
> >> * __boot_cpu_mode records what mode CPUs were booted in.
> >> @@ -80,6 +81,14 @@ static inline bool is_kernel_in_hyp_mode(void)
> >> return read_sysreg(CurrentEL) == CurrentEL_EL2;
> >> }
> >>
> >> +static inline bool has_vhe(void)
> >> +{
> >> + if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
> >> + return true;
> >> +
> >> + return false;
> >> +}
> >> +
> >
> > I was experimenting with using has_vhe for some of the optimization code
> > I was writing, and I saw a hyp crash as a result. That made me wonder
> > if this is really safe in Hyp mode?
> >
> > Specifically, there is no guarantee that this will actually be inlined
> > in the caller, right? At least that's what I can gather from trying to
> > understand the semantics of the inline keyword in the GCC manual.
>
> Indeed, there is no strict guarantee that this is enforced. We should
> probably have __always_inline instead. But having checked the generated
> code for __timer_restore_state, the function is definitely inlined
> (gcc 6.2). Happy to queue an extra patch changing that.
>
> > Further, are we guaranteed that the static branch gets compiled into
> > something that doesn't actually look at cpu_hwcap_keys, which is not
> > mapped in hyp mode?
>
> Here's the disassembly:
>
> ffff000008ad01d0 <__timer_restore_state>:
> ffff000008ad01d0: f9400001 ldr x1, [x0]
> ffff000008ad01d4: 9240bc21 and x1, x1, #0xffffffffffff
> ffff000008ad01d8: d503201f nop
> ffff000008ad01dc: d503201f nop
> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
> ffff000008ad01e4: 927ef842 and x2, x2, #0xfffffffffffffffd
> ffff000008ad01e8: b2400042 orr x2, x2, #0x1
> ffff000008ad01ec: d51ce102 msr cnthctl_el2, x2
> ffff000008ad01f0: d2834002 mov x2, #0x1a00 // #6656
> ffff000008ad01f4: 8b020000 add x0, x0, x2
> ffff000008ad01f8: 91038002 add x2, x0, #0xe0
> ffff000008ad01fc: 39425443 ldrb w3, [x2,#149]
> ffff000008ad0200: 34000103 cbz w3, ffff000008ad0220 <__timer_restore_state+0x50>
> ffff000008ad0204: f945a821 ldr x1, [x1,#2896]
> ffff000008ad0208: d51ce061 msr cntvoff_el2, x1
> ffff000008ad020c: f9400441 ldr x1, [x2,#8]
> ffff000008ad0210: d51be341 msr cntv_cval_el0, x1
> ffff000008ad0214: d5033fdf isb
> ffff000008ad0218: b940e000 ldr w0, [x0,#224]
> ffff000008ad021c: d51be320 msr cntv_ctl_el0, x0
> ffff000008ad0220: d65f03c0 ret
>
> The static branch resolves as such when VHE is enabled (taken from
> a running model):
>
> ffff000008ad01d0 <__timer_restore_state>:
> ffff000008ad01d0: f9400001 ldr x1, [x0]
> ffff000008ad01d4: 9240bc21 nop
> ffff000008ad01d8: d503201f nop
> ffff000008ad01dc: d503201f b ffff000008ad01f0
> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
> [...]
>
> That's using a toolchain that supports the "asm goto" feature that is used
> to implement static branches (and that's guaranteed not to generate any
> memory access other than the code patching itself).
>
> Now, with a toolchain that doesn't support this, such as gcc 4.8:
Hmm, I saw the error with 5.4.1, but perhaps I messed something else up,
because I cannot seem to reproduce this at the moment.
>
> ffff000008aa5168 <__timer_restore_state>:
> ffff000008aa5168: f9400001 ldr x1, [x0]
> ffff000008aa516c: 9240bc21 and x1, x1, #0xffffffffffff
> ffff000008aa5170: d503201f nop
> ffff000008aa5174: f00038a2 adrp x2, ffff0000091bc000 <reset_devices>
> ffff000008aa5178: 9113e042 add x2, x2, #0x4f8
> ffff000008aa517c: b9402c42 ldr w2, [x2,#44]
> ffff000008aa5180: 6b1f005f cmp w2, wzr
> ffff000008aa5184: 540000ac b.gt ffff000008aa5198 <__timer_restore_state+0x30>
> ffff000008aa5188: d53ce102 mrs x2, cnthctl_el2
> ffff000008aa518c: 927ef842 and x2, x2, #0xfffffffffffffffd
> ffff000008aa5190: b2400042 orr x2, x2, #0x1
> ffff000008aa5194: d51ce102 msr cnthctl_el2, x2
> ffff000008aa5198: 91400402 add x2, x0, #0x1, lsl #12
> ffff000008aa519c: 396dd443 ldrb w3, [x2,#2933]
> ffff000008aa51a0: 34000103 cbz w3, ffff000008aa51c0 <__timer_restore_state+0x58>
> ffff000008aa51a4: f945a821 ldr x1, [x1,#2896]
> ffff000008aa51a8: d51ce061 msr cntvoff_el2, x1
> ffff000008aa51ac: f9457441 ldr x1, [x2,#2792]
> ffff000008aa51b0: d51be341 msr cntv_cval_el0, x1
> ffff000008aa51b4: d5033fdf isb
> ffff000008aa51b8: b95ae000 ldr w0, [x0,#6880]
> ffff000008aa51bc: d51be320 msr cntv_ctl_el0, x0
> ffff000008aa51c0: d65f03c0 ret
>
> This is now controlled by some date located at FFFF0000091BC524:
>
> maz at approximate:~/Work/arm-platforms$ aarch64-linux-gnu-objdump -h vmlinux
>
> vmlinux: file format elf64-littleaarch64
>
> Sections:
> Idx Name Size VMA LMA File off Algn
> [...]
> 23 .bss 000da348 ffff0000091b8000 ffff0000091b8000 01147a00 2**12
> ALLOC
>
> That's the BSS, which we do map in HYP (fairly recent).
But we cannot map the BSS at the same address though, right? So
wouldn't this actually fail?
>
> But maybe we should have have some stronger guarantees that we'll
> always get things inlined, and that the "const" side is enforced:
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index b4989df..4710469 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
> }
>
> /* System capability check for constant caps */
> -static inline bool cpus_have_const_cap(int num)
> +static __always_inline bool cpus_have_const_cap(int num)
> {
> - if (num >= ARM64_NCAPS)
> - return false;
> + BUILD_BUG_ON(!__builtin_constant_p(num));
> + BUILD_BUG_ON(num >= ARM64_NCAPS);
> +
> return static_branch_unlikely(&cpu_hwcap_keys[num]);
> }
>
> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> index 439f6b5..1257701 100644
> --- a/arch/arm64/include/asm/virt.h
> +++ b/arch/arm64/include/asm/virt.h
> @@ -81,7 +81,7 @@ static inline bool is_kernel_in_hyp_mode(void)
> return read_sysreg(CurrentEL) == CurrentEL_EL2;
> }
>
> -static inline bool has_vhe(void)
> +static __always_inline bool has_vhe(void)
> {
> if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
> return true;
>
>
> But that's probably another patch or two. Thoughts?
>
Yes, if something needs fixing there, it should be a separate patch.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH 4/4] clk: rockchip: add rk3288 cif_out clock
From: Heiko Stuebner @ 2017-01-13 13:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484049560-14820-4-git-send-email-jacob-chen@iotwrt.com>
Hi Jacob,
Am Dienstag, 10. Januar 2017, 19:59:20 CET schrieb Jacob Chen:
> Add the clocks for the cif block of the rk3288
>
> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
> ---
> drivers/clk/rockchip/clk-rk3288.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c
> b/drivers/clk/rockchip/clk-rk3288.c index 8047cea..f071c24 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -192,6 +192,7 @@ PNAME(mux_uart1_p) = { "uart1_src", "uart1_frac",
> "xin24m" }; PNAME(mux_uart2_p) = { "uart2_src", "uart2_frac", "xin24m" };
> PNAME(mux_uart3_p) = { "uart3_src", "uart3_frac", "xin24m" };
> PNAME(mux_uart4_p) = { "uart4_src", "uart4_frac", "xin24m" };
> +PNAME(mux_cif_out_p) = { "cif_src", "xin24m" };
> PNAME(mux_vip_out_p) = { "vip_src", "xin24m" };
> PNAME(mux_mac_p) = { "mac_pll_src", "ext_gmac" };
> PNAME(mux_hsadcout_p) = { "hsadc_src", "ext_hsadc" };
> @@ -448,6 +449,12 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
> __initdata = { RK3288_CLKSEL_CON(6), 14, 2, MFLAGS, 8, 6, DFLAGS,
> RK3288_CLKGATE_CON(3), 15, GFLAGS),
>
> + COMPOSITE_NOGATE(0, "cif_src", mux_pll_src_cpll_gpll_p, 0,
> + RK3288_CLKSEL_CON(26), 8, 1, MFLAGS, 9, 5, DFLAGS),
> + COMPOSITE_NODIV(SCLK_CIF_OUT, "sclk_cif_out", mux_cif_out_p, 0,
> + RK3288_CLKSEL_CON(26), 15, 1, MFLAGS,
> + RK3288_CLKGATE_CON(3), 7, GFLAGS),
> +
we already have vip_src and sclk_vip_out defined, which I guess are the clocks
you are adding here and according to the TRM I have the names are also correct
(clock diagram 2).
But the clkid is not set yet, so I'd suggest adding SCLK_VIP_OUT and assigning
to the existing clocks.
Also, according to the clock diagram, the gate needs to be (and already is)
part of the vip_src clock and not the sclk_vip_out.
Heiko
^ permalink raw reply
* [PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
From: Marc Zyngier @ 2017-01-13 13:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113134640.GB31994@cbox>
On 13/01/17 13:46, Christoffer Dall wrote:
> On Fri, Jan 13, 2017 at 01:30:29PM +0000, Marc Zyngier wrote:
>> [+ Suzuki, who wrote the whole cpus_have_const_cap thing]
>>
>> On 13/01/17 12:36, Christoffer Dall wrote:
>>> On Fri, Jan 13, 2017 at 11:31:32AM +0000, Marc Zyngier wrote:
>>>> From: Jintack Lim <jintack@cs.columbia.edu>
>>>>
>>>> Current KVM world switch code is unintentionally setting wrong bits to
>>>> CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
>>>> timer. Bit positions of CNTHCTL_EL2 are changing depending on
>>>> HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
>>>> not set, but they are 11th and 10th bits respectively when E2H is set.
>>>>
>>>> In fact, on VHE we only need to set those bits once, not for every world
>>>> switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
>>>> 1, which makes those bits have no effect for the host kernel execution.
>>>> So we just set those bits once for guests, and that's it.
>>>>
>>>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>> arch/arm/include/asm/virt.h | 5 +++++
>>>> arch/arm/kvm/arm.c | 3 +++
>>>> arch/arm64/include/asm/virt.h | 9 +++++++++
>>>> include/kvm/arm_arch_timer.h | 1 +
>>>> virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
>>>> virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
>>>> 6 files changed, 62 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
>>>> index a2e75b8..6dae195 100644
>>>> --- a/arch/arm/include/asm/virt.h
>>>> +++ b/arch/arm/include/asm/virt.h
>>>> @@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
>>>> return false;
>>>> }
>>>>
>>>> +static inline bool has_vhe(void)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> /* The section containing the hypervisor idmap text */
>>>> extern char __hyp_idmap_text_start[];
>>>> extern char __hyp_idmap_text_end[];
>>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>>> index 1167678..9d74464 100644
>>>> --- a/arch/arm/kvm/arm.c
>>>> +++ b/arch/arm/kvm/arm.c
>>>> @@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
>>>> __cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
>>>> __cpu_init_stage2();
>>>>
>>>> + if (is_kernel_in_hyp_mode())
>>>> + kvm_timer_init_vhe();
>>>> +
>>>> kvm_arm_init_debug();
>>>> }
>>>>
>>>> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
>>>> index fea1073..439f6b5 100644
>>>> --- a/arch/arm64/include/asm/virt.h
>>>> +++ b/arch/arm64/include/asm/virt.h
>>>> @@ -47,6 +47,7 @@
>>>> #include <asm/ptrace.h>
>>>> #include <asm/sections.h>
>>>> #include <asm/sysreg.h>
>>>> +#include <asm/cpufeature.h>
>>>>
>>>> /*
>>>> * __boot_cpu_mode records what mode CPUs were booted in.
>>>> @@ -80,6 +81,14 @@ static inline bool is_kernel_in_hyp_mode(void)
>>>> return read_sysreg(CurrentEL) == CurrentEL_EL2;
>>>> }
>>>>
>>>> +static inline bool has_vhe(void)
>>>> +{
>>>> + if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
>>>> + return true;
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>
>>> I was experimenting with using has_vhe for some of the optimization code
>>> I was writing, and I saw a hyp crash as a result. That made me wonder
>>> if this is really safe in Hyp mode?
>>>
>>> Specifically, there is no guarantee that this will actually be inlined
>>> in the caller, right? At least that's what I can gather from trying to
>>> understand the semantics of the inline keyword in the GCC manual.
>>
>> Indeed, there is no strict guarantee that this is enforced. We should
>> probably have __always_inline instead. But having checked the generated
>> code for __timer_restore_state, the function is definitely inlined
>> (gcc 6.2). Happy to queue an extra patch changing that.
>>
>>> Further, are we guaranteed that the static branch gets compiled into
>>> something that doesn't actually look at cpu_hwcap_keys, which is not
>>> mapped in hyp mode?
>>
>> Here's the disassembly:
>>
>> ffff000008ad01d0 <__timer_restore_state>:
>> ffff000008ad01d0: f9400001 ldr x1, [x0]
>> ffff000008ad01d4: 9240bc21 and x1, x1, #0xffffffffffff
>> ffff000008ad01d8: d503201f nop
>> ffff000008ad01dc: d503201f nop
>> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
>> ffff000008ad01e4: 927ef842 and x2, x2, #0xfffffffffffffffd
>> ffff000008ad01e8: b2400042 orr x2, x2, #0x1
>> ffff000008ad01ec: d51ce102 msr cnthctl_el2, x2
>> ffff000008ad01f0: d2834002 mov x2, #0x1a00 // #6656
>> ffff000008ad01f4: 8b020000 add x0, x0, x2
>> ffff000008ad01f8: 91038002 add x2, x0, #0xe0
>> ffff000008ad01fc: 39425443 ldrb w3, [x2,#149]
>> ffff000008ad0200: 34000103 cbz w3, ffff000008ad0220 <__timer_restore_state+0x50>
>> ffff000008ad0204: f945a821 ldr x1, [x1,#2896]
>> ffff000008ad0208: d51ce061 msr cntvoff_el2, x1
>> ffff000008ad020c: f9400441 ldr x1, [x2,#8]
>> ffff000008ad0210: d51be341 msr cntv_cval_el0, x1
>> ffff000008ad0214: d5033fdf isb
>> ffff000008ad0218: b940e000 ldr w0, [x0,#224]
>> ffff000008ad021c: d51be320 msr cntv_ctl_el0, x0
>> ffff000008ad0220: d65f03c0 ret
>>
>> The static branch resolves as such when VHE is enabled (taken from
>> a running model):
>>
>> ffff000008ad01d0 <__timer_restore_state>:
>> ffff000008ad01d0: f9400001 ldr x1, [x0]
>> ffff000008ad01d4: 9240bc21 nop
>> ffff000008ad01d8: d503201f nop
>> ffff000008ad01dc: d503201f b ffff000008ad01f0
>> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
>> [...]
>>
>> That's using a toolchain that supports the "asm goto" feature that is used
>> to implement static branches (and that's guaranteed not to generate any
>> memory access other than the code patching itself).
>>
>> Now, with a toolchain that doesn't support this, such as gcc 4.8:
>
> Hmm, I saw the error with 5.4.1, but perhaps I messed something else up,
> because I cannot seem to reproduce this at the moment.
>
>>
>> ffff000008aa5168 <__timer_restore_state>:
>> ffff000008aa5168: f9400001 ldr x1, [x0]
>> ffff000008aa516c: 9240bc21 and x1, x1, #0xffffffffffff
>> ffff000008aa5170: d503201f nop
>> ffff000008aa5174: f00038a2 adrp x2, ffff0000091bc000 <reset_devices>
>> ffff000008aa5178: 9113e042 add x2, x2, #0x4f8
>> ffff000008aa517c: b9402c42 ldr w2, [x2,#44]
>> ffff000008aa5180: 6b1f005f cmp w2, wzr
>> ffff000008aa5184: 540000ac b.gt ffff000008aa5198 <__timer_restore_state+0x30>
>> ffff000008aa5188: d53ce102 mrs x2, cnthctl_el2
>> ffff000008aa518c: 927ef842 and x2, x2, #0xfffffffffffffffd
>> ffff000008aa5190: b2400042 orr x2, x2, #0x1
>> ffff000008aa5194: d51ce102 msr cnthctl_el2, x2
>> ffff000008aa5198: 91400402 add x2, x0, #0x1, lsl #12
>> ffff000008aa519c: 396dd443 ldrb w3, [x2,#2933]
>> ffff000008aa51a0: 34000103 cbz w3, ffff000008aa51c0 <__timer_restore_state+0x58>
>> ffff000008aa51a4: f945a821 ldr x1, [x1,#2896]
>> ffff000008aa51a8: d51ce061 msr cntvoff_el2, x1
>> ffff000008aa51ac: f9457441 ldr x1, [x2,#2792]
>> ffff000008aa51b0: d51be341 msr cntv_cval_el0, x1
>> ffff000008aa51b4: d5033fdf isb
>> ffff000008aa51b8: b95ae000 ldr w0, [x0,#6880]
>> ffff000008aa51bc: d51be320 msr cntv_ctl_el0, x0
>> ffff000008aa51c0: d65f03c0 ret
>>
>> This is now controlled by some date located at FFFF0000091BC524:
>>
>> maz at approximate:~/Work/arm-platforms$ aarch64-linux-gnu-objdump -h vmlinux
>>
>> vmlinux: file format elf64-littleaarch64
>>
>> Sections:
>> Idx Name Size VMA LMA File off Algn
>> [...]
>> 23 .bss 000da348 ffff0000091b8000 ffff0000091b8000 01147a00 2**12
>> ALLOC
>>
>> That's the BSS, which we do map in HYP (fairly recent).
>
> But we cannot map the BSS at the same address though, right? So
> wouldn't this actually fail?
We map it at the same relative offset, and use adrp to get the base
address (PC relative). So whatever context we're in, we should be OK.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v8 00/18] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Tomasz Nowicki @ 2017-01-13 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484127714-3263-1-git-send-email-eric.auger@redhat.com>
Hello Eric,
On 11.01.2017 10:41, Eric Auger wrote:
> Following LPC discussions, we now report reserved regions through
> the iommu-group sysfs reserved_regions attribute file.
>
> Reserved regions are populated through the IOMMU get_resv_region
> callback (former get_dm_regions), now implemented by amd-iommu,
> intel-iommu and arm-smmu:
> - the intel-iommu reports the [0xfee00000 - 0xfeefffff] MSI window
> as a reserved region and RMRR regions as direct-mapped regions.
> - the amd-iommu reports device direct mapped regions, the MSI region
> and HT regions.
> - the arm-smmu reports the MSI window (arbitrarily located at
> 0x8000000 and 1MB large).
>
> Unsafe interrupt assignment is tested by enumerating all MSI irq
> domains and checking MSI remapping is supported in the above hierarchy.
> This check is done in case we detect the iommu translates MSI
> (an IOMMU_RESV_MSI window exists). Otherwise the IRQ remapping
> capability is checked at IOMMU level. Obviously this is a defensive
> IRQ safety assessment: Assuming there are several MSI controllers
> in the system and at least one does not implement IRQ remapping,
> the assignment will be considered as unsafe (even if this controller
> is not acessible from the assigned devices).
>
> The series first patch stems from Robin's branch:
> http://linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/misc
>
> Best Regards
>
> Eric
>
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.10-rc3-reserved-v8
I tested the series on ThunderX with internal 10G VNIC and Intel IXGBE
NIC. Please feel free to add my:
Tested-by: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com>
Thanks,
Tomasz
^ permalink raw reply
* [PATCH v4 0/4] DMA Engine: switch PL330 driver to non-irq-safe runtime PM
From: Ulf Hansson @ 2017-01-13 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484292400-29730-1-git-send-email-m.szyprowski@samsung.com>
+Arnd
On 13 January 2017 at 08:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> This patchset changes the way the runtime PM is implemented in the PL330 DMA
> engine driver. The main goal of such change is to add support for the audio
> power domain to Exynos5 SoCs (5250, 542x, 5433, probably others) and let
> it to be properly turned off, when no audio is being used. Switching to
> non-irq-safe runtime PM is required to properly let power domain to be
> turned off (irq-safe runtime PM keeps power domain turned on all the time)
> and to integrate with clock controller's runtime PM (this cannot be
> workarounded any other way, PL330 uses clocks from the controller, which
> belongs to the same power domain).
>
> For more details of the proposed change to the PL330 driver see patch #4.
>
> Audio power domain on Exynos5 SoCs contains following hardware modules:
> 1. clock controller
> 2. pin controller
> 3. PL330 DMA controller
> 4. I2S audio controller
>
> Patches for adding or fixing runtime PM for each of the above devices is
> handled separately.
>
> Runtime PM patches for clock controllers is possible and has been proposed
> in the following thread (pending review): "[PATCH v4 0/4] Add runtime PM
> support for clocks (on Exynos SoC example)",
> http://www.spinics.net/lists/arm-kernel/msg550747.html
>
> Runtime PM support for Exynos pin controller has been posted in the
> following thread: "[PATCH 0/9] Runtime PM for Exynos pin controller driver",
> http://www.spinics.net/lists/arm-kernel/msg550161.html
>
> Exynos I2S driver supports runtime PM, but some fixes were needed for it
> and they are already queued to linux-next.
>
> This patchset is based on linux-next from 13th January 2017 with "dmaengine:
> pl330: fix double lock" patch applied.
>
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
Marek, this is great work! It's been on my TODO list forever, so I
really appreciate your work here.
I did only a brief review so far, particularly concentrating on the
changes for device links and runtime PM. I like it!
Perhaps we can get someone like Arnd/Vinod to comment in general idea
from a DT and DMA slave channel point of view. I don't know that stuff
good enough to give good opinion.
A couple of things that crosses my mind so far:
1) I have planned to extend pm_runtime_force_suspend|resume() to cover
also device links. Seems like that becomes really useful together with
these changes.
2) I think there will be some corner cases during system
suspend/resume for pl330. Not sure yet though. However, fixing 1) and
converting the driver to use pm_runtime_force_suspend|resume() should
probably work anyway.
Allow me to help out looking into 1) and 2). If not for pl330, I am
pretty sure it will be useful for other DMA controllers that
implements device links and runtime PM.
[...]
Kind regards
Uffe
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Andrew Lunn @ 2017-01-13 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112034121.27697-9-f.fainelli@gmail.com>
> index cd91070b5467..d326fc4afad7 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -81,17 +81,23 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
>
> static bool dsa_port_is_valid(struct dsa_port *port)
> {
> - return !!port->dn;
> + return !!(port->dn || port->name);
> }
Does this clash with Viviens recent change to make names optional and
have the kernel assign it?
I suppose you could use an name of "eth%d"? Is it worth adding a
comment to the platform data structure?
Andrew
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Andrew Lunn @ 2017-01-13 14:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112034121.27697-9-f.fainelli@gmail.com>
> static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
> {
> + struct dsa_chip_data *pdata = dev->platform_data;
> struct device_node *np = dev->of_node;
> struct dsa_switch_tree *dst;
> struct device_node *ports;
> u32 tree, index;
> int i, err;
>
> - err = dsa_parse_member_dn(np, &tree, &index);
> - if (err)
> - return err;
> + if (np) {
> + err = dsa_parse_member_dn(np, &tree, &index);
> + if (err)
> + return err;
>
> - ports = dsa_get_ports(ds, np);
> - if (IS_ERR(ports))
> - return PTR_ERR(ports);
> + ports = dsa_get_ports(ds, np);
> + if (IS_ERR(ports))
> + return PTR_ERR(ports);
>
> - err = dsa_parse_ports_dn(ports, ds);
> - if (err)
> - return err;
> + err = dsa_parse_ports_dn(ports, ds);
> + if (err)
> + return err;
> + } else {
> + err = dsa_parse_member(pdata, &tree, &index);
Hi Florian
Maybe it is hiding, but i don't see anywhere you check that pdata !=
NULL.
At least for x86 platforms, i don't expect we are booting using
platform data like ARM systems used to do. I think it is more likely a
glue module will be loaded. It looks up the MDIO bus and appends a
platform data to an MDIO device. The switch driver then needs to load
and use the platform data. But if things happen in a different order,
it could be the switch driver probes before the glue driver, meaning
pdata is NULL.
Do we even want to return -EPROBE_DEFERED?
Andrew
^ permalink raw reply
* [PATCH v7 00/15] ACPI platform MSI support and its example mbigen
From: Wei Xu @ 2017-01-13 14:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484147199-4267-1-git-send-email-hanjun.guo@linaro.org>
Hi Hanjun,
On 2017/1/11 15:06, Hanjun Guo wrote:
> With platform msi support landed in the kernel, and the introduction
> of IORT for GICv3 ITS (PCI MSI) and SMMU, the framework for platform msi
> is ready, this patch set add few patches to enable the ACPI platform
> msi support.
>
> For platform device connecting to ITS on arm platform, we have IORT
> table with the named componant node to describe the mappings of paltform
> device and ITS, so we can retrieve the dev id and find its parent
> irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).
>
> v6 -> v7:
> - Introduce iort_node_map_platform_id() to retrieve the
> dev id for both NC (named component) -> ITS/SMMU and
> NC -> SMMU -> ITS cases, suggested by Lorenzo;
>
> - Reorder the patches and rewrite some commit message;
>
> - Remove the test tags because it has major changes
> to retrieve the dev id, Sinan, Majun, Xinwei, could
> you please test them again on your platform?
>
> - rebased on top of 4.10-rc3 and Lorenzo's patch
> https://patchwork.kernel.org/patch/9507041/
>
> - Tested against Agustin's patch [1-2/3] "[PATCH V9 0/3] irqchip: qcom:
> Add IRQ combiner driver"
I tested this patch set on the Hisilicon D05 board with these patches:
[1] Agustin's V9 IRQ combiner driver patch set
https://patchwork.kernel.org/patch/9474751/
[2] Lorenzo's v2 iort_node_get_id fix patch
https://patchwork.kernel.org/patch/9507041/
The branch is at https://github.com/hisilicon/linux-hisi/tree/topic-acpi-mbigen.
The integrated XGE, SAS and PCIe controller works fine.
So with this patch set:
Tested-by: Wei Xu <xuwei5@hisilicon.com>
Thanks!
Best Regards,
Wei
^ permalink raw reply
* [PATCH v4 0/4] DMA Engine: switch PL330 driver to non-irq-safe runtime PM
From: Marek Szyprowski @ 2017-01-13 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFqDGs6OkG-NKiY9Xd3hHEJOJB1YJ2O0_3Tojrzv_xR-yw@mail.gmail.com>
Hi Ulf,
On 2017-01-13 14:59, Ulf Hansson wrote:
> On 13 January 2017 at 08:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> This patchset changes the way the runtime PM is implemented in the PL330 DMA
>> engine driver. The main goal of such change is to add support for the audio
>> power domain to Exynos5 SoCs (5250, 542x, 5433, probably others) and let
>> it to be properly turned off, when no audio is being used. Switching to
>> non-irq-safe runtime PM is required to properly let power domain to be
>> turned off (irq-safe runtime PM keeps power domain turned on all the time)
>> and to integrate with clock controller's runtime PM (this cannot be
>> workarounded any other way, PL330 uses clocks from the controller, which
>> belongs to the same power domain).
>>
>> For more details of the proposed change to the PL330 driver see patch #4.
>>
>> Audio power domain on Exynos5 SoCs contains following hardware modules:
>> 1. clock controller
>> 2. pin controller
>> 3. PL330 DMA controller
>> 4. I2S audio controller
>>
>> Patches for adding or fixing runtime PM for each of the above devices is
>> handled separately.
>>
>> Runtime PM patches for clock controllers is possible and has been proposed
>> in the following thread (pending review): "[PATCH v4 0/4] Add runtime PM
>> support for clocks (on Exynos SoC example)",
>> http://www.spinics.net/lists/arm-kernel/msg550747.html
>>
>> Runtime PM support for Exynos pin controller has been posted in the
>> following thread: "[PATCH 0/9] Runtime PM for Exynos pin controller driver",
>> http://www.spinics.net/lists/arm-kernel/msg550161.html
>>
>> Exynos I2S driver supports runtime PM, but some fixes were needed for it
>> and they are already queued to linux-next.
>>
>> This patchset is based on linux-next from 13th January 2017 with "dmaengine:
>> pl330: fix double lock" patch applied.
>>
>> Best regards
>> Marek Szyprowski
>> Samsung R&D Institute Poland
>
> Marek, this is great work! It's been on my TODO list forever, so I
> really appreciate your work here.
>
> I did only a brief review so far, particularly concentrating on the
> changes for device links and runtime PM. I like it!
>
> Perhaps we can get someone like Arnd/Vinod to comment in general idea
> from a DT and DMA slave channel point of view. I don't know that stuff
> good enough to give good opinion.
Arnd already said that it looks good:
http://www.spinics.net/lists/dmaengine/msg12186.html
> A couple of things that crosses my mind so far:
> 1) I have planned to extend pm_runtime_force_suspend|resume() to cover
> also device links. Seems like that becomes really useful together with
> these changes.
Is is really needed? I thought that this case is already handled by device
core. It works perfectly fine for Exynos IOMMU and its client devices for
suspend/resume too, which rely on pm_runtime_force_suspend|resume().
> 2) I think there will be some corner cases during system
> suspend/resume for pl330. Not sure yet though. However, fixing 1) and
> converting the driver to use pm_runtime_force_suspend|resume() should
> probably work anyway.
Do you have any particular case in mind? Device links ensures that pl330
will
suspended after its slave devices and waken before them. Is there anything
more needed here?
> Allow me to help out looking into 1) and 2). If not for pl330, I am
> pretty sure it will be useful for other DMA controllers that
> implements device links and runtime PM.
I'm open for suggestions.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH v1 1/2] Documentation: mtk-quadspi: update DT bindings
From: Boris Brezillon @ 2017-01-13 14:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484291609-20195-2-git-send-email-guochun.mao@mediatek.com>
On Fri, 13 Jan 2017 15:13:28 +0800
Guochun Mao <guochun.mao@mediatek.com> wrote:
> Add "mediatek,mt2701-nor" for nor flash node's compatible.
>
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
> .../devicetree/bindings/mtd/mtk-quadspi.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> index fb314f0..f83d31d 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -1,7 +1,9 @@
> * Serial NOR flash controller for MTK MT81xx (and similar)
>
> Required properties:
> -- compatible: should be "mediatek,mt8173-nor";
> +- compatible: should contain:
> + "mediatek,mt2701-nor" for MT2701,
> + "mediatek,mt8173-nor" for MT8173.
Do you need to define a new compatible? If the IPs are exactly the same
in both SoCs it shouldn't be needed.
> - reg: physical base address and length of the controller's register
> - clocks: the phandle of the clocks needed by the nor controller
> - clock-names: the names of the clocks
^ permalink raw reply
* [PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
From: Christoffer Dall @ 2017-01-13 14:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d591944f-8c12-afb9-e6c2-bce1bf526f56@arm.com>
On Fri, Jan 13, 2017 at 01:57:23PM +0000, Marc Zyngier wrote:
> On 13/01/17 13:46, Christoffer Dall wrote:
> > On Fri, Jan 13, 2017 at 01:30:29PM +0000, Marc Zyngier wrote:
> >> [+ Suzuki, who wrote the whole cpus_have_const_cap thing]
> >>
> >> On 13/01/17 12:36, Christoffer Dall wrote:
> >>> On Fri, Jan 13, 2017 at 11:31:32AM +0000, Marc Zyngier wrote:
> >>>> From: Jintack Lim <jintack@cs.columbia.edu>
> >>>>
> >>>> Current KVM world switch code is unintentionally setting wrong bits to
> >>>> CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
> >>>> timer. Bit positions of CNTHCTL_EL2 are changing depending on
> >>>> HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
> >>>> not set, but they are 11th and 10th bits respectively when E2H is set.
> >>>>
> >>>> In fact, on VHE we only need to set those bits once, not for every world
> >>>> switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
> >>>> 1, which makes those bits have no effect for the host kernel execution.
> >>>> So we just set those bits once for guests, and that's it.
> >>>>
> >>>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >>>> ---
> >>>> arch/arm/include/asm/virt.h | 5 +++++
> >>>> arch/arm/kvm/arm.c | 3 +++
> >>>> arch/arm64/include/asm/virt.h | 9 +++++++++
> >>>> include/kvm/arm_arch_timer.h | 1 +
> >>>> virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
> >>>> virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
> >>>> 6 files changed, 62 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
> >>>> index a2e75b8..6dae195 100644
> >>>> --- a/arch/arm/include/asm/virt.h
> >>>> +++ b/arch/arm/include/asm/virt.h
> >>>> @@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
> >>>> return false;
> >>>> }
> >>>>
> >>>> +static inline bool has_vhe(void)
> >>>> +{
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>> /* The section containing the hypervisor idmap text */
> >>>> extern char __hyp_idmap_text_start[];
> >>>> extern char __hyp_idmap_text_end[];
> >>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> >>>> index 1167678..9d74464 100644
> >>>> --- a/arch/arm/kvm/arm.c
> >>>> +++ b/arch/arm/kvm/arm.c
> >>>> @@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
> >>>> __cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
> >>>> __cpu_init_stage2();
> >>>>
> >>>> + if (is_kernel_in_hyp_mode())
> >>>> + kvm_timer_init_vhe();
> >>>> +
> >>>> kvm_arm_init_debug();
> >>>> }
> >>>>
> >>>> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> >>>> index fea1073..439f6b5 100644
> >>>> --- a/arch/arm64/include/asm/virt.h
> >>>> +++ b/arch/arm64/include/asm/virt.h
> >>>> @@ -47,6 +47,7 @@
> >>>> #include <asm/ptrace.h>
> >>>> #include <asm/sections.h>
> >>>> #include <asm/sysreg.h>
> >>>> +#include <asm/cpufeature.h>
> >>>>
> >>>> /*
> >>>> * __boot_cpu_mode records what mode CPUs were booted in.
> >>>> @@ -80,6 +81,14 @@ static inline bool is_kernel_in_hyp_mode(void)
> >>>> return read_sysreg(CurrentEL) == CurrentEL_EL2;
> >>>> }
> >>>>
> >>>> +static inline bool has_vhe(void)
> >>>> +{
> >>>> + if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
> >>>> + return true;
> >>>> +
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>
> >>> I was experimenting with using has_vhe for some of the optimization code
> >>> I was writing, and I saw a hyp crash as a result. That made me wonder
> >>> if this is really safe in Hyp mode?
> >>>
> >>> Specifically, there is no guarantee that this will actually be inlined
> >>> in the caller, right? At least that's what I can gather from trying to
> >>> understand the semantics of the inline keyword in the GCC manual.
> >>
> >> Indeed, there is no strict guarantee that this is enforced. We should
> >> probably have __always_inline instead. But having checked the generated
> >> code for __timer_restore_state, the function is definitely inlined
> >> (gcc 6.2). Happy to queue an extra patch changing that.
> >>
> >>> Further, are we guaranteed that the static branch gets compiled into
> >>> something that doesn't actually look at cpu_hwcap_keys, which is not
> >>> mapped in hyp mode?
> >>
> >> Here's the disassembly:
> >>
> >> ffff000008ad01d0 <__timer_restore_state>:
> >> ffff000008ad01d0: f9400001 ldr x1, [x0]
> >> ffff000008ad01d4: 9240bc21 and x1, x1, #0xffffffffffff
> >> ffff000008ad01d8: d503201f nop
> >> ffff000008ad01dc: d503201f nop
> >> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
> >> ffff000008ad01e4: 927ef842 and x2, x2, #0xfffffffffffffffd
> >> ffff000008ad01e8: b2400042 orr x2, x2, #0x1
> >> ffff000008ad01ec: d51ce102 msr cnthctl_el2, x2
> >> ffff000008ad01f0: d2834002 mov x2, #0x1a00 // #6656
> >> ffff000008ad01f4: 8b020000 add x0, x0, x2
> >> ffff000008ad01f8: 91038002 add x2, x0, #0xe0
> >> ffff000008ad01fc: 39425443 ldrb w3, [x2,#149]
> >> ffff000008ad0200: 34000103 cbz w3, ffff000008ad0220 <__timer_restore_state+0x50>
> >> ffff000008ad0204: f945a821 ldr x1, [x1,#2896]
> >> ffff000008ad0208: d51ce061 msr cntvoff_el2, x1
> >> ffff000008ad020c: f9400441 ldr x1, [x2,#8]
> >> ffff000008ad0210: d51be341 msr cntv_cval_el0, x1
> >> ffff000008ad0214: d5033fdf isb
> >> ffff000008ad0218: b940e000 ldr w0, [x0,#224]
> >> ffff000008ad021c: d51be320 msr cntv_ctl_el0, x0
> >> ffff000008ad0220: d65f03c0 ret
> >>
> >> The static branch resolves as such when VHE is enabled (taken from
> >> a running model):
> >>
> >> ffff000008ad01d0 <__timer_restore_state>:
> >> ffff000008ad01d0: f9400001 ldr x1, [x0]
> >> ffff000008ad01d4: 9240bc21 nop
> >> ffff000008ad01d8: d503201f nop
> >> ffff000008ad01dc: d503201f b ffff000008ad01f0
> >> ffff000008ad01e0: d53ce102 mrs x2, cnthctl_el2
> >> [...]
> >>
> >> That's using a toolchain that supports the "asm goto" feature that is used
> >> to implement static branches (and that's guaranteed not to generate any
> >> memory access other than the code patching itself).
> >>
> >> Now, with a toolchain that doesn't support this, such as gcc 4.8:
> >
> > Hmm, I saw the error with 5.4.1, but perhaps I messed something else up,
> > because I cannot seem to reproduce this at the moment.
> >
> >>
> >> ffff000008aa5168 <__timer_restore_state>:
> >> ffff000008aa5168: f9400001 ldr x1, [x0]
> >> ffff000008aa516c: 9240bc21 and x1, x1, #0xffffffffffff
> >> ffff000008aa5170: d503201f nop
> >> ffff000008aa5174: f00038a2 adrp x2, ffff0000091bc000 <reset_devices>
> >> ffff000008aa5178: 9113e042 add x2, x2, #0x4f8
> >> ffff000008aa517c: b9402c42 ldr w2, [x2,#44]
> >> ffff000008aa5180: 6b1f005f cmp w2, wzr
> >> ffff000008aa5184: 540000ac b.gt ffff000008aa5198 <__timer_restore_state+0x30>
> >> ffff000008aa5188: d53ce102 mrs x2, cnthctl_el2
> >> ffff000008aa518c: 927ef842 and x2, x2, #0xfffffffffffffffd
> >> ffff000008aa5190: b2400042 orr x2, x2, #0x1
> >> ffff000008aa5194: d51ce102 msr cnthctl_el2, x2
> >> ffff000008aa5198: 91400402 add x2, x0, #0x1, lsl #12
> >> ffff000008aa519c: 396dd443 ldrb w3, [x2,#2933]
> >> ffff000008aa51a0: 34000103 cbz w3, ffff000008aa51c0 <__timer_restore_state+0x58>
> >> ffff000008aa51a4: f945a821 ldr x1, [x1,#2896]
> >> ffff000008aa51a8: d51ce061 msr cntvoff_el2, x1
> >> ffff000008aa51ac: f9457441 ldr x1, [x2,#2792]
> >> ffff000008aa51b0: d51be341 msr cntv_cval_el0, x1
> >> ffff000008aa51b4: d5033fdf isb
> >> ffff000008aa51b8: b95ae000 ldr w0, [x0,#6880]
> >> ffff000008aa51bc: d51be320 msr cntv_ctl_el0, x0
> >> ffff000008aa51c0: d65f03c0 ret
> >>
> >> This is now controlled by some date located at FFFF0000091BC524:
> >>
> >> maz at approximate:~/Work/arm-platforms$ aarch64-linux-gnu-objdump -h vmlinux
> >>
> >> vmlinux: file format elf64-littleaarch64
> >>
> >> Sections:
> >> Idx Name Size VMA LMA File off Algn
> >> [...]
> >> 23 .bss 000da348 ffff0000091b8000 ffff0000091b8000 01147a00 2**12
> >> ALLOC
> >>
> >> That's the BSS, which we do map in HYP (fairly recent).
> >
> > But we cannot map the BSS at the same address though, right? So
> > wouldn't this actually fail?
>
> We map it at the same relative offset, and use adrp to get the base
> address (PC relative). So whatever context we're in, we should be OK.
>
Ah, right, I'll be shutting up now then.
(Will make a not to go back and carefully examine exactly why this
failed for me.)
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-13 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484291609-20195-3-git-send-email-guochun.mao@mediatek.com>
On Fri, 13 Jan 2017 15:13:29 +0800
Guochun Mao <guochun.mao@mediatek.com> wrote:
> Add Mediatek nor flash node.
>
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
> index 082ca88..85e5ae8 100644
> --- a/arch/arm/boot/dts/mt2701-evb.dts
> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> @@ -24,6 +24,31 @@
> };
> };
>
> +&nor_flash {
> + pinctrl-names = "default";
> + pinctrl-0 = <&nor_pins_default>;
> + status = "okay";
> + flash at 0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + };
> +};
> +
> +&pio {
> + nor_pins_default: nor {
> + pins1 {
> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
> + drive-strength = <MTK_DRIVE_4mA>;
> + bias-pull-up;
> + };
> + };
> +};
> +
> &uart0 {
> status = "okay";
> };
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index bdf8954..1eefce4 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -227,6 +227,18 @@
> status = "disabled";
> };
>
> + nor_flash: spi at 11014000 {
> + compatible = "mediatek,mt2701-nor",
> + "mediatek,mt8173-nor";
Why define both here? Is "mediatek,mt8173-nor" really providing a
subset of the features supported by "mediatek,mt2701-nor"?
> + reg = <0 0x11014000 0 0xe0>;
> + clocks = <&pericfg CLK_PERI_FLASH>,
> + <&topckgen CLK_TOP_FLASH_SEL>;
> + clock-names = "spi", "sf";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> mmsys: syscon at 14000000 {
> compatible = "mediatek,mt2701-mmsys", "syscon";
> reg = <0 0x14000000 0 0x1000>;
^ permalink raw reply
* [PATCH v2 18/18] insert build break
From: Christopher Bostic @ 2017-01-13 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113071545.GB12441@kroah.com>
On Fri, Jan 13, 2017 at 1:15 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 12, 2017 at 04:37:35PM -0600, christopher.lee.bostic at gmail.com wrote:
>> From: Chris Bostic <cbostic@us.ibm.com>
>>
>> Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
>
> I can not accept patches that have no changelog text, and this one is
> very odd:
>
>> ---
>> drivers/fsi/fsi-core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 28b82d1..db09836 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -42,6 +42,7 @@
>>
>> static DEFINE_IDA(master_ida);
>>
>> +
>
> Huh?
>
> Did something go wrong with your scripts?
Yes, had an error when sending out... Will correct this when I send
out version 3 today.
Sorry for the confusion.
-Chris
>
> greg k-h
^ permalink raw reply
* [PATCH v4 4/4] dmaengine: pl330: Don't require irq-safe runtime PM
From: Krzysztof Kozlowski @ 2017-01-13 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484292400-29730-5-git-send-email-m.szyprowski@samsung.com>
On Fri, Jan 13, 2017 at 08:26:40AM +0100, Marek Szyprowski wrote:
> This patch replaces irq-safe runtime PM with non-irq-safe version based on
> the new approach. Existing, irq-safe runtime PM implementation for PL330 was
> not bringing much benefits of its own - only clocks were enabled/disabled.
>
> Till now non-irq-safe runtime PM implementation was only possible by calling
> pm_runtime_get/put functions from alloc/free_chan_resources. All other DMA
> engine API functions cannot be called from a context, which permits sleeping.
> Such implementation, in practice would result in keeping DMA controller's
> device active almost all the time, because most of the slave device drivers
> (DMA engine clients) acquire DMA channel in their probe() function and
> released it during driver removal.
>
> This patch provides a new, different approach. It is based on an observation
> that there can be only one slave device using each DMA channel. PL330 hardware
> always has dedicated channels for each peripheral device. Using recently
> introduced device dependencies (links) infrastructure one can ensure proper
> runtime PM state of PL330 DMA controller basing on the runtime PM state of
> the slave device.
>
> In this approach in pl330_alloc_chan_resources() function a new dependency
> is being created between PL330 DMA controller device (as a supplier) and
> given slave device (as a consumer). This way PL330 DMA controller device
> runtime active counter is increased when the slave device is resumed and
> decreased the same time when given slave device is put to suspend. This way
> it has been ensured to keep PL330 DMA controller runtime active if there is
> an active used of any of its DMA channels. Slave device pointer is initially
> stored in per-channel data in of_dma_xlate callback. This is similar to what
> has been already implemented in Exynos IOMMU driver in commit 2f5f44f205cc95
> ("iommu/exynos: Use device dependency links to control runtime pm").
>
> If slave device doesn't implement runtime PM or keeps device runtime active
> all the time, then PL330 DMA controller will be runtime active all the time
> when channel is being allocated. The goal is however to have runtime PM
> added to all devices in the system, because it lets respective power
> domains to be turned off, what gives the best results in terms of power
> saving.
>
> If one requests memory-to-memory channel, runtime active counter is
> increased unconditionally. This might be a drawback of this approach, but
> PL330 is not really used for memory-to-memory operations due to poor
> performance in such operations compared to the CPU.
>
> Introducing non-irq-safe runtime power management finally allows to turn off
> audio power domain on Exynos5 SoCs.
>
> Removal of irq-safe runtime PM is based on the revert of the following
> commits:
> 1. commit 5c9e6c2b2ba3 "dmaengine: pl330: fix runtime pm support"
> 2. commit 81cc6edc0870 "dmaengine: pl330: Fix hang on dmaengine_terminate_all
> on certain boards"
> 3. commit ae43b3289186 "ARM: 8202/1: dmaengine: pl330: Add runtime Power
> Management support v12"
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/dma/pl330.c | 135 ++++++++++++++++++++++++++++------------------------
> 1 file changed, 72 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index c77a3494659c..a97cb02250ab 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -16,6 +16,7 @@
> #include <linux/init.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/string.h>
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> @@ -268,9 +269,6 @@ enum pl330_byteswap {
>
> #define NR_DEFAULT_DESC 16
>
> -/* Delay for runtime PM autosuspend, ms */
> -#define PL330_AUTOSUSPEND_DELAY 20
> -
> /* Populated by the PL330 core driver for DMA API driver's info */
> struct pl330_config {
> u32 periph_id;
> @@ -449,8 +447,8 @@ struct dma_pl330_chan {
> bool cyclic;
>
> /* for runtime pm tracking */
> - bool active;
> struct device *slave;
> + struct device_link *slave_link;
> };
>
> struct pl330_dmac {
> @@ -465,6 +463,9 @@ struct pl330_dmac {
> /* To protect desc_pool manipulation */
> spinlock_t pool_lock;
>
> + /* For runtime PM management of slave channels */
> + struct mutex rpm_lock;
Isn't synchronization provided by dmaengine core here? The
dma_chan_get/put (which call the alloc/free resources) are called under
dma_list_mutex(). Don't you trust the core in that matter?
Best regards,
Krzysztof
^ permalink raw reply
* [GIT PULL] ARM: exynos: Fixes for v4.10
From: Krzysztof Kozlowski @ 2017-01-13 14:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Just a couple of minor fixes for this release cycle.
Best regards,
Krzysztof
The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:
Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-fixes-4.10
for you to fetch changes up to 3ef01c968fbfb21c2f16281445d30a865ee4412c:
ARM: s3c2410_defconfig: Fix invalid values for NF_CT_PROTO_* (2017-01-02 21:05:34 +0200)
----------------------------------------------------------------
Samsung fixes for v4.10:
1. Update maintainers entry with Patchwork address.
2. Fix invalid values for NF_CT_PROTO_* in s3c2410 defconfig (these options
cannot be modules anymore).
----------------------------------------------------------------
Krzysztof Kozlowski (2):
MAINTAINERS: Add Patchwork URL to Samsung Exynos entry
ARM: s3c2410_defconfig: Fix invalid values for NF_CT_PROTO_*
MAINTAINERS | 1 +
arch/arm/configs/s3c2410_defconfig | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
^ permalink raw reply
* [PATCH 00/10] ARM: da850-lcdk: add SATA support
From: Sekhar Nori @ 2017-01-13 14:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484311084-31547-1-git-send-email-bgolaszewski@baylibre.com>
On Friday 13 January 2017 06:07 PM, Bartosz Golaszewski wrote:
> This series contains all the changes necessary to make SATA work on
> the da850-lcdk board.
>
> The first patch adds a clock lookup entry required for the ahci core
> to retrieve a functional clock.
>
> The second enables relevant config options for all davinci boards.
>
> The third adds device tree bindings for the ahci_da850 driver.
>
> The fourth adds a workaround for a SATA controller instability we
> detected after increasing the PLL0 frequency for proper LCD
> controller support.
>
> Patches 5 through 7 extend the ahci_da850 driver - add DT support,
> un-hardcode the clock multiplier value and add a workaround for
> a quirk present on the da850 SATA controller.
>
> Patches 8-10 add the device tree changes required to probe the driver.
>
> I'm posting the series as a whole to give all reviewers the full
> picture and visibility of the changes required, if needed I can resend
> the patches separately.
I just tested this series on my LCDK board using a Western Digital SATA
HDD and it works great with some basic read / write tests. Thanks!
For the non-platform patches which I wont be queuing:
Tested-by: Sekhar Nori <nsekhar@ti.com>
I will take a look at the series closely next week.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH -next] crypto: mediatek - make symbol of_crypto_id static
From: Herbert Xu @ 2017-01-13 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112150321.4187-1-weiyj.lk@gmail.com>
On Thu, Jan 12, 2017 at 03:03:21PM +0000, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fixes the following sparse warning:
>
> drivers/crypto/mediatek/mtk-platform.c:585:27: warning:
> symbol 'of_crypto_id' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH] crypto: arm/aes - avoid reserved 'tt' mnemonic in asm code
From: Herbert Xu @ 2017-01-13 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484296406-13224-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, Jan 13, 2017 at 08:33:26AM +0000, Ard Biesheuvel wrote:
> The ARMv8-M architecture introduces 'tt' and 'ttt' instructions,
> which means we can no longer use 'tt' as a register alias on recent
> versions of binutils for ARM. So replace the alias with 'ttab'.
>
> Fixes: 81edb4262975 ("crypto: arm/aes - replace scalar AES cipher")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 1/3] arm: dts: mt2701: Sort DT nodes by register address
From: Matthias Brugger @ 2017-01-13 14:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1482904006-44232-2-git-send-email-jamesjj.liao@mediatek.com>
On 28/12/16 06:46, James Liao wrote:
> This patch rearrange MT2701 DT nodes to keep them in ascending order.
>
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> ---
> arch/arm/boot/dts/mt2701.dtsi | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index 7eab6f4..73f4b7c 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -96,24 +96,6 @@
> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> };
>
> - pio: pinctrl at 10005000 {
> - compatible = "mediatek,mt2701-pinctrl";
> - reg = <0 0x1000b000 0 0x1000>;
> - mediatek,pctl-regmap = <&syscfg_pctl_a>;
> - pins-are-numbered;
> - gpio-controller;
> - #gpio-cells = <2>;
> - interrupt-controller;
> - #interrupt-cells = <2>;
> - interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> - };
Applied the whole series. I fixed the unit address of pio to 1000b000
and it's order in the file.
Please check v4.10-next/dts32
Thanks,
Mathias
> -
> - syscfg_pctl_a: syscfg at 10005000 {
> - compatible = "mediatek,mt2701-pctl-a-syscfg", "syscon";
> - reg = <0 0x10005000 0 0x1000>;
> - };
> -
> topckgen: syscon at 10000000 {
> compatible = "mediatek,mt2701-topckgen", "syscon";
> reg = <0 0x10000000 0 0x1000>;
> @@ -134,6 +116,24 @@
> #reset-cells = <1>;
> };
>
> + pio: pinctrl at 10005000 {
> + compatible = "mediatek,mt2701-pinctrl";
> + reg = <0 0x1000b000 0 0x1000>;
> + mediatek,pctl-regmap = <&syscfg_pctl_a>;
> + pins-are-numbered;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + syscfg_pctl_a: syscfg at 10005000 {
> + compatible = "mediatek,mt2701-pctl-a-syscfg", "syscon";
> + reg = <0 0x10005000 0 0x1000>;
> + };
> +
> watchdog: watchdog at 10007000 {
> compatible = "mediatek,mt2701-wdt",
> "mediatek,mt6589-wdt";
>
^ permalink raw reply
* [PATCH v4 0/4] DMA Engine: switch PL330 driver to non-irq-safe runtime PM
From: Ulf Hansson @ 2017-01-13 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <60596eee-12a9-58f3-387e-145a5bc28aaa@samsung.com>
[...]
>>
>> Marek, this is great work! It's been on my TODO list forever, so I
>> really appreciate your work here.
>>
>> I did only a brief review so far, particularly concentrating on the
>> changes for device links and runtime PM. I like it!
>>
>> Perhaps we can get someone like Arnd/Vinod to comment in general idea
>> from a DT and DMA slave channel point of view. I don't know that stuff
>> good enough to give good opinion.
>
>
> Arnd already said that it looks good:
> http://www.spinics.net/lists/dmaengine/msg12186.html
Great! Clearly, I haven't fully catched up since the holidays. :-)
>
>> A couple of things that crosses my mind so far:
>> 1) I have planned to extend pm_runtime_force_suspend|resume() to cover
>> also device links. Seems like that becomes really useful together with
>> these changes.
>
>
> Is is really needed? I thought that this case is already handled by device
> core. It works perfectly fine for Exynos IOMMU and its client devices for
> suspend/resume too, which rely on pm_runtime_force_suspend|resume().
Only parent devices are being considered in
pm_runtime_force_suspend|resume(). Meaning that the runtime PM usage
count will not be updated for a device's link, even if it should.
I guess what happens is that the pl330 controller will be brought up
to full power during system resume, even if it isn't necessary to do
so. If that's the case, it's not the end of the world, we can optimize
it later on.
Note, this is very theoretical and to be sure, I need to run some
tests and think more about it.
>
>> 2) I think there will be some corner cases during system
>> suspend/resume for pl330. Not sure yet though. However, fixing 1) and
>> converting the driver to use pm_runtime_force_suspend|resume() should
>> probably work anyway.
>
>
> Do you have any particular case in mind? Device links ensures that pl330
> will
> suspended after its slave devices and waken before them. Is there anything
> more needed here?
Likely we can consider them as optimizations. See comment above.
>
>> Allow me to help out looking into 1) and 2). If not for pl330, I am
>> pretty sure it will be useful for other DMA controllers that
>> implements device links and runtime PM.
>
>
> I'm open for suggestions.
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Kind regards
Uffe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox