* Re: [PATCH 00/59] KVM: arm64: ARMv8.3 Nested Virtualization support
From: Alexandru Elisei @ 2019-08-02 10:11 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
Cc: Andre Przywara, Dave P Martin
In-Reply-To: <20190621093843.220980-1-marc.zyngier@arm.com>
Hi,
On 6/21/19 10:37 AM, Marc Zyngier wrote:
> I've taken over the maintenance of this series originally written by
> Jintack and Christoffer. Since then, the series has been substantially
> reworked, new features (and most probably bugs) have been added, and
> the whole thing rebased multiple times. If anything breaks, please
> blame me, and nobody else.
>
> As you can tell, this is quite big. It is also remarkably incomplete
> (we're missing many critical bits for fully emulate EL2), but the idea
> is to start merging things early in order to reduce the maintenance
> headache. What we want to achieve is that with NV disabled, there is
> no performance overhead and no regression. The only thing I intend to
> merge ASAP is the first patch in the series, because it should have
> zero effect and is a reasonable cleanup.
>
> The series is roughly divided in 4 parts: exception handling, memory
> virtualization, interrupts and timers. There are of course some
> dependencies, but you'll hopefully get the gist of it.
>
> For the most courageous of you, I've put out a branch[1] containing this
> and a bit more. Of course, you'll need some userspace. Andre maintains
> a hacked version of kvmtool[1] that takes a --nested option, allowing
> the guest to be started at EL2. You can run the whole stack in the
> Foundation model. Don't be in a hurry ;-).
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/nv-wip-5.2-rc5
> [2] git://linux-arm.org/kvmtool.git nv/nv-wip-5.2-rc5
>
> Andre Przywara (4):
> KVM: arm64: nv: Handle virtual EL2 registers in
> vcpu_read/write_sys_reg()
> KVM: arm64: nv: Save/Restore vEL2 sysregs
> KVM: arm64: nv: Handle traps for timer _EL02 and _EL2 sysregs
> accessors
> KVM: arm64: nv: vgic: Allow userland to set VGIC maintenance IRQ
>
> Christoffer Dall (16):
> KVM: arm64: nv: Introduce nested virtualization VCPU feature
> KVM: arm64: nv: Reset VCPU to EL2 registers if VCPU nested virt is set
> KVM: arm64: nv: Allow userspace to set PSR_MODE_EL2x
> KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state
> KVM: arm64: nv: Handle trapped ERET from virtual EL2
> KVM: arm64: nv: Emulate PSTATE.M for a guest hypervisor
> KVM: arm64: nv: Trap EL1 VM register accesses in virtual EL2
> KVM: arm64: nv: Only toggle cache for virtual EL2 when SCTLR_EL2
> changes
> KVM: arm/arm64: nv: Support multiple nested stage 2 mmu structures
> KVM: arm64: nv: Implement nested Stage-2 page table walk logic
> KVM: arm64: nv: Handle shadow stage 2 page faults
> KVM: arm64: nv: Unmap/flush shadow stage 2 page tables
> KVM: arm64: nv: arch_timer: Support hyp timer emulation
> KVM: arm64: nv: vgic-v3: Take cpu_if pointer directly instead of vcpu
> KVM: arm64: nv: vgic: Emulate the HW bit in software
> KVM: arm64: nv: Add nested GICv3 tracepoints
>
> Dave Martin (1):
> KVM: arm64: Migrate _elx sysreg accessors to msr_s/mrs_s
>
> Jintack Lim (21):
> arm64: Add ARM64_HAS_NESTED_VIRT cpufeature
> KVM: arm64: nv: Add EL2 system registers to vcpu context
> KVM: arm64: nv: Support virtual EL2 exceptions
> KVM: arm64: nv: Inject HVC exceptions to the virtual EL2
> KVM: arm64: nv: Trap SPSR_EL1, ELR_EL1 and VBAR_EL1 from virtual EL2
> KVM: arm64: nv: Trap CPACR_EL1 access in virtual EL2
> KVM: arm64: nv: Set a handler for the system instruction traps
> KVM: arm64: nv: Handle PSCI call via smc from the guest
> KVM: arm64: nv: Respect virtual HCR_EL2.TWX setting
> KVM: arm64: nv: Respect virtual CPTR_EL2.TFP setting
> KVM: arm64: nv: Respect the virtual HCR_EL2.NV bit setting
> KVM: arm64: nv: Respect virtual HCR_EL2.TVM and TRVM settings
> KVM: arm64: nv: Respect the virtual HCR_EL2.NV1 bit setting
> KVM: arm64: nv: Emulate EL12 register accesses from the virtual EL2
> KVM: arm64: nv: Configure HCR_EL2 for nested virtualization
> KVM: arm64: nv: Pretend we only support larger-than-host page sizes
> KVM: arm64: nv: Introduce sys_reg_desc.forward_trap
> KVM: arm64: nv: Rework the system instruction emulation framework
> KVM: arm64: nv: Trap and emulate AT instructions from virtual EL2
> KVM: arm64: nv: Trap and emulate TLBI instructions from virtual EL2
> KVM: arm64: nv: Nested GICv3 Support
>
> Marc Zyngier (17):
> KVM: arm64: Move __load_guest_stage2 to kvm_mmu.h
> KVM: arm64: nv: Reset VMPIDR_EL2 and VPIDR_EL2 to sane values
> KVM: arm64: nv: Handle SPSR_EL2 specially
> KVM: arm64: nv: Refactor vcpu_{read,write}_sys_reg
> KVM: arm64: nv: Don't expose SVE to nested guests
> KVM: arm64: nv: Hide RAS from nested guests
> KVM: arm/arm64: nv: Factor out stage 2 page table data from struct kvm
> KVM: arm64: nv: Move last_vcpu_ran to be per s2 mmu
> KVM: arm64: nv: Don't always start an S2 MMU search from the beginning
> KVM: arm64: nv: Propagate CNTVOFF_EL2 to the virtual EL1 timer
> KVM: arm64: nv: Load timer before the GIC
> KVM: arm64: nv: Implement maintenance interrupt forwarding
> arm64: KVM: nv: Add handling of EL2-specific timer registers
> arm64: KVM: nv: Honor SCTLR_EL2.SPAN on entering vEL2
> arm64: KVM: nv: Handle SCTLR_EL2 RES0/RES1 bits
> arm64: KVM: nv: Restrict S2 RD/WR permissions to match the guest's
> arm64: KVM: nv: Allow userspace to request KVM_ARM_VCPU_NESTED_VIRT
>
> .../admin-guide/kernel-parameters.txt | 4 +
> .../virtual/kvm/devices/arm-vgic-v3.txt | 9 +
> arch/arm/include/asm/kvm_asm.h | 5 +-
> arch/arm/include/asm/kvm_emulate.h | 3 +
> arch/arm/include/asm/kvm_host.h | 31 +-
> arch/arm/include/asm/kvm_hyp.h | 25 +-
> arch/arm/include/asm/kvm_mmu.h | 83 +-
> arch/arm/include/asm/kvm_nested.h | 9 +
> arch/arm/include/uapi/asm/kvm.h | 1 +
> arch/arm/kvm/hyp/switch.c | 11 +-
> arch/arm/kvm/hyp/tlb.c | 13 +-
> arch/arm64/include/asm/cpucaps.h | 3 +-
> arch/arm64/include/asm/esr.h | 4 +-
> arch/arm64/include/asm/kvm_arm.h | 28 +-
> arch/arm64/include/asm/kvm_asm.h | 9 +-
> arch/arm64/include/asm/kvm_coproc.h | 2 +-
> arch/arm64/include/asm/kvm_emulate.h | 157 +-
> arch/arm64/include/asm/kvm_host.h | 105 +-
> arch/arm64/include/asm/kvm_hyp.h | 82 +-
> arch/arm64/include/asm/kvm_mmu.h | 62 +-
> arch/arm64/include/asm/kvm_nested.h | 68 +
> arch/arm64/include/asm/sysreg.h | 143 +-
> arch/arm64/include/uapi/asm/kvm.h | 2 +
> arch/arm64/kernel/cpufeature.c | 26 +
> arch/arm64/kvm/Makefile | 4 +
> arch/arm64/kvm/emulate-nested.c | 223 +++
> arch/arm64/kvm/guest.c | 6 +
> arch/arm64/kvm/handle_exit.c | 76 +-
> arch/arm64/kvm/hyp/Makefile | 1 +
> arch/arm64/kvm/hyp/at.c | 217 +++
> arch/arm64/kvm/hyp/switch.c | 86 +-
> arch/arm64/kvm/hyp/sysreg-sr.c | 267 ++-
> arch/arm64/kvm/hyp/tlb.c | 129 +-
> arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 2 +-
> arch/arm64/kvm/inject_fault.c | 12 -
> arch/arm64/kvm/nested.c | 551 +++++++
> arch/arm64/kvm/regmap.c | 4 +-
> arch/arm64/kvm/reset.c | 7 +
> arch/arm64/kvm/sys_regs.c | 1460 +++++++++++++++--
> arch/arm64/kvm/sys_regs.h | 6 +
> arch/arm64/kvm/trace.h | 58 +-
> include/kvm/arm_arch_timer.h | 6 +
> include/kvm/arm_vgic.h | 28 +-
> virt/kvm/arm/arch_timer.c | 158 +-
> virt/kvm/arm/arm.c | 62 +-
> virt/kvm/arm/hyp/vgic-v3-sr.c | 35 +-
> virt/kvm/arm/mmio.c | 12 +-
> virt/kvm/arm/mmu.c | 445 +++--
> virt/kvm/arm/trace.h | 6 +-
> virt/kvm/arm/vgic/vgic-init.c | 30 +
> virt/kvm/arm/vgic/vgic-kvm-device.c | 22 +
> virt/kvm/arm/vgic/vgic-nested-trace.h | 137 ++
> virt/kvm/arm/vgic/vgic-v2.c | 10 +-
> virt/kvm/arm/vgic/vgic-v3-nested.c | 236 +++
> virt/kvm/arm/vgic/vgic-v3.c | 40 +-
> virt/kvm/arm/vgic/vgic.c | 74 +-
> 56 files changed, 4683 insertions(+), 612 deletions(-)
> create mode 100644 arch/arm/include/asm/kvm_nested.h
> create mode 100644 arch/arm64/include/asm/kvm_nested.h
> create mode 100644 arch/arm64/kvm/emulate-nested.c
> create mode 100644 arch/arm64/kvm/hyp/at.c
> create mode 100644 arch/arm64/kvm/nested.c
> create mode 100644 virt/kvm/arm/vgic/vgic-nested-trace.h
> create mode 100644 virt/kvm/arm/vgic/vgic-v3-nested.c
>
When working on adding support for EL2 to kvm-unit-tests I was able to trigger
the following warning:
# ./lkvm run -f psci.flat -m 128 -c 8 --console serial --irqchip gicv3 --nested
# lkvm run --firmware psci.flat -m 128 -c 8 --name guest-151
Info: Placing fdt at 0x80200000 - 0x80210000
# Warning: The maximum recommended amount of VCPUs is 4
chr_testdev_init: chr-testdev: can't find a virtio-console
INFO: PSCI version 1.0
PASS: invalid-function
PASS: affinity-info-on
PASS: affinity-info-off
[ 24.381266] WARNING: CPU: 3 PID: 160 at
arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:170
kvm_timer_irq_can_fire+0xc/0x30
[ 24.381366] Modules linked in:
[ 24.381466] CPU: 3 PID: 160 Comm: kvm-vcpu-1 Not tainted
5.2.0-rc5-00060-g7dbce63bd1c7 #145
[ 24.381566] Hardware name: Foundation-v8A (DT)
[ 24.381566] pstate: 40400009 (nZcv daif +PAN -UAO)
[ 24.381666] pc : kvm_timer_irq_can_fire+0xc/0x30
[ 24.381766] lr : timer_emulate+0x24/0x98
[ 24.381766] sp : ffff000013d8b780
[ 24.381866] x29: ffff000013d8b780 x28: ffff80087a639b80
[ 24.381966] x27: ffff000010ba8648 x26: ffff000010b71b40
[ 24.382066] x25: ffff80087a63a100 x24: 0000000000000000
[ 24.382111] x23: 000080086ca54000 x22: ffff0000100ce260
[ 24.382166] x21: ffff800875e7c918 x20: ffff800875e7a800
[ 24.382275] x19: ffff800875e7ca08 x18: 0000000000000000
[ 24.382366] x17: 0000000000000000 x16: 0000000000000000
[ 24.382466] x15: 0000000000000000 x14: 0000000000002118
[ 24.382566] x13: 0000000000002190 x12: 0000000000002280
[ 24.382566] x11: 0000000000002208 x10: 0000000000000040
[ 24.382666] x9 : ffff000012dc3b38 x8 : 0000000000000000
[ 24.382766] x7 : 0000000000000000 x6 : ffff80087ac00248
[ 24.382866] x5 : 000080086ca54000 x4 : 0000000000002118
[ 24.382966] x3 : eeeeeeeeeeeeeeef x2 : ffff800875e7c918
[ 24.383066] x1 : 0000000000000001 x0 : ffff800875e7ca08
[ 24.383066] Call trace:
[ 24.383166] kvm_timer_irq_can_fire+0xc/0x30
[ 24.383266] kvm_timer_vcpu_load+0x9c/0x1a0
[ 24.383366] kvm_arch_vcpu_load+0xb0/0x1f0
[ 24.383366] kvm_sched_in+0x1c/0x28
[ 24.383466] finish_task_switch+0xd8/0x1d8
[ 24.383566] __schedule+0x248/0x4a0
[ 24.383666] preempt_schedule_irq+0x60/0x90
[ 24.383666] el1_irq+0xd0/0x180
[ 24.383766] kvm_handle_guest_abort+0x0/0x3a0
[ 24.383866] kvm_arch_vcpu_ioctl_run+0x41c/0x688
[ 24.383866] kvm_vcpu_ioctl+0x4c0/0x838
[ 24.383966] do_vfs_ioctl+0xb8/0x878
[ 24.384077] ksys_ioctl+0x84/0x90
[ 24.384166] __arm64_sys_ioctl+0x18/0x28
[ 24.384166] el0_svc_common.constprop.0+0xb0/0x168
[ 24.384266] el0_svc_handler+0x28/0x78
[ 24.384366] el0_svc+0x8/0xc
[ 24.384366] ---[ end trace 37a32293e43ac12c ]---
[ 24.384666] WARNING: CPU: 3 PID: 160 at
arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:170
kvm_timer_irq_can_fire+0xc/0x30
[ 24.384766] Modules linked in:
[ 24.384866] CPU: 3 PID: 160 Comm: kvm-vcpu-1 Tainted: G W
5.2.0-rc5-00060-g7dbce63bd1c7 #145
[ 24.384966] Hardware name: Foundation-v8A (DT)
[ 24.384966] pstate: 40400009 (nZcv daif +PAN -UAO)
[ 24.385066] pc : kvm_timer_irq_can_fire+0xc/0x30
[ 24.385166] lr : timer_emulate+0x24/0x98
[ 24.385166] sp : ffff000013d8b780
[ 24.385266] x29: ffff000013d8b780 x28: ffff80087a639b80
[ 24.385366] x27: ffff000010ba8648 x26: ffff000010b71b40
[ 24.385466] x25: ffff80087a63a100 x24: 0000000000000000
[ 24.385466] x23: 000080086ca54000 x22: ffff0000100ce260
[ 24.385566] x21: ffff800875e7c918 x20: ffff800875e7a800
[ 24.385666] x19: ffff800875e7ca80 x18: 0000000000000000
[ 24.385766] x17: 0000000000000000 x16: 0000000000000000
[ 24.385866] x15: 0000000000000000 x14: 0000000000002118
[ 24.385966] x13: 0000000000002190 x12: 0000000000002280
[ 24.385966] x11: 0000000000002208 x10: 0000000000000040
[ 24.386066] x9 : ffff000012dc3b38 x8 : 0000000000000000
[ 24.386166] x7 : 0000000000000000 x6 : ffff80087ac00248
[ 24.386266] x5 : 000080086ca54000 x4 : 0000000000002118
[ 24.386366] x3 : eeeeeeeeeeeeeeef x2 : ffff800875e7c918
[ 24.386466] x1 : 0000000000000001 x0 : ffff800875e7ca80
[ 24.386466] Call trace:
[ 24.386566] kvm_timer_irq_can_fire+0xc/0x30
[ 24.386666] kvm_timer_vcpu_load+0xa8/0x1a0
[ 24.386666] kvm_arch_vcpu_load+0xb0/0x1f0
[ 24.386898] kvm_sched_in+0x1c/0x28
[ 24.386966] finish_task_switch+0xd8/0x1d8
[ 24.387166] __schedule+0x248/0x4a0
[ 24.387354] preempt_schedule_irq+0x60/0x90
[ 24.387366] el1_irq+0xd0/0x180
[ 24.387466] kvm_handle_guest_abort+0x0/0x3a0
[ 24.387566] kvm_arch_vcpu_ioctl_run+0x41c/0x688
[ 24.387566] kvm_vcpu_ioctl+0x4c0/0x838
[ 24.387666] do_vfs_ioctl+0xb8/0x878
[ 24.387766] ksys_ioctl+0x84/0x90
[ 24.387866] __arm64_sys_ioctl+0x18/0x28
[ 24.387866] el0_svc_common.constprop.0+0xb0/0x168
[ 24.387966] el0_svc_handler+0x28/0x78
[ 24.388066] el0_svc+0x8/0xc
[ 24.388066] ---[ end trace 37a32293e43ac12d ]---
PASS: cpu-on
SUMMARY: 4 te[ 24.390266] WARNING: CPU: 3 PID: 160 at
arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:170
kvm_timer_irq_can_fire+0xc/0x30
s[ 24.390366] Modules linked in:
ts[ 24.390366] CPU: 3 PID: 160 Comm: kvm-vcpu-1 Tainted: G W
5.2.0-rc5-00060-g7dbce63bd1c7 #145
[ 24.390566] Hardware name: Foundation-v8A (DT)
[ 24.390795] pstate: 40400009 (nZcv daif +PAN -UAO)
[ 24.390866] pc : kvm_timer_irq_can_fire+0xc/0x30
[ 24.390966] lr : timer_emulate+0x24/0x98
[ 24.391066] sp : ffff000013d8b780
[ 24.391066] x29: ffff000013d8b780 x28: ffff80087a639b80
[ 24.391166] x27: ffff000010ba8648 x26: ffff000010b71b40
[ 24.391266] x25: ffff80087a63a100 x24: 0000000000000000
[ 24.391366] x23: 000080086ca54000 x22: 0000000000000003
[ 24.391466] x21: ffff800875e7c918 x20: ffff800875e7a800
[ 24.391466] x19: ffff800875e7ca08 x18: 0000000000000000
[ 24.391566] x17: 0000000000000000 x16: 0000000000000000
[ 24.391666] x15: 0000000000000000 x14: 0000000000002118
[ 24.391766] x13: 0000000000002190 x12: 0000000000002280
[ 24.391866] x11: 0000000000002208 x10: 0000000000000040
[ 24.391942] x9 : ffff000012dc3b38 x8 : 0000000000000000
[ 24.391966] x7 : 0000000000000000 x6 : ffff80087ac00248
[ 24.392066] x5 : 000080086ca54000 x4 : 0000000000002118
[ 24.392166] x3 : eeeeeeeeeeeeeeef x2 : ffff800875e7c918
[ 24.392269] x1 : 0000000000000001 x0 : ffff800875e7ca08
[ 24.392366] Call trace:
[ 24.392433] kvm_timer_irq_can_fire+0xc/0x30
[ 24.392466] kvm_timer_vcpu_load+0x9c/0x1a0
[ 24.392597] kvm_arch_vcpu_load+0xb0/0x1f0
[ 24.392666] kvm_sched_in+0x1c/0x28
[ 24.392766] finish_task_switch+0xd8/0x1d8
[ 24.392766] __schedule+0x248/0x4a0
[ 24.392866] preempt_schedule_irq+0x60/0x90
[ 24.392966] el1_irq+0xd0/0x180
[ 24.392966] kvm_handle_guest_abort+0x0/0x3a0
[ 24.393066] kvm_arch_vcpu_ioctl_run+0x41c/0x688
[ 24.393166] kvm_vcpu_ioctl+0x4c0/0x838
[ 24.393266] do_vfs_ioctl+0xb8/0x878
[ 24.393266] ksys_ioctl+0x84/0x90
[ 24.393366] __arm64_sys_ioctl+0x18/0x28
[ 24.393466] el0_svc_common.constprop.0+0xb0/0x168
[ 24.393566] el0_svc_handler+0x28/0x78
[ 24.393566] el0_svc+0x8/0xc
[ 24.393666] ---[ end trace 37a32293e43ac12e ]---
[ 24.393866] WARNING: CPU: 3 PID: 160 at
arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:170
kvm_timer_irq_can_fire+0xc/0x30
[ 24.394066] Modules linked in:
[ 24.394266] CPU: 3 PID: 160 Comm: kvm-vcpu-1 Tainted: G W
5.2.0-rc5-00060-g7dbce63bd1c7 #145
[ 24.394366] Hardware name: Foundation-v8A (DT)
[ 24.394466] pstate: 40400009 (nZcv daif +PAN -UAO)
[ 24.394466] pc : kvm_timer_irq_can_fire+0xc/0x30
[ 24.394566] lr : timer_emulate+0x24/0x98
[ 24.394666] sp : ffff000013d8b780
[ 24.394727] x29: ffff000013d8b780 x28: ffff80087a639b80
[ 24.394766] x27: ffff000010ba8648 x26: ffff000010b71b40
[ 24.394866] x25: ffff80087a63a100 x24: 0000000000000000
[ 24.394966] x23: 000080086ca54000 x22: 0000000000000003
[ 24.394966] x21: ffff800875e7c918 x20: ffff800875e7a800
[ 24.395066] x19: ffff800875e7ca80 x18: 0000000000000000
[ 24.395166] x17: 0000000000000000 x16: 0000000000000000
[ 24.395266] x15: 0000000000000000 x14: 0000000000002118
[ 24.395383] x13: 0000000000002190 x12: 0000000000002280
[ 24.395466] x11: 0000000000002208 x10: 0000000000000040
[ 24.395547] x9 : ffff000012dc3b38 x8 : 0000000000000000
[ 24.395666] x7 : 0000000000000000 x6 : ffff80087ac00248
[ 24.395866] x5 : 000080086ca54000 x4 : 0000000000002118
[ 24.395966] x3 : eeeeeeeeeeeeeeef x2 : ffff800875e7c918
[ 24.396066] x1 : 0000000000000001 x0 : ffff800875e7ca80
[ 24.396066] Call trace:
[ 24.396166] kvm_timer_irq_can_fire+0xc/0x30
[ 24.396266] kvm_timer_vcpu_load+0xa8/0x1a0
[ 24.396366] kvm_arch_vcpu_load+0xb0/0x1f0
[ 24.396366] kvm_sched_in+0x1c/0x28
[ 24.396466] finish_task_switch+0xd8/0x1d8
[ 24.396566] __schedule+0x248/0x4a0
[ 24.396666] preempt_schedule_irq+0x60/0x90
[ 24.396666] el1_irq+0xd0/0x180
[ 24.396766] kvm_handle_guest_abort+0x0/0x3a0
[ 24.396866] kvm_arch_vcpu_ioctl_run+0x41c/0x688
[ 24.396866] kvm_vcpu_ioctl+0x4c0/0x838
[ 24.397021] do_vfs_ioctl+0xb8/0x878
[ 24.397066] ksys_ioctl+0x84/0x90
[ 24.397166] __arm64_sys_ioctl+0x18/0x28
[ 24.397348] el0_svc_common.constprop.0+0xb0/0x168
[ 24.397366] el0_svc_handler+0x28/0x78
[ 24.397566] el0_svc+0x8/0xc
[ 24.397676] ---[ end trace 37a32293e43ac12f ]---
# KVM compatibility warning.
virtio-9p device was not detected.
While you have requested a virtio-9p device, the guest kernel did not
initialize it.
Please make sure that the guest kernel was compiled with
CONFIG_NET_9P_VIRTIO=y enabled in .config.
# KVM compatibility warning.
virtio-net device was not detected.
While you have requested a virtio-net device, the guest kernel did not
initialize it.
Please make sure that the guest kernel was compiled with CONFIG_VIRTIO_NET=y
enabled in .config.
These are the changes that I made to kvm-unit-tests (the diff can be applied on
top of upstream master, 2130fd4154ad ("tscdeadline_latency: Check condition
first before loop")):
diff --git a/arm/cstart64.S b/arm/cstart64.S
index b0e8baa1a23a..a7631b5a1801 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -51,6 +51,17 @@ start:
b 1b
1:
+ mrs x4, CurrentEL
+ cmp x4, CurrentEL_EL2
+ b.ne 1f
+ mrs x4, mpidr_el1
+ msr vmpidr_el2, x4
+ mrs x4, midr_el1
+ msr vpidr_el2, x4
+ ldr x4, =(HCR_EL2_TGE | HCR_EL2_E2H)
+ msr hcr_el2, x4
+ isb
+1:
/* set up stack */
mov x4, #1
msr spsel, x4
@@ -101,6 +112,17 @@ get_mmu_off:
.globl secondary_entry
secondary_entry:
+ mrs x0, CurrentEL
+ cmp x0, CurrentEL_EL2
+ b.ne 1f
+ mrs x0, mpidr_el1
+ msr vmpidr_el2, x0
+ mrs x0, midr_el1
+ msr vpidr_el2, x0
+ ldr x0, =(HCR_EL2_TGE | HCR_EL2_E2H)
+ msr hcr_el2, x0
+ isb
+1:
/* Enable FP/ASIMD */
mov x0, #(3 << 20)
msr cpacr_el1, x0
diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
index 7b956bf5987d..07297a27e0ce 100644
--- a/lib/arm/asm/psci.h
+++ b/lib/arm/asm/psci.h
@@ -3,6 +3,15 @@
#include <libcflat.h>
#include <linux/psci.h>
+enum psci_conduit {
+ PSCI_CONDUIT_HVC,
+ PSCI_CONDUIT_SMC,
+};
+
+extern void psci_init(void);
+extern void psci_set_conduit(enum psci_conduit conduit);
+extern enum psci_conduit psci_get_conduit(void);
+
extern int psci_invoke(unsigned long function_id, unsigned long arg0,
unsigned long arg1, unsigned long arg2);
extern int psci_cpu_on(unsigned long cpuid, unsigned long entry_point);
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index c3d399064ae3..20ad4b944738 100644
--- a/lib/arm/psci.c
+++ b/lib/arm/psci.c
@@ -6,13 +6,14 @@
*
* This work is licensed under the terms of the GNU LGPL, version 2.
*/
+#include <devicetree.h>
+#include <string.h>
#include <asm/psci.h>
#include <asm/setup.h>
#include <asm/page.h>
#include <asm/smp.h>
-__attribute__((noinline))
-int psci_invoke(unsigned long function_id, unsigned long arg0,
+static int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
unsigned long arg1, unsigned long arg2)
{
asm volatile(
@@ -22,6 +23,63 @@ int psci_invoke(unsigned long function_id, unsigned long arg0,
return function_id;
}
+static int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
+ unsigned long arg1, unsigned long arg2)
+{
+ asm volatile(
+ "smc #0"
+ : "+r" (function_id)
+ : "r" (arg0), "r" (arg1), "r" (arg2));
+ return function_id;
+}
+
+/*
+ * Initialize to something sensible, so the exit fallback psci_system_off still
+ * works before calling psci_init when booted at EL1.
+ */
+static enum psci_conduit psci_conduit = PSCI_CONDUIT_HVC;
+static int (*psci_fn)(unsigned long, unsigned long, unsigned long,
+ unsigned long) = &psci_invoke_hvc;
+
+void psci_set_conduit(enum psci_conduit conduit)
+{
+ psci_conduit = conduit;
+ if (conduit == PSCI_CONDUIT_HVC)
+ psci_fn = &psci_invoke_hvc;
+ else
+ psci_fn = &psci_invoke_smc;
+}
+
+enum psci_conduit psci_get_conduit(void)
+{
+ return psci_conduit;
+}
+
+int psci_invoke(unsigned long function_id, unsigned long arg0,
+ unsigned long arg1, unsigned long arg2)
+{
+ return psci_fn(function_id, arg0, arg1, arg2);
+}
+
+void psci_init(void)
+{
+ const char *conduit;
+ int ret;
+
+ ret = dt_get_psci_conduit(&conduit);
+ assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+
+ if (ret == -FDT_ERR_NOTFOUND)
+ conduit = "hvc";
+
+ assert(strcmp(conduit, "hvc") == 0 || strcmp(conduit, "smc") == 0);
+
+ if (strcmp(conduit, "hvc") == 0)
+ psci_set_conduit(PSCI_CONDUIT_HVC);
+ else
+ psci_set_conduit(PSCI_CONDUIT_SMC);
+}
+
int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
{
#ifdef __arm__
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 4f02fca85607..e0dc9e4801b0 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -21,6 +21,7 @@
#include <asm/setup.h>
#include <asm/page.h>
#include <asm/smp.h>
+#include <asm/psci.h>
#include "io.h"
@@ -164,7 +165,11 @@ void setup(const void *fdt)
freemem += initrd_size;
}
- /* call init functions */
+ /*
+ * call init functions. psci_init goes first so psci_system_off fallback
+ * works in case of an assert failure
+ */
+ psci_init();
mem_init(PAGE_ALIGN((unsigned long)freemem));
cpu_init();
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 1d9223f728a5..18c5d29ddd1f 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -16,6 +16,9 @@
#define SCTLR_EL1_A (1 << 1)
#define SCTLR_EL1_M (1 << 0)
+#define HCR_EL2_TGE (1 << 27)
+#define HCR_EL2_E2H (1 << 34)
+
#ifndef __ASSEMBLY__
#include <asm/ptrace.h>
#include <asm/esr.h>
diff --git a/lib/devicetree.c b/lib/devicetree.c
index 2b89178a109b..4e684c7100b2 100644
--- a/lib/devicetree.c
+++ b/lib/devicetree.c
@@ -263,6 +263,27 @@ int dt_get_bootargs(const char **bootargs)
return 0;
}
+int dt_get_psci_conduit(const char **conduit)
+{
+ const struct fdt_property *prop;
+ int node, len;
+
+ *conduit = NULL;
+
+ node = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-0.2");
+ if (node < 0)
+ return node;
+
+ prop = fdt_get_property(fdt, node, "method", &len);
+ if (!prop)
+ return len;
+ if (len < 4)
+ return -FDT_ERR_NOTFOUND;
+
+ *conduit = prop->data;
+ return 0;
+}
+
int dt_get_default_console_node(void)
{
const struct fdt_property *prop;
diff --git a/lib/devicetree.h b/lib/devicetree.h
index 93c7ebc63bd8..236035eb777d 100644
--- a/lib/devicetree.h
+++ b/lib/devicetree.h
@@ -211,6 +211,15 @@ extern int dt_get_reg(int fdtnode, int regidx, struct
dt_reg *reg);
extern int dt_get_bootargs(const char **bootargs);
/*
+ * dt_get_psci_conduit gets the conduit for PSCI function invocations from
+ * /psci/method
+ * returns
+ * - zero on success
+ * - a negative FDT_ERR_* value on failure, and @conduit will be set to null
+ */
+extern int dt_get_psci_conduit(const char **conduit);
+
+/*
* dt_get_default_console_node gets the node of the path stored in
* /chosen/stdout-path (or the deprecated /chosen/linux,stdout-path)
* returns
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Catalin Marinas @ 2019-08-02 10:08 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Will Deacon, linux-kernel, linux-mm, linux-kselftest,
Vincenzo Frascino, linux-arm-kernel
In-Reply-To: <b74e7ce7-d58a-68a0-2f28-6648ec6302c0@intel.com>
Hi Dave,
On Wed, Jul 31, 2019 at 09:43:46AM -0700, Dave Hansen wrote:
> On 7/25/19 6:50 AM, Vincenzo Frascino wrote:
> > With the relaxed ABI proposed through this document, it is now possible
> > to pass tagged pointers to the syscalls, when these pointers are in
> > memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().
>
> I don't see a lot of description of why this restriction is necessary.
> What's the problem with supporting MAP_SHARED?
We could support MAP_SHARED | MAP_ANONYMOUS (and based on some internal
discussions, this would be fine with the hardware memory tagging as
well). What we don't want in the ABI is to support file mmap() for
top-byte-ignore (or MTE). If you see a use-case, please let us know.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 4/4] serial: 8250: Don't check for mctrl_gpio_init() returning -ENOSYS
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
To: u.kleine-koenig@pengutronix.de, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com
Cc: geert+renesas@glider.be, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-serial@vger.kernel.org, Jiri Slaby,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190802100349.8659-1-frieder.schrempf@kontron.de>
From: Frieder Schrempf <frieder.schrempf@kontron.de>
Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
if CONFIG_GPIOLIB is disabled, we can safely remove this check.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Adjust the commit message and subject line
Changes in v2
=============
* Add a patch for the 8250 driver
---
drivers/tty/serial/8250/8250_core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index df3bcc0b2d74..e682390ce0de 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1026,10 +1026,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (!has_acpi_companion(uart->port.dev)) {
gpios = mctrl_gpio_init(&uart->port, 0);
if (IS_ERR(gpios)) {
- if (PTR_ERR(gpios) != -ENOSYS) {
- ret = PTR_ERR(gpios);
- goto out_unlock;
- }
+ ret = PTR_ERR(gpios);
+ goto out_unlock;
} else {
uart->gpios = gpios;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
To: u.kleine-koenig@pengutronix.de, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com, Greg Kroah-Hartman
Cc: geert+renesas@glider.be, linux-kernel@vger.kernel.org,
Schrempf Frieder, linux-serial@vger.kernel.org, Jiri Slaby,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190802100349.8659-1-frieder.schrempf@kontron.de>
From: Frieder Schrempf <frieder.schrempf@kontron.de>
As it is allowed to use the mctrl_gpio_* functions before
initialization (as the 8250 driver does according to 434be0ae7aa7),
it seems appropriate to have a NULL check in all of the functions.
Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
in a context that can lead to a NULL pointer dereference.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Move the changes in mctrl_gpio_to_gpiod() to a separate patch
---
drivers/tty/serial/serial_mctrl_gpio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 2b400189be91..54c43e02e375 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -61,6 +61,9 @@ EXPORT_SYMBOL_GPL(mctrl_gpio_set);
struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
enum mctrl_gpio_idx gidx)
{
+ if (gpios == NULL)
+ return NULL;
+
return gpios->gpio[gidx];
}
EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
To: u.kleine-koenig@pengutronix.de, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com, Greg Kroah-Hartman
Cc: geert+renesas@glider.be, linux-kernel@vger.kernel.org,
Schrempf Frieder, linux-serial@vger.kernel.org, Jiri Slaby,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190802100349.8659-1-frieder.schrempf@kontron.de>
From: Frieder Schrempf <frieder.schrempf@kontron.de>
Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
if CONFIG_GPIOLIB is disabled, we can safely remove this check.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Adjust the commit message and subject line
Changes in v2
=============
* Move the sh_sci changes to a separate patch
---
drivers/tty/serial/sh-sci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d18c680aa64b..249325b65ee0 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3287,7 +3287,7 @@ static int sci_probe_single(struct platform_device *dev,
return ret;
sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
- if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
+ if (IS_ERR(sciport->gpios))
return PTR_ERR(sciport->gpios);
if (sciport->has_rtscts) {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
To: u.kleine-koenig@pengutronix.de, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com, Greg Kroah-Hartman
Cc: geert+renesas@glider.be, linux-kernel@vger.kernel.org,
Schrempf Frieder, linux-serial@vger.kernel.org, Jiri Slaby,
linux-arm-kernel@lists.infradead.org
From: Frieder Schrempf <frieder.schrempf@kontron.de>
If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() and
mctrl_gpio_init_noauto() will currently return an error pointer with
-ENOSYS. As the mctrl GPIOs are usually optional, drivers need to
check for this condition to allow continue probing.
To avoid the need for this check in each driver, we return NULL
instead, as all the mctrl_gpio_*() functions are skipped anyway.
We also adapt mctrl_gpio_to_gpiod() to be in line with this change.
Reviewed-by: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Move the changes in mctrl_gpio_to_gpiod() to a separate patch
* Reorder tags
Changes in v2
=============
* Move the sh_sci changes to a separate patch
* Add Fabio's R-b tag
---
drivers/tty/serial/serial_mctrl_gpio.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index b7d3cca48ede..1b2ff503b2c2 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -114,19 +114,19 @@ static inline
struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
enum mctrl_gpio_idx gidx)
{
- return ERR_PTR(-ENOSYS);
+ return NULL;
}
static inline
struct mctrl_gpios *mctrl_gpio_init(struct uart_port *port, unsigned int idx)
{
- return ERR_PTR(-ENOSYS);
+ return NULL;
}
static inline
struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
{
- return ERR_PTR(-ENOSYS);
+ return NULL;
}
static inline
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 4.9 032/223] perf cs-etm: Properly set the value of old and head in snapshot mode
From: Greg Kroah-Hartman @ 2019-08-02 9:34 UTC (permalink / raw)
To: linux-kernel
Cc: Sasha Levin, Mathieu Poirier, Suzuki Poulouse, Alexander Shishkin,
Greg Kroah-Hartman, stable, Arnaldo Carvalho de Melo,
Peter Zijlstra, Leo Yan, Jiri Olsa, linux-arm-kernel
In-Reply-To: <20190802092238.692035242@linuxfoundation.org>
[ Upstream commit e45c48a9a4d20ebc7b639a62c3ef8f4b08007027 ]
This patch adds the necessary intelligence to properly compute the value
of 'old' and 'head' when operating in snapshot mode. That way we can
get the latest information in the AUX buffer and be compatible with the
generic AUX ring buffer mechanic.
Tester notes:
> Leo, have you had the chance to test/review this one? Suzuki?
Sure. I applied this patch on the perf/core branch (with latest
commit 3e4fbf36c1e3 'perf augmented_raw_syscalls: Move reading
filename to the loop') and passed testing with below steps:
# perf record -e cs_etm/@tmc_etr0/ -S -m,64 --per-thread ./sort &
[1] 19097
Bubble sorting array of 30000 elements
# kill -USR2 19097
# kill -USR2 19097
# kill -USR2 19097
[ perf record: Woken up 4 times to write data ]
[ perf record: Captured and wrote 0.753 MB perf.data ]
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Tested-by: Leo Yan <leo.yan@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20190605161633.12245-1-mathieu.poirier@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/perf/arch/arm/util/cs-etm.c | 127 +++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 47d584da5819..f6cff278aa5d 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -41,6 +41,8 @@ struct cs_etm_recording {
struct auxtrace_record itr;
struct perf_pmu *cs_etm_pmu;
struct perf_evlist *evlist;
+ int wrapped_cnt;
+ bool *wrapped;
bool snapshot_mode;
size_t snapshot_size;
};
@@ -458,16 +460,131 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
return 0;
}
-static int cs_etm_find_snapshot(struct auxtrace_record *itr __maybe_unused,
+static int cs_etm_alloc_wrapped_array(struct cs_etm_recording *ptr, int idx)
+{
+ bool *wrapped;
+ int cnt = ptr->wrapped_cnt;
+
+ /* Make @ptr->wrapped as big as @idx */
+ while (cnt <= idx)
+ cnt++;
+
+ /*
+ * Free'ed in cs_etm_recording_free(). Using realloc() to avoid
+ * cross compilation problems where the host's system supports
+ * reallocarray() but not the target.
+ */
+ wrapped = realloc(ptr->wrapped, cnt * sizeof(bool));
+ if (!wrapped)
+ return -ENOMEM;
+
+ wrapped[cnt - 1] = false;
+ ptr->wrapped_cnt = cnt;
+ ptr->wrapped = wrapped;
+
+ return 0;
+}
+
+static bool cs_etm_buffer_has_wrapped(unsigned char *buffer,
+ size_t buffer_size, u64 head)
+{
+ u64 i, watermark;
+ u64 *buf = (u64 *)buffer;
+ size_t buf_size = buffer_size;
+
+ /*
+ * We want to look the very last 512 byte (chosen arbitrarily) in
+ * the ring buffer.
+ */
+ watermark = buf_size - 512;
+
+ /*
+ * @head is continuously increasing - if its value is equal or greater
+ * than the size of the ring buffer, it has wrapped around.
+ */
+ if (head >= buffer_size)
+ return true;
+
+ /*
+ * The value of @head is somewhere within the size of the ring buffer.
+ * This can be that there hasn't been enough data to fill the ring
+ * buffer yet or the trace time was so long that @head has numerically
+ * wrapped around. To find we need to check if we have data at the very
+ * end of the ring buffer. We can reliably do this because mmap'ed
+ * pages are zeroed out and there is a fresh mapping with every new
+ * session.
+ */
+
+ /* @head is less than 512 byte from the end of the ring buffer */
+ if (head > watermark)
+ watermark = head;
+
+ /*
+ * Speed things up by using 64 bit transactions (see "u64 *buf" above)
+ */
+ watermark >>= 3;
+ buf_size >>= 3;
+
+ /*
+ * If we find trace data at the end of the ring buffer, @head has
+ * been there and has numerically wrapped around at least once.
+ */
+ for (i = watermark; i < buf_size; i++)
+ if (buf[i])
+ return true;
+
+ return false;
+}
+
+static int cs_etm_find_snapshot(struct auxtrace_record *itr,
int idx, struct auxtrace_mmap *mm,
- unsigned char *data __maybe_unused,
+ unsigned char *data,
u64 *head, u64 *old)
{
+ int err;
+ bool wrapped;
+ struct cs_etm_recording *ptr =
+ container_of(itr, struct cs_etm_recording, itr);
+
+ /*
+ * Allocate memory to keep track of wrapping if this is the first
+ * time we deal with this *mm.
+ */
+ if (idx >= ptr->wrapped_cnt) {
+ err = cs_etm_alloc_wrapped_array(ptr, idx);
+ if (err)
+ return err;
+ }
+
+ /*
+ * Check to see if *head has wrapped around. If it hasn't only the
+ * amount of data between *head and *old is snapshot'ed to avoid
+ * bloating the perf.data file with zeros. But as soon as *head has
+ * wrapped around the entire size of the AUX ring buffer it taken.
+ */
+ wrapped = ptr->wrapped[idx];
+ if (!wrapped && cs_etm_buffer_has_wrapped(data, mm->len, *head)) {
+ wrapped = true;
+ ptr->wrapped[idx] = true;
+ }
+
pr_debug3("%s: mmap index %d old head %zu new head %zu size %zu\n",
__func__, idx, (size_t)*old, (size_t)*head, mm->len);
- *old = *head;
- *head += mm->len;
+ /* No wrap has occurred, we can just use *head and *old. */
+ if (!wrapped)
+ return 0;
+
+ /*
+ * *head has wrapped around - adjust *head and *old to pickup the
+ * entire content of the AUX buffer.
+ */
+ if (*head >= mm->len) {
+ *old = *head - mm->len;
+ } else {
+ *head += mm->len;
+ *old = *head - mm->len;
+ }
return 0;
}
@@ -508,6 +625,8 @@ static void cs_etm_recording_free(struct auxtrace_record *itr)
{
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
+
+ zfree(&ptr->wrapped);
free(ptr);
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 06/34] drm/i915: convert put_page() to put_user_page*()
From: Joonas Lahtinen @ 2019-08-02 9:19 UTC (permalink / raw)
To: Andrew Morton, john.hubbard
Cc: linux-fbdev, Jan Kara, kvm, David Airlie, Dave Hansen,
Dave Chinner, dri-devel, linux-mm, sparclinux, Ira Weiny,
ceph-devel, devel, rds-devel, linux-rdma, x86, amd-gfx,
Christoph Hellwig, Jason Gunthorpe, xen-devel, devel, linux-media,
John Hubbard, intel-gfx, Jani Nikula, linux-block,
Jérôme Glisse, linux-rpi-kernel, Rodrigo Vivi,
Dan Williams, linux-arm-kernel, linux-nfs, netdev, LKML,
linux-xfs, linux-crypto, linux-fsdevel
In-Reply-To: <20190802022005.5117-7-jhubbard@nvidia.com>
Quoting john.hubbard@gmail.com (2019-08-02 05:19:37)
> From: John Hubbard <jhubbard@nvidia.com>
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Note that this effectively changes the code's behavior in
> i915_gem_userptr_put_pages(): it now calls set_page_dirty_lock(),
> instead of set_page_dirty(). This is probably more accurate.
We've already fixed this in drm-tip where the current code uses
set_page_dirty_lock().
This would conflict with our tree. Rodrigo is handling
drm-intel-next for 5.4, so you guys want to coordinate how
to merge.
Regards, Joonas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites
From: Michal Hocko @ 2019-08-02 9:12 UTC (permalink / raw)
To: john.hubbard
Cc: linux-fbdev, Jan Kara, kvm, Dave Hansen, Dave Chinner, dri-devel,
linux-mm, sparclinux, Ira Weiny, Dan Williams, devel, rds-devel,
linux-rdma, x86, amd-gfx, Christoph Hellwig, Jason Gunthorpe,
xen-devel, devel, linux-media, John Hubbard, intel-gfx,
linux-block, Jérôme Glisse, linux-rpi-kernel,
ceph-devel, linux-arm-kernel, linux-nfs, netdev, LKML, linux-xfs,
linux-crypto, linux-fsdevel, Andrew Morton
In-Reply-To: <20190802022005.5117-1-jhubbard@nvidia.com>
On Thu 01-08-19 19:19:31, john.hubbard@gmail.com wrote:
[...]
> 2) Convert all of the call sites for get_user_pages*(), to
> invoke put_user_page*(), instead of put_page(). This involves dozens of
> call sites, and will take some time.
How do we make sure this is the case and it will remain the case in the
future? There must be some automagic to enforce/check that. It is simply
not manageable to do it every now and then because then 3) will simply
be never safe.
Have you considered coccinele or some other scripted way to do the
transition? I have no idea how to deal with future changes that would
break the balance though.
--
Michal Hocko
SUSE Labs
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/1] coresight: Fix for v5.3-rc3
From: Greg KH @ 2019-08-02 9:10 UTC (permalink / raw)
To: Mathieu Poirier; +Cc: linux-arm-kernel, Suzuki K. Poulose
In-Reply-To: <CANLsYky1vC-=zNPvOmWt1wLd5A4jcD+sh=Hwdq0rwhW=b1cdwQ@mail.gmail.com>
On Thu, Aug 01, 2019 at 02:16:46PM -0600, Mathieu Poirier wrote:
> On Thu, 1 Aug 2019 at 12:17, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Aug 01, 2019 at 11:23:22AM -0600, Mathieu Poirier wrote:
> > > Good morning Greg,
> > >
> > > Here is a fix I'd like you to consider for this cycle. Do you think you
> > > could apply it to driver-core-next rather than char-misc-next?
> >
> > All of my -next branches are for 5.4-rc1, not 5.3 (i.e. the "next"
> > kernel).
> >
> > So either one of them isn't going to matter to you for 5.3-final.
> >
> > > My current
> > > coresight branch is based on driver-core-next to pick up Suzuki's
> > > generic device lookup helpers patchset [1]. Applying it to char-misc-next
> > > will create two different signature for the same patch, something that
> > > gives Stephen a hard time when building the linux-next tree.
> >
> > Why would Suzuki's patch matter for 5.3-final?
>
> There was a similar situation during the 5.2 cycle [1]. Here too we
> can fix a problem that is present in 5.3 rather than wait for 5.4.
>
> [1]. https://www.spinics.net/lists/arm-kernel/msg736274.html
But that has nothing to do with Suzuki's patch that is in my driver core
tree. I still fail to see the dependency here.
> > > I also think this patch should go in stable but haven't marked it as such since
> > > it doesn't apply to any of the stable trees. Once it is part of v5.3 I intend
> > > to send a new version of the patch that does apply cleanly to those trees. Let
> > > me know if you want me to proceed differently.
> > >
> > > Thanks,
> > > Mathieu
> > >
> > > [1]. https://www.spinics.net/lists/dri-devel/msg219674.html
> > >
> > >
> > > Suzuki K Poulose (1):
> > > coresight: Fix DEBUG_LOCKS_WARN_ON for uninitialized attribute
> > >
> > > drivers/hwtracing/coresight/coresight-etm-perf.c | 1 +
> > > 1 file changed, 1 insertion(+)
> >
> > Why would this patch depend on anything in any of my trees?
>
> I send you patches for inclusion in the next cycle and as such I
> thought it should be the same for fixes in the current cycle. If that
> is not the case, should I send them directly to Linus?
You can send me fixes to forward on to Linus for this current cycle, if
they are not depending on patches that are only for the -next release.
I always keep 2 branches in my git trees:
-linus : patches for Linus this release cycle
-next : patches for Linus the next release cycle
If you have bugfixes, make them against my -linus branch and send them
on. Odds are they don't have dependancies other than what is in Linus's
tree, so that's fine.
If you have patches for the next cycle, send them against my -next
branch.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board
From: Yannick FERTRE @ 2019-08-02 9:02 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-6-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This property is already defined into stm32mp157c.dtsi file.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 020ea0f..09f6e7b 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -17,7 +17,6 @@
> #address-cells = <1>;
> #size-cells = <0>;
> status = "okay";
> - phy-dsi-supply = <®18>;
>
> ports {
> #address-cells = <1>;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18
From: Yannick FERTRE @ 2019-08-02 9:02 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-5-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
> & stm32mp157c-dk2.dts to file stm32mp157c.dtsi.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c-dk2.dts | 8 --------
> arch/arm/boot/dts/stm32mp157c-ed1.dts | 16 ----------------
> arch/arm/boot/dts/stm32mp157c.dtsi | 16 ++++++++++++++++
> 3 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 20ea601..020ea0f 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -11,14 +11,6 @@
> / {
> model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
> compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> };
>
> &dsi {
> diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> index 62a8c78..f41189c 100644
> --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> @@ -27,22 +27,6 @@
> serial0 = &uart4;
> };
>
> - reg11: reg11 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg11";
> - regulator-min-microvolt = <1100000>;
> - regulator-max-microvolt = <1100000>;
> - regulator-always-on;
> - };
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> -
> sd_switch: regulator-sd_switch {
> compatible = "regulator-gpio";
> regulator-name = "sd_switch";
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 6b14f1e..aaac51cd 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -11,6 +11,22 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> + reg11: reg11 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg11";
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-always-on;
> + };
> +
> + reg18: reg18 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg18";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c
From: Yannick FERTRE @ 2019-08-02 9:01 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-4-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> The dsi physical layer is powered by the 1v8 power controller supply.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..6b14f1e 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1156,6 +1156,7 @@
> clock-names = "pclk", "ref", "px_clk";
> resets = <&rcc DSI_R>;
> reset-names = "apb";
> + phy-dsi-supply = <®18>;
> status = "disabled";
> };
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 07/20] ARM: dts: imx7-colibri: fix 1.8V/UHS support
From: Stefan Agner @ 2019-08-02 8:51 UTC (permalink / raw)
To: Philippe Schenker
Cc: mark.rutland, devicetree, michal.vokac, kernel, Stefan Agner,
Marcel Ziswiler, shawnguo, s.hauer, linux-kernel, robh+dt,
linux-imx, Max Krummenacher, festevam, linux-arm-kernel
In-Reply-To: <723f191c5893984c8fbe711163524dc7ebf09a5b.camel@toradex.com>
On 2019-07-31 16:52, Philippe Schenker wrote:
> On Wed, 2019-07-31 at 09:56 -0300, Fabio Estevam wrote:
>> On Wed, Jul 31, 2019 at 9:38 AM Philippe Schenker
>> <philippe.schenker@toradex.com> wrote:
>> > From: Stefan Agner <stefan.agner@toradex.com>
>> >
>> > Add pinmuxing and do not specify voltage restrictions in the
>> > module level device tree.
>>
>> It would be nice to explain the reason for doing this.
>
> This commit is in preparation of another patch that didn't made into this
> patchset (downstream stuff in there). But I will do another patch on top that
> will use this patch here. That should anyway be in mainline.
I guess what Fabio meant here is explain this patch.
The commit message really could be improved, e.g.:
Add pinmuxing and do not specify voltage restrictions for the usdhc
instance
available on the modules edge connector. This allows to use SD-cards
with
higher transfer modes if supported by the carrier board.
--
Stefan
>
> Philippe
>
>>
>> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>> > ---
>> >
>> > Changes in v2: None
>> >
>> > arch/arm/boot/dts/imx7-colibri.dtsi | 23 ++++++++++++++++++++++-
>> > 1 file changed, 22 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-
>> > colibri.dtsi
>> > index 16d1a1ed1aff..67f5e0c87fdc 100644
>> > --- a/arch/arm/boot/dts/imx7-colibri.dtsi
>> > +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
>> > @@ -326,7 +326,6 @@
>> > &usdhc1 {
>> > pinctrl-names = "default";
>> > pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>;
>> > - no-1-8-v;
>> > cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
>> > disable-wp;
>> > vqmmc-supply = <®_LDO2>;
>> > @@ -671,6 +670,28 @@
>> > >;
>> > };
>> >
>> > + pinctrl_usdhc1_100mhz: usdhc1grp_100mhz {
>> > + fsl,pins = <
>> > + MX7D_PAD_SD1_CMD__SD1_CMD 0x5a
>> > + MX7D_PAD_SD1_CLK__SD1_CLK 0x1a
>> > + MX7D_PAD_SD1_DATA0__SD1_DATA0 0x5a
>> > + MX7D_PAD_SD1_DATA1__SD1_DATA1 0x5a
>> > + MX7D_PAD_SD1_DATA2__SD1_DATA2 0x5a
>> > + MX7D_PAD_SD1_DATA3__SD1_DATA3 0x5a
>> > + >;
>> > + };
>> > +
>> > + pinctrl_usdhc1_200mhz: usdhc1grp_200mhz {
>> > + fsl,pins = <
>> > + MX7D_PAD_SD1_CMD__SD1_CMD 0x5b
>> > + MX7D_PAD_SD1_CLK__SD1_CLK 0x1b
>> > + MX7D_PAD_SD1_DATA0__SD1_DATA0 0x5b
>> > + MX7D_PAD_SD1_DATA1__SD1_DATA1 0x5b
>> > + MX7D_PAD_SD1_DATA2__SD1_DATA2 0x5b
>> > + MX7D_PAD_SD1_DATA3__SD1_DATA3 0x5b
>> > + >;
>> > + };
>>
>> You add the entries for 100MHz and 200MHz, but I don't see them being
>> referenced anywhere.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: stm32: add DFSDM pins to stm32mp157c
From: Olivier MOYSAN @ 2019-08-02 8:49 UTC (permalink / raw)
To: Alexandre TORGUE, linux-stm32@st-md-mailman.stormreply.com,
robh@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <a95e5d74-c8e3-42f9-cabf-f42623aee255@st.com>
Hi ALex,
On 8/2/19 10:09 AM, Alexandre Torgue wrote:
> Hi Olivier
>
> On 8/1/19 9:46 AM, Olivier Moysan wrote:
>> Add DFSDM pins to stm32mp157c.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
>> ---
>> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 39 +++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> index 9eaec9bf8cb8..f96a928cbc49 100644
>> --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> @@ -230,6 +230,45 @@
>> };
>> };
>>
>
> I use to only take pinconfig which are used in board. So please resend
> with the "board patch".
>
The DFSDM is one of the interface used in the STM32MP15 soundcard.
This soundcard also uses the Wolfson wm8994 audio codec.
The wm8994 codec driver requires adaptations, and the upstream of
these changes is not planned today.
So, the related board patches cannot be sent.
BRs
Olivier
> regards
> Alex
>
>
>> + dfsdm_clkout_pins_a: dfsdm-clkout-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('B', 13, AF3)>; /* DFSDM_CKOUT */
>> + bias-disable;
>> + drive-push-pull;
>> + slew-rate = <0>;
>> + };
>> + };
>> +
>> + dfsdm_clkout_sleep_pins_a: dfsdm-clkout-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('B', 13, ANALOG)>; /* DFSDM_CKOUT */
>> + };
>> + };
>> +
>> + dfsdm_data1_pins_a: dfsdm-data1-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('C', 3, AF3)>; /* DFSDM_DATA1 */
>> + };
>> + };
>> +
>> + dfsdm_data1_sleep_pins_a: dfsdm-data1-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('C', 3, ANALOG)>; /* DFSDM_DATA1 */
>> + };
>> + };
>> +
>> + dfsdm_data3_pins_a: dfsdm-data3-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('F', 13, AF6)>; /* DFSDM_DATA3 */
>> + };
>> + };
>> +
>> + dfsdm_data3_sleep_pins_a: dfsdm-data3-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('F', 13, ANALOG)>; /* DFSDM_DATA3 */
>> + };
>> + };
>> +
>> ethernet0_rgmii_pins_a: rgmii-0 {
>> pins1 {
>> pinmux = <STM32_PINMUX('G', 5, AF11)>, /* ETH_RGMII_CLK125 */
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH] spi: tle620x: remove stale driver
From: Masahiro Yamada @ 2019-08-02 8:42 UTC (permalink / raw)
To: Mark Brown
Cc: Arnd Bergmann, Linux Kernel Mailing List, Russell King, Ben Dooks,
linux-spi, linux-arm-kernel
In-Reply-To: <20190729105125.GA4787@sirena.org.uk>
On Mon, Jul 29, 2019 at 7:51 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Jul 29, 2019 at 01:13:04AM +0900, Masahiro Yamada wrote:
> > This driver seems obsolte because this driver needs platform data
> > but no one in upstream passes it.
>
> I've not noticed this driver getting in the way of anything?
I just happened to take a look at include/linux/spi/tle62x0.h
when I was working on my header-test feature of Kbuild.
Then, I asked this in ML, and Arnd was positive
for deleting this.
https://lkml.org/lkml/2019/7/21/19
IMHO, stale code should be cleaned away periodically.
Yet, if you are happy with keeping this driver,
that is fine.
If we could get comments from Ben Books,
that would be helpful.
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
From: Michael Nazzareno Trimarchi @ 2019-08-02 8:38 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, devicetree, David Airlie, Michael Turquette,
linux-sunxi, linux-kernel, dri-devel, Chen-Yu Tsai, Rob Herring,
Jagan Teki, Daniel Vetter, linux-amarula, linux-clk,
linux-arm-kernel
In-Reply-To: <CAOf5uwkvCs62zHcUoFuJwau_ZZFdnVf8ua6JY_wzUb9m8rLTTw@mail.gmail.com>
Hi Maxime
On Mon, Jul 29, 2019 at 8:59 AM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++++++++++---------
> > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > > > drm_display_mode *mode,
> > > > > > > }
> > > > > > >
> > > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > - const struct drm_display_mode *mode)
> > > > > > > + const struct drm_display_mode *mode,
> > > > > > > + u32 tcon_mul)
> > > > > > > {
> > > > > > > /* Configure the dot clock */
> > > > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > - u32 tcon_div;
> > > > > > > + u32 tcon_div, tcon_mul;
> > > > > > >
> > > > > > > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > + tcon->dclk_min_div = 4;
> > > > > > > + tcon->dclk_max_div = 127;
> > > > > > >
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + tcon_mul = bpp / lanes;
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > */
> > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
> > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > - block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > + block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > >
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 7;
> > > > > > > tcon->dclk_max_div = 7;
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 6;
> > > > > > > tcon->dclk_max_div = 127;
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > index 5c3ad5be0690..a07090579f84 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > @@ -13,8 +13,6 @@
> > > > > > > #include <drm/drm_encoder.h>
> > > > > > > #include <drm/drm_mipi_dsi.h>
> > > > > > >
> > > > > > > -#define SUN6I_DSI_TCON_DIV 4
> > > > > > > -
> > > > > > > struct sun6i_dsi {
> > > > > > > struct drm_connector connector;
> > > > > > > struct drm_encoder encoder;
> > > > > >
> > > > > > I had more something like this in mind:
> > > > > > http://code.bulix.org/nlp5a4-803511
> > > > >
> > > > > Worth to look at it. was it working on your panel? meanwhile I will check it.
> > > >
> > > > I haven't tested it.
> > > >
> > > > > We have updated with below change [1], seems working on but is
> > > > > actually checking the each divider as before start with 4... till 127.
> > > > >
> > > > > This new approach, is start looking the best divider from 4.. based on
> > > > > the idea vs rounded it will ended up best divider like [2]
> > > >
> > > > But why?
> > > >
> > > > I mean, it's not like it's the first time I'm asking this...
> > > >
> > > > If the issue is what Micheal described, then the divider has nothing
> > > > to do with it. We've had that discussion over and over again.
> > >
> > > This is what Michael is mentioned in above mail "tcon-pixel clock is
> > > the rate that you want to achive on display side and if you have 4
> > > lanes 32bit or lanes and different bit number that you need to have
> > > a clock that is able to put outside bits and speed equal to
> > > pixel-clock * bits / lanes. so If you want a pixel-clock of 40 mhz
> > > and you have 32bits and 4 lanes you need to have a clock of 40 * 32
> > > / 4 in no-burst mode. "
> >
> > Yeah, so we need to change the clock rate.
> >
> > > He is trying to manage the bpp/lanes into dclk_mul (in last mail)
> > > and it can multiply with pixel clock which is rate argument in
> > > sun4i_dclk_round_rate.
> > >
> > > The solution I have mentioned in dclk_min, max is bpp/lanes also
> > > multiple rate in dotclock sun4i_dclk_round_rate.
> > >
> > > In both cases the overall pll_rate depends on dividers, the one that I
> > > have on this patch is based on BSP and the Michael one is more generic
> > > way so-that it can not to touch other functionalities and looping
> > > dividers to find the best one.
> > >
> > > If dclk_min/max is bpp/lanes then dotclock directly using divider 6
> > > (assuming 24-bit and 4 lanes) and return the pll_rate and divider 6
> > > associated.
> > >
> > > if dclk_mul is bpp/lanes, on Michael new change, the dividers start
> > > with 4 and end with 127 but the constant ideal rate which rate *
> > > bpp/lanes but the loop from sun4i_dclk_round_rate computed the divider
> > > as 6 only, ie what I'm mentioned on the above mail.
> >
> > We've been over this a couple of times already.
> >
> > The clock is generated like this:
> >
> > PLL -> TCON Module Clock -> TCON DCLK
> >
> > You want the TCON DCLK to be at the pixel clock rate * bpp /
> > lanes. Fine, that makes sense.
> >
> > Except that the patch you've sent, instead of changing the rate
> > itself, changes the ratio between the module clock and DCLK.
> >
> > And this is where the issue lies. First, from a logical viewpoint, it
> > doesn't make sense. If you want to change the clock rate, then just do
> > it. Don't hack around the multipliers trying to fall back to something
> > that works for you.
> >
> > Then, the ratio itself needs to be set to 4. This is the part that
> > we've discussed way too many times already, but in the Allwinner BSP,
> > that ratio is hardcoded to 4, and we've had panels that need it at
> > that value.
> >
> > So, what you want to do is to have:
> >
> > TCON DCLK = pixel clock * bpp / lanes
> > TCON Module Clock = DCLK * 4
> > PLL = Module Clock * Module Clock Divider (which I believe is 1 in most cases)
>
> pll-mipi 1 1 1 178200000
> 0 0 50000
> tcon0 2 2 1 178200000
> 0 0 50000
> tcon-pixel-clock 1 1 1 29700000
> 0 0 50000
>
> This is an english problem from my side:
> tcon-pixel-clock is DCLK
> tcon0 must be tcon-pixel-clock * bpp / lanes because the logic need to
> put a bit every cycle.
>
> One solution can be:
> - set_rate_exclusive to tcon0 and calculate as display pixel clock *
> bpp / lanes
> - calculate the tcon-pixel-clock using all divider
>
> Problem is that the function that calculate tcon-pixel-clock does not
> have any constrain on the ideal value. What you are
> suggesting is not correct in my opinion or I'm not following your
> suggesstion. What I know is that if we have a pixel-clock
> of dvi display of 50Mhz and we have 4 lanes 32bit we need a clock in
> the logic of 400Mhz (this is the ideal throughtput).
> So tcon-pixel-clock is 50mhz and tcon0 is 400Mhz.
>
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 94f24c5e2dc5..ffb7906054e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -338,8 +338,9 @@ static void sun4i_tcon0_mode_set_cpu(struct
sun4i_tcon *tcon,
u32 block_space, start_delay;
u32 tcon_div;
- tcon->dclk_min_div = bpp/lanes;
- tcon->dclk_max_div = bpp/lanes;
+ tcon->dclk_min_div = 4;
+ tcon->dclk_max_div = 127;
+ clk_set_rate_exclusive(tcon->sclk0, mode->crtc_clock * 1000 * bpp / lanes);
sun4i_tcon0_mode_set_common(tcon, mode);
Something like this on top of jagan proposal
Michael
> Michael
>
>
> >
> > So you want to increase the PLL. Fortunately for use, this is exactly
> > what a call to clk_set_rate will end up doing.
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
>
>
>
> --
> | Michael Nazzareno Trimarchi Amarula Solutions BV |
> | COO - Founder Cruquiuskade 47 |
> | +31(0)851119172 Amsterdam 1018 AM NL |
> | [`as] http://www.amarulasolutions.com |
--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 4/4] ARM: dts: aspeed: Add Mihawk BMC platform
From: Ben Pai @ 2019-08-02 8:37 UTC (permalink / raw)
To: robh+dt, mark.rutland, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
Cc: Ben Pai, wangat
The Mihawk BMC is an ASPEED ast2500 based BMC that is part of an
OpenPower Power9 server.
Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts | 918 ++++++++++++++++++++
2 files changed, 919 insertions(+)
create mode 100755 arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index eb6de52c1936..cdfe0f43ffd3 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1275,6 +1275,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-lenovo-hr630.dtb \
aspeed-bmc-microsoft-olympus.dtb \
aspeed-bmc-opp-lanyang.dtb \
+ aspeed-bmc-opp-mihawk.dtb \
aspeed-bmc-opp-palmetto.dtb \
aspeed-bmc-opp-romulus.dtb \
aspeed-bmc-opp-swift.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
new file mode 100755
index 000000000000..bbf4a4671421
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
@@ -0,0 +1,918 @@
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/leds/leds-pca955x.h>
+
+/ {
+ model = "Mihawk BMC";
+ compatible = "ibm,mihawk-bmc", "aspeed,ast2500";
+
+
+ chosen {
+ stdout-path = &uart5;
+ bootargs = "console=ttyS4,115200 earlyprintk";
+ };
+
+ memory@80000000 {
+ reg = <0x80000000 0x20000000>; /* address and size of RAM(512MB) */
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ flash_memory: region@98000000 {
+ no-map;
+ reg = <0x98000000 0x04000000>; /* 64M */
+ };
+
+ gfx_memory: framebuffer {
+ size = <0x01000000>;
+ alignment = <0x01000000>;
+ compatible = "shared-dma-pool";
+ reusable;
+ };
+
+ video_engine_memory: jpegbuffer {
+ size = <0x02000000>; /* 32MM */
+ alignment = <0x01000000>;
+ compatible = "shared-dma-pool";
+ reusable;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ air-water {
+ label = "air-water";
+ gpios = <&gpio ASPEED_GPIO(F, 6) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(F, 6)>;
+ };
+
+ checkstop {
+ label = "checkstop";
+ gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(J, 2)>;
+ };
+
+ ps0-presence {
+ label = "ps0-presence";
+ gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(Z, 2)>;
+ };
+
+ ps1-presence {
+ label = "ps1-presence";
+ gpios = <&gpio ASPEED_GPIO(Z, 0) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(Z, 0)>;
+ };
+ id-button {
+ label = "id-button";
+ gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(F, 1)>;
+ };
+ };
+
+ gpio-keys-polled {
+ compatible = "gpio-keys-polled";
+ poll-interval = <1000>;
+
+ fan0-presence {
+ label = "fan0-presence";
+ gpios = <&pca9552 9 GPIO_ACTIVE_LOW>;
+ linux,code = <9>;
+ };
+
+ fan1-presence {
+ label = "fan1-presence";
+ gpios = <&pca9552 10 GPIO_ACTIVE_LOW>;
+ linux,code = <10>;
+ };
+
+ fan2-presence {
+ label = "fan2-presence";
+ gpios = <&pca9552 11 GPIO_ACTIVE_LOW>;
+ linux,code = <11>;
+ };
+
+ fan3-presence {
+ label = "fan3-presence";
+ gpios = <&pca9552 12 GPIO_ACTIVE_LOW>;
+ linux,code = <12>;
+ };
+
+ fan4-presence {
+ label = "fan4-presence";
+ gpios = <&pca9552 13 GPIO_ACTIVE_LOW>;
+ linux,code = <13>;
+ };
+
+ fan5-presence {
+ label = "fan5-presence";
+ gpios = <&pca9552 14 GPIO_ACTIVE_LOW>;
+ linux,code = <14>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ fault {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ power {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 1) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-id {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-g {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 4) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-ok {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(Y, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ fan0 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 0 GPIO_ACTIVE_LOW>;
+ };
+
+ fan1 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 1 GPIO_ACTIVE_LOW>;
+ };
+
+ fan2 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 2 GPIO_ACTIVE_LOW>;
+ };
+
+ fan3 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 3 GPIO_ACTIVE_LOW>;
+ };
+
+ fan4 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 4 GPIO_ACTIVE_LOW>;
+ };
+
+ fan5 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 5 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ fsi: gpio-fsi {
+ compatible = "fsi-master-gpio", "fsi-master";
+ #address-cells = <2>;
+ #size-cells = <0>;
+ no-gpio-delays;
+
+ clock-gpios = <&gpio ASPEED_GPIO(E, 6) GPIO_ACTIVE_HIGH>;
+ data-gpios = <&gpio ASPEED_GPIO(E, 7) GPIO_ACTIVE_HIGH>;
+ mux-gpios = <&gpio ASPEED_GPIO(E, 5) GPIO_ACTIVE_HIGH>;
+ enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
+ trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
+ };
+ iio-hwmon-12v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 0>;
+ };
+
+ iio-hwmon-5v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 1>;
+ };
+
+ iio-hwmon-3v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 2>;
+ };
+
+ iio-hwmon-vdd0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 3>;
+ };
+
+ iio-hwmon-vdd1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 4>;
+ };
+
+ iio-hwmon-vcs0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 5>;
+ };
+
+ iio-hwmon-vcs1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 6>;
+ };
+
+ iio-hwmon-vdn0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 7>;
+ };
+
+ iio-hwmon-vdn1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 8>;
+ };
+
+ iio-hwmon-vio0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 9>;
+ };
+
+ iio-hwmon-vio1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 10>;
+ };
+
+ iio-hwmon-vddra {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 11>;
+ };
+
+ iio-hwmon-battery {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 12>;
+ };
+
+ iio-hwmon-vddrb {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 13>;
+ };
+
+ iio-hwmon-vddrc {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 14>;
+ };
+
+ iio-hwmon-vddrd {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 15>;
+ };
+};
+
+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
+ &pinctrl_pwm2_default &pinctrl_pwm3_default
+ &pinctrl_pwm4_default &pinctrl_pwm5_default>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ };
+
+ fan@2 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ };
+
+ fan@3 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x03>;
+ };
+
+ fan@4 {
+ reg = <0x04>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x04>;
+ };
+
+ fan@5 {
+ reg = <0x05>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x05>;
+ };
+
+ fan@6 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x06>;
+ };
+
+ fan@7 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x07>;
+ };
+
+ fan@8 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x08>;
+ };
+
+ fan@9 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x09>;
+ };
+
+ fan@10 {
+ reg = <0x04>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0a>;
+ };
+
+ fan@11 {
+ reg = <0x05>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0b>;
+ };
+};
+
+&fmc {
+ status = "okay";
+ flash@0 {
+ status = "okay";
+ label = "bmc";
+ m25p,fast-read;
+ spi-max-frequency = <50000000>;
+ partitions {
+ #address-cells = < 1 >;
+ #size-cells = < 1 >;
+ compatible = "fixed-partitions";
+ u-boot@0 {
+ reg = < 0 0x60000 >;
+ label = "u-boot";
+ };
+ u-boot-env@60000 {
+ reg = < 0x60000 0x20000 >;
+ label = "u-boot-env";
+ };
+ obmc-ubi@80000 {
+ reg = < 0x80000 0x1F80000 >;
+ label = "obmc-ubi";
+ };
+ };
+ };
+ flash@1 {
+ status = "okay";
+ label = "alt-bmc";
+ m25p,fast-read;
+ spi-max-frequency = <50000000>;
+ partitions {
+ #address-cells = < 1 >;
+ #size-cells = < 1 >;
+ compatible = "fixed-partitions";
+ u-boot@0 {
+ reg = < 0 0x60000 >;
+ label = "alt-u-boot";
+ };
+ u-boot-env@60000 {
+ reg = < 0x60000 0x20000 >;
+ label = "alt-u-boot-env";
+ };
+ obmc-ubi@80000 {
+ reg = < 0x80000 0x1F80000 >;
+ label = "alt-obmc-ubi";
+ };
+ };
+ };
+};
+
+&spi1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1_default>;
+
+ flash@0 {
+ status = "okay";
+ label = "pnor";
+ m25p,fast-read;
+ spi-max-frequency = <100000000>;
+ };
+};
+
+&lpc_ctrl {
+ status = "okay";
+ memory-region = <&flash_memory>;
+ flash = <&spi1>;
+};
+
+&uart1 {
+ /* Rear RS-232 connector */
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd1_default
+ &pinctrl_rxd1_default
+ &pinctrl_nrts1_default
+ &pinctrl_ndtr1_default
+ &pinctrl_ndsr1_default
+ &pinctrl_ncts1_default
+ &pinctrl_ndcd1_default
+ &pinctrl_nri1_default>;
+};
+
+&uart2 {
+ /* APSS */
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
+};
+
+&uart5 {
+ status = "okay";
+};
+
+&mac0 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rmii1_default>;
+ use-ncsi;
+};
+
+&mac1 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
+};
+
+&i2c0 {
+ status = "disabled";
+};
+
+&i2c1 {
+ status = "disabled";
+};
+
+&i2c2 {
+ status = "okay";
+
+ /* SAMTEC P0 */
+ /* SAMTEC P1 */
+
+};
+
+&i2c3 {
+ status = "okay";
+
+ /* APSS */
+ /* CPLD */
+
+ /* PCA9516 (repeater) ->
+ * CLK Buffer 9FGS9092
+ * CLK Buffer 9DBL0651BKILFT
+ * CLK Buffer 9DBL0651BKILFT
+ * Power Supply 0
+ * Power Supply 1
+ * PCA 9552 LED
+ */
+
+ power-supply@58 {
+ compatible = "ibm,cffps1";
+ reg = <0x58>;
+ };
+
+ power-supply@5b {
+ compatible = "ibm,cffps1";
+ reg = <0x5b>;
+ };
+
+ pca9552: pca9552@60 {
+ compatible = "nxp,pca9552";
+ reg = <0x60>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ gpio@0 {
+ reg = <0>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@1 {
+ reg = <1>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@2 {
+ reg = <2>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@3 {
+ reg = <3>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@4 {
+ reg = <4>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@5 {
+ reg = <5>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@6 {
+ reg = <6>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@7 {
+ reg = <7>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@8 {
+ reg = <8>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@9 {
+ reg = <9>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@10 {
+ reg = <10>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@11 {
+ reg = <11>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@12 {
+ reg = <12>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@13 {
+ reg = <13>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@14 {
+ reg = <14>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@15 {
+ reg = <15>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+
+ };
+
+};
+
+&i2c4 {
+ status = "okay";
+
+ /* CP0 VDD & VCS : IR35221 */
+ /* CP0 VDN : IR35221 */
+ /* CP0 VIO : IR38064 */
+ /* CP0 VDDR : PXM1330 */
+
+ ir35221@70 {
+ compatible = "infineon,ir35221";
+ reg = <0x70>;
+ };
+
+ ir35221@72 {
+ compatible = "infineon,ir35221";
+ reg = <0x72>;
+ };
+
+};
+
+&i2c5 {
+ status = "okay";
+
+ /* CP0 VDD & VCS : IR35221 */
+ /* CP0 VDN : IR35221 */
+ /* CP0 VIO : IR38064 */
+ /* CP0 VDDR : PXM1330 */
+
+ ir35221@70 {
+ compatible = "infineon,ir35221";
+ reg = <0x70>;
+ };
+
+ ir35221@72 {
+ compatible = "infineon,ir35221";
+ reg = <0x72>;
+ };
+
+};
+
+&i2c6 {
+ status = "okay";
+
+ /* pca9548 -> NVMe1 to 8 */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+
+};
+
+&i2c7 {
+ status = "okay";
+
+ /* pca9548 -> NVMe9 to 16 */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+
+};
+
+&i2c8 {
+ status = "okay";
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+};
+
+&i2c9 {
+ status = "okay";
+
+ /* pca9545 Riser ->
+ * PCIe x8 Slot3
+ * PCIe x16 slot4
+ * PCIe x8 slot5
+ * I2C BMC RISER PCA9554
+ * BMC SCL/SDA PCA9554
+ * PCA9554
+ */
+
+ /* pca9545 ->
+ * PCIe x16 Slot1
+ * PCIe x8 slot2
+ * PEX8748
+ */
+
+ pca9545riser@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ pca9545@71 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x71>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+};
+
+&i2c10 {
+ status = "okay";
+
+ /* pca9545 Riser ->
+ * PCIe x8 Slot8
+ * PCIe x16 slot9
+ * PCIe x8 slot10
+ * I2C BMC RISER PCA9554
+ * BMC SCL/SDA PCA9554
+ * PCA9554
+ */
+
+ /* pca9545 ->
+ * PCIe x16 Slot1
+ * PCIe x8 slot2
+ * PEX8748
+ */
+
+ pca9545riser@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ pca9545@71 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x71>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+};
+
+&i2c11 {
+ status = "okay";
+
+ /* TPM */
+ /* RTC RX8900CE */
+ /* FPGA for power sequence */
+ /* TMP275A */
+ /* TMP275A */
+ /* EMC1462 */
+
+ tpm@57 {
+ compatible = "infineon,slb9645tt";
+ reg = <0x57>;
+ };
+
+ rtc@32 {
+ compatible = "epson,rx8900";
+ reg = <0x32>;
+ };
+
+ tmp275@48 {
+ compatible = "ti,tmp275";
+ reg = <0x48>;
+ };
+
+ tmp275@49 {
+ compatible = "ti,tmp275";
+ reg = <0x49>;
+ };
+
+ /* chip emc1462 use emc1403 driver */
+ emc1403@4c {
+ compatible = "smsc,emc1403";
+ reg = <0x4c>;
+ };
+
+};
+
+&i2c12 {
+ status = "okay";
+
+ /* pca9545 ->
+ * SAS BP1
+ * SAS BP2
+ * NVMe BP
+ * M.2 riser
+ */
+
+ pca9545@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+
+ tmp275@48 {
+ compatible = "ti,tmp275";
+ reg = <0x48>;
+ };
+ };
+
+ };
+
+};
+
+&i2c13 {
+ status = "okay";
+
+ /* pca9548 ->
+ * NVMe BP
+ * NVMe HDD17 to 24
+ */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+};
+
+&vuart {
+ status = "okay";
+};
+
+&gfx {
+ status = "okay";
+ memory-region = <&gfx_memory>;
+};
+
+&adc {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_adc0_default
+ &pinctrl_adc1_default
+ &pinctrl_adc2_default
+ &pinctrl_adc3_default
+ &pinctrl_adc4_default
+ &pinctrl_adc5_default
+ &pinctrl_adc6_default
+ &pinctrl_adc7_default
+ &pinctrl_adc8_default
+ &pinctrl_adc9_default
+ &pinctrl_adc10_default
+ &pinctrl_adc11_default
+ &pinctrl_adc12_default
+ &pinctrl_adc13_default
+ &pinctrl_adc14_default
+ &pinctrl_adc15_default>;
+};
+
+&wdt1 {
+ aspeed,reset-type = "none";
+ aspeed,external-signal;
+ aspeed,ext-push-pull;
+ aspeed,ext-active-high;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdtrst1_default>;
+};
+
+&wdt2 {
+ aspeed,alt-boot;
+};
+
+&ibt {
+ status = "okay";
+};
+
+&vhub {
+ status = "okay";
+};
+
+&video {
+ status = "okay";
+ memory-region = <&video_engine_memory>;
+};
+
+#include "ibm-power9-dual.dtsi"
+
--
2.17.1
---------------------------------------------------------------------------------------------------------------------------------------------------------------
This email contains confidential or legally privileged information and is for the sole use of its intended recipient.
Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
---------------------------------------------------------------------------------------------------------------------------------------------------------------
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-08-02 8:28 UTC (permalink / raw)
To: Jerry-ch Chen
Cc: devicetree, Sean.Cheng, laurent.pinchart+renesas, Rynn.Wu,
christie.yu, srv_heupstream, po-yang.huang, suleiman, shik,
jungo.lin, sj.huang, yuzhao, hans.verkuil, zwisler, frederic.chen,
matthias.bgg, linux-mediatek, mchehab, linux-arm-kernel,
linux-media
In-Reply-To: <1562661672-22439-5-git-send-email-Jerry-Ch.chen@mediatek.com>
Hi Jerry,
On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
>
> This patch adds the driver of Face Detection (FD) unit in
> Mediatek camera system, providing face detection function.
>
> The mtk-isp directory will contain drivers for multiple IP
> blocks found in Mediatek ISP system. It will include ISP Pass 1
> driver (CAM), sensor interface driver, DIP driver and face
> detection driver.
>
> Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> ---
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/mtk-isp/fd/Makefile | 5 +
> drivers/media/platform/mtk-isp/fd/mtk_fd.h | 157 +++
> drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1259 +++++++++++++++++++++++++
> include/uapi/linux/v4l2-controls.h | 4 +
> 5 files changed, 1427 insertions(+)
> create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile
> create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h
> create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
>
Thanks for the patch! I finally got a chance to fully review the code. Sorry
for the delay. Please check my comments inline.
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index e6deb25..8b817cc 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -94,6 +94,8 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP) += mtk-mdp/
>
> obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk-jpeg/
>
> +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-isp/fd/
> +
> obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom/camss/
>
> obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
> diff --git a/drivers/media/platform/mtk-isp/fd/Makefile b/drivers/media/platform/mtk-isp/fd/Makefile
> new file mode 100644
> index 0000000..9b1c501
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +mtk-fd-objs += mtk_fd_40.o
> +
> +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-fd.o
> \ No newline at end of file
> diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> new file mode 100644
> index 0000000..289999b
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +//
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +#ifndef __MTK_FD_HW_H__
> +#define __MTK_FD_HW_H__
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> +
> +/* Control the user defined image widths and heights
> + * to be scaled and performed face detection in FD HW.
> + * MTK FD support up to 14 user defined image sizes to perform face detection.
> + */
> +#define V4L2_CID_MTK_FD_SCALE_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 1)
> +#define V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 2)
> +
> +/* Control the numbers of user defined image sizes.
> + * The default value is 0 which means user is not going
> + * to define the specific image sizes.
> + */
> +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 3)
> +
> +/* Control the Face Pose to be detected.
> + * Here describe the value as following:
> + * {0, detect the front face with rotation from 0 to 270 degrees},
> + * {1, detect the front face with rotation from 0 to 240 and 300 degrees},
> + * {2, detect the front face with rotation from 0 to 240 and 330 degrees},
> + * {3, detect the front face with rotation from 0 to 240 and left side face}.
> + */
> +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 4)
> +#define V4L2_CID_MTK_FD_DETECT_SPEEDUP (V4L2_CID_USER_MTK_FD_BASE + 5)
> +#define V4L2_CID_MTK_FD_EXTRA_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6)
> +
> +/* We reserve 16 controls for this driver. */
> +#define V4L2_CID_MTK_FD_MAX 16
> +
> +#define ENABLE_FD 0x111
> +#define FD_HW_ENABLE 0x4
> +#define FD_INT_EN 0x15c
> +#define FD_INT 0x168
> +#define FD_RESULT 0x178
> +#define FD_IRQ_MASK 0x001
> +
> +#define RS_MAX_BUF_SIZE 2288788
> +#define FD_MAX_SPEEDUP 7
> +#define FD_MAX_POSE_VAL 0xfffffffffffffff
> +#define FD_DEF_POSE_VAL 0x3ff
> +#define MAX_FD_SEL_NUM 1026
> +
> +/* The max. number of face sizes could be detected, for feature scaling */
> +#define FACE_SIZE_NUM_MAX 14
> +/* FACE_SIZE_NUM_MAX + 1, first scale for input image W/H */
> +#define FD_SCALE_NUM 15
> +
> +enum fd_state {
> + FD_ENQ,
> + FD_CBD,
> +};
> +
> +enum fd_img_format {
> + FMT_VYUY = 2,
> + FMT_UYVY,
> + FMT_YVYU,
> + FMT_YUYV,
> + FMT_UNKNOWN
> +};
Please use #define macros for hardware/firmware interface definitions.
> +
> +struct fd_buffer {
> + __u32 scp_addr; /* used by SCP */
> + __u32 dma_addr; /* used by DMA HW */
> +} __packed;
> +
> +enum fd_scp_cmd {
> + FD_CMD_INIT,
> + FD_CMD_ENQUEUE,
> +};
Ditto.
> +
> +struct fd_user_output {
> + __u64 results[MAX_FD_SEL_NUM];
> + __u16 number;
> +};
> +
> +struct user_param {
> + u8 fd_pose;
> + u8 fd_speedup;
> + u8 fd_extra_model;
> + u8 scale_img_num;
> + u8 src_img_fmt;
> + __u16 scale_img_width[FD_SCALE_NUM];
> + __u16 scale_img_height[FD_SCALE_NUM];
> +} __packed;
> +
> +struct fd_hw_param {
> + struct fd_buffer src_img;
> + struct fd_buffer user_result;
> + struct user_param user_param;
> +} __packed;
> +
> +struct cmd_init_info {
> + struct fd_buffer fd_manager;
> + __u32 rs_dma_addr;
> +} __packed;
> +
> +struct ipi_message {
> + u8 cmd_id;
> + union {
> + struct cmd_init_info init_param;
> + struct fd_hw_param hw_param;
> + };
> +} __packed;
> +
> +struct mtk_fd_hw {
> + struct clk *fd_clk;
> + struct rproc *rproc_handle;
> + struct platform_device *scp_pdev;
> + struct fd_buffer scp_mem;
> + wait_queue_head_t wq;
> + void __iomem *fd_base;
> + atomic_t fd_user_cnt;
> + enum fd_state state;
> + u32 fd_irq_result;
> + /* Ensure only one job in hw */
> + struct mutex fd_hw_lock;
> +};
> +
> +struct mtk_fd_dev {
> + struct platform_device *pdev;
> + struct v4l2_device v4l2_dev;
> + struct v4l2_m2m_dev *m2m_dev;
> + struct media_device mdev;
> + struct video_device vfd;
> + struct mtk_fd_hw fd_hw;
Could we just put the contents of that struct directly here? That should
simplify dereference chains in the code.
> + /* Lock for V4L2 operations */
> + struct mutex vfd_lock;
> +};
> +
> +struct mtk_fd_ctx {
> + struct mtk_fd_dev *fd_dev;
> + struct device *dev;
> + struct v4l2_fh fh;
> + struct v4l2_ctrl_handler hdl;
> + struct v4l2_pix_format_mplane src_fmt;
> + struct v4l2_meta_format dst_fmt;
> + struct user_param user_param;
> +};
> +
> +#endif/*__MTK_FD_HW_H__*/
> diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> new file mode 100644
> index 0000000..246d3aa
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> @@ -0,0 +1,1259 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_data/mtk_scp.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/remoteproc.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <linux/wait.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-mem2mem.h>
> +#include <media/videobuf2-core.h>
> +
> +#include "mtk_fd.h"
> +
> +static struct v4l2_meta_format fw_param_fmts[] = {
const?
Also, isn't this the buffer with the detected faces rather than params?
> + {
> + .dataformat = V4L2_META_FMT_MTISP_PARAMS,
This should use its own format.
> + .buffersize = 1024 * 30,
Please define a C struct for this buffer and use sizeof() here.
> + },
> +};
Actually, is there a reason to have this array at all if there is only 1
format supported? I think we could just put the right constants directly in
the code.
> +
> +static const struct v4l2_pix_format_mplane in_img_fmts[] = {
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_VYUY,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_YUYV,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_YVYU,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_UYVY,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
If all the formats have the same values for a field, there is no reason to
have the field initialized here. Just make initialize them to the constant
values inside TRY_FMT.
Hmm, are the interleaved YUV formats the only formats supported by this
hardware? That would be quite unfortunate given the formats supported by the
other IP blocks on this SoC use the more typical planar formats.
> + },
> +};
> +
> +#define NUM_FORMATS ARRAY_SIZE(in_img_fmts)
> +
> +static inline struct mtk_fd_dev *mtk_fd_hw_to_dev(struct mtk_fd_hw *fd_hw)
> +{
> + return container_of(fd_hw, struct mtk_fd_dev, fd_hw);
> +}
> +
> +static inline struct mtk_fd_ctx *fh_to_ctx(struct v4l2_fh *fh)
> +{
> + return container_of(fh, struct mtk_fd_ctx, fh);
> +}
> +
> +static int mtk_fd_load_scp(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + phandle rproc_phandle;
> + int ret;
> +
> + /* init scp */
> + fd_hw->scp_pdev = scp_get_pdev(fd_dev->pdev);
> + if (!fd_hw->scp_pdev) {
> + dev_err(dev, "Failed to get scp device\n");
> + return -ENODEV;
> + }
> +
> + if (of_property_read_u32(fd_dev->pdev->dev.of_node, "mediatek,scp",
> + &rproc_phandle)) {
> + dev_err(dev, "Could not get scp device\n");
> + return -EINVAL;
> + }
> +
> + fd_hw->rproc_handle = rproc_get_by_phandle(rproc_phandle);
> + if (!fd_hw->rproc_handle) {
> + dev_err(dev, "Could not get FD's rproc_handle\n");
> + return -EINVAL;
> + }
> +
> + ret = rproc_boot(fd_hw->rproc_handle);
> + if (ret < 0) {
> + /**
> + * Return 0 if downloading firmware successfully,
> + * otherwise it is failed
> + */
> + dev_err(dev, "rproc_boot failed\n");
> + return -ENODEV;
> + }
> +
> + return ret;
> +}
> +
> +static dma_addr_t mtk_fd_hw_alloc_rs_dma_addr(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + void *va;
> + dma_addr_t dma_handle;
> +
> + va = dma_alloc_coherent(dev, RS_MAX_BUF_SIZE, &dma_handle, GFP_KERNEL);
I see this allocated here, but I don't see this freed anywhere.
> + if (!va) {
> + dev_err(dev, "dma_alloc null va\n");
> + return -ENOMEM;
> + }
> + memset(va, 0, RS_MAX_BUF_SIZE);
> +
> + return dma_handle;
> +}
> +
> +static int mtk_fd_send_ipi_init(struct mtk_fd_hw *fd_hw)
> +{
> + struct ipi_message fd_init_msg;
> + dma_addr_t rs_dma_addr;
> +
> + fd_init_msg.cmd_id = FD_CMD_INIT;
> +
> + fd_init_msg.init_param.fd_manager.scp_addr = fd_hw->scp_mem.scp_addr;
> + fd_init_msg.init_param.fd_manager.dma_addr = fd_hw->scp_mem.dma_addr;
> +
> + rs_dma_addr = mtk_fd_hw_alloc_rs_dma_addr(fd_hw);
> + if (!rs_dma_addr)
> + return -ENOMEM;
> +
> + fd_init_msg.init_param.rs_dma_addr = rs_dma_addr;
> +
> + return scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_init_msg,
> + sizeof(fd_init_msg), 0);
> +}
> +
> +static int mtk_fd_hw_enable(struct mtk_fd_hw *fd_hw)
> +{
> + int ret;
> +
> + ret = mtk_fd_load_scp(fd_hw);
> + if (ret)
> + return ret;
> +
> + ret = mtk_fd_send_ipi_init(fd_hw);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_hw_connect(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + s32 usercount;
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
First of all, we don't need a mutex here, because all the V4L2 ioctls are
already running with the video device mutex.
> + usercount = atomic_inc_return(&fd_hw->fd_user_cnt);
If we already use a mutex, there is no point in using atomic types.
> + if (usercount == 1) {
> + pm_runtime_get_sync(&fd_dev->pdev->dev);
> + if (mtk_fd_hw_enable(fd_hw)) {
> + pm_runtime_put_sync(&fd_dev->pdev->dev);
> + atomic_dec_return(&fd_hw->fd_user_cnt);
> + mutex_unlock(&fd_hw->fd_hw_lock);
> + return -EINVAL;
> + }
> + }
This is a simple mem-to-mem device, so there is no reason to keep it active
all the time it's streaming. Please just get the runtime PM counter when
queuing a job to the hardware and release it when the job finishes.
I guess we might still want to do the costly operations like rproc_boot()
when we start streaming, though.
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +
> + return 0;
> +}
> +
> +static int mtk_fd_wait_irq(struct mtk_fd_hw *fd_hw)
> +{
> + int timeout;
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> +
> + timeout = wait_event_interruptible_timeout
> + (fd_hw->wq, (fd_hw->fd_irq_result & FD_IRQ_MASK),
> + usecs_to_jiffies(1000000));
> + if (!timeout) {
> + dev_err(dev, "%s timeout, %d\n", __func__,
> + fd_hw->fd_irq_result);
> + return -EAGAIN;
> + } else if (!(fd_hw->fd_irq_result & FD_IRQ_MASK)) {
> + dev_err(dev, "%s No IRQ mask:0x%8x\n",
> + __func__, fd_hw->fd_irq_result);
> + return -EINVAL;
> + }
> + fd_hw->fd_irq_result = 0;
> +
> + return 0;
> +}
> +
> +static void mtk_fd_hw_disconnect(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + s32 usercount;
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
> + atomic_dec_return(&fd_hw->fd_user_cnt);
> + usercount = atomic_read(&fd_hw->fd_user_cnt);
> + if (usercount == 0) {
> + if (fd_hw->state == FD_ENQ)
> + mtk_fd_wait_irq(fd_hw);
This shouldn't be possible to happen as the queues should be already stopped
at this point.
> +
> + pm_runtime_put_sync(&fd_dev->pdev->dev);
Any reason to use pm_runtime_put_sync() over pm_runtime_put()?
> + rproc_shutdown(fd_hw->rproc_handle);
> + rproc_put(fd_hw->rproc_handle);
> + }
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +}
> +
> +static void mtk_fd_hw_job_finish(struct mtk_fd_hw *fd_hw,
> + struct fd_hw_param *fd_param,
> + enum vb2_buffer_state vb_state)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct mtk_fd_ctx *ctx;
> + struct device *dev = &fd_dev->pdev->dev;
> + struct vb2_buffer *src_vb, *dst_vb;
> + struct vb2_v4l2_buffer *src_vbuf = NULL, *dst_vbuf = NULL;
> +
> + ctx = v4l2_m2m_get_curr_priv(fd_dev->m2m_dev);
> + if (!ctx) {
> + dev_err(dev, "Instance released before end of transaction\n");
> + return;
> + }
This shouldn't be possible. I suspect you're seeing this because
.stop_streaming is not implemented correctly. See my comment there.
> +
> + src_vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + if (WARN_ON(!src_vb))
> + return;
This shouldn't be possible.
> + src_vbuf = to_vb2_v4l2_buffer(src_vb);
> +
> + dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + if (WARN_ON(!dst_vb))
> + return;
Ditto.
> + dst_vbuf = to_vb2_v4l2_buffer(dst_vb);
> +
> + dst_vbuf->vb2_buf.timestamp = src_vbuf->vb2_buf.timestamp;
> + dst_vbuf->timecode = src_vbuf->timecode;
> + dst_vbuf->flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
> + dst_vbuf->flags |= src_vbuf->flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
We should be able to just use v4l2_m2m_buf_copy_metadata().
> +
> + v4l2_m2m_buf_done(src_vbuf, vb_state);
> + v4l2_m2m_buf_done(dst_vbuf, vb_state);
> + v4l2_m2m_job_finish(fd_dev->m2m_dev, ctx->fh.m2m_ctx);
> +}
> +
> +static int mtk_fd_hw_job_exec(struct mtk_fd_hw *fd_hw,
> + struct fd_hw_param *fd_param,
> + void *output_vaddr)
> +{
> + struct fd_user_output *fd_output;
> + struct ipi_message fd_ipi_msg;
> + int ret;
> + u32 num;
> +
> + if (fd_param->user_param.src_img_fmt == FMT_UNKNOWN)
> + goto param_err;
Is this possible?
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
> + fd_hw->state = FD_ENQ;
What is this state for?
> + fd_ipi_msg.cmd_id = FD_CMD_ENQUEUE;
> + memcpy(&fd_ipi_msg.hw_param, fd_param, sizeof(fd_ipi_msg.hw_param));
> + ret = scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg,
> + sizeof(fd_ipi_msg), 0);
> + if (ret)
> + goto buf_err;
> +
> + ret = mtk_fd_wait_irq(fd_hw);
> + if (ret)
> + goto buf_err;
This function is called from device_run and it shouldn't be synchronous. It
should only queue the job to the hardware to be handled asynchronously when
it finishes.
> +
> + num = readl(fd_hw->fd_base + FD_RESULT);
> + /* Disable FD ISR */
> + writel(0x0, fd_hw->fd_base + FD_INT_EN);
> +
> + fd_output = (struct fd_user_output *)output_vaddr;
> + fd_output->number = num;
> + fd_hw->state = FD_CBD;
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +
> + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_DONE);
> + return 0;
> +
> +buf_err:
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +param_err:
> + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_ERROR);
> + return ret;
> +}
> +
> +static int mtk_fd_vb2_buf_out_validate(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> +
> + v4l2_buf->field = V4L2_FIELD_NONE;
We need to fail with -EINVAL if v4l2_buf->field was different than
V4L2_FIELD_ANY or _NONE.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_vb2_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vb2_queue *vq = vb->vb2_queue;
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct device *dev = ctx->dev;
> + struct v4l2_pix_format_mplane *pixfmt;
> +
> + switch (vq->type) {
> + case V4L2_BUF_TYPE_META_CAPTURE:
> + if (vb2_plane_size(vb, 0) < ctx->dst_fmt.buffersize) {
> + dev_err(dev, "meta size %d is too small\n");
Please make this dev_dbg(), because userspace misbehavior must not trigger
kernel error logs.
> + return -EINVAL;
> + }
> + break;
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + pixfmt = &ctx->src_fmt;
> +
> + if (vbuf->field == V4L2_FIELD_ANY)
> + vbuf->field = V4L2_FIELD_NONE;
> +
> + if (vb->num_planes != 1 || vbuf->field != V4L2_FIELD_NONE) {
> + dev_err(dev, "plane or field %d not supported\n",
> + vb->num_planes, vbuf->field);
Ditto.
> + return -EINVAL;
> + }
> + if (vb2_plane_size(vb, 0) < pixfmt->plane_fmt[0].sizeimage) {
> + dev_err(dev, "plane %d is too small\n");
Ditto.
> + return -EINVAL;
> + }
> + break;
> + default:
> + dev_err(dev, "invalid queue type: %d\n", vq->type);
> + return -EINVAL;
We shouldn't need to handle this.
> + }
> +
> + return 0;
> +}
> +
> +static void mtk_fd_vb2_buf_queue(struct vb2_buffer *vb)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> +}
> +
> +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> + unsigned int *num_buffers,
> + unsigned int *num_planes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct device *dev = ctx->dev;
> + unsigned int size;
> +
> + switch (vq->type) {
> + case V4L2_BUF_TYPE_META_CAPTURE:
> + size = ctx->dst_fmt.buffersize;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + size = ctx->src_fmt.plane_fmt[0].sizeimage;
> + break;
> + default:
> + dev_err(dev, "invalid queue type: %d\n", vq->type);
We should need to handle this.
> + return -EINVAL;
> + }
> +
> + if (!*num_planes) {
> + *num_planes = 1;
> + sizes[0] = size;
> + }
We need to handle the case when *num_planes is non-zero. We then need to
check if it's 1 and *sizes >= size and fail with -EINVAL if not.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + return mtk_fd_hw_connect(&ctx->fd_dev->fd_hw);
This would be called twice for every context, once for the VIDEO_OUTPUT
queue and once for the META_CAPTURE queue. Perhaps it would make sense to
just do this for the VIDEO_OUTPUT queue?
> +}
> +
> +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct vb2_buffer *vb;
How do we guarantee here that the hardware isn't still accessing the buffers
removed below?
> +
> + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + else
> + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> +
> + while (vb) {
> + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(vb), VB2_BUF_STATE_ERROR);
> + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + else
> + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + }
We can use v4l2_m2m_buf_remove(). Also we can move the call into the loop
condition:
while ((vb == v4l2_m2m_buf_remove(...)))
v4l2_m2m_buf_done(...);
> +
> + mtk_fd_hw_disconnect(&ctx->fd_dev->fd_hw);
This should also probably be done only for 1 queue. Since the VIDEO_OUTPUT
queue supports requestes, I'd consider it the master one.
> +}
> +
> +static void mtk_fd_vb2_request_complete(struct vb2_buffer *vb)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> +}
> +
> +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> + const struct v4l2_pix_format_mplane *sfmt)
> +{
> + dfmt->width = sfmt->width;
> + dfmt->height = sfmt->height;
> + dfmt->pixelformat = sfmt->pixelformat;
> + dfmt->field = sfmt->field;
> + dfmt->colorspace = sfmt->colorspace;
> + dfmt->num_planes = sfmt->num_planes;
> +
> + /* Use default */
> + dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> + dfmt->xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> + dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> + dfmt->plane_fmt[0].sizeimage =
> + dfmt->height * dfmt->plane_fmt[0].bytesperline;
> + memset(dfmt->reserved, 0, sizeof(dfmt->reserved));
> +}
Could we unify this function with mtk_fd_m2m_try_fmt_out_mp()? That function
should be almost directly reusable for the default format initialization +/-
the arguments passed to it.
> +
> +static const struct v4l2_pix_format_mplane *mtk_fd_find_fmt(u32 format)
> +{
> + unsigned int i;
> + const struct v4l2_pix_format_mplane *dev_fmt;
> +
> + for (i = 0; i < NUM_FORMATS; i++) {
> + dev_fmt = &in_img_fmts[i];
> + if (dev_fmt->pixelformat == format)
> + return dev_fmt;
> + }
> +
> + return NULL;
> +}
> +
> +static int mtk_fd_m2m_querycap(struct file *file, void *fh,
> + struct v4l2_capability *cap)
> +{
> + struct mtk_fd_dev *fd_dev = video_drvdata(file);
> + struct device *dev = &fd_dev->pdev->dev;
> +
> + strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
Please use dev_driver_string().
> + strscpy(cap->card, fd_dev->vfd.name, sizeof(cap->card));
I think we can also make this dev_driver_string().
> + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> + dev_name(&fd_dev->pdev->dev));
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_enum_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_fmtdesc *f)
> +{
> + int i;
> +
> + for (i = 0; i < NUM_FORMATS; ++i) {
> + if (i == f->index) {
> + f->pixelformat = in_img_fmts[i].pixelformat;
> + return 0;
> + }
> + }
Why don't we just check if f->index is within the [0, ARRAY_SIZE()-1] bounds
and then just use it to index the array directly?
> +
> + return -EINVAL;
> +}
> +
> +static int mtk_fd_m2m_try_fmt_out_mp(struct file *file,
> + void *fh,
> + struct v4l2_format *f)
I think we could just shorten the function prefixes to "mtk_fd_".
> +{
> + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> + const struct v4l2_pix_format_mplane *fmt;
> +
> + fmt = mtk_fd_find_fmt(pix_mp->pixelformat);
> + if (!fmt) {
> + /* Get default img fmt */
> + fmt = &in_img_fmts[0];
> + f->fmt.pix.pixelformat = fmt->pixelformat;
> + }
> +
> + /* Use default */
> + pix_mp->field = fmt->field;
> + pix_mp->colorspace = fmt->colorspace;
> + pix_mp->num_planes = fmt->num_planes;
> + pix_mp->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + pix_mp->quantization = V4L2_QUANTIZATION_DEFAULT;
> + pix_mp->xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(pix_mp->colorspace);
> +
> + /* Keep user setting as possible */
> + pix_mp->width = clamp(pix_mp->width,
> + MTK_FD_OUTPUT_MIN_WIDTH,
> + MTK_FD_OUTPUT_MAX_WIDTH);
> + pix_mp->height = clamp(pix_mp->height,
> + MTK_FD_OUTPUT_MIN_HEIGHT,
> + MTK_FD_OUTPUT_MAX_HEIGHT);
> +
> + pix_mp->plane_fmt[0].bytesperline = pix_mp->width * 2;
Please add a comment explaining why this is always 2.
> + pix_mp->plane_fmt[0].sizeimage =
> + pix_mp->plane_fmt[0].bytesperline * pix_mp->height;
> + memset(pix_mp->plane_fmt[0].reserved, 0,
> + sizeof(pix_mp->plane_fmt[0].reserved));
No need to zero this here. The core will handle it.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_g_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> +
> + f->fmt.pix_mp = ctx->src_fmt;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_s_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> + struct vb2_queue *vq;
> +
> + /* Change not allowed if queue is streaming. */
> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> + if (vb2_is_streaming(vq) || vb2_is_busy(vq)) {
vb2_is_busy() is a superset of vb2_is_streaming(), so it's enough to just
check the former.
> + dev_dbg(ctx->dev, "vb2_is_streaming or vb2_is_busy");
> + return -EBUSY;
> + }
> +
> + mtk_fd_m2m_try_fmt_out_mp(file, fh, f);
> + ctx->src_fmt = f->fmt.pix_mp;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_enum_fmt_meta_cap(struct file *file, void *fh,
> + struct v4l2_fmtdesc *f)
> +{
> + if (f->index)
> + return -EINVAL;
> +
> + strscpy(f->description, "Face detection result",
> + sizeof(f->description));
> + f->pixelformat = fw_param_fmts[0].dataformat;
> + f->flags = 0;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_g_fmt_meta_cap(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + f->fmt.meta.dataformat = fw_param_fmts[0].dataformat;
> + f->fmt.meta.buffersize = fw_param_fmts[0].buffersize;
> +
> + return 0;
> +}
> +
> +static const struct vb2_ops mtk_fd_vb2_ops = {
> + .queue_setup = mtk_fd_vb2_queue_setup,
> + .buf_out_validate = mtk_fd_vb2_buf_out_validate,
> + .buf_prepare = mtk_fd_vb2_buf_prepare,
> + .buf_queue = mtk_fd_vb2_buf_queue,
> + .start_streaming = mtk_fd_vb2_start_streaming,
> + .stop_streaming = mtk_fd_vb2_stop_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> + .buf_request_complete = mtk_fd_vb2_request_complete,
> +};
> +
> +static const struct v4l2_ioctl_ops mtk_fd_v4l2_video_out_ioctl_ops = {
> + .vidioc_querycap = mtk_fd_m2m_querycap,
> + .vidioc_enum_fmt_vid_out_mplane = mtk_fd_m2m_enum_fmt_out_mp,
> + .vidioc_g_fmt_vid_out_mplane = mtk_fd_m2m_g_fmt_out_mp,
> + .vidioc_s_fmt_vid_out_mplane = mtk_fd_m2m_s_fmt_out_mp,
> + .vidioc_try_fmt_vid_out_mplane = mtk_fd_m2m_try_fmt_out_mp,
> + .vidioc_enum_fmt_meta_cap = mtk_fd_m2m_enum_fmt_meta_cap,
> + .vidioc_g_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_s_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_try_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static int
> +mtk_fd_queue_init(void *priv, struct vb2_queue *src_vq,
> + struct vb2_queue *dst_vq)
> +{
> + struct mtk_fd_ctx *ctx = priv;
> + int ret;
> +
> + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> + src_vq->supports_requests = true;
> + src_vq->drv_priv = ctx;
> + src_vq->ops = &mtk_fd_vb2_ops;
> + src_vq->mem_ops = &vb2_dma_contig_memops;
> + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + src_vq->lock = &ctx->fd_dev->vfd_lock;
> + src_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> +
> + ret = vb2_queue_init(src_vq);
> + if (ret)
> + return ret;
> +
> + dst_vq->type = V4L2_BUF_TYPE_META_CAPTURE;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> + dst_vq->drv_priv = ctx;
> + dst_vq->ops = &mtk_fd_vb2_ops;
> + dst_vq->mem_ops = &vb2_dma_contig_memops;
> + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + dst_vq->lock = &ctx->fd_dev->vfd_lock;
> + dst_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> +
> + return vb2_queue_init(dst_vq);
> +}
> +
> +static int mtk_fd_dev_g_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct mtk_fd_ctx *ctx = ctrl->priv;
> + int i;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> + for (i = 0; i < ctrl->elems; i++)
> + ctrl->p_new.p_u16[i] =
> + ctx->user_param.scale_img_width[i];
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> + for (i = 0; i < ctrl->elems; i++)
> + ctrl->p_new.p_u16[i] =
> + ctx->user_param.scale_img_height[i];
> + break;
> + case V4L2_CID_MTK_FD_DETECT_POSE:
> + ctrl->val = ctx->user_param.fd_pose;
> + break;
> + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> + ctrl->val = ctx->user_param.fd_speedup;
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> + ctrl->val = ctx->user_param.scale_img_num;
> + break;
> + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> + ctrl->val = ctx->user_param.fd_extra_model;
> + break;
> + default:
> + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> + __func__, ctrl->id, ctrl->val);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_fd_dev_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct mtk_fd_ctx *ctx = ctrl->priv;
> + int i;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> + for (i = 0; i < ctrl->elems; i++)
> + ctx->user_param.scale_img_width[i] =
> + ctrl->p_new.p_u16[i];
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> + for (i = 0; i < ctrl->elems; i++)
> + ctx->user_param.scale_img_height[i] =
> + ctrl->p_new.p_u16[i];
> + break;
> + case V4L2_CID_MTK_FD_DETECT_POSE:
> + ctx->user_param.fd_pose = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> + ctx->user_param.fd_speedup = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> + ctx->user_param.scale_img_num = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> + ctx->user_param.fd_extra_model = ctrl->val;
> + break;
> + default:
> + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> + __func__, ctrl->id, ctrl->val);
> + return -EINVAL;
No need to handle this, as the framework will only pass the controls we
registered earlier to this function.
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops mtk_fd_dev_ctrl_ops = {
> + .g_volatile_ctrl = mtk_fd_dev_g_ctrl,
I don't see any volatile controls below. No need to implement this callback.
> + .s_ctrl = mtk_fd_dev_s_ctrl,
I think you might not even need to implement this function (or just provide
a dummy one that returns 0 if its required) if you just use the controls
directly when preparing the job for the hardware. Check v4l2_ctrl_find().
> +};
> +
> +struct v4l2_ctrl_config mtk_fd_controls[] = {
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_WIDTH,
> + .name = "FD scale image widths",
> + .type = V4L2_CTRL_TYPE_U16,
> + .min = MTK_FD_OUTPUT_MIN_WIDTH,
> + .max = MTK_FD_OUTPUT_MAX_WIDTH,
> + .step = 1,
> + .def = MTK_FD_OUTPUT_MAX_WIDTH,
> + .dims = { FD_SCALE_NUM },
Something wrong with indentation here.
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT,
> + .name = "FD scale image heights",
> + .type = V4L2_CTRL_TYPE_U16,
> + .min = MTK_FD_OUTPUT_MIN_HEIGHT,
> + .max = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .step = 1,
> + .def = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .dims = { FD_SCALE_NUM },
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_NUM,
> + .name = "FD scale size counts",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = FACE_SIZE_NUM_MAX,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_DETECT_POSE,
> + .name = "FD detect face pose",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = 3,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_DETECT_SPEEDUP,
> + .name = "FD detection speedup",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = FD_MAX_SPEEDUP,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_EXTRA_MODEL,
> + .name = "FD use extra model",
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 0,
> + },
> +};
> +
> +static int mtk_fd_ctrls_setup(struct mtk_fd_ctx *ctx)
> +{
> + struct v4l2_ctrl_handler *hdl = &ctx->hdl;
> + struct v4l2_ctrl *ctl;
> + int i;
> +
> + v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_FD_MAX);
> + if (hdl->error)
> + return hdl->error;
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_fd_controls); i++) {
> + ctl = v4l2_ctrl_new_custom(hdl, &mtk_fd_controls[i], ctx);
> + if (hdl->error) {
> + v4l2_ctrl_handler_free(hdl);
> + dev_err(ctx->dev, "Failed to register controls:%d", i);
> + return hdl->error;
> + }
> + }
> +
> + ctx->fh.ctrl_handler = &ctx->hdl;
> + v4l2_ctrl_handler_setup(hdl);
> +
> + return 0;
> +}
> +
> +static unsigned int get_fd_img_fmt(unsigned int fourcc)
> +{
> + switch (fourcc) {
> + case V4L2_PIX_FMT_VYUY:
> + return FMT_VYUY;
> + case V4L2_PIX_FMT_YUYV:
> + return FMT_YUYV;
> + case V4L2_PIX_FMT_YVYU:
> + return FMT_YVYU;
> + case V4L2_PIX_FMT_UYVY:
> + return FMT_UYVY;
> + default:
> + return FMT_UNKNOWN;
> + }
> +}
> +
> +static void init_ctx_fmt(struct mtk_fd_ctx *ctx)
> +{
> + const struct v4l2_pix_format_mplane *fmt;
> +
> + /* Initialize M2M source fmt */
> + fmt = &in_img_fmts[0];
> + mtk_fd_fill_pixfmt_mp(&ctx->src_fmt, fmt);
> +
> + /* Initialize M2M destination fmt */
> + ctx->dst_fmt.buffersize = fw_param_fmts[0].buffersize;
> + ctx->dst_fmt.dataformat = fw_param_fmts[0].dataformat;
Why not just make dst_fmt a pointer and assign &fw_params_fmt[0] there?
> +}
> +
> +/*
> + * V4L2 file operations.
> + */
> +static int mtk_vfd_open(struct file *filp)
> +{
> + struct mtk_fd_dev *fd_dev = video_drvdata(filp);
> + struct video_device *vdev = video_devdata(filp);
> + struct mtk_fd_ctx *ctx;
> + int ret;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->fd_dev = fd_dev;
> + ctx->dev = &fd_dev->pdev->dev;
> +
> + v4l2_fh_init(&ctx->fh, vdev);
> + filp->private_data = &ctx->fh;
> +
> + init_ctx_fmt(ctx);
> +
> + ret = mtk_fd_ctrls_setup(ctx);
> + if (ret) {
> + dev_err(ctx->dev, "Failed to set up controls:%d\n", ret);
> + goto err_fh_ctrl;
> + }
> +
> + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(fd_dev->m2m_dev, ctx,
> + &mtk_fd_queue_init);
> + if (IS_ERR(ctx->fh.m2m_ctx)) {
> + ret = PTR_ERR(ctx->fh.m2m_ctx);
> + goto err_init_ctx;
> + }
> +
> + v4l2_fh_add(&ctx->fh);
> +
> + return 0;
> +
> +err_init_ctx:
> + v4l2_ctrl_handler_free(&ctx->hdl);
> +err_fh_ctrl:
> + v4l2_fh_exit(&ctx->fh);
> + kfree(ctx);
> +
> + return ret;
> +}
> +
> +static int mtk_vfd_release(struct file *filp)
> +{
> + struct mtk_fd_ctx *ctx = container_of(filp->private_data,
> + struct mtk_fd_ctx, fh);
> +
> + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> +
> + v4l2_ctrl_handler_free(&ctx->hdl);
> + v4l2_fh_del(&ctx->fh);
> + v4l2_fh_exit(&ctx->fh);
> +
> + kfree(ctx);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_file_operations fd_video_fops = {
> + .owner = THIS_MODULE,
> + .open = mtk_vfd_open,
> + .release = mtk_vfd_release,
> + .poll = v4l2_m2m_fop_poll,
> + .unlocked_ioctl = video_ioctl2,
> + .mmap = v4l2_m2m_fop_mmap,
We also need the compat_ioctl here for 32-bit userspace on 64-bit kernels.
> +};
> +
> +static void mtk_fd_device_run(void *priv)
> +{
> + struct mtk_fd_ctx *ctx = priv;
> + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> + struct fd_hw_param fd_param;
> + void *fd_result_vaddr;
> +
> + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +
> + memset(&fd_param, 0, sizeof(fd_param));
> +
> + fd_param.src_img.dma_addr =
> + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> + fd_param.user_result.dma_addr =
> + vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> + fd_result_vaddr = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> +
> + ctx->user_param.src_img_fmt = get_fd_img_fmt(ctx->src_fmt.pixelformat);
> + memcpy(&fd_param.user_param, &ctx->user_param, sizeof(ctx->user_param));
> +
> + /* Complete request controls if any */
> + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> +
> + mtk_fd_hw_job_exec(&ctx->fd_dev->fd_hw, &fd_param, fd_result_vaddr);
> +}
> +
> +static struct v4l2_m2m_ops fd_m2m_ops = {
> + .device_run = mtk_fd_device_run,
> +};
> +
> +static int mtk_fd_request_validate(struct media_request *req)
> +{
> + unsigned int count;
> +
> + count = vb2_request_buffer_cnt(req);
> + if (!count)
> + return -ENOENT;
Why -ENOENT?
> + else if (count > 1)
> + return -EINVAL;
> +
> + return vb2_request_validate(req);
> +}
> +
> +static const struct media_device_ops fd_m2m_media_ops = {
> + .req_validate = mtk_fd_request_validate,
> + .req_queue = v4l2_m2m_request_queue,
> +};
> +
> +static int mtk_fd_video_device_register(struct mtk_fd_dev *fd_dev)
> +{
> + struct video_device *vfd = &fd_dev->vfd;
> + struct v4l2_m2m_dev *m2m_dev = fd_dev->m2m_dev;
> + struct device *dev = &fd_dev->pdev->dev;
> + int function, ret;
> +
> + vfd->fops = &fd_video_fops;
> + vfd->release = video_device_release;
> + vfd->lock = &fd_dev->vfd_lock;
> + vfd->v4l2_dev = &fd_dev->v4l2_dev;
> + vfd->vfl_dir = VFL_DIR_M2M;
> + vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> + V4L2_CAP_META_CAPTURE;
> + vfd->ioctl_ops = &mtk_fd_v4l2_video_out_ioctl_ops;
> +
> + strscpy(vfd->name, "MTK-FD-V4L2", sizeof(vfd->name));
Please use dev_driver_string().
> + video_set_drvdata(vfd, fd_dev);
> +
> + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> + if (ret) {
> + dev_err(dev, "Failed to register video device\n");
> + goto err_free_dev;
> + }
> +
> + function = MEDIA_ENT_F_PROC_VIDEO_DECODER;
MEDIA_ENT_F_PROC_VIDEO_STATISTICS fits here much more.
Also nit: Any reason to have this assigned to this intermediate variable
rather than just directly passed to the function?
> + ret = v4l2_m2m_register_media_controller(m2m_dev, vfd, function);
> + if (ret) {
> + dev_err(dev, "Failed to init mem2mem media controller\n");
> + goto err_unreg_video;
> + }
> + return 0;
> +
> +err_unreg_video:
> + video_unregister_device(vfd);
> +err_free_dev:
> + video_device_release(vfd);
> + return ret;
> +}
> +
> +static int mtk_fd_dev_v4l2_init(struct mtk_fd_dev *fd_dev)
> +{
> + struct media_device *mdev = &fd_dev->mdev;
> + struct device *dev = &fd_dev->pdev->dev;
> + int ret;
> +
> + ret = v4l2_device_register(dev, &fd_dev->v4l2_dev);
> + if (ret) {
> + dev_err(dev, "Failed to register v4l2 device\n");
> + return ret;
> + }
> +
> + fd_dev->m2m_dev = v4l2_m2m_init(&fd_m2m_ops);
> + if (IS_ERR(fd_dev->m2m_dev)) {
> + dev_err(dev, "Failed to init mem2mem device\n");
> + ret = PTR_ERR(fd_dev->m2m_dev);
> + goto fail_m2m_dev;
Please name the labels after the next clean-up step to be done, not the
failure.
> + }
> +
> + mdev->dev = dev;
> + strscpy(mdev->model, "MTK-FD-V4L2", sizeof(mdev->model));
Could we just use dev_driver_string() here instead?
> + snprintf(mdev->bus_info, sizeof(mdev->bus_info),
> + "platform:%s", dev_name(dev));
> + media_device_init(mdev);
> + mdev->ops = &fd_m2m_media_ops;
> + fd_dev->v4l2_dev.mdev = mdev;
> +
> + ret = mtk_fd_video_device_register(fd_dev);
> + if (ret) {
> + dev_err(dev, "Failed to register video device\n");
> + goto err_vdev;
> + }
> +
> + ret = media_device_register(mdev);
> + if (ret) {
> + dev_err(dev, "Failed to register mem2mem media device\n");
> + goto fail_mdev;
> + }
> +
> + return 0;
> +
> +fail_mdev:
> + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> + video_unregister_device(&fd_dev->vfd);
> + video_device_release(&fd_dev->vfd);
> +err_vdev:
> + media_device_cleanup(mdev);
> + v4l2_m2m_release(fd_dev->m2m_dev);
> +fail_m2m_dev:
> + v4l2_device_unregister(&fd_dev->v4l2_dev);
> + return ret;
> +}
> +
> +static void mtk_fd_dev_v4l2_release(struct mtk_fd_dev *fd_dev)
> +{
> + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> + video_unregister_device(&fd_dev->vfd);
> + video_device_release(&fd_dev->vfd);
> + media_device_cleanup(&fd_dev->mdev);
> + v4l2_m2m_release(fd_dev->m2m_dev);
> + v4l2_device_unregister(&fd_dev->v4l2_dev);
> +}
> +
> +static irqreturn_t mtk_fd_irq(int irq, void *data)
> +{
> + struct mtk_fd_hw *fd_hw = (struct mtk_fd_hw *)data;
> +
> + fd_hw->fd_irq_result = readl(fd_hw->fd_base + FD_INT);
We should be able to just handle the job completion directly from here.
> + wake_up_interruptible(&fd_hw->wq);
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_fd_hw_get_scp_mem(struct mtk_fd_hw *fd_hw,
> + struct fd_buffer *scp_mem)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + dma_addr_t addr;
> + u32 size;
> +
> + scp_mem->scp_addr = scp_get_reserve_mem_phys(SCP_FD_MEM_ID);
> + size = scp_get_reserve_mem_size(SCP_FD_MEM_ID);
> + if (!scp_mem->scp_addr || !size)
> + return -EPROBE_DEFER;
Why -EPROBE_DEFER? Should we have some way to distinguish between the SCP
driver not being initialized yet and some other error?
> +
> + /* get dma addr address */
> + addr = dma_map_page_attrs(dev, phys_to_page(scp_mem->scp_addr), 0,
> + size, DMA_BIDIRECTIONAL,
Should this be really bidirectional?
> + DMA_ATTR_SKIP_CPU_SYNC);
> + if (dma_mapping_error(dev, addr)) {
> + scp_mem->scp_addr = 0;
> + dev_err(dev, "Failed to map scp addr\n");
> + return -ENOMEM;
> + }
> + scp_mem->dma_addr = addr;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_probe(struct platform_device *pdev)
> +{
> + struct mtk_fd_dev *fd_dev;
nit: Perhaps just fd, to have shorter code?
> + struct device *dev = &pdev->dev;
> + struct mtk_fd_hw *fd_hw;
> + struct resource *res;
> + int irq;
> + int ret;
> +
> + fd_dev = devm_kzalloc(&pdev->dev, sizeof(*fd_dev), GFP_KERNEL);
> + if (!fd_dev)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, fd_dev);
> + fd_hw = &fd_dev->fd_hw;
> + fd_dev->pdev = pdev;
Is pdev useful for anything? Perhaps you want to store &pdev->dev instead?
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "no IRQ:%d resource info\n", irq);
> + return irq;
> + }
> + ret = devm_request_irq(dev, irq, mtk_fd_irq, IRQF_SHARED,
> + dev_driver_string(dev),
> + fd_hw);
Why not just fd_dev?
> + if (ret) {
> + dev_err(dev, "req_irq fail:%d\n", irq);
> + return ret;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + fd_hw->fd_base = devm_ioremap_resource(dev, res);
> + if (!fd_hw->fd_base) {
> + dev_err(dev, "unable to get fd reg base\n");
> + return PTR_ERR(fd_hw->fd_base);
> + }
> +
> + fd_hw->fd_clk = devm_clk_get(dev, "fd");
> + if (IS_ERR(fd_hw->fd_clk)) {
> + dev_err(dev, "cannot get fd_clk_img_fd clock\n");
> + return PTR_ERR(fd_hw->fd_clk);
> + }
> +
> + ret = mtk_fd_hw_get_scp_mem(fd_hw, &fd_hw->scp_mem);
> + if (ret) {
> + dev_err(dev, "scp memory init failed: %d\n", ret);
nit: Could we make the error messages a bit more consistent? For example
"Failed to get IRQ resource (%d)", "Failed to request IRQ (%d)", "Failed to
... (%d)", etc.
> + return ret;
> + }
> +
> + atomic_set(&fd_hw->fd_user_cnt, 0);
> + init_waitqueue_head(&fd_hw->wq);
> + mutex_init(&fd_dev->vfd_lock);
> + mutex_init(&fd_hw->fd_hw_lock);
> + pm_runtime_enable(dev);
> +
> + ret = mtk_fd_dev_v4l2_init(fd_dev);
> + if (ret) {
> + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> + mutex_destroy(&fd_dev->vfd_lock);
> + pm_runtime_disable(&pdev->dev);
Please move the clean-up to an error path on the bottom of the function.
> + dev_err(dev, "v4l2 init failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_fd_remove(struct platform_device *pdev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(&pdev->dev);
> +
> + mtk_fd_dev_v4l2_release(fd_dev);
> + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> + mutex_destroy(&fd_dev->vfd_lock);
> + pm_runtime_disable(&pdev->dev);
The order of calls in remove should normally be the opposite to probe.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_suspend(struct device *dev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + /* suspend FD HW */
> + writel(0x0, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> + writel(0x0, fd_dev->fd_hw.fd_base + FD_INT_EN);
> + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> + return 0;
> +}
> +
> +static int mtk_fd_resume(struct device *dev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + ret = clk_prepare_enable(fd_dev->fd_hw.fd_clk);
> + if (ret < 0) {
> + dev_dbg(dev, "open fd clk failed\n");
> + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> + }
> +
> + /* resume FD HW */
> + writel(ENABLE_FD, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> + if (fd_dev->fd_hw.state == FD_ENQ)
> + writel(0x1, fd_dev->fd_hw.fd_base + FD_INT_EN);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops mtk_fd_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(mtk_fd_suspend, mtk_fd_resume)
> + SET_RUNTIME_PM_OPS(mtk_fd_suspend, mtk_fd_resume, NULL)
We need separate runtime and system PM ops. Their behavior is expected to be
different.
For runtime PM ops, you the functions should just unconditionally power on
or power off the device.
For system PM ops, suspend needs to make sure that no further job is queued
to the hardware and that any job being already processed by the hardware is
completed. Resume needs to resume the processing if there are any further
jobs to be queued to the hardware.
> +};
> +
> +static const struct of_device_id mtk_fd_of_ids[] = {
> + { .compatible = "mediatek,mt8183-fd", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, mtk_fd_of_ids);
> +
> +static struct platform_driver mtk_fd_driver = {
> + .probe = mtk_fd_probe,
> + .remove = mtk_fd_remove,
> + .driver = {
> + .name = "mtk-fd-4.0",
> + .of_match_table = of_match_ptr(mtk_fd_of_ids),
> + .pm = &mtk_fd_pm_ops,
> + }
> +};
> +module_platform_driver(mtk_fd_driver);
> +
> +MODULE_DESCRIPTION("Mediatek FD driver");
> +MODULE_LICENSE("GPL");
GPL v2
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/3] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib
From: Uwe Kleine-König @ 2019-08-02 8:24 UTC (permalink / raw)
To: Schrempf Frieder
Cc: festevam@gmail.com, kernel@pengutronix.de,
geert+renesas@glider.be, Greg Kroah-Hartman,
s.hauer@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com, linux-serial@vger.kernel.org, Jiri Slaby,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <011ac0ac-571e-b898-2b2d-89b9a771b0c1@kontron.de>
Hello,
On Fri, Aug 02, 2019 at 07:56:54AM +0000, Schrempf Frieder wrote:
> On 01.08.19 22:33, Uwe Kleine-König wrote:
> > On Thu, Aug 01, 2019 at 06:45:21PM +0000, Schrempf Frieder wrote:
> >> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> >> index 2b400189be91..54c43e02e375 100644
> >> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> >> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> >> @@ -61,6 +61,9 @@ EXPORT_SYMBOL_GPL(mctrl_gpio_set);
> >> struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
> >> enum mctrl_gpio_idx gidx)
> >> {
> >> + if (gpios == NULL)
> >> + return NULL;
> >> +
> >
> > I wonder why you need this. If GPIOLIB is off this code isn't active and
> > with GPIOLIB calling mctrl_gpio_to_gpiod with a gpios == NULL is a bug
> > that IMHO should not be silently ignored.
> >
> > Am I missing something (again)?
>
> No, you're right. My thoughts were, that if the mctrl_gpio functions are
> allowed to be passed a NULL pointer in general, they all should have a
> NULL check, even if in the current context (GPIOLIB disabled) this code
> is not active. Apparently there are other cases when a NULL pointer is
> passed, see [1]. So you can't really consider gpios == NULL to be a bug
> as this seems to be allowed in general.
OK, then this is another separate commit, right? Preferably with a
comment pointing to drivers that use mctrl_gpio before being
initialized.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/5] dma-mapping: remove ARCH_NO_COHERENT_DMA_MMAP
From: Takashi Iwai @ 2019-08-02 8:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, Michal Simek, linux-parisc, linux-sh, Takashi Iwai,
Robin Murphy, x86, linux-kernel, iommu, linux-m68k, linuxppc-dev,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20190802070354.GA8280@lst.de>
On Fri, 02 Aug 2019 09:03:54 +0200,
Christoph Hellwig wrote:
>
> Takashi,
>
> any comments on the sounds/ side of this?
I wasn't careful enough to look at that change, sorry.
The code there tries to check whether dma_mmap_coherent() would always
fail on some platforms. Then the driver clears the mmap capability
flag at the device open time and notifies user-space to fall back to
the dumb read/write mode.
So I'm afraid that simply dropping the check would cause the behavior
regression, e.g. on PARISC.
Is there any simple way to test whether dma_mmap_coherent() would work
or not in general on the target platform? It's not necessarily in an
ifdef at all.
thanks,
Takashi
> On Thu, Jul 25, 2019 at 08:34:01AM +0200, Christoph Hellwig wrote:
> > Now that we never use a default ->mmap implementation, and non-coherent
> > architectures can control the presence of ->mmap support by enabling
> > ARCH_HAS_DMA_COHERENT_TO_PFN for the dma direct implementation there
> > is no need for a global config option to control the availability
> > of dma_common_mmap.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > arch/Kconfig | 3 ---
> > arch/c6x/Kconfig | 1 -
> > arch/m68k/Kconfig | 1 -
> > arch/microblaze/Kconfig | 1 -
> > arch/parisc/Kconfig | 1 -
> > arch/sh/Kconfig | 1 -
> > arch/xtensa/Kconfig | 1 -
> > kernel/dma/mapping.c | 4 ----
> > sound/core/pcm_native.c | 10 +---------
> > 9 files changed, 1 insertion(+), 22 deletions(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index a7b57dd42c26..ec2834206d08 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -790,9 +790,6 @@ config COMPAT_32BIT_TIME
> > This is relevant on all 32-bit architectures, and 64-bit architectures
> > as part of compat syscall handling.
> >
> > -config ARCH_NO_COHERENT_DMA_MMAP
> > - bool
> > -
> > config ARCH_NO_PREEMPT
> > bool
> >
> > diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig
> > index b4fb61c83494..e65e8d82442a 100644
> > --- a/arch/c6x/Kconfig
> > +++ b/arch/c6x/Kconfig
> > @@ -20,7 +20,6 @@ config C6X
> > select OF_EARLY_FLATTREE
> > select GENERIC_CLOCKEVENTS
> > select MODULES_USE_ELF_RELA
> > - select ARCH_NO_COHERENT_DMA_MMAP
> > select MMU_GATHER_NO_RANGE if MMU
> >
> > config MMU
> > diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> > index c518d695c376..614b355ae338 100644
> > --- a/arch/m68k/Kconfig
> > +++ b/arch/m68k/Kconfig
> > @@ -8,7 +8,6 @@ config M68K
> > select ARCH_HAS_DMA_PREP_COHERENT if HAS_DMA && MMU && !COLDFIRE
> > select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
> > select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
> > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU
> > select ARCH_NO_PREEMPT if !COLDFIRE
> > select BINFMT_FLAT_ARGVP_ENVP_ON_STACK
> > select DMA_DIRECT_REMAP if HAS_DMA && MMU && !COLDFIRE
> > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> > index d411de05b628..632c9477a0f6 100644
> > --- a/arch/microblaze/Kconfig
> > +++ b/arch/microblaze/Kconfig
> > @@ -9,7 +9,6 @@ config MICROBLAZE
> > select ARCH_HAS_SYNC_DMA_FOR_CPU
> > select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> > select ARCH_MIGHT_HAVE_PC_PARPORT
> > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU
> > select ARCH_WANT_IPC_PARSE_VERSION
> > select BUILDTIME_EXTABLE_SORT
> > select TIMER_OF
> > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> > index 6d732e451071..e9dd88b7f81e 100644
> > --- a/arch/parisc/Kconfig
> > +++ b/arch/parisc/Kconfig
> > @@ -52,7 +52,6 @@ config PARISC
> > select GENERIC_SCHED_CLOCK
> > select HAVE_UNSTABLE_SCHED_CLOCK if SMP
> > select GENERIC_CLOCKEVENTS
> > - select ARCH_NO_COHERENT_DMA_MMAP
> > select CPU_NO_EFFICIENT_FFS
> > select NEED_DMA_MAP_STATE
> > select NEED_SG_DMA_LENGTH
> > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> > index 6b1b5941b618..f356ee674d89 100644
> > --- a/arch/sh/Kconfig
> > +++ b/arch/sh/Kconfig
> > @@ -5,7 +5,6 @@ config SUPERH
> > select ARCH_HAS_PTE_SPECIAL
> > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> > select ARCH_MIGHT_HAVE_PC_PARPORT
> > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU
> > select HAVE_PATA_PLATFORM
> > select CLKDEV_LOOKUP
> > select DMA_DECLARE_COHERENT
> > diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> > index ebc135bda921..70653aed3005 100644
> > --- a/arch/xtensa/Kconfig
> > +++ b/arch/xtensa/Kconfig
> > @@ -5,7 +5,6 @@ config XTENSA
> > select ARCH_HAS_BINFMT_FLAT if !MMU
> > select ARCH_HAS_SYNC_DMA_FOR_CPU
> > select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> > - select ARCH_NO_COHERENT_DMA_MMAP if !MMU
> > select ARCH_USE_QUEUED_RWLOCKS
> > select ARCH_USE_QUEUED_SPINLOCKS
> > select ARCH_WANT_FRAME_POINTERS
> > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> > index 7dff1829c8c5..815446f76995 100644
> > --- a/kernel/dma/mapping.c
> > +++ b/kernel/dma/mapping.c
> > @@ -169,7 +169,6 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
> > void *cpu_addr, dma_addr_t dma_addr, size_t size,
> > unsigned long attrs)
> > {
> > -#ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
> > unsigned long user_count = vma_pages(vma);
> > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > unsigned long off = vma->vm_pgoff;
> > @@ -198,9 +197,6 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
> >
> > return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> > user_count << PAGE_SHIFT, vma->vm_page_prot);
> > -#else
> > - return -ENXIO;
> > -#endif /* !CONFIG_ARCH_NO_COHERENT_DMA_MMAP */
> > }
> >
> > /**
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index 860543a4c840..2dadc708343a 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -218,15 +218,7 @@ int snd_pcm_info_user(struct snd_pcm_substream *substream,
> >
> > static bool hw_support_mmap(struct snd_pcm_substream *substream)
> > {
> > - if (!(substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP))
> > - return false;
> > - /* architecture supports dma_mmap_coherent()? */
> > -#if defined(CONFIG_ARCH_NO_COHERENT_DMA_MMAP) || !defined(CONFIG_HAS_DMA)
> > - if (!substream->ops->mmap &&
> > - substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV)
> > - return false;
> > -#endif
> > - return true;
> > + return substream->runtime->hw.info & SNDRV_PCM_INFO_MMAP;
> > }
> >
> > static int constrain_mask_params(struct snd_pcm_substream *substream,
> > --
> > 2.20.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> ---end quoted text---
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/5] jump_label: Don't warn on __exit jump entries
From: Andrew Murray @ 2019-08-02 8:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Catalin Marinas, Boqun Feng, Will Deacon,
Ard.Biesheuvel, linux-arm-kernel
In-Reply-To: <20190731164156.GX31398@hirez.programming.kicks-ass.net>
On Wed, Jul 31, 2019 at 06:41:56PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 31, 2019 at 05:12:52PM +0100, Andrew Murray wrote:
> > On architectures that discard .exit.* sections at runtime, a
> > warning is printed for each jump label that is used within an
> > in-kernel __exit annotated function:
> >
> > can't patch jump_label at ehci_hcd_cleanup+0x8/0x3c
> > WARNING: CPU: 0 PID: 1 at kernel/jump_label.c:410 __jump_label_update+0x12c/0x138
> >
> > As these functions will never get executed (they are free'd along
> > with the rest of initmem) - we do not need to patch them and should
> > not display any warnings.
> >
> > The warning is displayed because the test required to satisfy
> > jump_entry_is_init is based on init_section_contains (__init_begin to
> > __init_end) whereas the test in __jump_label_update is based on
> > init_kernel_text (_sinittext to _einittext) via kernel_text_address).
> >
> > In addition to fixing this, we also remove an out-of-date comment
> > and use a WARN instead of a WARN_ONCE.
> >
> > Fixes: 19483677684b ("jump_label: Annotate entries that operate on __init code earlier")
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> > kernel/jump_label.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index df3008419a1d..c984078a5c28 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -407,7 +407,9 @@ static bool jump_label_can_update(struct jump_entry *entry, bool init)
> > return false;
> >
> > if (!kernel_text_address(jump_entry_code(entry))) {
> > - WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry));
> > + if (!jump_entry_is_init(entry))
> > + WARN_ONCE(1, "can't patch jump_label at %pS",
> > + (void *)jump_entry_code(entry));
>
> It seems to me we can writes that as:
>
> WARN_ONCE(!jump_entry_is_init(entry),
> "can't patch jump_label at %pS",
> (void *)jump_entry_code(entry));
>
> > return false;
> > }
>
> Other than that,
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Ah of course!
I'll make this change on my respin and add your ack. Thanks.
Thanks,
Andrew Murray
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Christoph Hellwig @ 2019-08-02 8:14 UTC (permalink / raw)
To: Will Deacon
Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, Russell King,
linux-kernel, iommu, Catalin Marinas, Robin Murphy,
Christoph Hellwig, linux-arm-kernel
In-Reply-To: <20190801164411.kmsl4japtfkgvzxe@willie-the-truck>
On Thu, Aug 01, 2019 at 05:44:12PM +0100, Will Deacon wrote:
> > > Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
> > > gets involved in the non-coherent case.
> >
> > A better name is welcome.
>
> How about arch_dma_noncoherent_mmap_pgprot() ? Too long?
Sounds a little long yes. And doesn't fix the additional problem that
we don't just it for mmap but also for the in-kernel remapping these
days.
> > But my worry is how this interacts with architectures that have an
> > uncached segment (mips, nios2, microblaze, extensa) where we'd have
> > the kernel access DMA_ATTR_WRITE_COMBINE mappigns using the uncached
> > segment, and userspace mmaps using pgprot_writecombine, which could
> > lead to aliasing issues. But then again mips already supports
> > DMA_ATTR_WRITE_COMBINE, so this must be ok somehow. I guess I'll
> > need to field that question to the relevant parties.
>
> Or it's always been busted and happens to work out in practice...
I've sent a ping to the mips folks. While we'are at it: arm64
and arm32 (optionally) map dma coherent allocations as write combine.
I suspect this hasn't always just been busted but intentional (of course!),
but is there any chance to get a quote from the arm architecture spec
on why this is fine as it looks rather confusion?
Also if we assume mips is buggy DMA_ATTR_WRITE_COMBINE really just seems
to be there for old arm platforms, which makes the scope pretty limited.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: stm32: add DFSDM pins to stm32mp157c
From: Alexandre Torgue @ 2019-08-02 8:09 UTC (permalink / raw)
To: Olivier Moysan, linux-stm32, robh, mark.rutland, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <1564645567-13156-1-git-send-email-olivier.moysan@st.com>
Hi Olivier
On 8/1/19 9:46 AM, Olivier Moysan wrote:
> Add DFSDM pins to stm32mp157c.
>
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> ---
> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 39 +++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> index 9eaec9bf8cb8..f96a928cbc49 100644
> --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> @@ -230,6 +230,45 @@
> };
> };
>
I use to only take pinconfig which are used in board. So please resend
with the "board patch".
regards
Alex
> + dfsdm_clkout_pins_a: dfsdm-clkout-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('B', 13, AF3)>; /* DFSDM_CKOUT */
> + bias-disable;
> + drive-push-pull;
> + slew-rate = <0>;
> + };
> + };
> +
> + dfsdm_clkout_sleep_pins_a: dfsdm-clkout-sleep-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('B', 13, ANALOG)>; /* DFSDM_CKOUT */
> + };
> + };
> +
> + dfsdm_data1_pins_a: dfsdm-data1-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('C', 3, AF3)>; /* DFSDM_DATA1 */
> + };
> + };
> +
> + dfsdm_data1_sleep_pins_a: dfsdm-data1-sleep-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('C', 3, ANALOG)>; /* DFSDM_DATA1 */
> + };
> + };
> +
> + dfsdm_data3_pins_a: dfsdm-data3-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('F', 13, AF6)>; /* DFSDM_DATA3 */
> + };
> + };
> +
> + dfsdm_data3_sleep_pins_a: dfsdm-data3-sleep-pins-0 {
> + pins {
> + pinmux = <STM32_PINMUX('F', 13, ANALOG)>; /* DFSDM_DATA3 */
> + };
> + };
> +
> ethernet0_rgmii_pins_a: rgmii-0 {
> pins1 {
> pinmux = <STM32_PINMUX('G', 5, AF11)>, /* ETH_RGMII_CLK125 */
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites
From: Peter Zijlstra @ 2019-08-02 8:05 UTC (permalink / raw)
To: john.hubbard
Cc: linux-fbdev, Jan Kara, kvm, Dave Hansen, Dave Chinner, dri-devel,
linux-mm, sparclinux, Ira Weiny, Dan Williams, devel, rds-devel,
linux-rdma, x86, amd-gfx, Christoph Hellwig, Jason Gunthorpe,
xen-devel, devel, linux-media, John Hubbard, intel-gfx,
linux-block, Jérôme Glisse, linux-rpi-kernel,
ceph-devel, linux-arm-kernel, linux-nfs, netdev, LKML, linux-xfs,
linux-crypto, linux-fsdevel, Andrew Morton
In-Reply-To: <20190802021653.4882-1-jhubbard@nvidia.com>
On Thu, Aug 01, 2019 at 07:16:19PM -0700, john.hubbard@gmail.com wrote:
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions"). That commit
> has an extensive description of the problem and the planned steps to
> solve it, but the highlites are:
That is one horridly mangled Changelog there :-/ It looks like it's
partially duplicated.
Anyway; no objections to any of that, but I just wanted to mention that
there are other problems with long term pinning that haven't been
mentioned, notably they inhibit compaction.
A long time ago I proposed an interface to mark pages as pinned, such
that we could run compaction before we actually did the pinning.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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