* [PATCH v7 00/26] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)
From: Ulf Hansson @ 2018-05-24 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1523531671-27491-1-git-send-email-ulf.hansson@linaro.org>
Sudeep, Lorenzo, Mark
On 12 April 2018 at 13:14, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v7:
> - Addressed comments concerning the PSCI changes from Mark Rutland, which moves
> the psci firmware driver to a new firmware subdir and change to force PSCI PC
> mode during boot to cope with kexec'ed booted kernels.
Are you guys happy in general with the PSCI specific parts from this
series? It would be nice if you could provide me with acks or
reviewed-by tags for the rest of them in such case.
I intend to re-post a new version very soon.
[...]
Kind regards
Uffe
^ permalink raw reply
* [PATCH v10 05/18] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Alex Bennée @ 2018-05-24 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180524085445.GP13470@e103592.cambridge.arm.com>
Dave Martin <Dave.Martin@arm.com> writes:
> On Thu, May 24, 2018 at 10:12:20AM +0200, Christoffer Dall wrote:
>> On Wed, May 23, 2018 at 08:35:13PM +0100, Alex Benn?e wrote:
>> >
>> > Dave Martin <Dave.Martin@arm.com> writes:
>> >
>> > > To make the lazy FPSIMD context switch trap code easier to hack on,
>> > > this patch converts it to C.
>> > >
>> > > This is not amazingly efficient, but the trap should typically only
>> > > be taken once per host context switch.
>> > >
>> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>> > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>> > > ---
>> > > arch/arm64/kvm/hyp/entry.S | 57 +++++++++++++++++----------------------------
>> > > arch/arm64/kvm/hyp/switch.c | 24 +++++++++++++++++++
>> > > 2 files changed, 46 insertions(+), 35 deletions(-)
>
> [...]
>
>> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> > > index d964523..c0796c4 100644
>> > > --- a/arch/arm64/kvm/hyp/switch.c
>> > > +++ b/arch/arm64/kvm/hyp/switch.c
>> > > @@ -318,6 +318,30 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
>> > > }
>> > > }
>> > >
>> > > +void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
>> > > + struct kvm_vcpu *vcpu)
>> > > +{
>> > > + kvm_cpu_context_t *host_ctxt;
>> > > +
>> > > + if (has_vhe())
>> > > + write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
>> > > + cpacr_el1);
>> > > + else
>> > > + write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
>> > > + cptr_el2);
>> >
>> > Is there no way to do alternative() in C or does it always come down to
>> > different inline asms?
>> >
>>
>> has_vhe() should resolve to a static key, and I prefer this over the
>> previous alternative construct we had for selecting function calls in C,
>> as that resultet in having to follow too many levels of indirection.
>
> I'll defer to Christoffer on that -- I was just following precedent :)
>
> The if (has_vhe()) approach has the benefit of being much more
> readable, and the static branch predictor in many CPUs will succeed in
> folding a short-range unconditional branch out entirely. There will be
> a small increase in I-cache pressure due to the larger inline code
> size, but probably not much beyond that.
Fair enough - it was mostly a curiosity. It seems most of the use of
alternative() are mostly at the low level instruction level anyway.
--
Alex Benn?e
^ permalink raw reply
* [PATCH v10 17/18] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
From: Christoffer Dall @ 2018-05-24 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527005119-6842-18-git-send-email-Dave.Martin@arm.com>
On Tue, May 22, 2018 at 05:05:18PM +0100, Dave Martin wrote:
> The entire tail of fixup_guest_exit() is contained in if statements
> of the form if (x && *exit_code == ARM_EXCEPTION_TRAP). As a result,
> we can check just once and bail out of the function early, allowing
> the remaining if conditions to be simplified.
>
> The only awkward case is where *exit_code is changed to
> ARM_EXCEPTION_EL1_SERROR in the case of an illegal GICv2 CPU
> interface access: in that case, the GICv3 trap handling code is
> skipped using a goto. This avoids pointlessly evaluating the
> static branch check for the GICv3 case, even though we can't have
> vgic_v2_cpuif_trap and vgic_v3_cpuif_trap true simultaneously
> unless we have a GICv3 and GICv2 on the host: that sounds stupid,
> but I haven't satisfied myself that it can't happen.
>
> No functional change.
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/hyp/switch.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 18d0faa..4fbee95 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -387,11 +387,13 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> * same PC once the SError has been injected, and replay the
> * trapping instruction.
> */
> - if (*exit_code == ARM_EXCEPTION_TRAP && !__populate_fault_info(vcpu))
> + if (*exit_code != ARM_EXCEPTION_TRAP)
> + goto exit;
> +
> + if (!__populate_fault_info(vcpu))
> return true;
>
> - if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
> - *exit_code == ARM_EXCEPTION_TRAP) {
> + if (static_branch_unlikely(&vgic_v2_cpuif_trap)) {
> bool valid;
>
> valid = kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_DABT_LOW &&
> @@ -417,11 +419,12 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
> *exit_code = ARM_EXCEPTION_EL1_SERROR;
> }
> +
> + goto exit;
> }
> }
>
> if (static_branch_unlikely(&vgic_v3_cpuif_trap) &&
> - *exit_code == ARM_EXCEPTION_TRAP &&
> (kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 ||
> kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
> int ret = __vgic_v3_perform_cpuif_access(vcpu);
> @@ -430,6 +433,7 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> return true;
> }
>
> +exit:
> /* Return to the host kernel and handle the exit */
> return false;
> }
> --
> 2.1.4
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* [PATCH v10 16/18] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
From: Christoffer Dall @ 2018-05-24 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527005119-6842-17-git-send-email-Dave.Martin@arm.com>
On Tue, May 22, 2018 at 05:05:17PM +0100, Dave Martin wrote:
> In fixup_guest_exit(), there are a couple of cases where after
> checking what the exit code was, we assign it explicitly with the
> value it already had.
>
> Assuming this is not indicative of a bug, these assignments are not
> needed.
>
> This patch removes the redundant assignments, and simplifies some
> if-nesting that becomes trivial as a result.
>
> No functional change.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> ---
> arch/arm64/kvm/hyp/switch.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index a6a8c7d..18d0faa 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -403,12 +403,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> if (valid) {
> int ret = __vgic_v2_perform_cpuif_access(vcpu);
>
> - if (ret == 1) {
> - if (__skip_instr(vcpu))
> - return true;
> - else
> - *exit_code = ARM_EXCEPTION_TRAP;
> - }
> + if (ret == 1 && __skip_instr(vcpu))
> + return true;
>
> if (ret == -1) {
> /* Promote an illegal access to an
> @@ -430,12 +426,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
> int ret = __vgic_v3_perform_cpuif_access(vcpu);
>
> - if (ret == 1) {
> - if (__skip_instr(vcpu))
> - return true;
> - else
> - *exit_code = ARM_EXCEPTION_TRAP;
> - }
> + if (ret == 1 && __skip_instr(vcpu))
> + return true;
> }
>
> /* Return to the host kernel and handle the exit */
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10 14/18] KVM: arm64: Save host SVE context as appropriate
From: Christoffer Dall @ 2018-05-24 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527005119-6842-15-git-send-email-Dave.Martin@arm.com>
On Tue, May 22, 2018 at 05:05:15PM +0100, Dave Martin wrote:
> This patch adds SVE context saving to the hyp FPSIMD context switch
> path. This means that it is no longer necessary to save the host
> SVE state in advance of entering the guest, when in use.
>
> In order to avoid adding pointless complexity to the code, VHE is
> assumed if SVE is in use. VHE is an architectural prerequisite for
> SVE, so there is no good reason to turn CONFIG_ARM64_VHE off in
> kernels that support both SVE and KVM.
>
> Historically, software models exist that can expose the
> architecturally invalid configuration of SVE without VHE, so if
> this situation is detected at kvm_init() time then KVM will be
> disabled.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>
> ---
>
> * Tags stripped since v8, please reconfirm if possible:
>
> Formerly-Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
> Formerly-Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Formerly-Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Changes since v9:
>
> Requested by Marc Zyngier:
>
> * Inline check for VHE if SVE is present into kvm_host.h.
>
> The check has been renamed to the more specific
> kvm_arch_check_sve_has_vhe(), and the kvm_pr_unimpl() moved back to
> arm.c (to avoid circular include issues).
>
> arm.c is not single-arch code, but it is all Arm-specific, so
> adding a hook like this doesn't seem too unreasonable.
>
> Changes since v8:
>
> * Add kvm_arch_check_supported() hook, and move arm64-specific check
> for SVE-implies-VHE into arch/arm64/.
>
> Due to circular header dependency problems, it is difficult to get
> the prototype for kvm_pr_*() functions in <asm/kvm_host.h>, so this
> patch puts arm64's kvm_arch_check_supported() hook out of line.
> This is not a hot function.
> ---
> arch/arm/include/asm/kvm_host.h | 1 +
> arch/arm64/Kconfig | 7 +++++++
> arch/arm64/include/asm/kvm_host.h | 13 +++++++++++++
> arch/arm64/kvm/fpsimd.c | 1 -
> arch/arm64/kvm/hyp/switch.c | 20 +++++++++++++++++++-
> virt/kvm/arm/arm.c | 7 +++++++
> 6 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index ac870b2..3b85bbb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -280,6 +280,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
> +static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
> static inline void kvm_arch_hardware_unsetup(void) {}
> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index eb2cf49..b0d3820 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1130,6 +1130,7 @@ endmenu
> config ARM64_SVE
> bool "ARM Scalable Vector Extension support"
> default y
> + depends on !KVM || ARM64_VHE
> help
> The Scalable Vector Extension (SVE) is an extension to the AArch64
> execution state which complements and extends the SIMD functionality
> @@ -1155,6 +1156,12 @@ config ARM64_SVE
> booting the kernel. If unsure and you are not observing these
> symptoms, you should assume that it is safe to say Y.
>
> + CPUs that support SVE are architecturally required to support the
> + Virtualization Host Extensions (VHE), so the kernel makes no
> + provision for supporting SVE alongside KVM without VHE enabled.
> + Thus, you will need to enable CONFIG_ARM64_VHE if you want to support
> + KVM in the same kernel image.
> +
> config ARM64_MODULE_PLTS
> bool
> select HAVE_MOD_ARCH_SPECIFIC
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index b3fe730..06d5a61 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -405,6 +405,19 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
> kvm_call_hyp(__kvm_set_tpidr_el2, tpidr_el2);
> }
>
> +static inline bool kvm_arch_check_sve_has_vhe(void)
> +{
> + /*
> + * The Arm architecture specifies that imlpementation of SVE
nit: implementation
> + * requires VHE also to be implemented. The KVM code for arm64
> + * relies on this when SVE is present:
> + */
> + if (system_supports_sve())
> + return has_vhe();
> + else
> + return true;
> +}
> +
> static inline void kvm_arch_hardware_unsetup(void) {}
> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 365933a..dc6ecfa 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -59,7 +59,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
> */
> void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> {
> - BUG_ON(system_supports_sve());
> BUG_ON(!current->mm);
>
> vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 118f300..a6a8c7d 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -21,6 +21,7 @@
>
> #include <kvm/arm_psci.h>
>
> +#include <asm/cpufeature.h>
> #include <asm/kvm_asm.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_host.h>
> @@ -28,6 +29,7 @@
> #include <asm/kvm_mmu.h>
> #include <asm/fpsimd.h>
> #include <asm/debug-monitors.h>
> +#include <asm/processor.h>
> #include <asm/thread_info.h>
>
> /* Check whether the FP regs were dirtied while in the host-side run loop: */
> @@ -329,6 +331,8 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
> void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> struct kvm_vcpu *vcpu)
> {
> + struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
> +
> if (has_vhe())
> write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
> cpacr_el1);
> @@ -339,7 +343,21 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> isb();
>
> if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
> - __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
> + /*
> + * In the SVE case, VHE is assumed: it is enforced by
> + * Kconfig and kvm_arch_init().
> + */
> + if (system_supports_sve() &&
> + (vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE)) {
> + struct thread_struct *thread = container_of(
> + host_fpsimd,
> + struct thread_struct, uw.fpsimd_state);
> +
> + sve_save_state(sve_pffr(thread), &host_fpsimd->fpsr);
> + } else {
> + __fpsimd_save_state(host_fpsimd);
> + }
> +
> vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
> }
>
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index bee226c..ce7c6f3 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -16,6 +16,7 @@
> * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> */
>
> +#include <linux/bug.h>
> #include <linux/cpu_pm.h>
> #include <linux/errno.h>
> #include <linux/err.h>
> @@ -41,6 +42,7 @@
> #include <asm/mman.h>
> #include <asm/tlbflush.h>
> #include <asm/cacheflush.h>
> +#include <asm/cpufeature.h>
> #include <asm/virt.h>
> #include <asm/kvm_arm.h>
> #include <asm/kvm_asm.h>
> @@ -1574,6 +1576,11 @@ int kvm_arch_init(void *opaque)
> return -ENODEV;
> }
>
> + if (!kvm_arch_check_sve_has_vhe()) {
> + kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
> + return -ENODEV;
> + }
> +
> for_each_online_cpu(cpu) {
> smp_call_function_single(cpu, check_kvm_target_cpu, &ret, 1);
> if (ret < 0) {
> --
> 2.1.4
>
>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
^ permalink raw reply
* [rjarzmik:work/dma_slave_map 5/13] drivers/mtd/nand/raw/marvell_nand.c:2628:52: sparse: incorrect type in argument 1 (different base types)
From: kbuild test robot @ 2018-05-24 9:07 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://github.com/rjarzmik/linux work/dma_slave_map
head: d18138ed977685f67fb18efcb5b5ef7084e3f1ae
commit: 38686a4ca1306465bae33d821a1128288f68ec6b [5/13] mtd: rawnand: marvell: remove the dmaengine compat need
reproduce:
# apt-get install sparse
git checkout 38686a4ca1306465bae33d821a1128288f68ec6b
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/mtd/nand/raw/marvell_nand.c:752:32: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:861:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:861:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:907:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:907:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:1572:41: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2238:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2238:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2240:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2240:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2262:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2263:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2264:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2265:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2266:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2267:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2268:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2271:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2272:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2273:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2277:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2279:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2282:25: sparse: expression using sizeof(void)
>> drivers/mtd/nand/raw/marvell_nand.c:2628:52: sparse: incorrect type in argument 1 (different base types) @@ expected struct device *dev @@ got sstruct device *dev @@
drivers/mtd/nand/raw/marvell_nand.c:2628:52: expected struct device *dev
drivers/mtd/nand/raw/marvell_nand.c:2628:52: got struct device **<noident>
drivers/mtd/nand/raw/marvell_nand.c: In function 'marvell_nfc_init_dma':
drivers/mtd/nand/raw/marvell_nand.c:2628:44: error: passing argument 1 of 'dma_request_slave_channel' from incompatible pointer type [-Werror=incompatible-pointer-types]
nfc->dma_chan = dma_request_slave_channel(&nfc->dev, "data");
^
In file included from drivers/mtd/nand/raw/marvell_nand.c:21:0:
include/linux/dmaengine.h:1315:18: note: expected 'struct device *' but argument is of type 'struct device **'
struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +2628 drivers/mtd/nand/raw/marvell_nand.c
2608
2609 static int marvell_nfc_init_dma(struct marvell_nfc *nfc)
2610 {
2611 struct platform_device *pdev = container_of(nfc->dev,
2612 struct platform_device,
2613 dev);
2614 struct dma_slave_config config = {};
2615 struct resource *r;
2616 int ret;
2617
2618 if (!IS_ENABLED(CONFIG_PXA_DMA)) {
2619 dev_warn(nfc->dev,
2620 "DMA not enabled in configuration\n");
2621 return -ENOTSUPP;
2622 }
2623
2624 ret = dma_set_mask_and_coherent(nfc->dev, DMA_BIT_MASK(32));
2625 if (ret)
2626 return ret;
2627
> 2628 nfc->dma_chan = dma_request_slave_channel(&nfc->dev, "data");
2629 if (!nfc->dma_chan) {
2630 dev_err(nfc->dev,
2631 "Unable to request data DMA channel\n");
2632 return -ENODEV;
2633 }
2634
2635 r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
2636 if (!r)
2637 return -ENXIO;
2638
2639 config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
2640 config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
2641 config.src_addr = r->start + NDDB;
2642 config.dst_addr = r->start + NDDB;
2643 config.src_maxburst = 32;
2644 config.dst_maxburst = 32;
2645 ret = dmaengine_slave_config(nfc->dma_chan, &config);
2646 if (ret < 0) {
2647 dev_err(nfc->dev, "Failed to configure DMA channel\n");
2648 return ret;
2649 }
2650
2651 /*
2652 * DMA must act on length multiple of 32 and this length may be
2653 * bigger than the destination buffer. Use this buffer instead
2654 * for DMA transfers and then copy the desired amount of data to
2655 * the provided buffer.
2656 */
2657 nfc->dma_buf = kmalloc(MAX_CHUNK_SIZE, GFP_KERNEL | GFP_DMA);
2658 if (!nfc->dma_buf)
2659 return -ENOMEM;
2660
2661 nfc->use_dma = true;
2662
2663 return 0;
2664 }
2665
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH v10 06/18] arm64: fpsimd: Generalise context saving for non-task contexts
From: Dave Martin @ 2018-05-24 9:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87in7et9gw.fsf@linaro.org>
On Wed, May 23, 2018 at 09:15:11PM +0100, Alex Benn?e wrote:
>
> Dave Martin <Dave.Martin@arm.com> writes:
>
> > In preparation for allowing non-task (i.e., KVM vcpu) FPSIMD
> > contexts to be handled by the fpsimd common code, this patch adapts
> > task_fpsimd_save() to save back the currently loaded context,
> > removing the explicit dependency on current.
> >
> > The relevant storage to write back to in memory is now found by
> > examining the fpsimd_last_state percpu struct.
> >
> > fpsimd_save() does nothing unless TIF_FOREIGN_FPSTATE is clear, and
> > fpsimd_last_state is updated under local_bh_disable() or
> > local_irq_disable() everywhere that TIF_FOREIGN_FPSTATE is cleared:
> > thus, fpsimd_save() will write back to the correct storage for the
> > loaded context.
> >
> > No functional change.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> > arch/arm64/kernel/fpsimd.c | 25 +++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 9d85373..3aa100a 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -270,13 +270,15 @@ static void task_fpsimd_load(void)
> > }
> >
> > /*
> > - * Ensure current's FPSIMD/SVE storage in thread_struct is up to date
> > - * with respect to the CPU registers.
> > + * Ensure FPSIMD/SVE storage in memory for the loaded context is up to
> > + * date with respect to the CPU registers.
> > *
> > * Softirqs (and preemption) must be disabled.
> > */
> > -static void task_fpsimd_save(void)
> > +static void fpsimd_save(void)
> > {
> > + struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
> > +
>
> I thought I was missing something but the only write I saw of this was:
>
> __this_cpu_write(fpsimd_last_state.st, NULL);
>
> which implied to me it is possible to have an invalid de-reference. I
> did figure it out eventually as fpsimd_bind_state_to_cpu uses a more
> indirect this_cpu_ptr idiom for tweaking this. I guess a reference to
> fpsimd_bind_[task|state]_to_cpu in the comment would have helped my
> confusion.
How about:
static void fpsimd_save(void)
{
struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
+ /* set by fpsimd_bind_to_cpu() */
WARN_ON(!in_softirq() && !irqs_disabled());
> Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Thanks
---Dave
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: A.s. Dong @ 2018-05-24 9:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFrZHB2ti75zQav_SSXndorV38TcXMCoRBvDzdE5SS-WhA@mail.gmail.com>
Hi Ulf,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> Sent: Thursday, May 24, 2018 5:00 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>
> On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote:
> > Hi Ulf,
> >
> > Thanks for the review.
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> >> Sent: Wednesday, May 9, 2018 3:16 AM
> >> To: A.s. Dong <aisheng.dong@nxp.com>
> >> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> >> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo
> >> <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> >> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki
> >> <rjw@rjwysocki.net>; Kevin Hilman <khilman@kernel.org>; Linux PM
> >> <linux-pm@vger.kernel.org>
> >> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
> >>
> >> [...]
> >>
> >> > +
> >> > +static int __init imx_sc_init_pm_domains(void) {
> >> > + struct generic_pm_domain *pd;
> >> > + struct device_node *np;
> >> > + sc_err_t sci_err;
> >> > +
> >> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> >> > + return 0;
> >> > +
> >> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
> >> > + if (sci_err != SC_ERR_NONE) {
> >> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
> >> > + return -ENODEV;
> >> > + }
> >> > +
> >> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
> >> > + pd = imx_sc_pm_add_one_domain(np, NULL);
> >> > + if (!IS_ERR(pd))
> >> > + imx_sc_pm_add_subdomains(np, pd);
> >> > + }
> >>
> >> Perhaps using of_genpd_add_subdomain() may help here and possibly
> >> could avoid some open coding!?
> >>
> >
> > Thanks for the suggestion. I thought of it a lot and the result is
> > that I'm not sure If it's quite suitable for i.MX cases. Currently
> > seems there's only one user, Samsung, in kernel using that API which
> > takes two struct of_phandle_args as arguments, parent and child. It looks
> needs special handling in code before using it, e.g.
> > register both parent and child domain first, which is somehow not like
> > i.MX flow of registration. It looks like to me not see much benefits
> > to enforce a big change to switch to it. Or am I missed anything?
>
> It's up to you. The idea was to make the code easier and a bit future proof.
> Perhaps that isn't suitable here.
>
> In any case, if you have a device node and all power-domain providers are
> listed below it, one can call for_each_child_of_node() and search for
> provider nodes hierarchically. I thought that is kind of nice.
>
> Something along the lines of what I done in psci_dt_set_genpd_topology(),
> from a patch I posted a while ago:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> chwork.kernel.org%2Fpatch%2F10338399%2F&data=02%7C01%7Caisheng.do
> ng%40nxp.com%7C0ce43cb8f66740a0f95908d5c154c330%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C1%7C636627492163349262&sdata=sk9U7c5VQ
> WKJN1UMOMUIXZh0nnpiWGX7ildx2Zzo4dc%3D&reserved=0
>
Thanks for the info. Will have a look at it.
> [...]
>
> If you decide to sticking to the existing way, anyway feel free to add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>
Thanks
Regards
Dong Aisheng
> Kind regards
> Uffe
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: Ulf Hansson @ 2018-05-24 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM0PR04MB42114331A1E75B3B90B07E56806A0@AM0PR04MB4211.eurprd04.prod.outlook.com>
On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote:
> Hi Ulf,
>
> Thanks for the review.
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
>> Sent: Wednesday, May 9, 2018 3:16 AM
>> To: A.s. Dong <aisheng.dong@nxp.com>
>> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
>> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
>> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
>> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
>> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
>> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>>
>> [...]
>>
>> > +
>> > +static int __init imx_sc_init_pm_domains(void) {
>> > + struct generic_pm_domain *pd;
>> > + struct device_node *np;
>> > + sc_err_t sci_err;
>> > +
>> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
>> > + return 0;
>> > +
>> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
>> > + if (sci_err != SC_ERR_NONE) {
>> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
>> > + return -ENODEV;
>> > + }
>> > +
>> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
>> > + pd = imx_sc_pm_add_one_domain(np, NULL);
>> > + if (!IS_ERR(pd))
>> > + imx_sc_pm_add_subdomains(np, pd);
>> > + }
>>
>> Perhaps using of_genpd_add_subdomain() may help here and possibly could
>> avoid some open coding!?
>>
>
> Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure
> If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung,
> in kernel using that API which takes two struct of_phandle_args as arguments,
> parent and child. It looks needs special handling in code before using it, e.g.
> register both parent and child domain first, which is somehow not like i.MX flow
> of registration. It looks like to me not see much benefits to enforce a big
> change to switch to it. Or am I missed anything?
It's up to you. The idea was to make the code easier and a bit future
proof. Perhaps that isn't suitable here.
In any case, if you have a device node and all power-domain providers
are listed below it, one can call for_each_child_of_node() and search
for provider nodes hierarchically. I thought that is kind of nice.
Something along the lines of what I done in
psci_dt_set_genpd_topology(), from a patch I posted a while ago:
https://patchwork.kernel.org/patch/10338399/
[...]
If you decide to sticking to the existing way, anyway feel free to add:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply
* [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
From: A.s. Dong @ 2018-05-24 8:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503124030.jkc7oox2vyjblk2y@pengutronix.de>
Hi Sascha,
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Thursday, May 3, 2018 8:41 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio Estevam
> <fabio.estevam@nxp.com>; shawnguo at kernel.org
> Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
>
> On Thu, May 03, 2018 at 12:29:40PM +0000, A.s. Dong wrote:
> > > -----Original Message-----
> > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > Sent: Thursday, May 3, 2018 7:06 PM
> > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > >
> > > On Wed, May 02, 2018 at 06:40:03PM +0000, A.s. Dong wrote:
> > > > > -----Original Message-----
> > > > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > > > Sent: Wednesday, May 2, 2018 6:04 PM
> > > > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > > > >
> > > > > On Sat, Apr 28, 2018 at 02:46:16AM +0800, Dong Aisheng wrote:
> > > > > > +/* Initialization of the MU code. */ int __init
> > > > > > +imx8_scu_init(void) {
> > > > > > + struct device_node *np, *mu_np;
> > > > > > + struct resource mu_res;
> > > > > > + sc_err_t sci_err;
> > > > > > + int ret;
> > > > > > +
> > > > > > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> > > > > > + return 0;
> > > > > > +
> > > > > > + np = of_find_compatible_node(NULL, NULL, "nxp,imx8qxp-
> scu");
> > > > > > + if (!np)
> > > > > > + return -ENODEV;
> > > > > > +
> > > > > > + mu_np = of_parse_phandle(np, "fsl,mu", 0);
> > > > > > + if (WARN_ON(!mu_np))
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + ret = of_address_to_resource(mu_np, 0, &mu_res);
> > > > > > + if (WARN_ON(ret))
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + /* we use mu physical address as IPC communication channel
> ID */
> > > > > > + sci_err = sc_ipc_open(&scu_ipc_handle,
> (sc_ipc_id_t)(mu_res.start));
> > > > > > + if (sci_err != SC_ERR_NONE) {
> > > > > > + pr_err("Cannot open MU channel to SCU\n");
> > > > > > + return sci_err;
> > > > > > + };
> > > > >
> > > > > Introducing private error codes always has the risk of just
> > > > > forwarding them as errno codes as done here.
> > > > >
> > > >
> > > > Yes, you're right.
> > > > We probably could do the same as scpi_to_linux_errno in
> > > > drivers/firmware/arm_scpi.c.
> > > > However, may can't fix the issue permanently.
> > > >
> > > > > > +
> > > > > > + of_node_put(mu_np);
> > > > > > +
> > > > > > + pr_info("NXP i.MX SCU Initialized (scu_ipc %u)\n",
> > > > > > +scu_ipc_handle);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +early_initcall(imx8_scu_init);
> > > > >
> > > > > This code shows that the separate 'scu' device node shouldn't exist.
> > > > > It is only used as a stepping stone to find the 'mu' node.
> > > > > Instead of providing a proper driver for the 'mu' node the scu
> > > > > code registers it
> > > with its physical address.
> > > > > I don't think it makes sense to separate mu and scu code in this way.
> > > > >
> > > >
> > > > I agree that may not look quite well.
> > > > It mainly because we want to use the MU physical address as a MU ID.
> > > > (can't use virtual address as sc_ipc_id_t is u32 defined by SCU firmware.
> > > >
> > > > If you have any better suggestion please let me know.
> > >
> > > What I'm suggesting is a single node:
> > >
> > > scu at 5d1b0000 {
> > > compatible = "fsl,imx8qxp-scu";
> > > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > > };
> > >
> > > Attach your code to this one, without any further separation between
> > > mu and scu code.
> > >
> >
> > A bit confused. You're suggesting a single node here without mu or
> > mailbox node phandle in it? Then how SCU use MU?
>
> ioremap the address and mu_receive_msg()/mu_send_msg() on it, just like
> you do already.
>
> >
> > > We discussed this internally and came to the conclusion that the SCU
> > > is not a generic user of a MU. The MU is designed to work together
> > > with a piece of SRAM to form a mailbox system. Instead of working as
> > > designed the SCU morses the messages through the doorbell (what the
> > > MU really is). For anything that uses the MU in the way it is
> > > designed I would suggest using the mailbox interface from
> > > drivers/mailbox/ along with the binding from
> Documentation/devicetree/bindings/mailbox/mailbox.txt.
> > >
> > > In the way I suggest there is no need for any MU ID.
> > >
> >
> > So you're suggesting switch to use mailbox instead of private MU
> > library function calls?
> > Something like:
> > scu {
> > compatible = "nxp,imx8qxp-scu", "simple-bus";
> > mboxes = <&mailbox 0>;
> > }
> > Then IPC is implemented based on mailbox?
> >
> > As I replied Oleksij Rempel in another mail in this thread, we've
> > tried mailbox but got performance regression issue and need more time
> > to investigate whether it's really quite suitable for i.MX SCU
> > firmware as SCU handling message quite fast in micro seconds. (Mailbox
> > polling method has much more latency than current MU sample polling we
> > used) and we want to avoid the SCU firmware API changes.
>
> I am not suggesting to do mailboxing (using shared memory) for the SCU.
> I am also not suggesting any API update for the SCU communication.
> I am just mentioning that doing mailboxing is the way the MU was originally
> designed for. Looking at the reference manual I see many MUs on the i.MX8.
> I guess most of them are for communication between the different cores on
> the system. For these it's probably worth writing some generic MU driver.
> The way the MU is used for communication with the SCU though is so special
> that it's not worth writing a generic driver, so just integrate the MU access
> functions in the SCU code.
>
I understand it.
One problem of the way you suggested may be that:
If we doing like below, we may lose flexibility to change the MU used
for SCU firmware communication.
scu at 5d1b0000 {
compatible = "fsl,imx8qxp-scu";
reg = <0x0 0x5d1b0000 0x0 0x10000>;
};
And current design is that the system supports multiple MU channels used
by various users at the same time, e.g. SCU, Power Domain, Clock and others.
User can flexibly change it under their nodes: And each MU channel is protected
by their private lock and not affect each others.
e.g.
scu {
compatible = "nxp,imx8qxp-scu", "simple-bus";
fsl,mu = <&lsio_mu0>;
clk: clk {
compatible = "fsl,imx8qxp-clk";
#clock-cells = <1>;
};
iomuxc: iomuxc {
compatible = "fsl,imx8qxp-iomuxc";
fsl,mu = <&lsio_mu3>;
};
imx8qx-pm {
#address-cells = <1>;
#size-cells = <0>;
fsl,mu = <&lsio_mu4>;
.............
}
The default code only uses MU0 which is used by SCU.
The change may affect this design. Any ideas?
Do you think we can keep the current way?
Regards
Dong Aisheng
> Sascha
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C266
> 24a5c38e5488a80b708d5b0f3107b%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C636609480354404338&sdata=XP%2BYdt9I7zURrQun2jRbempLhC
> XrYtMVMb3dEWrZuro%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v10 05/18] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Dave Martin @ 2018-05-24 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180524081220.GJ55598@C02W217FHV2R.local>
On Thu, May 24, 2018 at 10:12:20AM +0200, Christoffer Dall wrote:
> On Wed, May 23, 2018 at 08:35:13PM +0100, Alex Benn?e wrote:
> >
> > Dave Martin <Dave.Martin@arm.com> writes:
> >
> > > To make the lazy FPSIMD context switch trap code easier to hack on,
> > > this patch converts it to C.
> > >
> > > This is not amazingly efficient, but the trap should typically only
> > > be taken once per host context switch.
> > >
> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> > > ---
> > > arch/arm64/kvm/hyp/entry.S | 57 +++++++++++++++++----------------------------
> > > arch/arm64/kvm/hyp/switch.c | 24 +++++++++++++++++++
> > > 2 files changed, 46 insertions(+), 35 deletions(-)
[...]
> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > index d964523..c0796c4 100644
> > > --- a/arch/arm64/kvm/hyp/switch.c
> > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > @@ -318,6 +318,30 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
> > > }
> > > }
> > >
> > > +void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> > > + struct kvm_vcpu *vcpu)
> > > +{
> > > + kvm_cpu_context_t *host_ctxt;
> > > +
> > > + if (has_vhe())
> > > + write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
> > > + cpacr_el1);
> > > + else
> > > + write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
> > > + cptr_el2);
> >
> > Is there no way to do alternative() in C or does it always come down to
> > different inline asms?
> >
>
> has_vhe() should resolve to a static key, and I prefer this over the
> previous alternative construct we had for selecting function calls in C,
> as that resultet in having to follow too many levels of indirection.
I'll defer to Christoffer on that -- I was just following precedent :)
The if (has_vhe()) approach has the benefit of being much more
readable, and the static branch predictor in many CPUs will succeed in
folding a short-range unconditional branch out entirely. There will be
a small increase in I-cache pressure due to the larger inline code
size, but probably not much beyond that.
Cheers
---Dave
^ permalink raw reply
* [PATCH v3] arm64: allwinner: a64: Add Amarula A64-Relic initial support
From: Maxime Ripard @ 2018-05-24 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMty3ZC-N-6WzYg+kYd2+V3wbQdQmvU706Nog_Urd_EUaWnQjQ@mail.gmail.com>
On Wed, May 23, 2018 at 03:57:05PM +0530, Jagan Teki wrote:
> On Wed, May 23, 2018 at 1:48 PM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> > On Wed, May 23, 2018 at 11:44:56AM +0530, Jagan Teki wrote:
> >> On Tue, May 22, 2018 at 8:00 PM, Maxime Ripard
> >> <maxime.ripard@bootlin.com> wrote:
> >> > On Tue, May 22, 2018 at 06:52:28PM +0530, Jagan Teki wrote:
> >> >> Amarula A64-Relic is Allwinner A64 based IoT device, which support
> >> >> - Allwinner A64 Cortex-A53
> >> >> - Mali-400MP2 GPU
> >> >> - AXP803 PMIC
> >> >> - 1GB DDR3 RAM
> >> >> - 8GB eMMC
> >> >> - AP6330 Wifi/BLE
> >> >> - MIPI-DSI
> >> >> - CSI: OV5640 sensor
> >> >> - USB OTG
> >> >
> >> > You claim that this is doing OTG...
> >> >
> >> > [..]
> >> >
> >> >> +&usb_otg {
> >> >> + dr_mode = "peripheral";
> >> >> + status = "okay";
> >> >> +};
> >> >
> >> > ... and yet you're setting it as peripheral...
> >>
> >> Though it claims OTG, board doesn't have any USB ports to operate(not
> >> even Mini-AB) the only way to use the board as peripheral to transfer
> >> images from host.
> >
> > I'm not sure what you mean here. If there's no USB connector, why do
> > you even enable it?
>
> I'm saying there is no host port on board. Board has connector
> header[1] comes with few pins includes USB D+, D-, ID, VBUS, GROUND,
> UART0 TX, RX etc. we have to connect wires to these pins to make
> pluggable USB to host pc. and here USB pins used for transferring data
> from host pc to target board like in peripheral mode.
I'm still unsure how it's relevant. If the wire is plugged on your
board's header, is there anything that prevents using it as a host
assuming you have a proper connector on the other end of your wire?
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/65f20e5f/attachment.sig>
^ permalink raw reply
* [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Maxime Ripard @ 2018-05-24 8:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <218132669.UY7RKz0VPx@jernej-laptop>
On Mon, May 21, 2018 at 07:27:46PM +0200, Jernej ?krabec wrote:
> Hi,
>
> Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a):
> > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote:
> > > If SoC has TCON TOP unit, it has to be configured from TCON, since it
> > > has all information needed. Additionally, if it is TCON TV, it must also
> > > enable bus gate inside TCON TOP unit.
> >
> > Why?
>
> I'll explain my design decision below.
>
> >
> > > Add support for such TCONs.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++
> > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++
> > > 2 files changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev,
> > >
> > > dev_err(dev, "Couldn't get the TCON bus clock\n");
> > > return PTR_ERR(tcon->clk);
> > >
> > > }
> > >
> > > +
> > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) {
> > > + tcon->top_clk = devm_clk_get(dev, "tcon-top");
> > > + if (IS_ERR(tcon->top_clk)) {
> > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n");
> > > + return PTR_ERR(tcon->top_clk);
> > > + }
> > > + clk_prepare_enable(tcon->top_clk);
> > > + }
> > > +
> > >
> > > clk_prepare_enable(tcon->clk);
> > >
> > > if (tcon->quirks->has_channel_0) {
> > >
> > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev,
> > >
> > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon)
> > > {
> > >
> > > clk_disable_unprepare(tcon->clk);
> > >
> > > + clk_disable_unprepare(tcon->top_clk);
> > >
> > > }
> > >
> > > static int sun4i_tcon_init_irq(struct device *dev,
> > >
> > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct
> > > device *master,>
> > > tcon->id = engine->id;
> > > tcon->quirks = of_device_get_match_data(dev);
> > >
> > > + if (tcon->quirks->needs_tcon_top) {
> > > + struct device_node *np;
> > > +
> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0);
> > > + if (np) {
> > > + struct platform_device *pdev;
> > > +
> > > + pdev = of_find_device_by_node(np);
> > > + if (pdev)
> > > + tcon->tcon_top = platform_get_drvdata(pdev);
> > > + of_node_put(np);
> > > +
> > > + if (!tcon->tcon_top)
> > > + return -EPROBE_DEFER;
> > > + }
> > > + }
> > > +
> >
> > I might have missed it, but I've not seen the bindings additions for
> > that property. This shouldn't really be done that way anyway, instead
> > of using a direct phandle, you should be using the of-graph, with the
> > TCON-top sitting where it belongs in the flow of data.
>
> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt-
> bindings: display: sun4i-drm: Add R40 HDMI pipeline".
>
> As why I designed it that way - HW representation could be described that way
> (ASCII art makes sense when fixed width font is used to view it):
>
> / LCD0/LVDS0
> / TCON-LCD0
> | \ MIPI DSI
> mixer0 |
> \ / TCON-LCD1 - LCD1/LVDS1
> TCON-TOP
> / \ TCON-TV0 - TVE0/RGB
> mixer1 | \
> | TCON-TOP - HDMI
> | /
> \ TCON-TV1 - TVE1/RGB
>
> This is a bit simplified, since there is also TVE-TOP, which is responsible
> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/
> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you
> can arbitrarly choose which DAC is responsible for which signal, so there is a
> ton of possible end combinations, but I'm not 100% sure.
>
> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
> suggest more possibilities, although some of them seem wrong, like RGB feeding
> from LCD TCON. That is confirmed to be wrong when checking BSP code.
>
> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for
> pin muxing, although I'm not sure why is that needed at all, since according
> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT
> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC
> lines might be shared between TVE (when it works in RGB mode) and LCD. But
> that is just my guess since I'm not really familiar with RGB and LCD
> interfaces.
>
> I'm really not sure what would be the best representation in OF-graph. Can you
> suggest one?
Rob might disagree on this one, but I don't see anything wrong with
having loops in the graph. If the TCON-TOP can be both the input and
output of the TCONs, then so be it, and have it described that way in
the graph.
The code is already able to filter out nodes that have already been
added to the list of devices we need to wait for in the component
framework, so that should work as well.
And we'd need to describe TVE-TOP as well, even though we don't have a
driver for it yet. That will simplify the backward compatibility later
on.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/768dfd71/attachment.sig>
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Linus Walleij @ 2018-05-24 8:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2044895.BqI5yRtle6@diego>
On Thu, May 24, 2018 at 10:35 AM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Donnerstag, 24. Mai 2018, 10:28:44 CEST schrieb Linus Walleij:
>> On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
>> > So the gpio controller should definitly also be a subnode.
>> >
>> > The gpio in question is called "mute", so I'd think the gpio-syscon driver
>> > should just define a "rockchip,rk3328-gpio-mute" compatible and contain
>> > all the register voodoo in the driver itself and not define it in the dt.
>> >
>> > So it should probably look like
>> >
>> > grf: syscon at ff100000 {
>> >
>> > compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
>> >
>> > [all the other syscon sub-devices]
>> >
>> > gpio_mute: gpio-mute {
>> >
>> > compatible = "rockchip,rk3328-gpio-mute";
>> > gpio-controller;
>> > #gpio-cells = <2>;
>> >
>> > };
>>
>> I'm sceptic.
>>
>> That doesn't sound like "general purpose input output" at all.
>>
>> It sounds like special purpose, for a mute button.
>>
>> Does it use IRQ? I would recommend implementing
>> drivers/input/keyboard/syscon-keys.c in the same vein
>> as drivers/leds/leds-syscon.c so you can avoid indirection
>> through GPIO for no good reason at all.
>
> To quote Levin from the other mail:
> --------
> The "mute" pin is a output only GPIO, which is already supported by
> setting flags in the gpio-syscon
> driver. And yes, this pin has a defined function, but can also be used
> for general purpose operation.
> --------
>
> So to summarize, the documentation calls it "mute", but it is usable as
> a general pin, which is the reason Levin is working on it - because on his
> board this pin is used to switch between two voltages (aka a gpio-regulator)
> for the sdmmc controller [3.3V + 1.8V].
OK then, I was wrong! :)
Go ahead with this, sorry for the fuzz.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 05/15] drm/sun4i: Add TCON TOP driver
From: Maxime Ripard @ 2018-05-24 8:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5965231.6ucpJrPIQ5@jernej-laptop>
Hi,
On Mon, May 21, 2018 at 05:15:15PM +0200, Jernej ?krabec wrote:
> > > + /*
> > > + * Default register values might have some reserved bits set, which
> > > + * prevents TCON TOP from working properly. Set them to 0 here.
> > > + */
> > > + writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > > + writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > > +
> > > + for (i = 0; i < CLK_NUM; i++) {
> > > + const char *parent_name = "bus-tcon-top";
> >
> > I guess retrieving the parent's clock name at runtime would be more
> > flexible.
>
> It is, but will it ever be anything else?
Probably not, but when the complexity is exactly the same (using
__clk_get_name), we'd better use the more appropriate solution. If we
ever need to change that clock name, or to use the driver with an SoC
that wouldn't have the same clock name for whatever reason, it will
just work.
> > > + struct clk_init_data init;
> > > + struct clk_gate *gate;
> > > +
> > > + gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > > + if (!gate) {
> > > + ret = -ENOMEM;
> > > + goto err_disable_clock;
> > > + }
> > > +
> > > + init.name = gates[i].name;
> > > + init.ops = &clk_gate_ops;
> > > + init.flags = CLK_IS_BASIC;
> > > + init.parent_names = &parent_name;
> > > + init.num_parents = 1;
> > > +
> > > + gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > > + gate->bit_idx = gates[i].bit;
> > > + gate->lock = &tcon_top->reg_lock;
> > > + gate->hw.init = &init;
> > > +
> > > + ret = devm_clk_hw_register(dev, &gate->hw);
> > > + if (ret)
> > > + goto err_disable_clock;
> >
> > Isn't it what clk_hw_register_gate is doing?
>
> Almost, but not exactly. My goal was to use devm_* functions, so there is no
> need to do any special cleanup.
Is it the only difference? If so, you can just create a
devm_clk_hw_register gate.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/6ef507c2/attachment-0001.sig>
^ permalink raw reply
* [PATCH v2 2/4] drm/mediatek: Add support for mediatek SOC MT2712
From: Stu Hsieh @ 2018-05-24 8:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527125197.6406.10.camel@mtksdaap41>
Hi CK,
On Thu, 2018-05-24 at 09:26 +0800, CK Hu wrote:
> Hi, Stu:
>
> On Wed, 2018-05-23 at 17:28 +0800, Stu Hsieh wrote:
> > On Wed, 2018-05-23 at 13:23 +0800, CK Hu wrote:
> > > Hi, Stu:
> > >
> > > I've some inline comment.
> > >
> > > On Wed, 2018-05-23 at 10:25 +0800, Stu Hsieh wrote:
> > > > This patch add support for the Mediatek MT2712 DISP subsystem.
> > > > There are two OVL engine and three disp output in MT2712.
> > > >
> > > > Signed-off-by: Stu Hsieh <stu.hsieh@mediatek.com>
> > > > ---
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++++++++++++++++++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 +++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 7 ++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 +++++++++++++++++++++++++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 ++--
> > > > 5 files changed, 108 insertions(+), 11 deletions(-)
> > > >
> > > > +#define MT2712_MUTEX_MOD_DISP_AAL0 BIT(20)
> > > > +#define MT2712_MUTEX_MOD_DISP_UFOE BIT(22)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM0 BIT(23)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM1 BIT(24)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM2 BIT(10)
> > > > +#define MT2712_MUTEX_MOD_DISP_OD0 BIT(25)
> > > > +/* modules more than 32, add BIT(31) when using DISP_REG_MUTEX_MOD2 bit */
> > > > +#define MT2712_MUTEX_MOD2_DISP_AAL1 (BIT(1) | BIT(31))
> > >
> > > I think a better definition is
> > >
> > > #define MT2712_MUTEX_MOD2_DISP_AAL1 BIT(33)
> > >
> > > when you need to access this register,
> > >
> > > if (ddp->mutex_mod[id] < BIT(32)) {
> > > offset = DISP_REG_MUTEX_MOD(mutex->id);
> > > reg = readl_relaxed(ddp->regs + offset);
> > > reg |= ddp->mutex_mod[id];
> > > writel_relaxed(reg, ddp->regs + offset);
> > > } else {
> > > offset = DISP_REG_MUTEX_MOD2(mutex->id);
> > > reg = readl_relaxed(ddp->regs + offset);
> > > reg |= (ddp->mutex_mod[id] >> 32);
> > > writel_relaxed(reg, ddp->regs + offset);
> > > }
> > >
> > > because DISP_REG_MUTEX_MOD BIT(31) could be used for some module.
> >
> > This modification is workable, but result some build warning like
> > following:
> > 1. #define BIT(nr) (1UL << (nr)) in include/linux/bitops.h
> > 2. [DDP_COMPONENT_AAL1] = MT2712_MUTEX_MOD2_DISP_AAL1,
> > => we need to modify the definition about "static const unsigned int
> > mt2712_mutex_mod"
> >
>
> Currently, mutex_mod is a bitwise definition. I think it could be
> changed to index definition such as
>
>
> #define MT2712_MUTEX_MOD_DISP_PWM2 10
> #define MT2712_MUTEX_MOD_DISP_OD0 25
> #define MT2712_MUTEX_MOD2_DISP_AAL1 33
>
> when you need to access this register,
>
> if (ddp->mutex_mod[id] < 32) {
> offset = DISP_REG_MUTEX_MOD(mutex->id);
> reg = readl_relaxed(ddp->regs + offset);
> reg |= 1 << ddp->mutex_mod[id];
> writel_relaxed(reg, ddp->regs + offset);
> } else {
> offset = DISP_REG_MUTEX_MOD2(mutex->id);
> reg = readl_relaxed(ddp->regs + offset);
> reg |= 1 << (ddp->mutex_mod[id] - 32);
> writel_relaxed(reg, ddp->regs + offset);
> }
>
> Regards,
> CK
This modification has no build warning.
I would also change the definition about 2701 and 8173 from bitwise to
index.
Regards,
Stu
>
> > > > +#define MT2712_MUTEX_MOD2_DISP_OD1 (BIT(2) | BIT(31))
> > > > +
> > > > #define MT2701_MUTEX_MOD_DISP_OVL BIT(3)
> > > > #define MT2701_MUTEX_MOD_DISP_WDMA BIT(6)
> > > > #define MT2701_MUTEX_MOD_DISP_COLOR BIT(7)
> > > > @@ -74,6 +96,7 @@
> > > >
> > > >
> > >
> > >
> >
> >
>
>
^ permalink raw reply
* [PATCHv2] arm64: Make sure permission updates happen for pmd/pud
From: Peter Robinson @ 2018-05-24 8:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523184346.487-1-labbott@redhat.com>
On Wed, May 23, 2018 at 7:43 PM, Laura Abbott <labbott@redhat.com> wrote:
> Commit 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
> disallowed block mappings for ioremap since that code does not honor
> break-before-make. The same APIs are also used for permission updating
> though and the extra checks prevent the permission updates from happening,
> even though this should be permitted. This results in read-only permissions
> not being fully applied. Visibly, this can occasionaly be seen as a failure
> on the built in rodata test when the test data ends up in a section or
> as an odd RW gap on the page table dump. Fix this by using
> pgattr_change_is_safe instead of p*d_present for determining if the
> change is permitted.
>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Fixes: 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
> Signed-off-by: Laura Abbott <labbott@redhat.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>
Tested on Macbin, mustang, pine64, RPi3+ and db410c and fixes the issue I saw.
> ---
> v2: Switch to using pgattr_change_is_safe per suggestion of Will
> ---
> arch/arm64/mm/mmu.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2dbb2c9f1ec1..493ff75670ff 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -933,13 +933,15 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> + pud_t new_pud = pfn_pud(__phys_to_pfn(phys), sect_prot);
>
> - /* ioremap_page_range doesn't honour BBM */
> - if (pud_present(READ_ONCE(*pudp)))
> + /* Only allow permission changes for now */
> + if (!pgattr_change_is_safe(READ_ONCE(pud_val(*pudp)),
> + pud_val(new_pud)))
> return 0;
>
> BUG_ON(phys & ~PUD_MASK);
> - set_pud(pudp, pfn_pud(__phys_to_pfn(phys), sect_prot));
> + set_pud(pudp, new_pud);
> return 1;
> }
>
> @@ -947,13 +949,15 @@ int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PMD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> + pmd_t new_pmd = pfn_pmd(__phys_to_pfn(phys), sect_prot);
>
> - /* ioremap_page_range doesn't honour BBM */
> - if (pmd_present(READ_ONCE(*pmdp)))
> + /* Only allow permission changes for now */
> + if (!pgattr_change_is_safe(READ_ONCE(pmd_val(*pmdp)),
> + pmd_val(new_pmd)))
> return 0;
>
> BUG_ON(phys & ~PMD_MASK);
> - set_pmd(pmdp, pfn_pmd(__phys_to_pfn(phys), sect_prot));
> + set_pmd(pmdp, new_pmd);
> return 1;
> }
>
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: A.s. Dong @ 2018-05-24 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFowdpxwNMmDjdD3qcqnAcQZA9-MYWnTcZL-oSbwg8WYMg@mail.gmail.com>
Hi Ulf,
Thanks for the review.
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> Sent: Wednesday, May 9, 2018 3:16 AM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>
> [...]
>
> > +
> > +static int __init imx_sc_init_pm_domains(void) {
> > + struct generic_pm_domain *pd;
> > + struct device_node *np;
> > + sc_err_t sci_err;
> > +
> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> > + return 0;
> > +
> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
> > + if (sci_err != SC_ERR_NONE) {
> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
> > + return -ENODEV;
> > + }
> > +
> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
> > + pd = imx_sc_pm_add_one_domain(np, NULL);
> > + if (!IS_ERR(pd))
> > + imx_sc_pm_add_subdomains(np, pd);
> > + }
>
> Perhaps using of_genpd_add_subdomain() may help here and possibly could
> avoid some open coding!?
>
Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure
If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung,
in kernel using that API which takes two struct of_phandle_args as arguments,
parent and child. It looks needs special handling in code before using it, e.g.
register both parent and child domain first, which is somehow not like i.MX flow
of registration. It looks like to me not see much benefits to enforce a big
change to switch to it. Or am I missed anything?
If you have better idea please let me know.
> > +
> > + return 0;
> > +}
> > +early_initcall(imx_sc_init_pm_domains);
>
> Otherwise this looks good to me!
>
Thanks
Regards
Dong Aisheng
> Kind regards
> Uffe
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Heiko Stübner @ 2018-05-24 8:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYCdNZ=ASw1uz4zKXC4AMd1EGbvG_G5K5cwSpH5eGPgKA@mail.gmail.com>
Hi Linus,
Am Donnerstag, 24. Mai 2018, 10:28:44 CEST schrieb Linus Walleij:
> On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
> > So the gpio controller should definitly also be a subnode.
> >
> > The gpio in question is called "mute", so I'd think the gpio-syscon driver
> > should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> > all the register voodoo in the driver itself and not define it in the dt.
> >
> > So it should probably look like
> >
> > grf: syscon at ff100000 {
> >
> > compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
> >
> > [all the other syscon sub-devices]
> >
> > gpio_mute: gpio-mute {
> >
> > compatible = "rockchip,rk3328-gpio-mute";
> > gpio-controller;
> > #gpio-cells = <2>;
> >
> > };
>
> I'm sceptic.
>
> That doesn't sound like "general purpose input output" at all.
>
> It sounds like special purpose, for a mute button.
>
> Does it use IRQ? I would recommend implementing
> drivers/input/keyboard/syscon-keys.c in the same vein
> as drivers/leds/leds-syscon.c so you can avoid indirection
> through GPIO for no good reason at all.
To quote Levin from the other mail:
--------
The "mute" pin is a output only GPIO, which is already supported by
setting flags in the gpio-syscon
driver. And yes, this pin has a defined function, but can also be used
for general purpose operation.
--------
So to summarize, the documentation calls it "mute", but it is usable as
a general pin, which is the reason Levin is working on it - because on his
board this pin is used to switch between two voltages (aka a gpio-regulator)
for the sdmmc controller [3.3V + 1.8V].
Available pin settings are output-enable + of course the high/low setting
and I think I remember there is even a pull setting for it in the GRF
somewhere - but my memory might be fuzzy here.
Heiko
^ permalink raw reply
* [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks
From: Christoffer Dall @ 2018-05-24 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523150337.GO13470@e103592.cambridge.arm.com>
On Wed, May 23, 2018 at 04:03:37PM +0100, Dave Martin wrote:
> On Wed, May 23, 2018 at 03:56:57PM +0100, Catalin Marinas wrote:
> > On Wed, May 23, 2018 at 02:31:59PM +0100, Dave P Martin wrote:
> > > On Wed, May 23, 2018 at 01:48:12PM +0200, Christoffer Dall wrote:
> > > > On Tue, May 22, 2018 at 05:05:08PM +0100, Dave Martin wrote:
> > > > > This is true by construction however: TIF_FOREIGN_FPSTATE is never
> > > > > cleared except when returning to userspace or returning from a
> > > > > signal: thus, for a true kernel thread no FPSIMD context is ever
> > > > > loaded, TIF_FOREIGN_FPSTATE will remain set and no context will
> > > > > ever be saved.
> > > >
> > > > I don't understand this construction proof; from looking at the patch
> > > > below it is not obvious to me why fpsimd_thread_switch() can never have
> > > > !wrong_task && !wrong_cpu and therefore clear TIF_FOREIGN_FPSTATE for a
> > > > kernel thread?
> > >
> > > Looking at this again, I think it is poorly worded. This patch aims to
> > > make it true by construction, but it isn't prior to the patch.
> > >
> > > I'm tempted to delete the paragraph: the assertion of both untrue and
> > > not the best way to justify that this patch works.
> > >
> > >
> > > How about:
> > >
> > > -8<-
> > >
> > > The context switch logic already isolates user threads from each other.
> > > This, it is sufficient for isolating user threads from the kernel,
s/This/Thus/ ?
I don't understand what 'it' refers to here?
> > > since the goal either way is to ensure that code executing in userspace
> > > cannot see any FPSIMD state except its own. Thus, there is no special
> > > property of kernel threads that we care about except that it is
> > > pointless to save or load FPSIMD register state for them.
Actually, I'm not really sure what this paragraph is getting at.
> > >
> > > At worst, the removal of all the kernel thread special cases by this
> > > patch would thus spuriously load and save state for kernel threads when
> > > unnecessary.
> > >
> > > But the context switch logic is already deliberately optimised to defer
> > > reloads of the regs until ret_to_user (or sigreturn as a special case),
> > > which kernel threads by definition never reach.
> > >
> > > ->8-
> >
> > The "at worst" paragraph makes it look like it could happen (at least
> > until you reach the last paragraph). Maybe you can just say that
> > wrong_task and wrong_cpu (with the fpsimd_cpu = NR_CPUS addition) are
> > always true for kernel threads. You should probably mention this in a
> > comment in the code as well.
>
> What if I just delete the second paragraph, and remove the "But" from
> the start of the third, and append:
>
> "As a result, the wrong_task and wrong_cpu tests in
> fpsimd_thread_switch() will always yield false for kernel threads."
>
> ...with a similar comment in the code?
...with a risk of being a bit over-pedantic and annoying, may I suggest
the following complete commit text:
------8<------
Currently the FPSIMD handling code uses the condition task->mm ==
NULL as a hint that task has no FPSIMD register context.
The ->mm check is only there to filter out tasks that cannot
possibly have FPSIMD context loaded, for optimisation purposes.
However, TIF_FOREIGN_FPSTATE must always be checked anyway before
saving FPSIMD context back to memory. For this reason, the ->mm
checks are not useful, providing that that TIF_FOREIGN_FPSTATE is
maintained properly for kernel threads.
FPSIMD context is never preserved for kernel threads across a context
switch and therefore TIF_FOREIGN_FPSTATE should always be true for
kernel threads. This is indeed the case, as the wrong_task and
wrong_cpu tests in fpsimd_thread_switch() will always yield false for
kernel threads.
Further, the context switch logic is already deliberately optimised to
defer reloads of the FPSIMD context until ret_to_user (or sigreturn as a
special case), which kernel threads by definition never reach, and
therefore this change introduces no additional work in the critical
path.
This patch removes the redundant checks and special-case code.
------8<------
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] PM / runtime: Drop usage count for suppliers at device link removal
From: Ulf Hansson @ 2018-05-24 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In the case consumer device is runtime resumed, while the link to the
supplier is removed, the earlier call to pm_runtime_get_sync() made from
rpm_get_suppliers() does not get properly balanced with a corresponding
call to pm_runtime_put(). This leads to that suppliers remains to be
runtime resumed forever, while they don't need to.
Let's fix the behaviour by calling rpm_put_suppliers() when dropping a
device link. Not that, since rpm_put_suppliers() checks the
link->rpm_active flag, we can correctly avoid to call pm_runtime_put() in
cases when we shouldn't.
Reported-by: Todor Tomov <todor.tomov@linaro.org>
Fixes: 21d5c57b3726 ("PM / runtime: Use device links")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Rafael, I am not sure if this is safe from locking point of view. The device
link write lock has been taken when pm_runtime_drop_link() is called, hence I
assume calling rpm_put_suppliers() should be fine!? If not, can you please
advise how to change?
Kind regards
Uffe
---
drivers/base/power/runtime.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 8bef3cb..beb85c3 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1607,6 +1607,8 @@ void pm_runtime_new_link(struct device *dev)
void pm_runtime_drop_link(struct device *dev)
{
+ rpm_put_suppliers(dev);
+
spin_lock_irq(&dev->power.lock);
WARN_ON(dev->power.links_count == 0);
dev->power.links_count--;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] pinctrl: at91-pio4: add missing of_node_put
From: Linus Walleij @ 2018-05-24 8:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527102436-13447-2-git-send-email-Julia.Lawall@lip6.fr>
On Wed, May 23, 2018 at 9:07 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> The device node iterators perform an of_node_get on each iteration, so a
> jump out of the loop requires an of_node_put.
>
> The semantic patch that fixes this problem is as follows
> (http://coccinelle.lip6.fr):
>
> // <smpl>
> @@
> expression root,e;
> local idexpression child;
> iterator name for_each_child_of_node;
> @@
>
> for_each_child_of_node(root, child) {
> ... when != of_node_put(child)
> when != e = child
> + of_node_put(child);
> ? break;
> ...
> }
> ... when != child
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Patch applied with Ludovic's ACK!
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Linus Walleij @ 2018-05-24 8:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1685755.J6GI985WX3@diego>
On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
> So the gpio controller should definitly also be a subnode.
>
> The gpio in question is called "mute", so I'd think the gpio-syscon driver
> should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> all the register voodoo in the driver itself and not define it in the dt.
>
> So it should probably look like
>
> grf: syscon at ff100000 {
> compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
>
> [all the other syscon sub-devices]
>
> gpio_mute: gpio-mute {
> compatible = "rockchip,rk3328-gpio-mute";
> gpio-controller;
> #gpio-cells = <2>;
> };
I'm sceptic.
That doesn't sound like "general purpose input output" at all.
It sounds like special purpose, for a mute button.
Does it use IRQ? I would recommend implementing
drivers/input/keyboard/syscon-keys.c in the same vein
as drivers/leds/leds-syscon.c so you can avoid indirection
through GPIO for no good reason at all.
I already have other good uses for such a generic
input driver.
Yours,
Linus Walleij
^ permalink raw reply
* [linux-sunxi] Re: [PATCH 12/15] drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver
From: Maxime Ripard @ 2018-05-24 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4018914.loHx9iTxYh@jernej-laptop>
On Mon, May 21, 2018 at 05:02:23PM +0200, Jernej ?krabec wrote:
> Hi,
>
> Dne ponedeljek, 21. maj 2018 ob 10:12:53 CEST je Maxime Ripard napisal(a):
> > On Sat, May 19, 2018 at 08:31:24PM +0200, Jernej Skrabec wrote:
> > > Expand HDMI PHY clock driver to support second clock parent.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 6 +-
> > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 29 ++++++-
> > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++++++++++++++------
> > > 3 files changed, 98 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 801a17222762..aadbe0a10b0c
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > @@ -98,7 +98,8 @@
> > >
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)
> > >
> > > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26)
> > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
> > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26
> > >
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20)
> > >
> > > @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
> > > *hdmi);
> > >
> > > void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
> > > const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
> > >
> > > -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev);
> > > +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev,
> > > + bool second_parent);
> > >
> > > #endif /* _SUN8I_DW_HDMI_H_ */
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index deba47ed69d8..7a911f0a3ae3
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
> > > *hdmi,>
> > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
> > >
> > > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
> > >
> > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
> > > + /*
> > > + * NOTE: We have to be careful not to overwrite PHY parent
> > > + * clock selection bit and clock divider.
> > > + */
> > > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> > > + ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> > > + pll_cfg1_init);
> >
> > It feels like it belongs in a separate patch.
>
> Why? clk_set_rate() on HDMI PHY clock is called before this function, so
> SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL bit is already set to correct value. Original
> code doesn't take this into account and sets it to 0 here in all cases, which
> obviously is not right.
>
> If you insist on splitting this in new patch, it has to be applied before
> clock changes. It would also need to include "reset PLL clock configuration"
> chunk (next change in this patch), otherwise other SoCs with only one parent
> could get 1 there, which is obviously wrong. Note that I didn't really tested
> if default value of this bit is 0 or 1, but I wouldn't really like to depend
> on default values.
I don't have a strong feeling about this, but to me, the fact that you
don't want to overwrite the muxing bit because the clock might use it
is sensible in itself, disregarding the fact that the clock *will* use
it.
>
> >
> > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
> > >
> > > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
> > > pll_cfg2_init);
> > >
> > > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct
> > > sun8i_hdmi_phy *phy)>
> > > SUN8I_HDMI_PHY_ANA_CFG3_SCLEN |
> > > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN);
> > >
> > > + /* reset PLL clock configuration */
> > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0);
> > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0);
> > > +
> >
> > Ditto,
> >
> > > +
> > > + /*
> > > + * Even though HDMI PHY clock doesn't have enable/disable
> > > + * handlers, we have to enable it. Otherwise it could happen
> > > + * that parent PLL is not enabled by clock framework in a
> > > + * highly unlikely event when parent PLL is used solely for
> > > + * HDMI PHY clock.
> > > + */
> > > + clk_prepare_enable(phy->clk_phy);
> >
> > The implementation of the clock doesn't really matter in our API
> > usage. If we're using a clock, we have to call
> > clk_prepare_enable. That's documented everywhere, and mentionning how
> > the clock is implemented is an abstraction leakage and it's
> > irrelevant. I'd simply remove the comment here.
> >
> > And it should be in a separate patch as well.
>
> OK. Should I add Fixes tag, since current code obviously is not by the specs?
> It could still get in 4.17...
Yep!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/374eaf4a/attachment.sig>
^ permalink raw reply
* [PATCH v4 3/3] arm64: Introduce command line parameter to disable CNP
From: Vladimir Murzin @ 2018-05-24 8:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523171721.ppazcyf576koimhl@armageddon.cambridge.arm.com>
On 23/05/18 18:17, Catalin Marinas wrote:
> On Fri, May 18, 2018 at 11:07:02AM +0100, Vladimir Murzin wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28e..8f59d47 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2636,6 +2636,10 @@
>>
>> noclflush [BUGS=X86] Don't use the CLFLUSH instruction
>>
>> + nocnp [ARM64]
>> + Disable CNP (Common not Private translations)
>> + even if it is supported by processor.
>> +
>> nodelayacct [KNL] Disable per-task delay accounting
>>
>> nodsp [SH] Disable hardware DSP at boot time.
>
> Do we actually have a use-case for this command line option? I'm not
> considering hardware errata as these are handled separately in the
> kernel.
>
Well, I cannot count all cases, yet we might see CnP support advertised
by CPU via ID register (where CPU meant to be part of bL) but not really
doing optimisations in hardware.
Probably, some userspace (benchmarks) might not benefit of CnP; otoh maybe
better way for such case would be user-space asking kernel to {dis,en}able
CnP...
I have no strong opinion on patch, so I'm fine to drop it and come back
when/if we get results from real hardware.
Cheers
Vladimir
^ 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