[PATCH 1/4] signal: add a helper to restore a process state from sigcontex
It will be used to implement process_vm_exec. Signed-off-by: Andrei Vagin --- arch/x86/kernel/signal.c | 78 ++-- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index be0d7d4152ec..cc269a20dd5f 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -79,51 +79,43 @@ static void force_valid_ss(struct pt_regs *regs) # define CONTEXT_COPY_SIZE sizeof(struct sigcontext) #endif -static int restore_sigcontext(struct pt_regs *regs, - struct sigcontext __user *usc, +static int __restore_sigcontext(struct pt_regs *regs, + struct sigcontext __user *sc, unsigned long uc_flags) { - struct sigcontext sc; - - /* Always make any pending restarted system calls return -EINTR */ - current->restart_block.fn = do_no_restart_syscall; - - if (copy_from_user(, usc, CONTEXT_COPY_SIZE)) - return -EFAULT; - #ifdef CONFIG_X86_32 - set_user_gs(regs, sc.gs); - regs->fs = sc.fs; - regs->es = sc.es; - regs->ds = sc.ds; + set_user_gs(regs, sc->gs); + regs->fs = sc->fs; + regs->es = sc->es; + regs->ds = sc->ds; #endif /* CONFIG_X86_32 */ - regs->bx = sc.bx; - regs->cx = sc.cx; - regs->dx = sc.dx; - regs->si = sc.si; - regs->di = sc.di; - regs->bp = sc.bp; - regs->ax = sc.ax; - regs->sp = sc.sp; - regs->ip = sc.ip; + regs->bx = sc->bx; + regs->cx = sc->cx; + regs->dx = sc->dx; + regs->si = sc->si; + regs->di = sc->di; + regs->bp = sc->bp; + regs->ax = sc->ax; + regs->sp = sc->sp; + regs->ip = sc->ip; #ifdef CONFIG_X86_64 - regs->r8 = sc.r8; - regs->r9 = sc.r9; - regs->r10 = sc.r10; - regs->r11 = sc.r11; - regs->r12 = sc.r12; - regs->r13 = sc.r13; - regs->r14 = sc.r14; - regs->r15 = sc.r15; + regs->r8 = sc->r8; + regs->r9 = sc->r9; + regs->r10 = sc->r10; + regs->r11 = sc->r11; + regs->r12 = sc->r12; + regs->r13 = sc->r13; + regs->r14 = sc->r14; + regs->r15 = sc->r15; #endif /* CONFIG_X86_64 */ /* Get CS/SS and force CPL3 */ - regs->cs = sc.cs | 0x03; - regs->ss = sc.ss | 0x03; + regs->cs = sc->cs | 0x03; + regs->ss = sc->ss | 0x03; - regs->flags = (regs->flags & ~FIX_EFLAGS) | (sc.flags & FIX_EFLAGS); + regs->flags = (regs->flags & ~FIX_EFLAGS) | (sc->flags & FIX_EFLAGS); /* disable syscall checks */ regs->orig_ax = -1; @@ -136,10 +128,26 @@ static int restore_sigcontext(struct pt_regs *regs, force_valid_ss(regs); #endif - return fpu__restore_sig((void __user *)sc.fpstate, + return fpu__restore_sig((void __user *)sc->fpstate, IS_ENABLED(CONFIG_X86_32)); } +static int restore_sigcontext(struct pt_regs *regs, + struct sigcontext __user *usc, + unsigned long uc_flags) +{ + struct sigcontext sc; + + /* Always make any pending restarted system calls return -EINTR */ + current->restart_block.fn = do_no_restart_syscall; + + if (copy_from_user(, usc, CONTEXT_COPY_SIZE)) + return -EFAULT; + + return __restore_sigcontext(regs, , uc_flags); +} + + static __always_inline int __unsafe_setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate, struct pt_regs *regs, unsigned long mask) -- 2.29.2
[PATCH 4/4] selftests: add tests for process_vm_exec
Output: $ make run_tests TAP version 13 1..4 # selftests: process_vm_exec: process_vm_exec # 1..1 # ok 1 275 ns/syscall # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 ok 1 selftests: process_vm_exec: process_vm_exec # selftests: process_vm_exec: process_vm_exec_fault # 1..1 # ok 1 789 ns/signal # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 ok 2 selftests: process_vm_exec: process_vm_exec_fault # selftests: process_vm_exec: ptrace_vm_exec # 1..1 # ok 1 1378 ns/syscall# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 ok 3 selftests: process_vm_exec: ptrace_vm_exec # selftests: process_vm_exec: process_vm_exec_syscall # 1..1 # ok 1 write works as expectd # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 ok 4 selftests: process_vm_exec: process_vm_exec_syscall Signed-off-by: Andrei Vagin --- .../selftests/process_vm_exec/Makefile| 7 ++ tools/testing/selftests/process_vm_exec/log.h | 26 .../process_vm_exec/process_vm_exec.c | 105 + .../process_vm_exec/process_vm_exec_fault.c | 111 ++ .../process_vm_exec/process_vm_exec_syscall.c | 81 + .../process_vm_exec/ptrace_vm_exec.c | 111 ++ 6 files changed, 441 insertions(+) create mode 100644 tools/testing/selftests/process_vm_exec/Makefile create mode 100644 tools/testing/selftests/process_vm_exec/log.h create mode 100644 tools/testing/selftests/process_vm_exec/process_vm_exec.c create mode 100644 tools/testing/selftests/process_vm_exec/process_vm_exec_fault.c create mode 100644 tools/testing/selftests/process_vm_exec/process_vm_exec_syscall.c create mode 100644 tools/testing/selftests/process_vm_exec/ptrace_vm_exec.c diff --git a/tools/testing/selftests/process_vm_exec/Makefile b/tools/testing/selftests/process_vm_exec/Makefile new file mode 100644 index ..bdf7fcf0fdd3 --- /dev/null +++ b/tools/testing/selftests/process_vm_exec/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 + +UNAME_M := $(shell uname -m) +TEST_GEN_PROGS_x86_64 := process_vm_exec process_vm_exec_fault ptrace_vm_exec process_vm_exec_syscall +TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M)) + +include ../lib.mk diff --git a/tools/testing/selftests/process_vm_exec/log.h b/tools/testing/selftests/process_vm_exec/log.h new file mode 100644 index ..ef268c2cf2b8 --- /dev/null +++ b/tools/testing/selftests/process_vm_exec/log.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __SELFTEST_PROCESS_VM_EXEC_LOG_H__ +#define __SELFTEST_PROCESS_VM_EXEC_LOG_H__ + +#define pr_msg(fmt, lvl, ...) \ + ksft_print_msg("[%s] (%s:%d)\t" fmt "\n", \ + lvl, __FILE__, __LINE__, ##__VA_ARGS__) + +#define pr_p(func, fmt, ...) func(fmt ": %m", ##__VA_ARGS__) + +#define pr_err(fmt, ...) \ + ({ \ + ksft_test_result_error(fmt "\n", ##__VA_ARGS__); \ + -1; \ + }) + +#define pr_fail(fmt, ...) \ + ({ \ + ksft_test_result_fail(fmt "\n", ##__VA_ARGS__); \ + -1; \ + }) + +#define pr_perror(fmt, ...)pr_p(pr_err, fmt, ##__VA_ARGS__) + +#endif diff --git a/tools/testing/selftests/process_vm_exec/process_vm_exec.c b/tools/testing/selftests/process_vm_exec/process_vm_exec.c new file mode 100644 index ..aa4009c43e01 --- /dev/null +++ b/tools/testing/selftests/process_vm_exec/process_vm_exec.c @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-2.0 + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "asm/unistd.h" +#include +#include + +#include "../kselftest.h" +#include "log.h" + +#ifndef __NR_process_vm_exec +#define __NR_process_vm_exec 441 +#endif + +#define TEST_SYSCALL 123 +#define TEST_SYSCALL_RET 456 +#define TEST_MARKER 789 +#define TEST_TIMEOUT 5 +#define TEST_STACK_SIZE 65536 + +static inline long __syscall1(long n, long a1) +{ + unsigned long ret; + + __asm__ __volatile__ ("syscall" : "=a"(ret) : "a"(n), "D"(a1) : "rcx", "r11", "memory"); + + return ret; +} + +int marker; + +static void guest(void) +{ + while (1) + if (__syscall1(TEST_SYSCALL, marker) != TEST_SYSCALL_RET) + abort(); +} + +int main(int argc, char **argv) +{ + struct sigcontext ctx = {}; + struct timespec start, cur; + int status, ret; + pid_t pid; + long sysnr; + void *stack; + + ksft_set_plan(1); + + stack = mmap(NULL, TEST_STACK_SIZE, PROT_READ | PROT_WRITE,
Re: [syzbot] KASAN: use-after-free Read in get_wchan
On Wed, Apr 14, 2021 at 7:52 AM syzbot wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit:b2b3d18f riscv: Make NUMA depend on MMU > git tree: git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git > fixes > console output: https://syzkaller.appspot.com/x/log.txt?x=12b59d16d0 > kernel config: https://syzkaller.appspot.com/x/.config?x=81b3e7c68dad6e > dashboard link: https://syzkaller.appspot.com/bug?extid=0806291048161061627c > userspace arch: riscv64 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+08062910481610616...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in walk_stackframe > arch/riscv/kernel/stacktrace.c:60 [inline] > BUG: KASAN: use-after-free in get_wchan+0x156/0x196 > arch/riscv/kernel/stacktrace.c:136 > Read of size 8 at addr ffe0058e3d90 by task syz-executor.0/4667 > > CPU: 1 PID: 4667 Comm: syz-executor.0 Not tainted > 5.12.0-rc5-syzkaller-00721-gb2b3d18fc20e #0 > Hardware name: riscv-virtio,qemu (DT) > Call Trace: > [] walk_stackframe+0x0/0x23c arch/riscv/kernel/traps.c:201 > [] dump_backtrace+0x40/0x4e > arch/riscv/kernel/stacktrace.c:113 > [] show_stack+0x22/0x2e arch/riscv/kernel/stacktrace.c:118 > [] __dump_stack lib/dump_stack.c:79 [inline] > [] dump_stack+0x148/0x1d8 lib/dump_stack.c:120 > [] print_address_description.constprop.0+0x52/0x31e > mm/kasan/report.c:232 > [] __kasan_report mm/kasan/report.c:399 [inline] > [] kasan_report+0x16e/0x18c mm/kasan/report.c:416 > [] check_region_inline mm/kasan/generic.c:180 [inline] > [] __asan_load8+0x6e/0x80 mm/kasan/generic.c:253 > [] walk_stackframe arch/riscv/kernel/stacktrace.c:60 > [inline] If it's walking the stack of another task, then it needs to use READ_ONCE_NOCHECK. See x86/arm64/s390 for examples: https://elixir.bootlin.com/linux/v5.12-rc7/A/ident/READ_ONCE_NOCHECK > [] get_wchan+0x156/0x196 arch/riscv/kernel/stacktrace.c:136 > [] proc_pid_wchan+0x48/0xa4 fs/proc/base.c:390 > [] proc_single_show+0x9c/0x13c fs/proc/base.c:774 > [] seq_read_iter+0x2e0/0x8f2 fs/seq_file.c:227 > [] seq_read+0x200/0x298 fs/seq_file.c:159 > [] vfs_read+0x108/0x2ac fs/read_write.c:494 > [] ksys_read+0xb4/0x1b8 fs/read_write.c:634 > [] __do_sys_read fs/read_write.c:644 [inline] > [] sys_read+0x28/0x36 fs/read_write.c:642 > [] ret_from_syscall+0x0/0x2 > > The buggy address belongs to the page: > page:ffcf0216b8c0 refcount:0 mapcount:0 mapping: > index:0x0 pfn:0x85ae3 > flags: 0xffe() > raw: 0ffe ffcf0216b8c8 ffcf0216b8c8 > raw: > page dumped because: kasan: bad access detected > > Memory state around the buggy address: > ffe0058e3c80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ffe0058e3d00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > >ffe0058e3d80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ^ > ffe0058e3e00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ffe0058e3e80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > == > > > --- > This report is generated by a bot. It may contain errors. > See https://goo.gl/tpsmEJ for more information about syzbot. > syzbot engineers can be reached at syzkal...@googlegroups.com. > > syzbot will keep track of this issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > > -- > You received this message because you are subscribed to the Google Groups > "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to syzkaller-bugs+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/syzkaller-bugs/9862e005bfe859c8%40google.com.
[PATCH 2/4] arch/x86: implement the process_vm_exec syscall
This change introduces the new system call: process_vm_exec(pid_t pid, struct sigcontext *uctx, unsigned long flags, siginfo_t * uinfo, sigset_t *sigmask, size_t sizemask) process_vm_exec allows to execute the current process in an address space of another process. process_vm_exec swaps the current address space with an address space of a specified process, sets a state from sigcontex and resumes the process. When a process receives a signal or calls a system call, process_vm_exec saves the process state back to sigcontext, restores the origin address space, restores the origin process state, and returns to userspace. If it was interrupted by a signal and the signal is in the user_mask, the signal is dequeued and information about it is saved in uinfo. If process_vm_exec is interrupted by a system call, a synthetic siginfo for the SIGSYS signal is generated. The behavior of this system call is similar to PTRACE_SYSEMU but everything is happing in the context of one process, so process_vm_exec shows a better performance. PTRACE_SYSEMU is primarily used to implement sandboxes (application kernels) like User-mode Linux or gVisor. These type of sandboxes intercepts applications system calls and acts as the guest kernel. A simple benchmark, where a "tracee" process executes systems calls in a loop and a "tracer" process traps syscalls and handles them just incrementing the tracee instruction pointer to skip the syscall instruction shows that process_vm_exec works more than 5 times faster than PTRACE_SYSEMU. Signed-off-by: Andrei Vagin --- arch/Kconfig | 15 +++ arch/x86/Kconfig | 1 + arch/x86/entry/common.c| 16 +++ arch/x86/entry/syscalls/syscall_64.tbl | 1 + arch/x86/include/asm/sigcontext.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/process_vm_exec.c | 133 + arch/x86/kernel/signal.c | 47 + include/linux/process_vm_exec.h| 15 +++ include/linux/sched.h | 7 ++ include/linux/syscalls.h | 6 ++ include/uapi/asm-generic/unistd.h | 4 +- kernel/fork.c | 9 ++ kernel/sys_ni.c| 2 + 14 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 arch/x86/kernel/process_vm_exec.c create mode 100644 include/linux/process_vm_exec.h diff --git a/arch/Kconfig b/arch/Kconfig index ba4e966484ab..3ed9b8fb1727 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -514,6 +514,21 @@ config SECCOMP_FILTER See Documentation/userspace-api/seccomp_filter.rst for details. +config HAVE_ARCH_PROCESS_VM_EXEC + bool + help + An arch should select this symbol to support the process_vm_exec system call. + +config PROCESS_VM_EXEC + prompt "Enable the process_vm_exec syscall" + def_bool y + depends on HAVE_ARCH_PROCESS_VM_EXEC + help + process_vm_exec allows executing code and system calls in a specified + address space. + + If unsure, say Y. + config HAVE_ARCH_STACKLEAK bool help diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index fbf26e0f7a6a..1c7ebb58865e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -27,6 +27,7 @@ config X86_64 select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 select ARCH_USE_CMPXCHG_LOCKREF select HAVE_ARCH_SOFT_DIRTY + select HAVE_ARCH_PROCESS_VM_EXEC select MODULES_USE_ELF_RELA select NEED_DMA_MAP_STATE select SWIOTLB diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 870efeec8bda..42eac459b25b 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -19,6 +19,7 @@ #include #include #include +#include #ifdef CONFIG_XEN_PV #include @@ -38,6 +39,21 @@ #ifdef CONFIG_X86_64 __visible noinstr void do_syscall_64(unsigned long nr, struct pt_regs *regs) { +#ifdef CONFIG_PROCESS_VM_EXEC + if (current->exec_mm && current->exec_mm->ctx) { + kernel_siginfo_t info = { + .si_signo = SIGSYS, + .si_call_addr = (void __user *)KSTK_EIP(current), + .si_arch = syscall_get_arch(current), + .si_syscall = nr, + }; + restore_vm_exec_context(regs); + regs->ax = copy_siginfo_to_user(current->exec_mm->siginfo, ); + syscall_exit_to_user_mode(regs); + return; + } +#endif + nr = syscall_enter_from_user_mode(regs, nr); instrumentation_begin(); diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index 379819244b91..2a8e27b2d87e 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -362,6 +362,7 @@ 438common pidfd_getfd sys_pidfd_getfd 439common
[PATCH 0/4 POC] Allow executing code and syscalls in another address space
We already have process_vm_readv and process_vm_writev to read and write to a process memory faster than we can do this with ptrace. And now it is time for process_vm_exec that allows executing code in an address space of another process. We can do this with ptrace but it is much slower. = Use-cases = Here are two known use-cases. The first one is “application kernel” sandboxes like User-mode Linux and gVisor. In this case, we have a process that runs the sandbox kernel and a set of stub processes that are used to manage guest address spaces. Guest code is executed in the context of stub processes but all system calls are intercepted and handled in the sandbox kernel. Right now, these sort of sandboxes use PTRACE_SYSEMU to trap system calls, but the process_vm_exec can significantly speed them up. Another use-case is CRIU (Checkpoint/Restore in User-space). Several process properties can be received only from the process itself. Right now, we use a parasite code that is injected into the process. We do this with ptrace but it is slow, unsafe, and tricky. process_vm_exec can simplify the process of injecting a parasite code and it will allow pre-dump memory without stopping processes. The pre-dump here is when we enable a memory tracker and dump the memory while a process is continue running. On each interaction we dump memory that has been changed from the previous iteration. In the final step, we will stop processes and dump their full state. Right now the most effective way to dump process memory is to create a set of pipes and splice memory into these pipes from the parasite code. With process_vm_exec, we will be able to call vmsplice directly. It means that we will not need to stop a process to inject the parasite code. = How it works = process_vm_exec has two modes: * Execute code in an address space of a target process and stop on any signal or system call. * Execute a system call in an address space of a target process. int process_vm_exec(pid_t pid, struct sigcontext uctx, unsigned long flags, siginfo_t siginfo, sigset_t *sigmask, size_t sizemask) PID - target process identification. We can consider to use pidfd instead of PID here. sigcontext contains a process state with what the process will be resumed after switching the address space and then when a process will be stopped, its sate will be saved back to sigcontext. siginfo is information about a signal that has interrupted the process. If a process is interrupted by a system call, signfo will contain a synthetic siginfo of the SIGSYS signal. sigmask is a set of signals that process_vm_exec returns via signfo. # How fast is it In the fourth patch, you can find two benchmarks that execute a function that calls system calls in a loop. ptrace_vm_exe uses ptrace to trap system calls, proces_vm_exec uses the process_vm_exec syscall to do the same thing. ptrace_vm_exec: 1446 ns/syscall ptrocess_vm_exec: 289 ns/syscall PS: This version is just a prototype. Its goal is to collect the initial feedback, to discuss the interfaces, and maybe to get some advice on implementation.. Cc: Andrew Morton Cc: Andy Lutomirski Cc: Anton Ivanov Cc: Christian Brauner Cc: Dmitry Safonov <0x7f454...@gmail.com> Cc: Ingo Molnar Cc: Jeff Dike Cc: Mike Rapoport Cc: Michael Kerrisk (man-pages) Cc: Oleg Nesterov Cc: Peter Zijlstra Cc: Richard Weinberger Cc: Thomas Gleixner Andrei Vagin (4): signal: add a helper to restore a process state from sigcontex arch/x86: implement the process_vm_exec syscall arch/x86: allow to execute syscalls via process_vm_exec selftests: add tests for process_vm_exec arch/Kconfig | 15 ++ arch/x86/Kconfig | 1 + arch/x86/entry/common.c | 19 +++ arch/x86/entry/syscalls/syscall_64.tbl| 1 + arch/x86/include/asm/sigcontext.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/process_vm_exec.c | 160 ++ arch/x86/kernel/signal.c | 125 ++ include/linux/entry-common.h | 2 + include/linux/process_vm_exec.h | 17 ++ include/linux/sched.h | 7 + include/linux/syscalls.h | 6 + include/uapi/asm-generic/unistd.h | 4 +- include/uapi/linux/process_vm_exec.h | 8 + kernel/entry/common.c | 2 +- kernel/fork.c | 9 + kernel/sys_ni.c | 2 + .../selftests/process_vm_exec/Makefile| 7 + tools/testing/selftests/process_vm_exec/log.h | 26 +++ .../process_vm_exec/process_vm_exec.c | 105 .../process_vm_exec/process_vm_exec_fault.c | 111 .../process_vm_exec/process_vm_exec_syscall.c | 81 + .../process_vm_exec/ptrace_vm_exec.c
[PATCH] i2c: busses: remove unused including
Fix the following versioncheck warnings: ./drivers/i2c/busses/i2c-xgene-slimpro.c: 22 linux/version.h not needed. ./drivers/i2c/busses/i2c-brcmstb.c: 25 linux/version.h not needed. Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/i2c/busses/i2c-brcmstb.c | 1 - drivers/i2c/busses/i2c-xgene-slimpro.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c index ba766d2..490ee39 100644 --- a/drivers/i2c/busses/i2c-brcmstb.c +++ b/drivers/i2c/busses/i2c-brcmstb.c @@ -22,7 +22,6 @@ #include #include #include -#include #define N_DATA_REGS8 diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c index 63cbb9c7..bba08cb 100644 --- a/drivers/i2c/busses/i2c-xgene-slimpro.c +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c @@ -19,7 +19,6 @@ #include #include #include -#include #define MAILBOX_OP_TIMEOUT 1000/* Operation time out in ms */ #define MAILBOX_I2C_INDEX 0 -- 1.8.3.1
Re: [Nouveau] [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
On Tue, Apr 13, 2021 at 10:24 AM Roy Spliet wrote: > > Op 13-04-2021 om 01:10 schreef Karol Herbst: > > On Mon, Apr 12, 2021 at 9:36 PM Roy Spliet wrote: > >> > >> Hello Aaron, > >> > >> Thanks for your insights. A follow-up query and some observations in-line. > >> > >> Op 12-04-2021 om 20:06 schreef Aaron Plattner: > >>> On 4/10/21 1:48 PM, Roy Spliet wrote: > Op 10-04-2021 om 20:23 schreef Lukas Wunner: > > On Sat, Apr 10, 2021 at 04:51:27PM +0100, Roy Spliet wrote: > >> Can I ask someone with more > >> technical knowledge of snd_hda_intel and vgaswitcheroo to brainstorm > >> about > >> the possible challenges of nouveau taking matters into its own hand > >> rather > >> than keeping this PCI quirk around? > > > > It sounds to me like the HDA is not powered if no cable is plugged in. > > What is reponsible then for powering it up or down, firmware code on > > the GPU or in the host's BIOS? > > Sometimes the BIOS, but definitely unconditionally the PCI quirk code: > https://github.com/torvalds/linux/blob/master/drivers/pci/quirks.c#L5289 > > (CC Aaron Plattner) > >>> > >>> My basic understanding is that the audio function stops responding > >>> whenever the graphics function is powered off. So the requirement here > >>> is that the audio driver can't try to talk to the audio function while > >>> the graphics function is asleep, and must trigger a graphics function > >>> wakeup before trying to communicate with the audio function. > >> > >> I believe that vgaswitcheroo takes care of this for us. > >> > > > > yeah, and also: why would the driver want to do stuff? If the GPU is > > turned off, there is no point in communicating with the audio device > > anyway. The driver should do the initial probe and leave the device be > > unless it's actively used. Also there is no such thing as "use the > > audio function, but not the graphics one" > > > >>> I think > >>> there are also requirements about the audio function needing to be awake > >>> when the graphics driver is updating the ELD, but I'm not sure. > >>> > > > > well, it's one physical device anyway, so technically the audio > > function is powered on. > > > >>> This is harder on Windows because the audio driver lives in its own > >>> little world doing its own thing but on Linux we can do better. > >>> > > Ideally, we should try to find out how to control HDA power from the > > operating system rather than trying to cooperate with whatever firmware > > is doing. If we have that capability, the OS should power the HDA up > > and down as it sees fit. > >>> > >>> After system boot, I don't think there's any firmware involved, but I'm > >>> not super familiar with the low-level details and it's possible the > >>> situation changed since I last looked at it. > >>> > >>> I think the problem with having nouveau write this quirk is that the > >>> kernel will need to re-probe the PCI device to notice that it has > >>> suddenly become a multi-function device with an audio function, and > >>> hotplug the audio driver. I originally looked into trying to do that but > >>> it was tricky because the PCI subsystem didn't really have a mechanism > >>> for a single-function device to become a multi-function device on the > >>> fly and it seemed easier to enable it early on during bus enumeration. > >>> That way the kernel sees both functions all the time without anything > >>> else having to be special about this configuration. > > > > Well, we do have this pci/quirk.c thing, no? Nouveau does flip the > > bit, but I am actually not sure if that's even doing something > > anymore. Maybe in the runtime_resume case it's still relevant but not > > sure _when_ DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY is triggered, it does > > seem to be called even in the runtime_resume case though. > > > >> > >> Right, so for a little more context: a while ago I noticed that my > >> laptop (lucky me, Asus K501UB) has a 940M with HDA but no codec. Seems > >> legit, given how this GPU has no displays attached; they're all hooked > >> up to the Intel integrated GPU. That threw off the snd_hda_intel > >> mid-probe, and as a result didn't permit runpm, keeping the entire GPU, > >> PCIe bus and thus the CPU package awake. A bit of hackerly later we > >> decided to continue probing without a codec, and now my laptop is happy, > >> but... > >> A new problem popped up with several other NVIDIA GPUs that expose their > >> HDA subdevice, but somehow its inaccessible. Relevant lines from a > >> users' log: > >> > >> [3.031222] MXM: GUID detected in BIOS > >> [3.031280] ACPI BIOS Error (bug): AE_AML_PACKAGE_LIMIT, Index > >> (0x3) is beyond end of object (length 0x0) (20200925/exoparg2-393) > >> [3.031352] ACPI Error: Aborting method \_SB.PCI0.GFX0._DSM due to > >> previous error (AE_AML_PACKAGE_LIMIT) (20200925/psparse-529) > >> [3.031419] ACPI: \_SB_.PCI0.GFX0: failed to evaluate _DSM
[PATCH] rsi: remove unused including
Fix the following versioncheck warning: ./drivers/net/wireless/rsi/rsi_91x_ps.c: 19 linux/version.h not needed. Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/net/wireless/rsi/rsi_91x_ps.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_ps.c b/drivers/net/wireless/rsi/rsi_91x_ps.c index fdaa5a7..a029049 100644 --- a/drivers/net/wireless/rsi/rsi_91x_ps.c +++ b/drivers/net/wireless/rsi/rsi_91x_ps.c @@ -16,7 +16,6 @@ #include #include -#include #include "rsi_debugfs.h" #include "rsi_mgmt.h" #include "rsi_common.h" -- 1.8.3.1
perf arm64 --topdown support (was "perf arm64 metricgroup support")
On 08/04/2021 13:06, Jiri Olsa wrote: perf stat --topdown is not supported, as this requires the CPU PMU to expose (alias) events for the TopDown L1 metrics from sysfs, which arm does not do. To get that to work, we probably need to make perf use the pmu-events cpumap to learn about those alias events. Hi guys, About supporting --topdown command for other archs apart from x86, it seems not possible today. Support there is based on kernel support for "topdown" CPU events used in the metric calculations. However, arm64, for example, does not support these "topdown" events. It seems to me that we can change to use pmu-events framework + metricgroup support here, rather than hardcoded events - has anyone considered this approach previously? Seems a pretty big job, so thought I'd ask first ... Thanks, John
Re: [v9,2/7] PCI: Export pci_pio_to_address() for module use
On Wed, Mar 24, 2021 at 10:09:42AM +0100, Pali Rohár wrote: > On Wednesday 24 March 2021 11:05:05 Jianjun Wang wrote: > > This interface will be used by PCI host drivers for PIO translation, > > export it to support compiling those drivers as kernel modules. > > > > Signed-off-by: Jianjun Wang > > --- > > drivers/pci/pci.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 16a17215f633..12bba221c9f2 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -4052,6 +4052,7 @@ phys_addr_t pci_pio_to_address(unsigned long pio) > > > > return address; > > } > > +EXPORT_SYMBOL(pci_pio_to_address); > > Hello! I'm not sure if EXPORT_SYMBOL is correct because file has GPL-2.0 > header. Should not be in this case used only EXPORT_SYMBOL_GPL? Maybe > other people would know what is correct? I think this should be EXPORT_SYMBOL_GPL(), I can make this change but this requires Bjorn's ACK to go upstream (Bjorn, it is my fault, it was assigned to me on patchwork, now updated, please have a look). Thanks, Lorenzo > > > > unsigned long __weak pci_address_to_pio(phys_addr_t address) > > { > > -- > > 2.25.1 > >
Re: bl_list and lockdep
On Tue, Apr 13, 2021 at 01:18:35AM +0200, Thomas Gleixner wrote: > Dave, > > On Tue, Apr 13 2021 at 08:15, Dave Chinner wrote: > > On Mon, Apr 12, 2021 at 05:20:53PM +0200, Thomas Gleixner wrote: > >> On Wed, Apr 07 2021 at 07:22, Dave Chinner wrote: > >> > And, FWIW, I'm also aware of the problems that RT kernels have with > >> > the use of bit spinlocks and being unable to turn them into sleeping > >> > mutexes by preprocessor magic. I don't care about that either, > >> > because dentry cache... > >> > >> In the dentry cache it's a non-issue. > > > > Incorrect. > > I'm impressed about your detailed knowledge of something you do not care > about in the first place. There's a difference between "don't care because don't understand" and "don't care because I know how complex real-time is and I know I can't validate any code I write to be RT safe". Indeed, just because I work on filesystems now doesn't mean I don't know what real-time is - I spent the best part of a decade as an industrial control engineer building systems that provided water and electricity to populations of millions of people before I started working on filesystems > >> RT does not have a problem with bit spinlocks per se, it depends on how > >> they are used and what nests inside. Most of them are just kept as bit > >> spinlocks because the lock held, and therefore preempt disabled times > >> are small and no other on RT conflicting operations happen inside. > >> > >> In the case at hand this is going to be a problem because inode->i_lock > >> nests inside the bit spinlock and we can't make inode->i_lock a raw > >> spinlock because it protects way heavier weight code pathes as well. > > > > Yes, that's exactly the "problem" I'm refering to. And I don't care, > > precisely because, well, dentry cache > > > > THat is, the dcache calls wake_up_all() from under the > > hlist_bl_lock() in __d_lookup_done(). That ends up in > > __wake_up_common_lock() which takes a spin lock embedded inside a > > wait_queue_head. That's not a raw spinlock, either, so we already > > have this "spinlock inside bit lock" situation with the dcache usage > > of hlist_bl. > > Sure, but you are missing that RT solves that by substituting the > wait_queue with a swait_queue, which does not suffer from that. But that > can't be done for the inode::i_lock case for various reasons. I didn't know about that specific forklift replacement. But, really that simply adds weight to my comment below > > FYI, this dentry cache behaviour was added to the dentry cache in > > 2016 by commit d9171b934526 ("parallel lookups machinery, part 4 > > (and last)"), so it's not like it's a new thing, either. > > Really? I wasn't aware of that. Thanks for the education. > > > If you want to make hlist_bl RT safe, then re-implement it behind > > the scenes for RT enabled kernels. All it takes is more memory > > usage for the hash table + locks, but that's something that non-RT > > people should not be burdened with caring about ... because if RT devs are willing to forklift replace core kernel functionality like wait queues to provide RT kernels with a completely different locking schema to vanilla kernels, then slightly modifying the hlist-bl structure in a RT compatible way is child's play > I'm well aware that anything outside of @fromorbit universe is not > interesting to you, but I neverless want to take the opportunity to > express my appreciation for your truly caring and collaborative attitude > versus interests of others who unfortunately do no share that universe. I'm being realistic. I dont' have the time or mental bandwidth to solve RT kernel problems. I don't have any way to test RT kernels, and lockdep is a crock of shit for validating RT locking on vanilla kernels because of the forklift upgrade games like the above that give the RT kernel a different locking schema. Because I have sufficient knowledge of the real-time game, I know *I'm not an RT expert* these days. I know that I don't know all the games it plays, nor do I have the time (or patience) to learn about all of them, nor the resources or knowledge to test whether the code I write follows all the rules I don't know about, whether I introduced interrupt hold-offs longer than 50us, etc. IOWs, I chose not to care about RT because I know I don't know enough about it to write solid, well tested RT compatible kernel code. I can write untested shit as well as any other programmer, but I have much higher professional standards than that. And I also know there are paid professionals who are RT experts who are supposed to take care of this stuff so random kernel devs like myself *don't need to care about the details of how RT kernels do their magic*. So for solving the inode cache scalability issue with RT in mind, we're left with these choices: a) increase memory consumption and cacheline misses for everyone by adding a spinlock per hash chain so that RT kernels can do their
linux-next: Tree for Apr 13
Hi all, Changes since 20210412: New tree: xilinx The arm-soc tree lost its build failure (actually yesterday). The kvm-arm tree gained conflicts against the arm64 tree. The usb tree lost its build failure. The akpm-current tree gained conflicts against the arm64 tree. Non-merge commits (relative to Linus' tree): 11352 10244 files changed, 557584 insertions(+), 257771 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig and htmldocs. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 338 trees (counting Linus' and 87 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (89698becf06d Merge tag 'm68knommu-for-v5.12-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/gerg/m68knommu) Merging fixes/fixes (e71ba9452f0b Linux 5.11-rc2) Merging kbuild-current/fixes (bcbcf50f5218 kbuild: fix ld-version.sh to not be affected by locale) Merging arc-current/for-curr (163630b2d95b arc: Fix typos/spellos) Merging arm-current/fixes (30e3b4f256b4 ARM: footbridge: fix PCI interrupt mapping) Merging arm64-fixes/for-next/fixes (2decad92f473 arm64: mte: Ensure TIF_MTE_ASYNC_FAULT is set atomically) Merging arm-soc-fixes/arm/fixes (b9a9786a13ea Merge tag 'omap-for-v5.12/fixes-rc6-signed' of git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap into arm/fixes) Merging drivers-memory-fixes/fixes (a38fd8748464 Linux 5.12-rc2) Merging m68k-current/for-linus (a65a802aadba m68k: Fix virt_addr_valid() W=1 compiler warnings) Merging powerpc-fixes/fixes (791f9e36599d powerpc/vdso: Make sure vdso_wrapper.o is rebuilt everytime vdso.so is rebuilt) Merging s390-fixes/fixes (a994eddb947e s390/entry: save the caller of psw_idle) Merging sparc/master (05a59d79793d Merge git://git.kernel.org:/pub/scm/linux/kernel/git/netdev/net) Merging fscrypt-current/for-stable (d19d8d345eec fscrypt: fix inline encryption not used on new files) Merging net/master (f33b0e196ed7 ethtool: fix kdoc attr name) Merging bpf/master (afd0be729953 libbpf: Fix potential NULL pointer dereference) Merging ipsec/master (6628ddfec758 net: geneve: check skb is large enough for IPv4/IPv6 header) Merging netfilter/master (d163a925ebbc netfilter: arp_tables: add pre_exit hook for table unregister) Merging ipvs/master (fbea31808ca1 netfilter: conntrack: do not print icmpv6 as unknown via /proc) Merging wireless-drivers/master (65db391dd874 iwlwifi: mvm: fix beacon protection checks) Merging mac80211/master (864db232dc70 net: ipv6: check for validity before dereferencing cfg->fc_nlinfo.nlh) Merging rdma-fixes/for-rc (d434405aaab7 Linux 5.12-rc7) Merging sound-current/for-linus (c8426b2700b5 ALSA: hda/realtek: Fix speaker amp setup on Acer Aspire E1) Merging sound-asoc-fixes/for-linus (49065ed5ad5b Merge remote-tracking branch 'asoc/for-5.12' into asoc-linus) Merging regmap-fixes/for-linus (78d889705732 Merge remote-tracking branch 'regmap/for-5.12' into regmap-linus) Merging regulator-fixes/for-linus (6068cc31dedd Merge remote-tracking branch 'regulator/for-5.12' into regulator-linus) Merging spi-fixes/for-linus (c730b40940f9 Merge remote-tracking branch 'spi/for-5.12' into spi-linus) Merging pci-current/for-linus (cf673bd0cc97 PCI: switchtec: Fix Spectre v1 vulnerability) Merging driver-core.current/driver-core-linus (d434405aaab7 Linux 5.12-rc7) Merging tty.current/tty-linus (e49d033bddf5 Linux
[PATCH v2] watchdog: add new parameter to start the watchdog on module insertion
The new parameter "start_enabled" starts the watchdog at the same time of the module insertion. This feature is very useful in embedded systems, to avoid cases where the system hangs before reaching userspace. This feature can be enabled in the kernel config, so it can be also used when the watchdog driver is build as "built-in". This parameter involves the "core" section of the watchdog driver; in this way it is common for all the watchdog hardware implementations. Signed-off-by: Flavio Suligoi --- v2: - check WDOG_HW_RUNNING before starting watchdog; - remove useless comments in commit text, watchdog-parameters.rst and Kconfig; v1: - first version; Documentation/watchdog/watchdog-parameters.rst | 3 +++ drivers/watchdog/Kconfig | 9 + drivers/watchdog/watchdog_core.c | 12 3 files changed, 24 insertions(+) diff --git a/Documentation/watchdog/watchdog-parameters.rst b/Documentation/watchdog/watchdog-parameters.rst index 223c99361a30..7780d0c1fb4a 100644 --- a/Documentation/watchdog/watchdog-parameters.rst +++ b/Documentation/watchdog/watchdog-parameters.rst @@ -21,6 +21,9 @@ watchdog core: timeout. Setting this to a non-zero value can be useful to ensure that either userspace comes up properly, or the board gets reset and allows fallback logic in the bootloader to try something else. +start_enabled: + Watchdog is started on module insertion. This option can be also + selected by kernel config (default=kernel config parameter). - diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 0470dc15c085..1c480f4c7f94 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -47,6 +47,15 @@ config WATCHDOG_NOWAYOUT get killed. If you say Y here, the watchdog cannot be stopped once it has been started. +config WATCHDOG_START_ENABLED + bool "Start watchdog on module insertion" + help + Say Y if you want to start the watchdog at the same time when the + driver is loaded. + This feature is very useful in embedded systems, to avoid cases where + the system could hang before reaching userspace. + This parameter applies to all watchdog drivers. + config WATCHDOG_HANDLE_BOOT_ENABLED bool "Update boot-enabled watchdog until userspace takes over" default y diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c index 5df0a22e2cb4..8a1e2e9331ee 100644 --- a/drivers/watchdog/watchdog_core.c +++ b/drivers/watchdog/watchdog_core.c @@ -43,6 +43,11 @@ static int stop_on_reboot = -1; module_param(stop_on_reboot, int, 0444); MODULE_PARM_DESC(stop_on_reboot, "Stop watchdogs on reboot (0=keep watching, 1=stop)"); +static bool start_enabled = IS_ENABLED(CONFIG_WATCHDOG_START_ENABLED); +module_param(start_enabled, bool, 0444); +MODULE_PARM_DESC(start_enabled, "Start watchdog on module insertion (default=" + __MODULE_STRING(IS_ENABLED(CONFIG_WATCHDOG_START_ENABLED)) ")"); + /* * Deferred Registration infrastructure. * @@ -224,6 +229,13 @@ static int __watchdog_register_device(struct watchdog_device *wdd) * corrupted in a later stage then we expect a kernel panic! */ + /* If required, start the watchdog immediately */ + if (start_enabled && !watchdog_hw_running(wdd)) { + set_bit(WDOG_HW_RUNNING, >status); + wdd->ops->start(wdd); + pr_info("Watchdog enabled\n"); + } + /* Use alias for watchdog id if possible */ if (wdd->parent) { ret = of_alias_get_id(wdd->parent->of_node, "watchdog"); -- 2.25.1
[Outreachy patch] [PATCH] staging: rtl8188eu: Move channel_table away from rtw_mlme_ext.h
Moved "static const struct channel_table[]" from include/rtw_mlme_ext.h to core/rtw_ioctl_set.c because the latter is the only file that uses that array of struct(s) in the whole driver. "make rtl8188eu/ W=1" output several warnings about "'channel_table' defined but not used [-Wunused-const-variable=]". Signed-off-by: Fabio M. De Francesco --- drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 8 drivers/staging/rtl8188eu/include/rtw_mlme_ext.h | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c index 1ef32ff900a9..17b999f45132 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c +++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c @@ -11,6 +11,14 @@ #include #include +static const struct { +int channel_plan; +char *name; +} channel_table[] = { { RT_CHANNEL_DOMAIN_FCC, "US" }, +{ RT_CHANNEL_DOMAIN_ETSI, "EU" }, +{ RT_CHANNEL_DOMAIN_MKK, "JP" }, +{ RT_CHANNEL_DOMAIN_CHINA, "CN"} }; + extern void indicate_wx_scan_complete_event(struct adapter *padapter); u8 rtw_do_join(struct adapter *padapter) diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h index 77eb5e3ef172..03d55eb7dc16 100644 --- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h +++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h @@ -171,14 +171,6 @@ struct rt_channel_plan_map { unsigned char Index2G; }; -static const struct { - int channel_plan; - char *name; -} channel_table[] = { { RT_CHANNEL_DOMAIN_FCC, "US" }, - { RT_CHANNEL_DOMAIN_ETSI, "EU" }, - { RT_CHANNEL_DOMAIN_MKK, "JP" }, - { RT_CHANNEL_DOMAIN_CHINA, "CN"} }; - enum Associated_AP { atherosAP = 0, broadcomAP = 1, -- 2.31.1
RE: linux-next: build warning after merge of the mac80211-next tree
Hi, > > Hi all, > > After merging the mac80211-next tree, today's linux-next build (htmldocs) > produced this warning: > > include/net/cfg80211.h:6643: warning: expecting prototype for > wiphy_rfkill_set_hw_state(). Prototype was for > wiphy_rfkill_set_hw_state_reason() instead > include/net/cfg80211.h:6643: warning: expecting prototype for Ouch Johannes, this is the fix: diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 3b296f2b7a2c..c6134220dd8f 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -6634,7 +6634,7 @@ void cfg80211_notify_new_peer_candidate(struct net_device *dev, */ /** - * wiphy_rfkill_set_hw_state - notify cfg80211 about hw block state + * wiphy_rfkill_set_hw_state_reason - notify cfg80211 about hw block state * @wiphy: the wiphy * @blocked: block status * @reason: one of reasons in rfkill_hard_block_reasons Do you want a patch or you amend the original commit? It is not in net-next yet.
Re: [PATCH 5.4 097/111] net: sched: bump refcount for new action in ACT replace mode
On Tue, Apr 13, 2021 at 11:36:57AM +0100, Sudip Mukherjee wrote: > Hi Greg, > > On Mon, Apr 12, 2021 at 10:41:15AM +0200, Greg Kroah-Hartman wrote: > > From: Kumar Kartikeya Dwivedi > > > > commit 6855e8213e06efcaf7c02a15e12b1ae64b9a7149 upstream. > > This has been reverted upstream by: > 4ba86128ba07 ("Revert "net: sched: bump refcount for new action in ACT > replace mode"") Ah, I added that to the 5.10 and 5.11 queues, but not 4.19 or 5.4, odd, my fault, thanks for letting me know, I'll go fix that up... greg k-h
Re: [PATCH v2 0/7] remove different PHY fixups
On Tue, Apr 13, 2021 at 12:00:45PM +0200, Lucas Stach wrote: > I agree with the opinion that those PHY fixups introduce more harm than > good. Essentially they are pushing board specific configuration values > into the PHY, without any checks that the fixup is even running on the > specific board it was targeted at. Yes and no. The problem is, that's an easy statement to make when one doesn't understand what they're all doing. Some are "board specific" in that the normal setup for e.g. iMX6 would be to enable clock output from the AR8035 PHY and feed that into the iMX6 - as far as I'm aware, that's the only working configuration for that SoC and PHY. However, it's also true that this fixup should not be applied unconditionally. Then there's SmartEEE - it has been found that the PHY defaults for this lead to link drops independent of the board and SoC that it is connected to. It seems that the PHY is essentially broken - it powers up with SmartEEE enabled, and when connected to another SmartEEE supporting device, it seems guaranteed that it will result in link drops in its default configuration. Freescale's approach has apparently been to unconditionally disable SmartEEE for all their platforms because of this. With a bit of research however (as has been done by Jon and myself) we've found that increasing the Tw parameter for 1G connections results in a much more stable link. So, just saying that these are bad without actually understanding what they are doing is _also_ bad. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v4 05/13] module: Add printk formats to add module build ID to stacktraces
On Mon, Apr 12, 2021 at 12:29:05PM -0700, Stephen Boyd wrote: > Quoting Andy Shevchenko (2021-04-12 04:58:02) > > On Fri, Apr 09, 2021 at 06:52:52PM -0700, Stephen Boyd wrote: > > > Let's make kernel stacktraces easier to identify by including the build > > > ID[1] of a module if the stacktrace is printing a symbol from a module. > > > This makes it simpler for developers to locate a kernel module's full > > > debuginfo for a particular stacktrace. Combined with > > > scripts/decode_stracktrace.sh, a developer can download the matching > > > debuginfo from a debuginfod[2] server and find the exact file and line > > > number for the functions plus offsets in a stacktrace that match the > > > module. This is especially useful for pstore crash debugging where the > > > kernel crashes are recorded in something like console-ramoops and the > > > recovery kernel/modules are different or the debuginfo doesn't exist on > > > the device due to space concerns (the debuginfo can be too large for > > > space limited devices). > > > > > > Originally, I put this on the %pS format, but that was quickly rejected > > > given that %pS is used in other places such as ftrace where build IDs > > > aren't meaningful. There was some discussions on the list to put every > > > module build ID into the "Modules linked in:" section of the stacktrace > > > message but that quickly becomes very hard to read once you have more > > > than three or four modules linked in. It also provides too much > > > information when we don't expect each module to be traversed in a > > > stacktrace. Having the build ID for modules that aren't important just > > > makes things messy. Splitting it to multiple lines for each module > > > quickly explodes the number of lines printed in an oops too, possibly > > > wrapping the warning off the console. And finally, trying to stash away > > > each module used in a callstack to provide the ID of each symbol printed > > > is cumbersome and would require changes to each architecture to stash > > > away modules and return their build IDs once unwinding has completed. > > > > > > Instead, we opt for the simpler approach of introducing new printk > > > formats '%pS[R]b' for "pointer symbolic backtrace with module build ID" > > > and '%pBb' for "pointer backtrace with module build ID" and then > > > updating the few places in the architecture layer where the stacktrace > > > is printed to use this new format. > > > > > > Example: > > > > Can you trim a bit the example, so we will see only important lines. > > In such case you may provide "before" and "after" variants. > > > > ... > > > > > - if (modname) > > > - len += sprintf(buffer + len, " [%s]", modname); > > > + if (modname) { > > > + len += sprintf(buffer + len, " [%s", modname); > > > > > + /* build ID should match length of sprintf below */ > > > + BUILD_BUG_ON(BUILD_ID_SIZE_MAX != 20); > > > > First of all, why not static_assert() defined near to the actual macro? > > Which macro? BUILD_ID_SIZE_MAX? Yes. > I tried static_assert() and it didn't > work for me but maybe I missed something. Sounds weird. static_assert() is a good one. Check, for example, lib/vsprintf.c on how to use it. > Why is static_assert() > preferred? Because it's cleaner way to achieve it and as a bonus it can be put outside of the functions (be in the header or so). > > > + if (IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID) && add_buildid > > > && buildid) > > > + len += sprintf(buffer + len, " %20phN", buildid); > > > > len += sprintf(buffer + len, " %*phN", > > BUILD_ID_SIZE_MAX, buildid); > > > > Are you suggesting to use sprintf format here so that the size is part > of the printf? Sounds good to me. Thanks. I prefer %20phN when the size is carved in stone (for example by specification), but if you are really expecting that it may be changed in the future, use variadic approach as I showed above. -- With Best Regards, Andy Shevchenko
Re: [PATCH v3] hwmon: Add driver for fsp-3y PSUs and PDUs
út 13. 4. 2021 v 7:36 odesílatel Guenter Roeck napsal: > > On 4/12/21 9:31 PM, Václav Kubernát wrote: > [ ... ] > > Okay, I have made some additional testing. Some of the registers > > return 0x, some don't. These are the ones that pmbus_core queries > > when the driver is loading (with values I got from > > i2c_smbus_read_word_data): > > PMBUS_POUT_MAX 0x > > PMBUS_FAN_COMMAND_1 0x0 > > PMBUS_VOUT_OV_FAULT_LIMIT 0x > > PMBUS_VOUT_OV_WARN_LIMIT 0x > > PMBUS_VOUT_UV_WARN_LIMIT 0x > > PMBUS_VOUT_UV_FAULT_LIMIT 0x > > PMBUS_IOUT_OC_FAULT_LIMIT 0x > > PMBUS_IOUT_OC_WARN_LIMIT 0x10 > > PMBUS_IOUT_UC_FAULT_LIMIT 0x > > PMBUS_OT_FAULT_LIMIT 0x > > PMBUS_OT_WARN_LIMIT 0xFFB6 > > PMBUS_UT_WARN_LIMIT 0x > > PMBUS_UT_FAULT_LIMIT 0x > > PMBUS_VIN_OV_FAULT_LIMIT 0x > > PMBUS_VIN_OV_WARN_LIMIT 0x > > PMBUS_VIN_UV_WARN_LIMIT 0x > > PMBUS_VIN_UV_FAULT_LIMIT 0x > > PMBUS_IIN_OC_FAULT_LIMIT 0x > > PMBUS_IIN_OC_WARN_LIMIT 0x0 > > PMBUS_POUT_OP_FAULT_LIMIT 0x > > PMBUS_POUT_OP_WARN_LIMIT 0xFFB6 > > PMBUS_PIN_OP_WARN_LIMIT 0x0 > > PMBUS_READ_FAN_SPEED_2 0x0 > > PMBUS_MFR_VIN_MIN 0x5A > > PMBUS_MFR_VIN_MAX 0x108 > > PMBUS_MFR_IIN_MAX 0x3 > > PMBUS_MFR_PIN_MAX 0xC8 > > PMBUS_MFR_VOUT_MIN 0x1748 > > PMBUS_MFR_VOUT_MAX 0x18B0 > > PMBUS_MFR_IOUT_MAX 0xF > > PMBUS_MFR_POUT_MAX 0x96 > > PMBUS_MFR_MAX_TEMP_1 0x > > PMBUS_MFR_MAX_TEMP_2 0x > > > > The question is really what the status register reports for those. > I bet that PMBUS_STATUS_CML will set the "invalid command" bit > for many if not all registers returning 0x. Those will > be filtered out by the PMBus core automatically. That leaves the > ones returning some data. Of course it would be desirable > to have those supported, but I can understand if you don't know > the encoding. The reason for masking those needs to be explained. > I thought I sent the v4 patch before, but it seems that I only sent it to myself. Also, I realized I've sent some of the emails only to you. That wasn't my intention. Sorry. I'm bad at emailing. Anyway, I have posted the v4 patch right now and it includes a comment about the masking. Václav > Thanks, > Guenter
Re: [PATCH v2 3/4] staging: media: intel-ipu3: reduce length of line
On Tue, Apr 13, 2021 at 01:44:32PM +0300, Sakari Ailus wrote: > On Tue, Apr 13, 2021 at 04:13:04PM +0530, Mitali Borkar wrote: > > On Tue, Apr 13, 2021 at 01:01:34PM +0300, Sakari Ailus wrote: > > > Hi Mitali, > > > > > > Thanks for the update. > > > > > > On Tue, Apr 13, 2021 at 10:46:06AM +0530, Mitali Borkar wrote: > > > > Reduced length of the line under 80 characters to meet linux-kernel > > > > coding style. > > > > > > > > Signed-off-by: Mitali Borkar > > > > --- > > > > > > > > Changes from v1:- Reduced length of the line under 80 characters > > > > > > > > drivers/staging/media/ipu3/include/intel-ipu3.h | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > index 6a72c81d2b67..52dcc6cdcffc 100644 > > > > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > @@ -247,7 +247,8 @@ struct ipu3_uapi_ae_ccm { > > > > */ > > > > struct ipu3_uapi_ae_config { > > > > struct ipu3_uapi_ae_grid_config grid_cfg __aligned(32); > > > > - struct ipu3_uapi_ae_weight_elem weights[IPU3_UAPI_AE_WEIGHTS] > > > > __aligned(32); > > > > + struct ipu3_uapi_ae_weight_elem weights[IPU3_UAPI_AE_WEIGHTS] > > > > + __aligned(32); > > > > > > Do you still have the other two patches in your tree? This doesn't apply > > > here due to the different attribute syntax. > > > > > I have patch 1/6 and 2/6 in my tree which you asked me to drop. > > Could you drop them and then submit v3? > I am extremely sorry Sir, but I am still learning to use git, drop them means to delete those commits? Even if I delete those, this patch was made after those, so the changes I made then will remain as it is, so what to do now? > Thanks. > > -- > Sakari Ailus
RE: [PATCH] riscv: locks: introduce ticket-based spinlock implementation
From: Catalin Marinas > Sent: 13 April 2021 11:45 ... > This indeed needs some care. IIUC RISC-V has similar restrictions as arm > here, no load/store instructions are allowed between LR and SC. You > can't guarantee that the compiler won't spill some variable onto the > stack. You can probably never guarantee the compiler won't spill to stack. Especially if someone compiles with -O0. Which probably means that anything using LR/SC must be written in asm and the C wrappers disabled. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH v2 0/7] remove different PHY fixups
Hi Russell, sorry for the noise of this arriving in your inbox twice. Apparently I messed up and replied in private in my last mail. Am Dienstag, dem 13.04.2021 um 11:51 +0100 schrieb Russell King - ARM Linux admin: > On Tue, Apr 13, 2021 at 12:00:45PM +0200, Lucas Stach wrote: > > I agree with the opinion that those PHY fixups introduce more harm than > > good. Essentially they are pushing board specific configuration values > > into the PHY, without any checks that the fixup is even running on the > > specific board it was targeted at. > > Yes and no. The problem is, that's an easy statement to make when one > doesn't understand what they're all doing. > > Some are "board specific" in that the normal setup for e.g. iMX6 would > be to enable clock output from the AR8035 PHY and feed that into the > iMX6 - as far as I'm aware, that's the only working configuration for > that SoC and PHY. However, it's also true that this fixup should not > be applied unconditionally. > > Then there's SmartEEE - it has been found that the PHY defaults for > this lead to link drops independent of the board and SoC that it is > connected to. It seems that the PHY is essentially broken - it powers > up with SmartEEE enabled, and when connected to another SmartEEE > supporting device, it seems guaranteed that it will result in link > drops in its default configuration. > > Freescale's approach has apparently been to unconditionally disable > SmartEEE for all their platforms because of this. With a bit of > research however (as has been done by Jon and myself) we've found > that increasing the Tw parameter for 1G connections results in a > much more stable link. > > So, just saying that these are bad without actually understanding what > they are doing is _also_ bad. I'm not saying the fixups are bad per se. What I'm saying is that they are inherently board specific and the right way to apply them is either via DT properties, or if absolutely necessary via a fixup that at least checks that it is running on the specific board it was targeted at. While SmartEEE disabling will cause no big harm, aside from a bit more power consumption, a wrong clock configuration can cause major confusion. Especially if the configuration in DT and values put into the PHY via fixups differ from each other. Regards, Lucas
Re: [PATCH 0/2] Relax cpuset validation for cgroup v2
On Tue, Apr 13, 2021 at 11:02:33AM +0200, Odin Ugedal wrote: > Two small validation relaxations for cgroup v2, making it easier to > manage the hierarchy without the current pain points. Both changes > work for both mems and cpus (but I have no NUMA machine to test mems). > > Hopefully the patches has an ok description about what change they > provide, and why they are helpful. I'm generally in favor of removing configuration constraints but let's hear what Li thinks. Thanks. -- tejun
an Orphan needs your help
Hello Friend, Greetings and thanks for your reply. I am Janete moon but unfortunately, I 'm now an orphan,the only child and Daughter of late Mr and Mrs Joseph moon, from Ivory Coast Abidjan. I know it may have sounded very strange to you on why I contact you as you are a complete stranger to me and I must tell you this, It will be very difficult for me to get in touch with someone here who knows me because of the ugly circumstance that surrounds the demise of my lovely parents with whom my future ws looking very bright from all looks. My uncle conspired with my father's business rivals and poisoned my parents during a business lunch hour and their motive for eliminating them was to take over their businesses and inherit their wealth. In one of their letter's that I stumbled into, they were asking my uncle to give them their own part of the deal so While reading that letter, I fainted and my uncle came in and caught me with the letter. I'm afraid that they might decide to kill me or poison me as they did to my parents in order to keep me silent for the evil they did to my beloved late parents. For safety, I decided to run away from the house. I'm now hiding in a neigbouring country called Burkina Faso. My purpose of contacting you is because I need to come to your country secretly so that my uncle will not know my where about. I got information from my father before he died in the hospital about the secret fund (15.5 Million US Dollars only) he kept in a finance house here in Burkina Faso West Africa. I have verified this with them before contacting you. I shall require your help in transfering this money to your country for investment purpose like buying of company shares,Real Estate Investment Trust funds,Jewels or Diamond, and to continue my studies from where I stopped as Immediately after the transfer I will come to live in your country. I will give you more information once I hear from you as I am in sincere desire of your humble assistance in this regard. Your suggestions and ideas will be highly welcomed and I am willing to offer you 15% of the total fund once it is transferred to your account . Now permit me to ask these few questions 1. Can you honestly help me from your heart? 2. Can I completely trust you? Thank you and God bless. Yours affectionately Miss. Janete moon.
Re: How to handle concurrent access to /dev/ttyprintk ?
Dne 13.04.2021 (tor) ob 11:41 +0200 je Petr Mladek napisal(a): > On Mon 2021-04-12 14:41:27, Samo Pogačnik wrote: > > Dne 12.04.2021 (pon) ob 19:39 +0900 je Tetsuo Handa napisal(a): > > > What is the intended usage of /dev/ttyprintk ? > > > > > > > The intended use of 'ttyprintk' is to redirect console to /dev/ttyprintk > > via the TIOCCONS ioctl. After successfull redirection, all console > > messages get "merged" with kernel messages and as such automatically > > processed > > (stored/transferred) by the syslog service for example. > > The same can be achieved by /dev/kmsg that was created by systemd > developers. > 'kmsg' and 'ttyprintk' are different types of drivers and as such rather complementary than exclusive. The 'ttyprintk' being a tty driver allows for a system wide automatic redirection of anything written to the console. On the other hand 'kmsg' is probably better suited for a per process output redirection/injection of its output into kernel messages. Maybe i am wrong, but 'systemd' could also find 'ttyprintk' usefull? best regards, Samo
Re: linux-next: manual merge of the kvm-arm tree with the arm64 tree
On Tue, 13 Apr 2021 at 07:43, Stephen Rothwell wrote: > > Hi all, > > Today's linux-next merge of the kvm-arm tree got a conflict in: > > arch/arm64/include/asm/assembler.h > > between commits: > > 27248fe1abb2 ("arm64: assembler: remove conditional NEON yield macros") > 13150149aa6d ("arm64: fpsimd: run kernel mode NEON with softirqs disabled") > > from the arm64 tree Those patches are on a topic branch 'for-next/neon-softirqs-disabled' in the arm64 tree, so probably best to just pull that into kvm-arm and fix the conflicts there. > and commits: > > 8f4de66e247b ("arm64: asm: Provide set_sctlr_el2 macro") > 755db23420a1 ("KVM: arm64: Generate final CTR_EL0 value when running in > Protected mode") > > from the kvm-arm tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > -- > Cheers, > Stephen Rothwell > > diff --cc arch/arm64/include/asm/assembler.h > index ab569b0b45fc,34ddd8a0f3dd.. > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@@ -15,7 -15,7 +15,8 @@@ > #include > > #include > +#include > + #include > #include > #include > #include > @@@ -701,25 -705,95 +714,33 @@@ USER(\label, ic ivau, \tmp2) > // inval > isb > .endm > > + .macro set_sctlr_el1, reg > + set_sctlr sctlr_el1, \reg > + .endm > + > + .macro set_sctlr_el2, reg > + set_sctlr sctlr_el2, \reg > + .endm > + > -/* > - * Check whether to yield to another runnable task from kernel mode NEON > code > - * (which runs with preemption disabled). > - * > - * if_will_cond_yield_neon > - *// pre-yield patchup code > - * do_cond_yield_neon > - *// post-yield patchup code > - * endif_yield_neon > - * > - * where is optional, and marks the point where execution will > resume > - * after a yield has been performed. If omitted, execution resumes right > after > - * the endif_yield_neon invocation. Note that the entire sequence, including > - * the provided patchup code, will be omitted from the image if > - * CONFIG_PREEMPTION is not defined. > - * > - * As a convenience, in the case where no patchup code is required, the > above > - * sequence may be abbreviated to > - * > - * cond_yield_neon > - * > - * Note that the patchup code does not support assembler directives that > change > - * the output section, any use of such directives is undefined. > - * > - * The yield itself consists of the following: > - * - Check whether the preempt count is exactly 1 and a reschedule is also > - * needed. If so, calling of preempt_enable() in kernel_neon_end() will > - * trigger a reschedule. If it is not the case, yielding is pointless. > - * - Disable and re-enable kernel mode NEON, and branch to the yield fixup > - * code. > - * > - * This macro sequence may clobber all CPU state that is not guaranteed by > the > - * AAPCS to be preserved across an ordinary function call. > - */ > - > - .macro cond_yield_neon, lbl > - if_will_cond_yield_neon > - do_cond_yield_neon > - endif_yield_neon\lbl > - .endm > - > - .macro if_will_cond_yield_neon > -#ifdef CONFIG_PREEMPTION > - get_current_taskx0 > - ldr x0, [x0, #TSK_TI_PREEMPT] > - sub x0, x0, #PREEMPT_DISABLE_OFFSET > - cbz x0, .Lyield_\@ > - /* fall through to endif_yield_neon */ > - .subsection 1 > -.Lyield_\@ : > -#else > - .section".discard.cond_yield_neon", "ax" > -#endif > - .endm > - > - .macro do_cond_yield_neon > - bl kernel_neon_end > - bl kernel_neon_begin > - .endm > - > - .macro endif_yield_neon, lbl > - .ifnb \lbl > - b \lbl > - .else > - b .Lyield_out_\@ > - .endif > - .previous > -.Lyield_out_\@ : > - .endm > - > /* > - * Check whether preempt-disabled code should yield as soon as it > - * is able. This is the case if re-enabling preemption a single > - * time results in a preempt count of zero, and the TIF_NEED_RESCHED > - * flag is set. (Note that the latter is stored negated in the > - * top word of the thread_info::preempt_count field) > + * Check whether preempt/bh-disabled asm code should yield as soon as > + * it is able. This is the case if we are currently running in task > + * context, and either a softirq is pending, or the TIF_NEED_RESCHED > + * flag is set and re-enabling
Re: Subject: [PATCH v2] staging: media: meson: vdec: declare u32 as static const appropriately
On 13/04/2021 13:09, Mitali Borkar wrote: > On Tue, Apr 13, 2021 at 09:26:01AM +0200, Hans Verkuil wrote: >> On 13/04/2021 08:27, Mitali Borkar wrote: >>> Declared 32 bit unsigned int as static constant inside a function >>> appropriately. >>> >>> Reported-by: kernel test robot >>> Signed-off-by: Mitali Borkar >>> --- >>> >>> Changes from v1:- Rectified the mistake by declaring u32 as static const >>> properly. >>> >>> drivers/staging/media/meson/vdec/codec_h264.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/media/meson/vdec/codec_h264.c >>> b/drivers/staging/media/meson/vdec/codec_h264.c >>> index ea86e9e1c447..80141b89a9f6 100644 >>> --- a/drivers/staging/media/meson/vdec/codec_h264.c >>> +++ b/drivers/staging/media/meson/vdec/codec_h264.c >>> @@ -287,8 +287,8 @@ static void codec_h264_resume(struct amvdec_session >>> *sess) >>> struct amvdec_core *core = sess->core; >>> struct codec_h264 *h264 = sess->priv; >>> u32 mb_width, mb_height, mb_total; >>> - static const u32[] canvas3 = { ANCO_CANVAS_ADDR, 0 }; >>> - static const u32[] canvas4 = { 24, 0 }; >>> + static const u32 canvas3[] = { ANCO_CANVAS_ADDR, 0 }; >>> + static const u32 canvas4[] = { 24, 0 }; >> >> This is a patch on top of your previous (v1) patch. That won't work >> since the v1 is not merged, you need to make a patch against the current >> mainline code. >> > But Sir, since I have made changes in the code, and committed them, now, > if I open that file, it will contain those changes. Then should I > rewrite the patch body more accurately? You only committed the v1 change in your own repository, it's not in the upstream repository. And the patches you post must be against the upstream repository, not your own. 'git rebase -i' can be your friend here, it makes it easy to fold the second patch into the first, and then you only have to post the final version. Regards, Hans > >> Regards, >> >> Hans >> >>> >>> amvdec_set_canvases(sess, canvas3, canvas4); >>> >>> >>
Re: [PATCH v7 3/3] bio: add limit_bio_size sysfs
> On Tue, Apr 13, 2021 at 11:55:02AM +0900, Changheun Lee wrote: > > Add limit_bio_size block sysfs node to limit bio size. > > Queue flag QUEUE_FLAG_LIMIT_BIO_SIZE will be set if limit_bio_size is set. > > And bio max size will be limited by queue max sectors via > > QUEUE_FLAG_LIMIT_BIO_SIZE set. > > > > Signed-off-by: Changheun Lee > > --- > > Documentation/ABI/testing/sysfs-block | 10 ++ > > Documentation/block/queue-sysfs.rst | 7 +++ > > block/blk-sysfs.c | 3 +++ > > 3 files changed, 20 insertions(+) > > Isn't it too late to change the sysfs entry after the device has been > probed and initialized by the kernel as the kernel does not look at this > value after that? Kernel will take a look this when page is merged to bio. And bio size will be limited dynamically. > > Why do you need a userspace knob for this? What tool is going to ever > change this, and what logic is it going to use to change it? Why can't > the kernel also just "do the right thing" and properly detect this > option as well as userspace can? Actually I didn't considerate userspace knob at first. And there is no tool, no logic to change it. It's a kind of policy about bio max size. One time setting will be OK on system boot time actually. At the first, I suggested 1MB bio max size. It is same with bio max size before applying of multipage bvec. But there are requests of big bio size on another system environment, and feedback of knob for utility too. So I made this as a optional for each system, and a knob too. > > thanks, > > greg k-h Thanks, Changheun Lee
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:214:2: error: implicit declaration of function 'enable_kernel_altivec'
Hi Alex, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 89698becf06d341a700913c3d89ce2a914af69a2 commit: 582e2ce5b4ece37055c6ebe58ab48a4817d30b10 drm/amdgpu/display: FP fixes for DCN3.x (v4) date: 5 months ago config: powerpc64-randconfig-r013-20210413 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc64 cross compiling tool for clang build # apt-get install binutils-powerpc64-linux-gnu # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=582e2ce5b4ece37055c6ebe58ab48a4817d30b10 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 582e2ce5b4ece37055c6ebe58ab48a4817d30b10 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:214:2: error: implicit declaration of function 'enable_kernel_vsx' [-Werror,-Wimplicit-function-declaration] DC_FP_START(); ^ drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:68:3: note: expanded from macro 'DC_FP_START' enable_kernel_vsx(); \ ^ drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:214:2: note: did you mean 'enable_kernel_fp'? drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:68:3: note: expanded from macro 'DC_FP_START' enable_kernel_vsx(); \ ^ arch/powerpc/include/asm/switch_to.h:40:13: note: 'enable_kernel_fp' declared here extern void enable_kernel_fp(void); ^ >> drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:214:2: >> error: implicit declaration of function 'enable_kernel_altivec' >> [-Werror,-Wimplicit-function-declaration] DC_FP_START(); ^ drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:71:3: note: expanded from macro 'DC_FP_START' enable_kernel_altivec(); \ ^ drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:214:2: note: did you mean 'enable_kernel_fp'? drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:71:3: note: expanded from macro 'DC_FP_START' enable_kernel_altivec(); \ ^ arch/powerpc/include/asm/switch_to.h:40:13: note: 'enable_kernel_fp' declared here extern void enable_kernel_fp(void); ^ drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:216:2: error: implicit declaration of function 'disable_kernel_vsx' [-Werror,-Wimplicit-function-declaration] DC_FP_END(); ^ drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:79:3: note: expanded from macro 'DC_FP_END' disable_kernel_vsx(); \ ^ drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:216:2: note: did you mean 'disable_kernel_fp'? drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:79:3: note: expanded from macro 'DC_FP_END' disable_kernel_vsx(); \ ^ arch/powerpc/include/asm/switch_to.h:44:20: note: 'disable_kernel_fp' declared here static inline void disable_kernel_fp(void) ^ >> drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:216:2: >> error: implicit declaration of function 'disable_kernel_altivec' >> [-Werror,-Wimplicit-function-declaration] DC_FP_END(); ^ drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:82:3: note: expanded from macro 'DC_FP_END' disable_kernel_altivec(); \ ^ drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c:216:2: note: did you mean 'disable_kernel_fp'? drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:82:3: note: expanded from macro 'DC_FP_END' disable_kernel_altivec(); \ ^ arch/powerpc/include/asm/switch_to.h:44:20: note: 'disable_kernel_fp' declared here static inline void disable_kernel_fp(void) ^ 4 errors generated. -- drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_hwseq.h:244:2: note: expanded from macro 'HWSEQ_DCN2_REG_LIST' SR(MPC_CRC_RESULT_C), \ ^~~~ drivers/gpu/drm/amd/amdgpu/.
[PATCH v2 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same function to initialise the hdr_output_metadata property, and while the creation of that property is in a helper, every driver uses the same code to attach it. Provide a helper for it as well Reviewed-by: Harry Wentland Reviewed-by: Jernej Skrabec Signed-off-by: Maxime Ripard --- Changes from v1: - Rebased on latest drm-misc-next tag - Added the tags --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +-- drivers/gpu/drm/drm_connector.c | 21 +++ drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +-- include/drm/drm_connector.h | 1 + 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 55e39b462a5e..1e22ce718010 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7078,9 +7078,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, if (connector_type == DRM_MODE_CONNECTOR_HDMIA || connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_eDP) { - drm_object_attach_property( - >base.base, - dm->ddev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(>base); if (!aconnector->mst_port) drm_connector_attach_vrr_capable_property(>base); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..f24bbb840dbf 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi) drm_connector_attach_max_bpc_property(connector, 8, 16); if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe) - drm_object_attach_property(>base, - connector->dev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(connector); drm_connector_attach_encoder(connector, hdmi->bridge.encoder); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 7631f76e7f34..a4aa2d87af35 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2150,6 +2150,27 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector, } EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); +/** + * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property + * @connector: connector to attach the property on. + * + * This is used to allow the userspace to send HDR Metadata to the + * driver. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + struct drm_property *prop = dev->mode_config.hdr_output_metadata_property; + + drm_object_attach_property(>base, prop, 0); + + return 0; +} +EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property); + /** * drm_connector_set_vrr_capable_property - sets the variable refresh rate * capable property for a connector diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 95919d325b0b..f2f1b025e6ba 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2965,8 +2965,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c drm_connector_attach_content_type_property(connector); if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) - drm_object_attach_property(>base, - connector->dev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(connector); if (!HAS_GMCH(dev_priv)) drm_connector_attach_max_bpc_property(connector, 8, 12); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 1922b278ffad..32172dab8427 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); int drm_connector_attach_vrr_capable_property( struct drm_connector *connector); +int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector); int drm_mode_create_aspect_ratio_property(struct drm_device *dev); int
Re: [PATCH 5.4 097/111] net: sched: bump refcount for new action in ACT replace mode
On Tue, Apr 13, 2021 at 12:51:10PM +0200, Greg Kroah-Hartman wrote: > On Tue, Apr 13, 2021 at 11:36:57AM +0100, Sudip Mukherjee wrote: > > Hi Greg, > > > > On Mon, Apr 12, 2021 at 10:41:15AM +0200, Greg Kroah-Hartman wrote: > > > From: Kumar Kartikeya Dwivedi > > > > > > commit 6855e8213e06efcaf7c02a15e12b1ae64b9a7149 upstream. > > > > This has been reverted upstream by: > > 4ba86128ba07 ("Revert "net: sched: bump refcount for new action in ACT > > replace mode"") > > Ah, I added that to the 5.10 and 5.11 queues, but not 4.19 or 5.4, odd, > my fault, thanks for letting me know, I'll go fix that up... I see why I did it that way. The revert is there because of additional patches in the tree that means that the original patch is not needed anymore. But those patches were not backported to 4.19 and 5.4, so the revert shouldn't be needed either. Right? It would be great if someone else could verify this... thanks, greg k-h
Re: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic warning on PCI_IOBASE
On Tue, 2021-04-13 at 14:26 +0200, Arnd Bergmann wrote: > On Tue, Apr 13, 2021 at 1:54 PM Niklas Schnelle > wrote: > > When PCI_IOBASE is not defined, it is set to 0 such that it is ignored > > in calls to the readX/writeX primitives. While mathematically obvious > > this triggers clang's -Wnull-pointer-arithmetic warning. > > Indeed, this is an annoying warning. > > > An additional complication is that PCI_IOBASE is explicitly typed as > > "void __iomem *" which causes the type conversion that converts the > > "unsigned long" port/addr parameters to the appropriate pointer type. > > As non pointer types are used by drivers at the callsite since these are > > dealing with I/O port numbers, changing the parameter type would cause > > further warnings in drivers. Instead use "uintptr_t" for PCI_IOBASE > > 0 and explicitly cast to "void __iomem *" when calling readX/writeX. > > > > Signed-off-by: Niklas Schnelle > > --- > > include/asm-generic/io.h | 26 +- > > 1 file changed, 13 insertions(+), 13 deletions(-) > > > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h > > index c6af40ce03be..8eb00bdef7ad 100644 > > --- a/include/asm-generic/io.h > > +++ b/include/asm-generic/io.h > > @@ -441,7 +441,7 @@ static inline void writesq(volatile void __iomem *addr, > > const void *buffer, > > #endif /* CONFIG_64BIT */ > > > > #ifndef PCI_IOBASE > > -#define PCI_IOBASE ((void __iomem *)0) > > +#define PCI_IOBASE ((uintptr_t)0) > > #endif > > > > #ifndef IO_SPACE_LIMIT > > Your patch looks wrong in the way it changes the type of one of the > definitions, > but not the type of any of the architecture specific ones. It's also > awkward since > 'void __iomem *' is really the correct type, while 'uintptr_t' is not! Yeah I see your point. The way I justified it for myself is that the above define really only serves to ignore the PCI_IOBASE and the explicit cast in the function makes the actual type more clear since the parameters have the "wrong" type too. I do agree that this still leaves things somewhat awkward. > > I think the real underlying problem is that '0' is a particularly bad > default value, > we should not have used this one in asm-generic, or maybe have left it as > mandatory to be defined by an architecture to a sane value. Note that > architectures that don't actually support I/O ports will cause a NULL > pointer dereference when loading a legacy driver, which is exactly what clang > is warning about here. Architectures that to support I/O ports in PCI > typically > map them at a fixed location in the virtual address space and should put that > location here, rather than adding the pointer value to the port resources. > > What we might do instead here would be move the definition into those > architectures that actually define the base to be at address zero, with a > comment explaining why this is a bad location, and then changing the > inb/outb style helpers to either an empty function or a WARN_ONCE(). > > On which architectures do you see the problem? How is the I/O port > actually mapped there? > > Arnd I'm seeing this on s390 which indeed has no I/O port support at all. I'm not sure how many others there are but I feel like us having to define these functions as empty is also kind of awkward. Maybe we could put them into the asm-generic/io.h for the case that PCI_IOBASE is not defined? Then at least every platform not supporting I/O ports would share them.
Re: Linux 5.12-rc7
On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote: > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet wrote: > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds > > wrote: > > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck wrote: > > > > > > > > Qemu test results: > > > > total: 460 pass: 459 fail: 1 > > > > Failed tests: > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs > > > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull > > > > payload in > > > > skb->head"). It is a spurious problem - the test passes roughly every > > > > other > > > > time. When the failure is seen, udhcpc fails to get an IP address and > > > > aborts > > > > with SIGTERM. So far I have only seen this with the "sh" architecture. > > > > > > Hmm. Let's add in some more of the people involved in that commit, and > > > also netdev. > > > > > > Nothing in there looks like it should have any interaction with > > > architecture, so that "it happens on sh" sounds odd, but maybe it's > > > some particular interaction with the qemu environment. > > > > Yes, maybe. > > > > I spent few hours on this, and suspect a buggy memcpy() implementation > > on SH, but this was not conclusive. > > > > By pulling one extra byte, the problem goes away. > > > > Strange thing is that the udhcpc process does not go past sendto(). > > This is the patch working around the issue. Unfortunately I was not > able to root-cause it (I really suspect something on SH) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655 > 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct > virtnet_info *vi, > > /* Copy all frame if it fits skb->head, otherwise > * we let virtio_net_hdr_to_skb() and GRO pull headers as needed. > +* > +* Apparently, pulling only the Ethernet Header triggers a bug > on qemu-system-sh4. > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 bytes > +* more to work around this bug : These 20 bytes can not belong > +* to UDP/TCP payload. > +* As a bonus, this makes GRO slightly faster for IPv4 (one less > copy). > */ Question: do we still want to do this for performance reasons? We also have the hdr_len coming from the device which is just skb_headlen on the host. > if (len <= skb_tailroom(skb)) > copy = len; > else > - copy = ETH_HLEN + metasize; > + copy = ETH_HLEN + sizeof(struct iphdr) + metasize; > skb_put_data(skb, p, copy); > > if (metasize) {
Re: [PATCH 3/4] staging: rtl8712: remove space after cast
This patch has already been applied, which is fine, but really all these casts are totally pointless and should be removed. regards, dan carpenter
RE: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic warning on PCI_IOBASE
From: Arnd Bergmann > Sent: 13 April 2021 13:27 > > On Tue, Apr 13, 2021 at 1:54 PM Niklas Schnelle > wrote: > > > > When PCI_IOBASE is not defined, it is set to 0 such that it is ignored > > in calls to the readX/writeX primitives. While mathematically obvious > > this triggers clang's -Wnull-pointer-arithmetic warning. > > Indeed, this is an annoying warning. > > > An additional complication is that PCI_IOBASE is explicitly typed as > > "void __iomem *" which causes the type conversion that converts the > > "unsigned long" port/addr parameters to the appropriate pointer type. > > As non pointer types are used by drivers at the callsite since these are > > dealing with I/O port numbers, changing the parameter type would cause > > further warnings in drivers. Instead use "uintptr_t" for PCI_IOBASE > > 0 and explicitly cast to "void __iomem *" when calling readX/writeX. Since the definitions make no sense when PCI_IOBASE in undefined maybe the functions should either not be defined, be stubs that do nothing or stubs that are BUG(). David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node_range
Hi, On Tue, 13 Apr 2021 18:03:24 +0800 Jisheng Zhang wrote: > Use the __vmalloc_node_range() to simplify x86's alloc_insn_page() > implementation. Have you checked this is equivarent to the original code on all architecture? IIRC, some arch has a special module_alloc(), thus I NACKed similar patch previously. Thank you, > > Signed-off-by: Jisheng Zhang > --- > arch/x86/kernel/kprobes/core.c | 24 > 1 file changed, 4 insertions(+), 20 deletions(-) > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > index df776cdca327..75081f3dbe44 100644 > --- a/arch/x86/kernel/kprobes/core.c > +++ b/arch/x86/kernel/kprobes/core.c > @@ -383,26 +383,10 @@ static int prepare_boost(kprobe_opcode_t *buf, struct > kprobe *p, > /* Make page to RO mode when allocate it */ > void *alloc_insn_page(void) > { > - void *page; > - > - page = module_alloc(PAGE_SIZE); > - if (!page) > - return NULL; > - > - set_vm_flush_reset_perms(page); > - /* > - * First make the page read-only, and only then make it executable to > - * prevent it from being W+X in between. > - */ > - set_memory_ro((unsigned long)page, 1); > - > - /* > - * TODO: Once additional kernel code protection mechanisms are set, > ensure > - * that the page was not maliciously altered and it is still zeroed. > - */ > - set_memory_x((unsigned long)page, 1); > - > - return page; > + return __vmalloc_node_range(PAGE_SIZE, PAGE_SIZE, VMALLOC_START, > + VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX, > + VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > + __builtin_return_address(0)); > } > > /* Recover page to RW mode before releasing it */ > -- > 2.31.0 > -- Masami Hiramatsu
Re: [PATCH 3/4] kbuild: spilt cc-option and friends to scripts/Makefile.compiler
On 13.04.21 14:51, Janosch Frank wrote: On 2/28/21 7:10 AM, Masahiro Yamada wrote: scripts/Kbuild.include is included everywhere, but macros such as cc-option are needed by build targets only. For example, when 'make clean' traverses the tree, it does not need to evaluate $(call cc-option,). Split cc-option, ld-option, etc. to scripts/Makefile.compiler, which is only included from the top Makefile and scripts/Makefile.build. Signed-off-by: Masahiro Yamada This commit broke the KVM selftest compilation under s390 in linux-next for me. Funny enough the compilation is only broken on Ubuntu, under Fedora the test fails with an assertion. FEDORA: [root@fedora kvm]# ./set_memory_region_test Allowed number of memory slots: 32767 Test Assertion Failure lib/kvm_util.c:142: vm->fd >= 0 pid=1541645 tid=1541645 - Invalid argument 1 0x01002f4b: vm_open at kvm_util.c:142 2 (inlined by) vm_create at kvm_util.c:258 3 0x010015ef: test_add_max_memory_regions at set_memory_region_test.c:351 4 (inlined by) main at set_memory_region_test.c:397 5 0x03ffa3d2bb89: ?? ??:0 6 0x010017ad: .annobin_abi_note.c.hot at crt1.o:? KVM_CREATE_VM ioctl failed, rc: -1 errno: 22 Ubuntu: make[1]: Leaving directory '/mnt/dev/linux' gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE -I../../../../tools/include -I../../../../tools/arch/s390/include -I../../../../usr/include/ -Iinclude -Ilib -Iinclude/s390x -I.. -c lib/sparsebit.c -o /mnt/dev/linux/tools/testing/selftests/kvm/lib/sparsebit.o gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE -I../../../../tools/include -I../../../../tools/arch/s390/include -I../../../../usr/include/ -Iinclude -Ilib -Iinclude/s390x -I.. -c lib/kvm_util.c -o /mnt/dev/linux/tools/testing/selftests/kvm/lib/kvm_util.o gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE -I../../../../tools/include -I../../../../tools/arch/s390/include -I../../../../usr/include/ -Iinclude -Ilib/s390x -Iinclude/s390x -I.. -c lib/s390x/diag318_test_handler.c -o /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.o ar crs /mnt/dev/linux/tools/testing/selftests/kvm/libkvm.a /mnt/dev/linux/tools/testing/selftests/kvm/lib/assert.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/elf.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/io.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/kvm_util.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/sparsebit.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/test_util.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/guest_modes.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/perf_test_util.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/processor.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/ucall.o /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.o gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE -I../../../../tools/include -I../../../../tools/arch/s390/include -I../../../../usr/include/ -Iinclude -Is390x -Iinclude/s390x -I.. -pthreads390x/memop.c /mnt/dev/linux/tools/testing/selftests/kvm/libkvm.a -o /mnt/dev/linux/tools/testing/selftests/kvm/s390x/memop /usr/bin/ld: /tmp/ccFU8WYF.o: `stdout@@GLIBC_2.2' non-PLT reloc for symbol defined in shared library and accessed from executable (rebuild file with -fPIC ?) /usr/bin/ld: final link failed: bad value collect2: error: ld returned 1 exit status make: *** [../lib.mk:139: /mnt/dev/linux/tools/testing/selftests/kvm/s390x/memop] Error 1 It looks like that from tools/testing/selftests/kvm/Makefile additional linker flags are being ignored with this patch. no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \ $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie) # On s390, build the testcases KVM-enabled pgste-option = $(call try-run, echo 'int main() { return 0; }' | \ $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste) LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
Re: [PATCH net-next 1/2] net: phy: marvell-88x2222: check that link is operational
> Indeed - it should be a logical and operation - there is light present > _and_ the PHY recognises the signal. This is what the commit achieves, > although (iirc) doesn't cater for the case where there is no SFP cage > attached. Hi Russell Is there something like this in the marvell10 driver? Also, do you know when there is an SFP cage? Do we need a standardised DT property for this? Andrew
Re: [PATCH] spi: SPI_HISI_KUNPENG should depend on ARCH_HISI
On Tue, Apr 13, 2021 at 08:59:02PM +0800, Jay Fang wrote: > On 2021/4/13 20:47, Mark Brown wrote: > > On Tue, Apr 13, 2021 at 02:27:23PM +0200, Geert Uytterhoeven wrote: > >> The HiSilicon Kunpeng SPI controller is only present on HiSilicon > >> Kunpeng SoCs. Hence add a dependency on ARCH_HISI, to prevent asking > >> the user about this driver when configuring a kernel without Hisilicon > >> platform support. > > Are you *sure* about this? HiSilicon produce a wide range of SoCs with > > very diverse target markets, this driver looks like it's for enterprise > > stuff while most things guarded by that config option look like they're > > for embedded applications. > SPI_HISI_KUNPENG does not depend on ARCH_HISI. Right, but that's what Geert is proposing to change - the question is does it make sense to do so or not? signature.asc Description: PGP signature
Re: [Outreachy kernel] [PATCH] staging: rtl8192u: ieee80211: Replaced strncpy() with strscpy()
On Tuesday, April 13, 2021 2:59:29 PM CEST Greg Kroah-Hartman wrote: > On Tue, Apr 13, 2021 at 02:30:41PM +0200, Fabio M. De Francesco wrote: > > Replaced strncpy() with strscpy() because of compilation time warnings > > about possible truncation of output [-Wstringop-truncation]. > > build warnings? What build warnings? > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:1388:5: warning: ‘strncpy’ output may be truncated copying 32 bytes from a string of length 32 [-Wstringop-truncation] 1388 | strncpy(tmp_ssid, ieee->current_network.ssid, IW_ESSID_MAX_SIZE); > > > Furthermore, according to the Linux official documentation, strscpy() > > is > > preferred to strncpy. > > > > Signed-off-by: Fabio M. De Francesco > > --- > > > > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c > > b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c index > > 25ea8e1b6b65..aa58eedf5e86 100644 > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c > > @@ -1385,12 +1385,12 @@ inline void ieee80211_softmac_new_net(struct > > ieee80211_device *ieee, struct ieee> > > * essid provided by the user. > > */ > > > > if (!ssidbroad) { > > > > - strncpy(tmp_ssid, ieee- >current_network.ssid, IW_ESSID_MAX_SIZE); > > + strscpy(tmp_ssid, ieee- >current_network.ssid, IW_ESSID_MAX_SIZE); > > Are you sure you can just replace this like this? > I surely was... but now I'm not anymore, since your review :) Maybe you mean I have to check possible return of -E2BIG? Did you mean something else? May you please elaborate further? Thanks, Fabio > > > tmp_ssid_len = ieee- >current_network.ssid_len; > > > > } > > memcpy(>current_network, net, sizeof(struct > > ieee80211_network)); > > > > - strncpy(ieee->current_network.ssid, tmp_ssid, IW_ESSID_MAX_SIZE); > > + strscpy(ieee->current_network.ssid, tmp_ssid, IW_ESSID_MAX_SIZE); > > Same here, are you sure? > > thanks, > > greg k-h
memory leak in ext4_multi_mount_protect
Hi! I've done debugging on this issue https://syzkaller.appspot.com/bug?id=420258a304e5d92cfef6b0097f87b42506e1db08 and I want to ask you about proper way of fixing it. The problem was in case sbi->s_mmp_tsk hasn’t started at the time of kthread_stop() call. In that case allocated data won't be freed. I wrote fix patch, but I am confused about it, because I didn't find any kernel code like this. I don't think, that adding new members to struct super_block is good idea, that's why I came to that decision: diff --git a/fs/ext4/super.c b/fs/ext4/super.c index b9693680463a..9c33e97bd5c5 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5156,8 +5156,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) failed_mount3: flush_work(>s_error_work); del_timer_sync(>s_err_report); - if (sbi->s_mmp_tsk) - kthread_stop(sbi->s_mmp_tsk); + if (sbi->s_mmp_tsk) { + if (kthread_stop(sbi->s_mmp_tsk) == -EINTR) + kfree(kthread_data(sbi->s_mmp_tsk)); + } failed_mount2: rcu_read_lock(); group_desc = rcu_dereference(sbi->s_group_desc); I look forward to hearing your perspective on this patch :) With regards, Pavel Skripkin
Re: [PATCH v3 1/4] i2c: mpc: use device managed APIs
On Tue, Apr 13, 2021 at 4:31 PM Andy Shevchenko wrote: > > On Tue, Apr 13, 2021 at 3:21 PM Wolfram Sang wrote: > > > > > > > Yongjun[1] into the original patch. If Wei's patch is applied on top > > > of whats already in i2c/for-next then this patch can be ignored. > > > > I applied Wei's patch instead. It was easier. > > Where is it? i2c/for-next shows the v2 of this series being applied. Ah, I see it's a follow up. I thought it was a replacement. -- With Best Regards, Andy Shevchenko
Re: [PATCH v8] RISC-V: enable XIP
Le 4/13/21 à 2:35 AM, Alexandre Ghiti a écrit : From: Vitaly Wool Introduce XIP (eXecute In Place) support for RISC-V platforms. It allows code to be executed directly from non-volatile storage directly addressable by the CPU, such as QSPI NOR flash which can be found on many RISC-V platforms. This makes way for significant optimization of RAM footprint. The XIP kernel is not compressed since it has to run directly from flash, so it will occupy more space on the non-volatile storage. The physical flash address used to link the kernel object files and for storing it has to be known at compile time and is represented by a Kconfig option. XIP on RISC-V will for the time being only work on MMU-enabled kernels. Signed-off-by: Alexandre Ghiti [ Rebase on top of "Move kernel mapping outside the linear mapping" ] Signed-off-by: Vitaly Wool --- I forgot the changes history: Changes in v2: - dedicated macro for XIP address fixup when MMU is not enabled yet o both for 32-bit and 64-bit RISC-V - SP is explicitly set to a safe place in RAM before __copy_data call - removed redundant alignment requirements in vmlinux-xip.lds.S - changed long -> uintptr_t typecast in __XIP_FIXUP macro. Changes in v3: - rebased against latest for-next - XIP address fixup macro now takes an argument - SMP related fixes Changes in v4: - rebased against the current for-next - less #ifdef's in C/ASM code - dedicated XIP_FIXUP_OFFSET assembler macro in head.S - C-specific definitions moved into #ifndef __ASSEMBLY__ - Fixed multi-core boot Changes in v5: - fixed build error for non-XIP kernels Changes in v6: - XIP_PHYS_RAM_BASE config option renamed to PHYS_RAM_BASE - added PHYS_RAM_BASE_FIXED config flag to allow usage of PHYS_RAM_BASE in non-XIP configurations if needed - XIP_FIXUP macro rewritten with a tempoarary variable to avoid side effects - fixed crash for non-XIP kernels that don't use built-in DTB Changes in v7: - Fix pfn_base that required FIXUP - Fix copy_data which lacked + 1 in size to copy - Fix pfn_valid for FLATMEM - Rebased on top of "Move kernel mapping outside the linear mapping": this is the biggest change and affected mm/init.c, kernel/vmlinux-xip.lds.S and include/asm/pgtable.h: XIP kernel is now mapped like 'normal' kernel at the end of the address space. Changes in v8: - XIP_KERNEL now depends on SPARSEMEM - FLATMEM related: pfn_valid and pfn_base removal arch/riscv/Kconfig | 55 +++- arch/riscv/Makefile | 8 +- arch/riscv/boot/Makefile| 13 +++ arch/riscv/include/asm/page.h | 21 + arch/riscv/include/asm/pgtable.h| 25 +- arch/riscv/kernel/head.S| 46 +- arch/riscv/kernel/head.h| 3 + arch/riscv/kernel/setup.c | 10 ++- arch/riscv/kernel/vmlinux-xip.lds.S | 133 arch/riscv/kernel/vmlinux.lds.S | 6 ++ arch/riscv/mm/init.c| 115 ++-- 11 files changed, 418 insertions(+), 17 deletions(-) create mode 100644 arch/riscv/kernel/vmlinux-xip.lds.S diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 8ea60a0a19ae..7c7efdd67a10 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -28,7 +28,7 @@ config RISCV select ARCH_HAS_PTE_SPECIAL select ARCH_HAS_SET_DIRECT_MAP select ARCH_HAS_SET_MEMORY - select ARCH_HAS_STRICT_KERNEL_RWX if MMU + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT @@ -441,7 +441,7 @@ config EFI_STUB config EFI bool "UEFI runtime support" - depends on OF + depends on OF && !XIP_KERNEL select LIBFDT select UCS2_STRING select EFI_PARAMS_FROM_FDT @@ -465,11 +465,60 @@ config STACKPROTECTOR_PER_TASK def_bool y depends on STACKPROTECTOR && CC_HAVE_STACKPROTECTOR_TLS +config PHYS_RAM_BASE_FIXED + bool "Explicitly specified physical RAM address" + default n + +config PHYS_RAM_BASE + hex "Platform Physical RAM address" + depends on PHYS_RAM_BASE_FIXED + default "0x8000" + help + This is the physical address of RAM in the system. It has to be + explicitly specified to run early relocations of read-write data + from flash to RAM. + +config XIP_KERNEL + bool "Kernel Execute-In-Place from ROM" + depends on MMU && SPARSEMEM + select PHYS_RAM_BASE_FIXED + help + Execute-In-Place allows the kernel to run from non-volatile storage + directly addressable by the CPU, such as NOR flash. This saves RAM + space since the text section of the kernel is not loaded from flash + to RAM. Read-write sections, such as the data section and stack, + are still copied to
aoe: kernel crash on blk_update_request: I/O error, BUG: scheduling while atomic
Dear Maintainers, It seems we found a race condition in the aoe driver that leads to a kernel crash. It is triggered when an aoe device is unavailable and therefore produces an I/O error in the code that tries to remove the device. (drivers/block/aoe/aoedev.c: aoedev_downdev) I reproduced this behaviour successfully with kernels 5.10.26 and 5.10.28 from the debian archives (bullseye an unstable). Kernel 4.19.181 from debian buster is confirmed not to be affected. The bug was also reported to debian [1] and kernel.org [2]. example process to reproduce: * add an aoe-target to a lvm2 volume group * make the aoe target unavailable (e.g. set the network dev down) but don't flush it * run a command that scans all physical volumes, e.g. 'vgs' * wait for aoe to time out (default for aoe_deadsecs is 180s) result: kernel crash relevant dmesg output: [] [ 183.855191] mlx4_en: enp65s0d1: Close port called [ 183.931534] mlx4_en: enp65s0d1: Link Down [ 408.620155] blk_update_request: I/O error, dev etherd/e42.0, sector 4096 op 0x0:(READ) flags 0x0 phys_seg 2 prio class 0 [ 408.620235] blk_update_request: I/O error, dev etherd/e42.0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 2 prio class 0 [ 408.620290] BUG: scheduling while atomic: swapper/16/0/0x0100 [ 408.620325] Modules linked in: sctp bridge 8021q garp stp mrp llc psmouse dlm configfs aoe ipmi_ssif amd64_edac_mod edac_mce_amd amd_energy kvm_amd kvm irqbypass ghash_clmulni_intel aesni_intel libaes crypto_simd cryptd glue_helper rapl pcspkr ast drm_vram_helper drm_ttm_helper ttm drm_kms_helper cec drm evdev joydev ccp sg sp5100_tco rng_core watchdog k10temp acpi_ipmi ipmi_si ipmi_devintf ipmi_msghandler acpi_cpufreq button ext4 crc16 mbcache jbd2 dm_mod raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c crc32c_generic raid0 multipath linear mlx4_ib ib_uverbs mlx4_en raid1 md_mod sd_mod t10_pi crc_t10dif crct10dif_generic ib_core hid_generic usbhid hid crct10dif_pclmul crct10dif_common crc32_pclmul crc32c_intel xhci_pci igb mpt3sas xhci_hcd ahci libahci i2c_algo_bit dca ptp libata pps_core raid_class usbcore scsi_transport_sas mlx4_core scsi_mod i2c_piix4 usb_common [ 408.620422] CPU: 16 PID: 0 Comm: swapper/16 Not tainted 5.10.0-5-amd64 #1 Debian 5.10.26-1 [ 408.620424] Hardware name: Supermicro AS -2013S-C0R/H11SSL-C, BIOS 2.1 02/21/2020 [ 408.620425] Call Trace: [ 408.620428] [ 408.620437] dump_stack+0x6b/0x83 [ 408.620442] __schedule_bug.cold+0x4c/0x58 [ 408.620446] __schedule+0x719/0x870 [ 408.620449] schedule+0x46/0xb0 [ 408.620453] blk_mq_freeze_queue_wait+0x62/0x90 [ 408.620458] ? add_wait_queue_exclusive+0x70/0x70 [ 408.620466] aoedev_downdev+0x106/0x150 [aoe] [ 408.620471] rexmit_timer+0x4ea/0x500 [aoe] [ 408.620476] ? rexmit_deferred+0x380/0x380 [aoe] [ 408.620480] call_timer_fn+0x29/0xf0 [ 408.620483] __run_timers.part.0+0x1d3/0x240 [ 408.620485] ? ktime_get+0x38/0xa0 [ 408.620488] ? lapic_next_event+0x1d/0x20 [ 408.620491] ? clockevents_program_event+0x8d/0xf0 [ 408.620494] run_timer_softirq+0x26/0x50 [ 408.620496] __do_softirq+0xc5/0x275 [ 408.620499] asm_call_irq_on_stack+0x12/0x20 [ 408.620501] [ 408.620505] do_softirq_own_stack+0x37/0x40 [ 408.620509] irq_exit_rcu+0x8e/0xc0 [ 408.620512] sysvec_apic_timer_interrupt+0x36/0x80 [ 408.620515] asm_sysvec_apic_timer_interrupt+0x12/0x20 [ 408.620520] RIP: 0010:cpuidle_enter_state+0xc7/0x350 [ 408.620523] Code: 8b 3d dd 5b b7 6b e8 d8 4f a2 ff 49 89 c5 0f 1f 44 00 00 31 ff e8 69 5a a2 ff 45 84 ff 0f 85 fa 00 00 00 fb 66 0f 1f 44 00 00 <45> 85 f6 0f 88 06 01 00 00 49 63 c6 4c 2b 2c 24 48 8d 14 40 48 8d [ 408.620525] RSP: 0018:ba890038fea8 EFLAGS: 0246 [ 408.620527] RAX: 9c18afc2bc00 RBX: 0002 RCX: 001f [ 408.620529] RDX: RSI: 3677d46d RDI: [ 408.620530] RBP: 9c28d634b000 R08: 005f23a8382d R09: 0018 [ 408.620531] R10: 0dd5 R11: 1169 R12: 955b8fa0 [ 408.620532] R13: 005f23a8382d R14: 0002 R15: [ 408.620537] ? cpuidle_enter_state+0xb7/0x350 [ 408.620540] cpuidle_enter+0x29/0x40 [ 408.620543] do_idle+0x1ef/0x2b0 [ 408.620546] cpu_startup_entry+0x19/0x20 [ 408.620550] secondary_startup_64_no_verify+0xb0/0xbb [ 408.620561] bad: scheduling from the idle thread! [ 408.620591] CPU: 16 PID: 0 Comm: swapper/16 Tainted: GW 5.10.0-5-amd64 #1 Debian 5.10.26-1 [ 408.620593] Hardware name: Supermicro AS -2013S-C0R/H11SSL-C, BIOS 2.1 02/21/2020 [ 408.620601] Call Trace: [ 408.620609] [ 408.620617] dump_stack+0x6b/0x83 [ 408.620625] dequeue_task_idle+0x28/0x40 [ 408.620632] __schedule+0x3bf/0x870 [ 408.620641] schedule+0x46/0xb0 [ 408.620648] blk_mq_freeze_queue_wait+0x62/0x90 [ 408.620657] ? add_wait_queue_exclusive+0x70/0x70 [ 408.620666] aoedev_downdev+0x106/0x150 [aoe] [ 408.620679]
[PATCH v2 1/7] rpmsg: char: Export eptdev create an destroy functions
To prepare the split of the code related to the control (ctrldev) and the endpoint (eptdev) devices in 2 separate files: - Rename and export the functions in rpmsg_char.h. - Suppress the dependency with the rpmsg_ctrldev struct in the rpmsg_chrdev_create_eptdev function. - The rpmsg class is provided as parameter in rpmsg_chrdev_create_eptdev, because the class is associated to the control part. Suggested-by: Mathieu Poirier Signed-off-by: Arnaud Pouliquen --- drivers/rpmsg/rpmsg_char.c | 19 +-- drivers/rpmsg/rpmsg_char.h | 50 ++ 2 files changed, 61 insertions(+), 8 deletions(-) create mode 100644 drivers/rpmsg/rpmsg_char.h diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index 2bebc9b2d163..b9df8dc4365f 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* + * Copyright (C) 2021, STMicroelectronics * Copyright (c) 2016, Linaro Ltd. * Copyright (c) 2012, Michal Simek * Copyright (c) 2012, PetaLogix @@ -22,6 +23,7 @@ #include #include +#include "rpmsg_char.h" #include "rpmsg_internal.h" #define RPMSG_DEV_MAX (MINORMASK + 1) @@ -78,7 +80,7 @@ struct rpmsg_eptdev { wait_queue_head_t readq; }; -static int rpmsg_eptdev_destroy(struct device *dev, void *data) +int rpmsg_chrdev_destroy_eptdev(struct device *dev, void *data) { struct rpmsg_eptdev *eptdev = dev_to_eptdev(dev); @@ -97,6 +99,7 @@ static int rpmsg_eptdev_destroy(struct device *dev, void *data) return 0; } +EXPORT_SYMBOL(rpmsg_chrdev_destroy_eptdev); static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len, void *priv, u32 addr) @@ -280,7 +283,7 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd, if (cmd != RPMSG_DESTROY_EPT_IOCTL) return -EINVAL; - return rpmsg_eptdev_destroy(>dev, NULL); + return rpmsg_chrdev_destroy_eptdev(>dev, NULL); } static const struct file_operations rpmsg_eptdev_fops = { @@ -339,10 +342,9 @@ static void rpmsg_eptdev_release_device(struct device *dev) kfree(eptdev); } -static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev, - struct rpmsg_channel_info chinfo) +int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent, + struct rpmsg_channel_info chinfo, struct class *rpmsg_class) { - struct rpmsg_device *rpdev = ctrldev->rpdev; struct rpmsg_eptdev *eptdev; struct device *dev; int ret; @@ -362,7 +364,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev, device_initialize(dev); dev->class = rpmsg_class; - dev->parent = >dev; + dev->parent = parent; dev->groups = rpmsg_eptdev_groups; dev_set_drvdata(dev, eptdev); @@ -405,6 +407,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev, return ret; } +EXPORT_SYMBOL(rpmsg_chrdev_create_eptdev); static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp) { @@ -444,7 +447,7 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd, chinfo.src = eptinfo.src; chinfo.dst = eptinfo.dst; - return rpmsg_eptdev_create(ctrldev, chinfo); + return rpmsg_chrdev_create_eptdev(ctrldev->rpdev, >dev, chinfo, rpmsg_class); }; static const struct file_operations rpmsg_ctrldev_fops = { @@ -530,7 +533,7 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev) int ret; /* Destroy all endpoints */ - ret = device_for_each_child(>dev, NULL, rpmsg_eptdev_destroy); + ret = device_for_each_child(>dev, NULL, rpmsg_chrdev_destroy_eptdev); if (ret) dev_warn(>dev, "failed to nuke endpoints: %d\n", ret); diff --git a/drivers/rpmsg/rpmsg_char.h b/drivers/rpmsg/rpmsg_char.h new file mode 100644 index ..379d2ae2bee8 --- /dev/null +++ b/drivers/rpmsg/rpmsg_char.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Copyright (C) STMicroelectronics 2021. + */ + +#ifndef __RPMSG_CHRDEV_H__ +#define __RPMSG_CHRDEV_H__ + +#if IS_REACHABLE(CONFIG_RPMSG_CHAR) +/** + * rpmsg_chrdev_create_eptdev() - register char device based on an endpoint + * @rpdev: prepared rpdev to be used for creating endpoints + * @parent: parent device + * @chinfo: assiated endpoint channel information. + * + * This function create a new rpmsg char endpoint device to instantiate a new + * endpoint based on chinfo information. + */ +int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent, + struct rpmsg_channel_info chinfo, struct class *rpmsg_class); + +/** + * rpmsg_chrdev_destroy_eptdev() - destroy created char device endpoint. + * @data: private data associated to the endpoint device + * + * This function
[PATCH v2 0/7] Restructure the rpmsg char and introduce the rpmsg-raw channel
update from V1 [1] - fix issues reported by Mathieu Poirier This series is the second step in the division of the series [2]: "Introducing a Generic IOCTL Interface for RPMsg Channel Management". The purpose of this patchset is to: - split the control code related to the control and the endpoint. - define the rpmsg-raw channel, associated with the rpmsg char device to allow it to be instantiated using a name service announcement. An important point to keep in mind for this patchset is that the concept of channel is associated with a default endpoint. To facilitate communication with the remote side, this default endpoint must have a fixed address. Consequently, for this series, I made a design choice to fix the endpoint on the "rpmsg-raw" channel probe, and not allow to create/destroy an endpoint on FS open/close. This is only applicable for channels probed by the rpmsg bus. The behavior, using the RPMSG_CREATE_EPT_IOCTL and RPMSG_DESTROY_EPT_IOCTL controls, is preserved. The next steps should be to correct this: Introduce the IOCTLs RPMSG_CREATE_DEV_IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate the rpmsg devices [1]: https://patchwork.kernel.org/project/linux-remoteproc/list/?series=453805 [2]: https://patchwork.kernel.org/project/linux-remoteproc/list/?series=435523 Arnaud Pouliquen (7): rpmsg: char: Export eptdev create an destroy functions rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl rpmsg: Update rpmsg_chrdev_register_device function rpmsg: char: Introduce __rpmsg_chrdev_create_eptdev function rpmsg: char: Introduce a rpmsg driver for the rpmsg char device rpmsg: char: No dynamic endpoint management for the default one rpmsg: char: Return error if user tries to destroy a default endpoint. drivers/rpmsg/Kconfig | 9 ++ drivers/rpmsg/Makefile| 1 + drivers/rpmsg/qcom_glink_native.c | 2 +- drivers/rpmsg/qcom_smd.c | 2 +- drivers/rpmsg/rpmsg_char.c| 222 +--- drivers/rpmsg/rpmsg_char.h| 52 +++ drivers/rpmsg/rpmsg_ctrl.c| 231 ++ drivers/rpmsg/rpmsg_internal.h| 8 +- drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- 9 files changed, 368 insertions(+), 161 deletions(-) create mode 100644 drivers/rpmsg/rpmsg_char.h create mode 100644 drivers/rpmsg/rpmsg_ctrl.c -- 2.17.1
[PATCH v2 6/7] rpmsg: char: No dynamic endpoint management for the default one
Do not dynamically manage the default endpoint associated to the rpmsg device. The ept address must not change. This update is needed to manage the rpmsg-raw channel. In this case a default endpoint is used and its address must not change or been reused by another service. Signed-off-by: Arnaud Pouliquen --- drivers/rpmsg/rpmsg_char.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index 4606787b7011..fa59abfa8878 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -112,17 +112,26 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp) struct rpmsg_endpoint *ept; struct rpmsg_device *rpdev = eptdev->rpdev; struct device *dev = >dev; + u32 addr = eptdev->chinfo.src; if (eptdev->ept) return -EBUSY; get_device(dev); - ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo); - if (!ept) { - dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); - put_device(dev); - return -EINVAL; + /* +* The RPMsg device can has been created by a ns announcement. In this +* case a default endpoint has been created. Reuse it to avoid to manage +* a new address on each open/close. +*/ + ept = rpdev->ept; + if (!ept || addr != ept->addr) { + ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo); + if (!ept) { + dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); + put_device(dev); + return -EINVAL; + } } eptdev->ept = ept; @@ -134,12 +143,17 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp) static int rpmsg_eptdev_release(struct inode *inode, struct file *filp) { struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev); + struct rpmsg_device *rpdev = eptdev->rpdev; struct device *dev = >dev; - /* Close the endpoint, if it's not already destroyed by the parent */ + /* +* Close the endpoint, if it's not already destroyed by the parent and it is not the +* default one. +*/ mutex_lock(>ept_lock); if (eptdev->ept) { - rpmsg_destroy_ept(eptdev->ept); + if (eptdev->ept != rpdev->ept) + rpmsg_destroy_ept(eptdev->ept); eptdev->ept = NULL; } mutex_unlock(>ept_lock); -- 2.17.1
[PATCH v2 2/7] rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
Create the rpmsg_ctrl.c module and move the code related to the rpmsg_ctrldev device in this new module. Add the dependency between rpmsg_char and rpmsg_ctrl in the kconfig file. Signed-off-by: Arnaud Pouliquen --- update from v1: - keep "rpmsg_chrdev" driver name in rpmsg_ctrl, driver will be renamed in next path that renames the rpmsg_chrdev_create_eptdev. - rename the chardev regions - move RPMSG_DEV_MAX to rpmsg_char.h --- drivers/rpmsg/Kconfig | 9 ++ drivers/rpmsg/Makefile | 1 + drivers/rpmsg/rpmsg_char.c | 181 + drivers/rpmsg/rpmsg_char.h | 2 + drivers/rpmsg/rpmsg_ctrl.c | 231 + 5 files changed, 245 insertions(+), 179 deletions(-) create mode 100644 drivers/rpmsg/rpmsg_ctrl.c diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig index 0b4407abdf13..d822ec9ec692 100644 --- a/drivers/rpmsg/Kconfig +++ b/drivers/rpmsg/Kconfig @@ -10,11 +10,20 @@ config RPMSG_CHAR tristate "RPMSG device interface" depends on RPMSG depends on NET + select RPMSG_CTRL help Say Y here to export rpmsg endpoints as device files, usually found in /dev. They make it possible for user-space programs to send and receive rpmsg packets. +config RPMSG_CTRL + tristate "RPMSG control interface" + depends on RPMSG + help + Say Y here to enable the support of the /dev/rpmsg_ctrlX API. This API + allows user-space programs to create endpoints with specific service name, + source and destination addresses. + config RPMSG_NS tristate "RPMSG name service announcement" depends on RPMSG diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile index 8d452656f0ee..58e3b382e316 100644 --- a/drivers/rpmsg/Makefile +++ b/drivers/rpmsg/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_RPMSG)+= rpmsg_core.o obj-$(CONFIG_RPMSG_CHAR) += rpmsg_char.o +obj-$(CONFIG_RPMSG_CTRL) += rpmsg_ctrl.o obj-$(CONFIG_RPMSG_NS) += rpmsg_ns.o obj-$(CONFIG_RPMSG_MTK_SCP)+= mtk_rpmsg.o qcom_glink-objs:= qcom_glink_native.o qcom_glink_ssr.o diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index b9df8dc4365f..21ef9d9eccd7 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -26,33 +26,14 @@ #include "rpmsg_char.h" #include "rpmsg_internal.h" -#define RPMSG_DEV_MAX (MINORMASK + 1) - static dev_t rpmsg_major; -static struct class *rpmsg_class; -static DEFINE_IDA(rpmsg_ctrl_ida); static DEFINE_IDA(rpmsg_ept_ida); static DEFINE_IDA(rpmsg_minor_ida); #define dev_to_eptdev(dev) container_of(dev, struct rpmsg_eptdev, dev) #define cdev_to_eptdev(i_cdev) container_of(i_cdev, struct rpmsg_eptdev, cdev) -#define dev_to_ctrldev(dev) container_of(dev, struct rpmsg_ctrldev, dev) -#define cdev_to_ctrldev(i_cdev) container_of(i_cdev, struct rpmsg_ctrldev, cdev) - -/** - * struct rpmsg_ctrldev - control device for instantiating endpoint devices - * @rpdev: underlaying rpmsg device - * @cdev: cdev for the ctrl device - * @dev: device for the ctrl device - */ -struct rpmsg_ctrldev { - struct rpmsg_device *rpdev; - struct cdev cdev; - struct device dev; -}; - /** * struct rpmsg_eptdev - endpoint device context * @dev: endpoint device @@ -409,169 +390,13 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent } EXPORT_SYMBOL(rpmsg_chrdev_create_eptdev); -static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp) -{ - struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev); - - get_device(>dev); - filp->private_data = ctrldev; - - return 0; -} - -static int rpmsg_ctrldev_release(struct inode *inode, struct file *filp) -{ - struct rpmsg_ctrldev *ctrldev = cdev_to_ctrldev(inode->i_cdev); - - put_device(>dev); - - return 0; -} - -static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd, - unsigned long arg) -{ - struct rpmsg_ctrldev *ctrldev = fp->private_data; - void __user *argp = (void __user *)arg; - struct rpmsg_endpoint_info eptinfo; - struct rpmsg_channel_info chinfo; - - if (cmd != RPMSG_CREATE_EPT_IOCTL) - return -EINVAL; - - if (copy_from_user(, argp, sizeof(eptinfo))) - return -EFAULT; - - memcpy(chinfo.name, eptinfo.name, RPMSG_NAME_SIZE); - chinfo.name[RPMSG_NAME_SIZE-1] = '\0'; - chinfo.src = eptinfo.src; - chinfo.dst = eptinfo.dst; - - return rpmsg_chrdev_create_eptdev(ctrldev->rpdev, >dev, chinfo, rpmsg_class); -}; - -static const struct file_operations rpmsg_ctrldev_fops = { - .owner = THIS_MODULE, - .open = rpmsg_ctrldev_open, - .release = rpmsg_ctrldev_release, - .unlocked_ioctl = rpmsg_ctrldev_ioctl, -
[PATCH v2 4/7] rpmsg: char: Introduce __rpmsg_chrdev_create_eptdev function
Introduce the __rpmsg_chrdev_create_eptdev internal function that returns the rpmsg_eptdev context structure. This patch prepares the introduction of a rpmsg channel device for the char device. The rpmsg device will need a reference to the context. Signed-off-by: Arnaud Pouliquen --- update from V1 - fix __rpmsg_chrdev_create_eptdev function header indentation. --- drivers/rpmsg/rpmsg_char.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index 21ef9d9eccd7..a64249d83172 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -323,8 +323,9 @@ static void rpmsg_eptdev_release_device(struct device *dev) kfree(eptdev); } -int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent, - struct rpmsg_channel_info chinfo, struct class *rpmsg_class) +static struct rpmsg_eptdev * +__rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent, +struct rpmsg_channel_info chinfo, struct class *rpmsg_class) { struct rpmsg_eptdev *eptdev; struct device *dev; @@ -332,7 +333,7 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL); if (!eptdev) - return -ENOMEM; + return ERR_PTR(-ENOMEM); dev = >dev; eptdev->rpdev = rpdev; @@ -376,7 +377,7 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent put_device(dev); } - return ret; + return eptdev; free_ept_ida: ida_simple_remove(_ept_ida, dev->id); @@ -386,7 +387,19 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent put_device(dev); kfree(eptdev); - return ret; + return ERR_PTR(ret); +} + +int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent, + struct rpmsg_channel_info chinfo, struct class *rpmsg_class) +{ + struct rpmsg_eptdev *eptdev; + + eptdev = __rpmsg_chrdev_create_eptdev(rpdev, parent, chinfo, rpmsg_class); + if (IS_ERR(eptdev)) + return PTR_ERR(eptdev); + + return 0; } EXPORT_SYMBOL(rpmsg_chrdev_create_eptdev); -- 2.17.1
[PATCH v2 5/7] rpmsg: char: Introduce a rpmsg driver for the rpmsg char device
A rpmsg char device allows to probe the endpoint device on a remote name service announcement. With this patch the /dev/rpmsgX interface is created either by a user application or by the remote firmware. Signed-off-by: Arnaud Pouliquen --- update from V1: - add missing unregister_rpmsg_driver call on module exit. --- drivers/rpmsg/rpmsg_char.c | 59 +- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index a64249d83172..4606787b7011 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -26,6 +26,8 @@ #include "rpmsg_char.h" #include "rpmsg_internal.h" +#define RPMSG_CHAR_DEVNAME "rpmsg-raw" + static dev_t rpmsg_major; static DEFINE_IDA(rpmsg_ept_ida); @@ -403,13 +405,67 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent } EXPORT_SYMBOL(rpmsg_chrdev_create_eptdev); +static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev) +{ + struct rpmsg_channel_info chinfo; + struct rpmsg_eptdev *eptdev; + + if (!rpdev->ept) + return -EINVAL; + + memcpy(chinfo.name, RPMSG_CHAR_DEVNAME, sizeof(RPMSG_CHAR_DEVNAME)); + chinfo.src = rpdev->src; + chinfo.dst = rpdev->dst; + + eptdev = __rpmsg_chrdev_create_eptdev(rpdev, >dev, chinfo, NULL); + if (IS_ERR(eptdev)) + return PTR_ERR(eptdev); + + /* Set the private field of the default endpoint to retrieve context on callback. */ + rpdev->ept->priv = eptdev; + + return 0; +} + +static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev) +{ + int ret; + + ret = device_for_each_child(>dev, NULL, rpmsg_chrdev_destroy_eptdev); + if (ret) + dev_warn(>dev, "failed to destroy endpoints: %d\n", ret); +} + +static struct rpmsg_device_id rpmsg_chrdev_id_table[] = { + { .name = RPMSG_CHAR_DEVNAME }, + { }, +}; + +static struct rpmsg_driver rpmsg_chrdev_driver = { + .probe = rpmsg_chrdev_probe, + .remove = rpmsg_chrdev_remove, + .id_table = rpmsg_chrdev_id_table, + .callback = rpmsg_ept_cb, + .drv = { + .name = "rpmsg_chrdev", + }, +}; + static int rpmsg_chrdev_init(void) { int ret; ret = alloc_chrdev_region(_major, 0, RPMSG_DEV_MAX, "rpmsg_char"); - if (ret < 0) + if (ret < 0) { pr_err("rpmsg: failed to allocate char dev region\n"); + return ret; + } + + ret = register_rpmsg_driver(_chrdev_driver); + if (ret < 0) { + pr_err("rpmsg: failed to register rpmsg raw driver\n"); + unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX); + } return ret; } @@ -417,6 +473,7 @@ postcore_initcall(rpmsg_chrdev_init); static void rpmsg_chrdev_exit(void) { + unregister_rpmsg_driver(_chrdev_driver); unregister_chrdev_region(rpmsg_major, RPMSG_DEV_MAX); } module_exit(rpmsg_chrdev_exit); -- 2.17.1
Re: [PATCH 1/1] staging: ipu3-imgu: Move the UAPI header from include under include/uapi
Hi Sakari, Thank you for the patch. On Mon, Apr 12, 2021 at 02:11:20PM +0300, Sakari Ailus wrote: > The header defines the user space interface but may be mistaken as > kernel-only header due to its location. Add "uapi" directory under > driver's include directory and move the header there. > > Suggested-by: Greg KH > Signed-off-by: Sakari Ailus > --- > Documentation/admin-guide/media/ipu3.rst | 35 ++- > .../media/v4l/pixfmt-meta-intel-ipu3.rst | 2 +- > .../ipu3/include/{ => uapi}/intel-ipu3.h | 0 > drivers/staging/media/ipu3/ipu3-abi.h | 2 +- > 4 files changed, 20 insertions(+), 19 deletions(-) > rename drivers/staging/media/ipu3/include/{ => uapi}/intel-ipu3.h (100%) > > diff --git a/Documentation/admin-guide/media/ipu3.rst > b/Documentation/admin-guide/media/ipu3.rst > index f59697c7b374..d6454f637ff4 100644 > --- a/Documentation/admin-guide/media/ipu3.rst > +++ b/Documentation/admin-guide/media/ipu3.rst > @@ -234,22 +234,23 @@ The IPU3 ImgU pipelines can be configured using the > Media Controller, defined at > Running mode and firmware binary selection > -- > > -ImgU works based on firmware, currently the ImgU firmware support run 2 > pipes in > -time-sharing with single input frame data. Each pipe can run at certain mode > - > -"VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture, > and > -"STILL" is used for still frame capture. However, you can also select > "VIDEO" to > -capture still frames if you want to capture images with less system load and > -power. For "STILL" mode, ImgU will try to use smaller BDS factor and output > -larger bayer frame for further YUV processing than "VIDEO" mode to get high > -quality images. Besides, "STILL" mode need XNR3 to do noise reduction, hence > -"STILL" mode will need more power and memory bandwidth than "VIDEO" mode. TNR > -will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is > running at > -“VIDEO” mode by default, the user can use v4l2 control > V4L2_CID_INTEL_IPU3_MODE > -(currently defined in drivers/staging/media/ipu3/include/intel-ipu3.h) to > query > -and set the running mode. For user, there is no difference for buffer > queueing > -between the "VIDEO" and "STILL" mode, mandatory input and main output node > -should be enabled and buffers need be queued, the statistics and the > view-finder > -queues are optional. > +ImgU works based on firmware, currently the ImgU firmware support run 2 pipes > +in time-sharing with single input frame data. Each pipe can run at certain > mode > +- "VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture, > +and "STILL" is used for still frame capture. However, you can also select > +"VIDEO" to capture still frames if you want to capture images with less > system > +load and power. For "STILL" mode, ImgU will try to use smaller BDS factor and > +output larger bayer frame for further YUV processing than "VIDEO" mode to get > +high quality images. Besides, "STILL" mode need XNR3 to do noise reduction, > +hence "STILL" mode will need more power and memory bandwidth than "VIDEO" > mode. > +TNR will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is > +running at “VIDEO” mode by default, the user can use v4l2 control > +V4L2_CID_INTEL_IPU3_MODE (currently defined in > +drivers/staging/media/ipu3/include/uapi/intel-ipu3.h) to query and set the > +running mode. For user, there is no difference for buffer queueing between > the > +"VIDEO" and "STILL" mode, mandatory input and main output node should be > +enabled and buffers need be queued, the statistics and the view-finder queues > +are optional. The reflow of the whole paragraph is a bit painful to review. Reviewed-by: Laurent Pinchart > > The firmware binary will be selected according to current running mode, such > log > "using binary if_to_osys_striped " or "using binary > if_to_osys_primary_striped" > @@ -586,7 +587,7 @@ preserved. > References > == > > -.. [#f5] drivers/staging/media/ipu3/include/intel-ipu3.h > +.. [#f5] drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > > .. [#f1] https://github.com/intel/nvt > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > b/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > index 5f33d35532ef..84d81dd7a7b5 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > @@ -78,4 +78,4 @@ hardware and algorithm details. > Intel IPU3 ImgU uAPI data types > === > > -.. kernel-doc:: drivers/staging/media/ipu3/include/intel-ipu3.h > +.. kernel-doc:: drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > similarity index 100% > rename
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Mon, 2021-04-12 at 10:50 +0100, Russell King - ARM Linux admin wrote: > On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote: > > +#define B100T1_PMAPMD_CTL 0x0834 > > +#define B100T1_PMAPMD_CONFIG_ENBIT(15) > > +#define B100T1_PMAPMD_MASTER BIT(14) > > +#define MASTER_MODE(B100T1_PMAPMD_CONFIG_EN | > > B100T1_PMAPMD_MASTER) > > +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN) > > + > > +#define DEVICE_CONTROL 0x0040 > > +#define DEVICE_CONTROL_RESET BIT(15) > > +#define DEVICE_CONTROL_CONFIG_GLOBAL_ENBIT(14) > > +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13) > > +#define RESET_POLL_NS (250 * NSEC_PER_MSEC) > > + > > +#define PHY_CONTROL0x8100 > > +#define PHY_CONFIG_EN BIT(14) > > +#define PHY_START_OP BIT(0) > > + > > +#define PHY_CONFIG 0x8108 > > +#define PHY_CONFIG_AUTOBIT(0) > > + > > +#define SIGNAL_QUALITY 0x8320 > > +#define SQI_VALID BIT(14) > > +#define SQI_MASK GENMASK(2, 0) > > +#define MAX_SQISQI_MASK > > + > > +#define CABLE_TEST 0x8330 > > +#define CABLE_TEST_ENABLE BIT(15) > > +#define CABLE_TEST_START BIT(14) > > +#define CABLE_TEST_VALID BIT(13) > > +#define CABLE_TEST_OK 0x00 > > +#define CABLE_TEST_SHORTED 0x01 > > +#define CABLE_TEST_OPEN0x02 > > +#define CABLE_TEST_UNKNOWN 0x07 > > + > > +#define PORT_CONTROL 0x8040 > > +#define PORT_CONTROL_ENBIT(14) > > + > > +#define PORT_INFRA_CONTROL 0xAC00 > > +#define PORT_INFRA_CONTROL_EN BIT(14) > > + > > +#define VND1_RXID 0xAFCC > > +#define VND1_TXID 0xAFCD > > +#define ID_ENABLE BIT(15) > > + > > +#define ABILITIES 0xAFC4 > > +#define RGMII_ID_ABILITY BIT(15) > > +#define RGMII_ABILITY BIT(14) > > +#define RMII_ABILITY BIT(10) > > +#define REVMII_ABILITY BIT(9) > > +#define MII_ABILITYBIT(8) > > +#define SGMII_ABILITY BIT(0) > > + > > +#define MII_BASIC_CONFIG 0xAFC6 > > +#define MII_BASIC_CONFIG_REV BIT(8) > > +#define MII_BASIC_CONFIG_SGMII 0x9 > > +#define MII_BASIC_CONFIG_RGMII 0x7 > > +#define MII_BASIC_CONFIG_RMII 0x5 > > +#define MII_BASIC_CONFIG_MII 0x4 > > + > > +#define SYMBOL_ERROR_COUNTER 0x8350 > > +#define LINK_DROP_COUNTER 0x8352 > > +#define LINK_LOSSES_AND_FAILURES 0x8353 > > +#define R_GOOD_FRAME_CNT 0xA950 > > +#define R_BAD_FRAME_CNT0xA952 > > +#define R_RXER_FRAME_CNT 0xA954 > > +#define RX_PREAMBLE_COUNT 0xAFCE > > +#define TX_PREAMBLE_COUNT 0xAFCF > > +#define RX_IPG_LENGTH 0xAFD0 > > +#define TX_IPG_LENGTH 0xAFD1 > > +#define COUNTERS_ENBIT(15) > > + > > +#define CLK_25MHZ_PS_PERIOD4UL > > +#define PS_PER_DEGREE (CLK_25MHZ_PS_PERIOD / 360) > > +#define MIN_ID_PS 8222U > > +#define MAX_ID_PS 11300U > > Maybe include some prefix as to which MMD each of these registers is > located? I will add the MMD as prefix. Thank you. > > > +static bool nxp_c45_can_sleep(struct phy_device *phydev) > > +{ > > + int reg; > > + > > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT1); > > + if (reg < 0) > > + return false; > > + > > + return !!(reg & MDIO_STAT1_LPOWERABLE); > > +} > > This looks like it could be useful as a generic helper function - > nothing in this function is specific to this PHY. > > > +static int nxp_c45_resume(struct phy_device *phydev) > > +{ > > + int reg; > > + > > + if (!nxp_c45_can_sleep(phydev)) > > + return -EOPNOTSUPP; > > + > > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1); > > + reg &= ~MDIO_CTRL1_LPOWER; > > + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg); > > + > > + return 0; > > +} > > + > > +static int nxp_c45_suspend(struct phy_device *phydev) > > +{ > > + int reg; > > + > > + if (!nxp_c45_can_sleep(phydev)) > > + return -EOPNOTSUPP; > > + > > + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1); > > + reg |= MDIO_CTRL1_LPOWER; > > + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg); > > + > > + return 0; > > +} > > These too look like potential generic helper functions. That's true. Should I implement them as
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On 4/13/2021 3:30 PM, Andrew Lunn wrote: On Tue, Apr 13, 2021 at 08:56:30AM +0200, Christian Herber wrote: Hi Andrew, On 4/12/2021 6:52 PM, Andrew Lunn wrote: So what you are say is, you don't care if the IP is completely different, it all goes in one driver. So lets put this driver into nxp-tja11xx.c. And then we avoid all the naming issues. Andrew As this seems to be a key question, let me try and shed some more light on this. The original series of BASE-T1 PHYs includes TJA110, TJA1101, and TJA1102. They are covered by the existing driver, which has the unfortunate naming TJA11xx. Unfortunate, because the use of wildcards is a bit to generous. Yes, that does happen. Naming is difficult. But i really think nxp-c45.c is much worse. It gives no idea at all what it supports. Or in the future, what it does not support, and you actually need nxp-c45-ng.c. Developers are going to look at a board, see a tja1XYZ chip, see the nxp-tja11xx.c and enable it. Does the chip have a big C45 symbol on it? Anything to give the idea it should use the nxp-c45 driver? Maybe we should actually swing the complete opposite direction. Name it npx-tja1103.c. There are lots of drivers which have a specific name, but actually support a lot more devices. The developer sees they have an tja1XYZ, there are two drivers which look about right, and enable them both? Andrew Ok, we can agree that there will not be a perfect naming. Would it be a possibility to rename the existing TJA11xx driver to TJA1100-1-2 or is that unwanted? I agree that it should be easy to find the right driver. Right now, there is another device called the SJA1110, which has a very similar IP integrated. It would be possible to use the driver for that device also, even if this is outside of the scope of this submission. Going for wildcards, we get to xJA11xx, which is really undesirable to me. In the end my hope was that people will look up the correct driver through LKDDb or similar and will find the matching devices from there. I am open to any suggestion that leads to users finding the right driver and that also work for future devices. Using your example of an NG device: My assumption is that the things that change are covered by IEEE standardized registers, and thus should be implemented as part of generic helper functions. The things that are outside of the IEEE scope, e.g xMII interface configuration are generic and can be contained in a single driver if the registers are kept software compatible which we intend to do. If nxp-c45.c is to generic (I take from your comments that' your conclusion), we could at least lean towards nxp-c45-bt1.c? Unfortunately, the product naming schemes are not sufficiently methodical to have a a good driver name based on product names. Christian
Re: Linux 5.12-rc7
On Tue, Apr 13, 2021 at 03:42:24PM +0200, Eric Dumazet wrote: > On Tue, Apr 13, 2021 at 3:38 PM Michael S. Tsirkin wrote: > > > > On Tue, Apr 13, 2021 at 03:33:40PM +0200, Eric Dumazet wrote: > > > On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet wrote: > > > > > > > > On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin > > > > wrote: > > > > > > > > > > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote: > > > > > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet > > > > > > wrote: > > > > > > > > > > > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds > > > > > > > wrote: > > > > > > > > > > > > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Qemu test results: > > > > > > > > > total: 460 pass: 459 fail: 1 > > > > > > > > > Failed tests: > > > > > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs > > > > > > > > > > > > > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do > > > > > > > > > not pull payload in > > > > > > > > > skb->head"). It is a spurious problem - the test passes > > > > > > > > > roughly every other > > > > > > > > > time. When the failure is seen, udhcpc fails to get an IP > > > > > > > > > address and aborts > > > > > > > > > with SIGTERM. So far I have only seen this with the "sh" > > > > > > > > > architecture. > > > > > > > > > > > > > > > > Hmm. Let's add in some more of the people involved in that > > > > > > > > commit, and > > > > > > > > also netdev. > > > > > > > > > > > > > > > > Nothing in there looks like it should have any interaction with > > > > > > > > architecture, so that "it happens on sh" sounds odd, but maybe > > > > > > > > it's > > > > > > > > some particular interaction with the qemu environment. > > > > > > > > > > > > > > Yes, maybe. > > > > > > > > > > > > > > I spent few hours on this, and suspect a buggy memcpy() > > > > > > > implementation > > > > > > > on SH, but this was not conclusive. > > > > > > > > > > > > > > By pulling one extra byte, the problem goes away. > > > > > > > > > > > > > > Strange thing is that the udhcpc process does not go past > > > > > > > sendto(). > > > > > > > > > > > > This is the patch working around the issue. Unfortunately I was not > > > > > > able to root-cause it (I really suspect something on SH) > > > > > > > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > > > > index > > > > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655 > > > > > > 100644 > > > > > > --- a/drivers/net/virtio_net.c > > > > > > +++ b/drivers/net/virtio_net.c > > > > > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct > > > > > > virtnet_info *vi, > > > > > > > > > > > > /* Copy all frame if it fits skb->head, otherwise > > > > > > * we let virtio_net_hdr_to_skb() and GRO pull headers as > > > > > > needed. > > > > > > +* > > > > > > +* Apparently, pulling only the Ethernet Header triggers a > > > > > > bug > > > > > > on qemu-system-sh4. > > > > > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 > > > > > > bytes > > > > > > +* more to work around this bug : These 20 bytes can not > > > > > > belong > > > > > > +* to UDP/TCP payload. > > > > > > +* As a bonus, this makes GRO slightly faster for IPv4 (one > > > > > > less copy). > > > > > > */ > > > > > > > > > > Question: do we still want to do this for performance reasons? > > > > > We also have the hdr_len coming from the device which is > > > > > just skb_headlen on the host. > > > > > > > > Well, putting 20 bytes in skb->head will disable frag0 optimization. > > > > > > > > The change would only benefit to sh architecture :) > > > > > > > > About hdr_len, I suppose we could try it, with appropriate safety > > > > checks. > > > > > > I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am > > > using. > > > > > > Have I understood you correctly ? > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > index > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894 > > > 100644 > > > --- a/drivers/net/virtio_net.c > > > +++ b/drivers/net/virtio_net.c > > > @@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct > > > virtnet_info *vi, > > > hdr_padded_len = sizeof(struct padded_vnet_hdr); > > > > > > /* hdr_valid means no XDP, so we can copy the vnet header */ > > > - if (hdr_valid) > > > + if (hdr_valid) { > > > memcpy(hdr, p, hdr_len); > > > - > > > + pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len); > > > + } > > > len -= hdr_len; > > > offset += hdr_padded_len; > > > p += hdr_padded_len; > > > > > > Depends on how you connect qemu on the host. It's filled by host tap, > > see virtio_net_hdr_from_skb. If
[PATCH v2 3/7] rpmsg: Update rpmsg_chrdev_register_device function
The rpmsg_chrdev driver has been replaced by the rpmsg_ctrl driver for the /dev/rpmsg_ctrlX devices management. The reference for the driver override is now the rpmsg_ctrl. Update the rpmsg_chrdev_register_device function to reflect the update, and rename the function to use the rpmsg_ctrldev prefix. The platform drivers are updated accordingly. Signed-off-by: Arnaud Pouliquen --- update from v1 - move the rename of the rpmsg_ctrl driver from previous patch to this one. --- drivers/rpmsg/qcom_glink_native.c | 2 +- drivers/rpmsg/qcom_smd.c | 2 +- drivers/rpmsg/rpmsg_ctrl.c| 2 +- drivers/rpmsg/rpmsg_internal.h| 8 drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index 05533c71b10e..7d7e809800ec 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -1642,7 +1642,7 @@ static int qcom_glink_create_chrdev(struct qcom_glink *glink) rpdev->dev.parent = glink->dev; rpdev->dev.release = qcom_glink_device_release; - return rpmsg_chrdev_register_device(rpdev); + return rpmsg_ctrldev_register_device(rpdev); } struct qcom_glink *qcom_glink_native_probe(struct device *dev, diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c index 8da1b5cb31b3..d223e438d17c 100644 --- a/drivers/rpmsg/qcom_smd.c +++ b/drivers/rpmsg/qcom_smd.c @@ -1113,7 +1113,7 @@ static int qcom_smd_create_chrdev(struct qcom_smd_edge *edge) qsdev->rpdev.dev.parent = >dev; qsdev->rpdev.dev.release = qcom_smd_release_device; - return rpmsg_chrdev_register_device(>rpdev); + return rpmsg_ctrldev_register_device(>rpdev); } /* diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c index a5bb9ed97f32..059c228d0045 100644 --- a/drivers/rpmsg/rpmsg_ctrl.c +++ b/drivers/rpmsg/rpmsg_ctrl.c @@ -180,7 +180,7 @@ static struct rpmsg_driver rpmsg_ctrldev_driver = { .probe = rpmsg_ctrldev_probe, .remove = rpmsg_ctrldev_remove, .drv = { - .name = "rpmsg_chrdev", + .name = "rpmsg_ctrl", }, }; diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h index a76c344253bf..8c500a8f29aa 100644 --- a/drivers/rpmsg/rpmsg_internal.h +++ b/drivers/rpmsg/rpmsg_internal.h @@ -82,16 +82,16 @@ struct rpmsg_device *rpmsg_create_channel(struct rpmsg_device *rpdev, int rpmsg_release_channel(struct rpmsg_device *rpdev, struct rpmsg_channel_info *chinfo); /** - * rpmsg_chrdev_register_device() - register chrdev device based on rpdev + * rpmsg_ctrl_register_device() - register a char device for control based on rpdev * @rpdev: prepared rpdev to be used for creating endpoints * * This function wraps rpmsg_register_device() preparing the rpdev for use as * basis for the rpmsg chrdev. */ -static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev) +static inline int rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev) { - strcpy(rpdev->id.name, "rpmsg_chrdev"); - rpdev->driver_override = "rpmsg_chrdev"; + strcpy(rpdev->id.name, "rpmsg_ctrl"); + rpdev->driver_override = "rpmsg_ctrl"; return rpmsg_register_device(rpdev); } diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index 8e49a3bacfc7..e42234a3e2ab 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -840,7 +840,7 @@ static struct rpmsg_device *rpmsg_virtio_add_ctrl_dev(struct virtio_device *vdev rpdev_ctrl->dev.release = virtio_rpmsg_release_device; rpdev_ctrl->little_endian = virtio_is_little_endian(vrp->vdev); - err = rpmsg_chrdev_register_device(rpdev_ctrl); + err = rpmsg_ctrldev_register_device(rpdev_ctrl); if (err) { kfree(vch); return ERR_PTR(err); -- 2.17.1
[PATCH v2 7/7] rpmsg: char: Return error if user tries to destroy a default endpoint.
Using the RPMSG_DESTROY_EPT_IOCTL control, user application can destroy an endpoint. This patch prevents to destroy a default endpoint associated to a channel. This update is needed to manage the "rpmsg-raw" channel. In this case a default endpoint is used and destroying it without the channel does not make sense. Signed-off-by: Arnaud Pouliquen --- drivers/rpmsg/rpmsg_char.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index fa59abfa8878..d4316bb904f2 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -280,6 +280,10 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd, if (cmd != RPMSG_DESTROY_EPT_IOCTL) return -EINVAL; + /* Don't allow to destroy a default endpoint. */ + if (!eptdev->rpdev || eptdev->ept == eptdev->rpdev->ept) + return -EPERM; + return rpmsg_chrdev_destroy_eptdev(>dev, NULL); } -- 2.17.1
Re: [PATCH net-next 1/2] net: phy: marvell-88x2222: check that link is operational
On Tue, Apr 13, 2021 at 10:23:48AM +0100, Russell King - ARM Linux admin wrote: > On Tue, Apr 13, 2021 at 10:19:30AM +0300, Ivan Bornyakov wrote: > > On Tue, Apr 13, 2021 at 01:40:32AM +0200, Andrew Lunn wrote: > > > On Mon, Apr 12, 2021 at 03:16:59PM +0300, Ivan Bornyakov wrote: > > > > Some SFP modules uses RX_LOS for link indication. In such cases link > > > > will be always up, even without cable connected. RX_LOS changes will > > > > trigger link_up()/link_down() upstream operations. Thus, check that SFP > > > > link is operational before actual read link status. > > > > > > Sorry, but this is not making much sense to me. > > > > > > LOS just indicates some sort of light is coming into the device. You > > > have no idea what sort of light. The transceiver might be able to > > > decode that light and get sync, it might not. It is important that > > > mv_read_status() returns the line side status. Has it been able to > > > achieve sync? That should be independent of LOS. Or are you saying the > > > transceiver is reporting sync, despite no light coming in? > > > > > > Andrew > > > > Yes, with some SFP modules transceiver is reporting sync despite no > > light coming in. So, the idea is to check that link is somewhat > > operational before determing line-side status. > > Indeed - it should be a logical and operation - there is light present > _and_ the PHY recognises the signal. This is what the commit achieves, > although (iirc) doesn't cater for the case where there is no SFP cage > attached. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! Correct, it does not, I only have HW with SFP cage attached.
[PATCH 1/2] clk: Introduce a clock request API
It's not unusual to find clocks being shared across multiple devices that need to change the rate depending on what the device is doing at a given time. The SoC found on the RaspberryPi4 (BCM2711) is in such a situation between its two HDMI controllers that share a clock that needs to be raised depending on the output resolution of each controller. The current clk_set_rate API doesn't really allow to support that case since there's really no synchronisation between multiple users, it's essentially a fire-and-forget solution. clk_set_min_rate does allow for such a synchronisation, but has another drawback: it doesn't allow to reduce the clock rate once the work is over. In our previous example, this means that if we were to raise the resolution of one HDMI controller to the largest resolution and then changing for a smaller one, we would still have the clock running at the largest resolution rate resulting in a poor power-efficiency. In order to address both issues, let's create an API that allows user to create temporary requests to increase the rate to a minimum, before going back to the initial rate once the request is done. This introduces mainly two side-effects: * There's an interaction between clk_set_rate and requests. This has been addressed by having clk_set_rate increasing the rate if it's greater than what the requests asked for, and in any case changing the rate the clock will return to once all the requests are done. * Similarly, clk_round_rate has been adjusted to take the requests into account and return a rate that will be greater or equal to the requested rates. Signed-off-by: Maxime Ripard --- drivers/clk/clk.c | 121 include/linux/clk.h | 4 ++ 2 files changed, 125 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 5052541a0986..4fd91a57e6db 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -77,12 +77,14 @@ struct clk_core { unsigned intprotect_count; unsigned long min_rate; unsigned long max_rate; + unsigned long default_request_rate; unsigned long accuracy; int phase; struct clk_duty duty; struct hlist_head children; struct hlist_node child_node; struct hlist_head clks; + struct list_headpending_requests; unsigned intnotifier_count; #ifdef CONFIG_DEBUG_FS struct dentry *dentry; @@ -105,6 +107,12 @@ struct clk { struct hlist_node clks_node; }; +struct clk_request { + struct list_head list; + struct clk *clk; + unsigned long rate; +}; + /*** runtime pm ***/ static int clk_pm_runtime_get(struct clk_core *core) { @@ -1434,10 +1442,14 @@ unsigned long clk_hw_round_rate(struct clk_hw *hw, unsigned long rate) { int ret; struct clk_rate_request req; + struct clk_request *clk_req; clk_core_get_boundaries(hw->core, _rate, _rate); req.rate = rate; + list_for_each_entry(clk_req, >core->pending_requests, list) + req.min_rate = max(clk_req->rate, req.min_rate); + ret = clk_core_round_rate_nolock(hw->core, ); if (ret) return 0; @@ -1458,6 +1470,7 @@ EXPORT_SYMBOL_GPL(clk_hw_round_rate); long clk_round_rate(struct clk *clk, unsigned long rate) { struct clk_rate_request req; + struct clk_request *clk_req; int ret; if (!clk) @@ -1471,6 +1484,9 @@ long clk_round_rate(struct clk *clk, unsigned long rate) clk_core_get_boundaries(clk->core, _rate, _rate); req.rate = rate; + list_for_each_entry(clk_req, >core->pending_requests, list) + req.min_rate = max(clk_req->rate, req.min_rate); + ret = clk_core_round_rate_nolock(clk->core, ); if (clk->exclusive_count) @@ -1938,6 +1954,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, unsigned long new_rate; unsigned long min_rate; unsigned long max_rate; + struct clk_request *req; int p_index = 0; long ret; @@ -1952,6 +1969,9 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, clk_core_get_boundaries(core, _rate, _rate); + list_for_each_entry(req, >pending_requests, list) + min_rate = max(req->rate, min_rate); + /* find the closest rate and parent clk/rate */ if (clk_core_can_round(core)) { struct clk_rate_request req; @@ -2156,6 +2176,7 @@ static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core, { int ret, cnt; struct clk_rate_request req; + struct clk_request *clk_req; lockdep_assert_held(_lock); @@ -2170,6 +2191,9 @@ static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core,
[PATCH 2/2] drm/vc4: hdmi: Convert to the new clock request API
The new clock request API allows us to increase the rate of the HSM clock to match our pixel rate requirements while decreasing it when we're done, resulting in a better power-efficiency. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 19 --- drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 1fda574579af..244053de6150 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -473,7 +473,9 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE); clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock); + clk_request_done(vc4_hdmi->bvb_req); clk_disable_unprepare(vc4_hdmi->hsm_clock); + clk_request_done(vc4_hdmi->hsm_req); clk_disable_unprepare(vc4_hdmi->pixel_clock); ret = pm_runtime_put(_hdmi->pdev->dev); @@ -778,9 +780,9 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, * pixel clock, but HSM ends up being the limiting factor. */ hsm_rate = max_t(unsigned long, 12000, (pixel_rate / 100) * 101); - ret = clk_set_min_rate(vc4_hdmi->hsm_clock, hsm_rate); - if (ret) { - DRM_ERROR("Failed to set HSM clock rate: %d\n", ret); + vc4_hdmi->hsm_req = clk_request_start(vc4_hdmi->hsm_clock, hsm_rate); + if (IS_ERR(vc4_hdmi->hsm_req)) { + DRM_ERROR("Failed to set HSM clock rate: %ld\n", PTR_ERR(vc4_hdmi->hsm_req)); return; } @@ -797,10 +799,11 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, * FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup * at 300MHz. */ - ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, - (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 : 7500)); - if (ret) { - DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); + vc4_hdmi->bvb_req = clk_request_start(vc4_hdmi->pixel_bvb_clock, + (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 : 7500)); + if (IS_ERR(vc4_hdmi->bvb_req)) { + DRM_ERROR("Failed to set pixel bvb clock rate: %ld\n", PTR_ERR(vc4_hdmi->bvb_req)); + clk_request_done(vc4_hdmi->hsm_req); clk_disable_unprepare(vc4_hdmi->hsm_clock); clk_disable_unprepare(vc4_hdmi->pixel_clock); return; @@ -809,6 +812,8 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); if (ret) { DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret); + clk_request_done(vc4_hdmi->bvb_req); + clk_request_done(vc4_hdmi->hsm_req); clk_disable_unprepare(vc4_hdmi->hsm_clock); clk_disable_unprepare(vc4_hdmi->pixel_clock); return; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3cebd1fd00fc..9ac4a2c751df 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -167,6 +167,9 @@ struct vc4_hdmi { struct reset_control *reset; + struct clk_request *bvb_req; + struct clk_request *hsm_req; + struct debugfs_regset32 hdmi_regset; struct debugfs_regset32 hd_regset; }; -- 2.30.2
Re: [PATCH] gpio: mxs: remove useless function
On Mon, Apr 12, 2021 at 4:16 AM Jiapeng Chong wrote: > > Fix the following gcc warning: > > drivers/gpio/gpio-mxs.c:63:19: warning: kernel/sys_ni.cunused function > 'is_imx28_gpio'. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpio/gpio-mxs.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpio/gpio-mxs.c b/drivers/gpio/gpio-mxs.c > index dfc0c1e..524b668 100644 > --- a/drivers/gpio/gpio-mxs.c > +++ b/drivers/gpio/gpio-mxs.c > @@ -60,11 +60,6 @@ static inline int is_imx23_gpio(struct mxs_gpio_port *port) > return port->devid == IMX23_GPIO; > } > > -static inline int is_imx28_gpio(struct mxs_gpio_port *port) > -{ > - return port->devid == IMX28_GPIO; > -} > - > /* Note: This driver assumes 32 GPIOs are handled in one register */ > > static int mxs_gpio_set_irq_type(struct irq_data *d, unsigned int type) > -- > 1.8.3.1 > Patch applied, thanks! Bartosz
[PATCH] Drivers: hv: vmbus: Use after free in __vmbus_open()
The "open_info" variable is added to the _connection.chn_msg_list, but the error handling frees "open_info" without removing it from the list. This will result in a use after free. First remove it from the list, and then free it. Fixes: 6f3d791f3006 ("Drivers: hv: vmbus: Fix rescind handling issues") Signed-off-by: Dan Carpenter --- >From static analysis. Untested etc. There is almost certainly a good reason to add it to the list before checking "newchannel->rescind" but I don't know the code well enough to know what the reason is. drivers/hv/channel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index db30be8f9cce..1c5a418c1962 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -653,7 +653,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, if (newchannel->rescind) { err = -ENODEV; - goto error_free_info; + goto error_clean_msglist; } err = vmbus_post_msg(open_msg, -- 2.30.2
[PATCH v3 00/33] media: atmel: atmel-isc: add support for xisc
Hello, This series adds support for a variant of the ISC named XISC. This block is present in the product named sama7g5. I started by moving code around, the code which was specialized for sama5d2 type of ISC, to have it inside the dedicated sama5d2 file. I added several new pipeline elements to the code base, which would be common to sama5d2 and the new sama7g5, but only used by the new style pipeline. I separated the input and output formats on a per-product separate array. I added the new sama7g5 compatible driver for the xisc, which is similar with the sama5d2, but with differences in terms of DT, clocks and callbacks to specific operations. I converted the atmel-isc binding to yaml format, and added the new binding in yaml format. Feedback is appreciated. Thanks, Eugen Changes in v3: - Adapted commit messages for several commits to explain several acronyms, especially for submodule names - Converted atmel-isc binding to yaml - Converted microchip-xisc binding to yaml - Updated MAINTAINERS Changes in v2: - Fixed krobot warnings with W=1 regarding functions with no prototype - Fixed new sama7g5 driver to use the new subdev fwnode API in kernel 5.12. my driver was based on old 5.10 style API. Eugen Hristev (33): media: atmel: atmel-isc: specialize gamma table into product specific media: atmel: atmel-isc: specialize driver name constant media: atmel: atmel-isc: add checks for limiting frame sizes media: atmel: atmel-isc: specialize max width and max height media: atmel: atmel-isc: specialize dma cfg media: atmel: atmel-isc: extract CSC submodule config into separate function media: atmel: atmel-isc-base: add id to clock debug message media: atmel: atmel-isc: create register offsets struct media: atmel: atmel-isc: extract CBC submodule config into separate function media: atmel: atmel-isc: add CBC to the reg offsets struct media: atmel: atmel-isc: add SUB422 and SUB420 to register offsets media: atmel: atmel-isc: add RLP to register offsets media: atmel: atmel-isc: add HIS to register offsets media: atmel: atmel-isc: add DMA to register offsets media: atmel: atmel-isc: add support for version register media: atmel: atmel-isc: add his_entry to register offsets media: atmel: atmel-isc: add register description for additional modules media: atmel: atmel-isc: extend pipeline with extra modules media: atmel: atmel-isc: add CC initialization function media: atmel: atmel-isc: create product specific v4l2 controls config media: atmel: atmel-isc: create callback for DPC submodule product specific media: atmel: atmel-isc: create callback for GAM submodule product specific media: atmel: atmel-isc: create callback for RLP submodule product specific media: atmel: atmel-isc: move the formats list into product specific code media: atmel: atmel-isc: create an adapt pipeline callback for product specific media: atmel: atmel-isc-regs: add additional fields for sama7g5 type pipeline media: atmel: atmel-isc-base: add support for more formats and additional pipeline modules media: atmel: atmel-isc-sama5d2: remove duplicate define dt-bindings: media: atmel-isc: convert to yaml dt-bindings: media: add microchip,xisc device bindings media: atmel: atmel-isc: add microchip-xisc driver MAINTAINERS: update ISC driver bindings file MAINTAINERS: add xisc files to isc driver entry .../devicetree/bindings/media/atmel,isc.yaml | 115 .../devicetree/bindings/media/atmel-isc.txt | 65 -- .../bindings/media/microchip,xisc.yaml| 129 MAINTAINERS | 4 +- drivers/media/platform/Makefile | 1 + drivers/media/platform/atmel/Kconfig | 11 + drivers/media/platform/atmel/Makefile | 2 + drivers/media/platform/atmel/atmel-isc-base.c | 381 --- drivers/media/platform/atmel/atmel-isc-regs.h | 133 +++- drivers/media/platform/atmel/atmel-isc.h | 122 +++- .../media/platform/atmel/atmel-sama5d2-isc.c | 311 - .../media/platform/atmel/atmel-sama7g5-isc.c | 643 ++ 12 files changed, 1574 insertions(+), 343 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/atmel,isc.yaml delete mode 100644 Documentation/devicetree/bindings/media/atmel-isc.txt create mode 100644 Documentation/devicetree/bindings/media/microchip,xisc.yaml create mode 100644 drivers/media/platform/atmel/atmel-sama7g5-isc.c -- 2.25.1
[PATCH v3 08/33] media: atmel: atmel-isc: create register offsets struct
Create a struct that holds register offsets that are product specific. Add initially the CSC register. This allows each product that contains a variant of the ISC to add their own register offset. Signed-off-by: Eugen Hristev --- drivers/media/platform/atmel/atmel-isc-base.c | 2 +- drivers/media/platform/atmel/atmel-isc-regs.h | 3 +++ drivers/media/platform/atmel/atmel-isc.h | 12 +++ .../media/platform/atmel/atmel-sama5d2-isc.c | 20 +-- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c index bc036e8ac4fe..019d931d1367 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -2311,7 +2311,7 @@ int isc_pipeline_init(struct isc_device *isc) REG_FIELD(ISC_GAM_CTRL, 1, 1), REG_FIELD(ISC_GAM_CTRL, 2, 2), REG_FIELD(ISC_GAM_CTRL, 3, 3), - REG_FIELD(ISC_CSC_CTRL, 0, 0), + REG_FIELD(ISC_CSC_CTRL + isc->offsets.csc, 0, 0), REG_FIELD(ISC_CBC_CTRL, 0, 0), REG_FIELD(ISC_SUB422_CTRL, 0, 0), REG_FIELD(ISC_SUB420_CTRL, 0, 0), diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h index f1e160ed4351..5a65600c5f88 100644 --- a/drivers/media/platform/atmel/atmel-isc-regs.h +++ b/drivers/media/platform/atmel/atmel-isc-regs.h @@ -153,6 +153,9 @@ /* ISC_Gamma Correction Green Entry Register */ #define ISC_GAM_RENTRY 0x0298 +/* Offset for CSC register specific to sama5d2 product */ +#define ISC_SAMA5D2_CSC_OFFSET 0 + /* Color Space Conversion Control Register */ #define ISC_CSC_CTRL0x0398 diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h index bb0b4419deff..ef3a0451192d 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -144,6 +144,14 @@ struct isc_ctrls { #define ISC_PIPE_LINE_NODE_NUM 11 +/* + * struct isc_reg_offsets - ISC device register offsets + * @csc: Offset for the CSC register + */ +struct isc_reg_offsets { + u32 csc; +}; + /* * struct isc_device - ISC device driver data/config struct * @regmap:Register map @@ -195,6 +203,8 @@ struct isc_ctrls { * * @config_csc:pointer to a function that initializes product * specific CSC module + * + * @offsets: struct holding the product specific register offsets */ struct isc_device { struct regmap *regmap; @@ -266,6 +276,8 @@ struct isc_device { struct { void (*config_csc)(struct isc_device *isc); }; + + struct isc_reg_offsets offsets; }; extern struct isc_format formats_list[]; diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c index f041bd75090e..9e557d17e731 100644 --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c @@ -61,12 +61,18 @@ void isc_sama5d2_config_csc(struct isc_device *isc) struct regmap *regmap = isc->regmap; /* Convert RGB to YUV */ - regmap_write(regmap, ISC_CSC_YR_YG, 0x42 | (0x81 << 16)); - regmap_write(regmap, ISC_CSC_YB_OY, 0x19 | (0x10 << 16)); - regmap_write(regmap, ISC_CSC_CBR_CBG, 0xFDA | (0xFB6 << 16)); - regmap_write(regmap, ISC_CSC_CBB_OCB, 0x70 | (0x80 << 16)); - regmap_write(regmap, ISC_CSC_CRR_CRG, 0x70 | (0xFA2 << 16)); - regmap_write(regmap, ISC_CSC_CRB_OCR, 0xFEE | (0x80 << 16)); + regmap_write(regmap, ISC_CSC_YR_YG + isc->offsets.csc, +0x42 | (0x81 << 16)); + regmap_write(regmap, ISC_CSC_YB_OY + isc->offsets.csc, +0x19 | (0x10 << 16)); + regmap_write(regmap, ISC_CSC_CBR_CBG + isc->offsets.csc, +0xFDA | (0xFB6 << 16)); + regmap_write(regmap, ISC_CSC_CBB_OCB + isc->offsets.csc, +0x70 | (0x80 << 16)); + regmap_write(regmap, ISC_CSC_CRR_CRG + isc->offsets.csc, +0x70 | (0xFA2 << 16)); + regmap_write(regmap, ISC_CSC_CRB_OCR + isc->offsets.csc, +0xFEE | (0x80 << 16)); } /* Gamma table with gamma 1/2.2 */ @@ -215,6 +221,8 @@ static int atmel_isc_probe(struct platform_device *pdev) isc->config_csc = isc_sama5d2_config_csc; + isc->offsets.csc = ISC_SAMA5D2_CSC_OFFSET; + /* sama5d2-isc - 8 bits per beat */ isc->dcfg = ISC_DCFG_YMBSIZE_BEATS8 | ISC_DCFG_CMBSIZE_BEATS8; -- 2.25.1
Re: [PATCH 0/6] KVM: x86: Make the cause of instruction emulation available to user-space
On Monday, 2021-04-12 at 11:34:33 -07, Jim Mattson wrote: > On Mon, Apr 12, 2021 at 6:09 AM David Edmondson > wrote: >> >> Instruction emulation happens for a variety of reasons, yet on error >> we have no idea exactly what triggered it. Add a cause of emulation to >> the various originators and pass it upstream when emulation fails. > > What is userspace going to do with this information? It's hard to say > whether or not this is the right ABI without more context. Logging for debug purposes, see reply to Sean. dme. -- You make me feel like a natural woman.
[PATCH v3 09/33] media: atmel: atmel-isc: extract CBC submodule config into separate function
The CBC submodule is a part of the atmel-isc pipeline, and stands for Contrast Brightness Control. It is used to apply gains and offsets to the luma (Y) and chroma (U, V) components of the YUV elements. The CBC submodule should be initialized in the product specific driver as it's product specific. Other products can implement it differently Signed-off-by: Eugen Hristev --- Changes in v3: - added module description explanation Changes in v2: - addded function prototype to avoid warning with W=1 drivers/media/platform/atmel/atmel-isc-base.c| 4 +--- drivers/media/platform/atmel/atmel-isc.h | 3 +++ drivers/media/platform/atmel/atmel-sama5d2-isc.c | 10 ++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c index 019d931d1367..446fe232956b 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -647,9 +647,7 @@ static void isc_set_pipeline(struct isc_device *isc, u32 pipeline) regmap_bulk_write(regmap, ISC_GAM_RENTRY, gamma, GAMMA_ENTRIES); isc->config_csc(isc); - - regmap_write(regmap, ISC_CBC_BRIGHT, ctrls->brightness); - regmap_write(regmap, ISC_CBC_CONTRAST, ctrls->contrast); + isc->config_cbc(isc); } static int isc_update_profile(struct isc_device *isc) diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h index ef3a0451192d..cb47932197b1 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -203,6 +203,8 @@ struct isc_reg_offsets { * * @config_csc:pointer to a function that initializes product * specific CSC module + * @config_cbc:pointer to a function that initializes product + * specific CBC module * * @offsets: struct holding the product specific register offsets */ @@ -275,6 +277,7 @@ struct isc_device { struct { void (*config_csc)(struct isc_device *isc); + void (*config_cbc)(struct isc_device *isc); }; struct isc_reg_offsets offsets; diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c index 9e557d17e731..66b92fa1c752 100644 --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c @@ -55,6 +55,7 @@ #define ISC_CLK_MAX_DIV255 void isc_sama5d2_config_csc(struct isc_device *isc); +void isc_sama5d2_config_cbc(struct isc_device *isc); void isc_sama5d2_config_csc(struct isc_device *isc) { @@ -75,6 +76,14 @@ void isc_sama5d2_config_csc(struct isc_device *isc) 0xFEE | (0x80 << 16)); } +void isc_sama5d2_config_cbc(struct isc_device *isc) +{ + struct regmap *regmap = isc->regmap; + + regmap_write(regmap, ISC_CBC_BRIGHT, isc->ctrls.brightness); + regmap_write(regmap, ISC_CBC_CONTRAST, isc->ctrls.contrast); +} + /* Gamma table with gamma 1/2.2 */ const u32 isc_sama5d2_gamma_table[][GAMMA_ENTRIES] = { /* 0 --> gamma 1/1.8 */ @@ -220,6 +229,7 @@ static int atmel_isc_probe(struct platform_device *pdev) isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT; isc->config_csc = isc_sama5d2_config_csc; + isc->config_cbc = isc_sama5d2_config_cbc; isc->offsets.csc = ISC_SAMA5D2_CSC_OFFSET; -- 2.25.1
Re: [PATCH 4/6] staging: rtl8192e: matched alignment with open parenthesis
On Fri, Apr 09, 2021 at 07:31:53PM -0700, Joe Perches wrote: > On Sat, 2021-04-10 at 07:55 +0530, Mitali Borkar wrote: > > On Fri, Apr 09, 2021 at 07:07:09PM -0700, Joe Perches wrote: > > > On Sat, 2021-04-10 at 07:05 +0530, Mitali Borkar wrote: > > > > Matched the alignment with open parenthesis to meet linux kernel coding > > > > style. > > > > Reported by checkpatch. > > > [] > > > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c > > > > b/drivers/staging/rtl8192e/rtl819x_HTProc.c > > > [] > > > > @@ -154,7 +154,7 @@ bool IsHTHalfNmodeAPs(struct rtllib_device *ieee) > > > > (net->ralink_cap_exist)) > > > > retValue = true; > > > > else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) || > > > > - !memcmp(net->bssid, > > > > LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) || > > > > +!memcmp(net->bssid, > > > > LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) || > > > > !memcmp(net->bssid, > > > > LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3) || > > > > (net->broadcom_cap_exist)) > > > > > > checkpatch is a stupid script. > > > Look further at the code not just at what checkpatch reports. > > > Align all the contination lines, not just the first one. > > > > > Sir, I have aligned them in last patch of this patchset. > > This sort of change should not require an additional patch. > So now should I compile this as a patchset of 5, removing the last patch of alignment? >
[PATCH v3 06/33] media: atmel: atmel-isc: extract CSC submodule config into separate function
The CSC submodule is a part of the atmel-isc pipeline, and stands for Color Space Conversion. It is used to apply a matrix transformation to RGB pixels to convert them to the YUV components. The CSC submodule should be initialized in the product specific driver as it's product specific. Other products can implement it differently. Signed-off-by: Eugen Hristev --- Changes in v3: - added explanation regarding CSC acronym Changes in v2: - addded function prototype to avoid warning with W=1 drivers/media/platform/atmel/atmel-isc-base.c | 8 +--- drivers/media/platform/atmel/atmel-isc.h| 7 +++ .../media/platform/atmel/atmel-sama5d2-isc.c| 17 + 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c index ff40ee2e2759..31f63ba90c71 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -646,13 +646,7 @@ static void isc_set_pipeline(struct isc_device *isc, u32 pipeline) regmap_bulk_write(regmap, ISC_GAM_GENTRY, gamma, GAMMA_ENTRIES); regmap_bulk_write(regmap, ISC_GAM_RENTRY, gamma, GAMMA_ENTRIES); - /* Convert RGB to YUV */ - regmap_write(regmap, ISC_CSC_YR_YG, 0x42 | (0x81 << 16)); - regmap_write(regmap, ISC_CSC_YB_OY, 0x19 | (0x10 << 16)); - regmap_write(regmap, ISC_CSC_CBR_CBG, 0xFDA | (0xFB6 << 16)); - regmap_write(regmap, ISC_CSC_CBB_OCB, 0x70 | (0x80 << 16)); - regmap_write(regmap, ISC_CSC_CRR_CRG, 0x70 | (0xFA2 << 16)); - regmap_write(regmap, ISC_CSC_CRB_OCR, 0xFEE | (0x80 << 16)); + isc->config_csc(isc); regmap_write(regmap, ISC_CBC_BRIGHT, ctrls->brightness); regmap_write(regmap, ISC_CBC_CONTRAST, ctrls->contrast); diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h index d14ae096fbf6..bb0b4419deff 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -192,6 +192,9 @@ struct isc_ctrls { * * @max_width: maximum frame width, dependent on the internal RAM * @max_height:maximum frame height, dependent on the internal RAM + * + * @config_csc:pointer to a function that initializes product + * specific CSC module */ struct isc_device { struct regmap *regmap; @@ -259,6 +262,10 @@ struct isc_device { u32 max_width; u32 max_height; + + struct { + void (*config_csc)(struct isc_device *isc); + }; }; extern struct isc_format formats_list[]; diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c index 6d9942dcd7c1..f041bd75090e 100644 --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c @@ -54,6 +54,21 @@ #define ISC_CLK_MAX_DIV255 +void isc_sama5d2_config_csc(struct isc_device *isc); + +void isc_sama5d2_config_csc(struct isc_device *isc) +{ + struct regmap *regmap = isc->regmap; + + /* Convert RGB to YUV */ + regmap_write(regmap, ISC_CSC_YR_YG, 0x42 | (0x81 << 16)); + regmap_write(regmap, ISC_CSC_YB_OY, 0x19 | (0x10 << 16)); + regmap_write(regmap, ISC_CSC_CBR_CBG, 0xFDA | (0xFB6 << 16)); + regmap_write(regmap, ISC_CSC_CBB_OCB, 0x70 | (0x80 << 16)); + regmap_write(regmap, ISC_CSC_CRR_CRG, 0x70 | (0xFA2 << 16)); + regmap_write(regmap, ISC_CSC_CRB_OCR, 0xFEE | (0x80 << 16)); +} + /* Gamma table with gamma 1/2.2 */ const u32 isc_sama5d2_gamma_table[][GAMMA_ENTRIES] = { /* 0 --> gamma 1/1.8 */ @@ -198,6 +213,8 @@ static int atmel_isc_probe(struct platform_device *pdev) isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH; isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT; + isc->config_csc = isc_sama5d2_config_csc; + /* sama5d2-isc - 8 bits per beat */ isc->dcfg = ISC_DCFG_YMBSIZE_BEATS8 | ISC_DCFG_CMBSIZE_BEATS8; -- 2.25.1
[PATCH v3 10/33] media: atmel: atmel-isc: add CBC to the reg offsets struct
The CBC submodule is a part of the atmel-isc pipeline, and stands for Contrast Brightness Control. It is used to apply gains and offsets to the luma (Y) and chroma (U, V) components of the YUV elements. Add cbc to the reg offsets struct. This will allow different products to have a different reg offset for this particular module. Signed-off-by: Eugen Hristev --- Changes in v3: - add module explanation in commit message drivers/media/platform/atmel/atmel-isc-base.c| 2 +- drivers/media/platform/atmel/atmel-isc-regs.h| 3 +++ drivers/media/platform/atmel/atmel-isc.h | 2 ++ drivers/media/platform/atmel/atmel-sama5d2-isc.c | 7 +-- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c index 446fe232956b..d4bf7fd5929f 100644 --- a/drivers/media/platform/atmel/atmel-isc-base.c +++ b/drivers/media/platform/atmel/atmel-isc-base.c @@ -2310,7 +2310,7 @@ int isc_pipeline_init(struct isc_device *isc) REG_FIELD(ISC_GAM_CTRL, 2, 2), REG_FIELD(ISC_GAM_CTRL, 3, 3), REG_FIELD(ISC_CSC_CTRL + isc->offsets.csc, 0, 0), - REG_FIELD(ISC_CBC_CTRL, 0, 0), + REG_FIELD(ISC_CBC_CTRL + isc->offsets.cbc, 0, 0), REG_FIELD(ISC_SUB422_CTRL, 0, 0), REG_FIELD(ISC_SUB420_CTRL, 0, 0), }; diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h index 5a65600c5f88..a5e2fe01ba9f 100644 --- a/drivers/media/platform/atmel/atmel-isc-regs.h +++ b/drivers/media/platform/atmel/atmel-isc-regs.h @@ -177,6 +177,9 @@ /* Color Space Conversion CRB OCR Register */ #define ISC_CSC_CRB_OCR0x03b0 +/* Offset for CBC register specific to sama5d2 product */ +#define ISC_SAMA5D2_CBC_OFFSET 0 + /* Contrast And Brightness Control Register */ #define ISC_CBC_CTRL0x03b4 diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h index cb47932197b1..b1fe93c93c61 100644 --- a/drivers/media/platform/atmel/atmel-isc.h +++ b/drivers/media/platform/atmel/atmel-isc.h @@ -147,9 +147,11 @@ struct isc_ctrls { /* * struct isc_reg_offsets - ISC device register offsets * @csc: Offset for the CSC register + * @cbc: Offset for the CBC register */ struct isc_reg_offsets { u32 csc; + u32 cbc; }; /* diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c index 66b92fa1c752..c3037244e01e 100644 --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c @@ -80,8 +80,10 @@ void isc_sama5d2_config_cbc(struct isc_device *isc) { struct regmap *regmap = isc->regmap; - regmap_write(regmap, ISC_CBC_BRIGHT, isc->ctrls.brightness); - regmap_write(regmap, ISC_CBC_CONTRAST, isc->ctrls.contrast); + regmap_write(regmap, ISC_CBC_BRIGHT + isc->offsets.cbc, +isc->ctrls.brightness); + regmap_write(regmap, ISC_CBC_CONTRAST + isc->offsets.cbc, +isc->ctrls.contrast); } /* Gamma table with gamma 1/2.2 */ @@ -232,6 +234,7 @@ static int atmel_isc_probe(struct platform_device *pdev) isc->config_cbc = isc_sama5d2_config_cbc; isc->offsets.csc = ISC_SAMA5D2_CSC_OFFSET; + isc->offsets.cbc = ISC_SAMA5D2_CBC_OFFSET; /* sama5d2-isc - 8 bits per beat */ isc->dcfg = ISC_DCFG_YMBSIZE_BEATS8 | ISC_DCFG_CMBSIZE_BEATS8; -- 2.25.1
Re: Subject: [PATCH v2] staging: media: meson: vdec: declare u32 as static const appropriately
On Tue, Apr 13, 2021 at 09:26:01AM +0200, Hans Verkuil wrote: > On 13/04/2021 08:27, Mitali Borkar wrote: > > Declared 32 bit unsigned int as static constant inside a function > > appropriately. > > > > Reported-by: kernel test robot > > Signed-off-by: Mitali Borkar > > --- > > > > Changes from v1:- Rectified the mistake by declaring u32 as static const > > properly. > > > > drivers/staging/media/meson/vdec/codec_h264.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/media/meson/vdec/codec_h264.c > > b/drivers/staging/media/meson/vdec/codec_h264.c > > index ea86e9e1c447..80141b89a9f6 100644 > > --- a/drivers/staging/media/meson/vdec/codec_h264.c > > +++ b/drivers/staging/media/meson/vdec/codec_h264.c > > @@ -287,8 +287,8 @@ static void codec_h264_resume(struct amvdec_session > > *sess) > > struct amvdec_core *core = sess->core; > > struct codec_h264 *h264 = sess->priv; > > u32 mb_width, mb_height, mb_total; > > - static const u32[] canvas3 = { ANCO_CANVAS_ADDR, 0 }; > > - static const u32[] canvas4 = { 24, 0 }; > > + static const u32 canvas3[] = { ANCO_CANVAS_ADDR, 0 }; > > + static const u32 canvas4[] = { 24, 0 }; > > This is a patch on top of your previous (v1) patch. That won't work > since the v1 is not merged, you need to make a patch against the current > mainline code. > But Sir, since I have made changes in the code, and committed them, now, if I open that file, it will contain those changes. Then should I rewrite the patch body more accurately? > Regards, > > Hans > > > > > amvdec_set_canvases(sess, canvas3, canvas4); > > > > >
Re: [PATCH 1/1] staging: ipu3-imgu: Move the UAPI header from include under include/uapi
Reviewed-by: Bingbu Cao On 4/12/21 7:11 PM, Sakari Ailus wrote: > The header defines the user space interface but may be mistaken as > kernel-only header due to its location. Add "uapi" directory under > driver's include directory and move the header there. > > Suggested-by: Greg KH > Signed-off-by: Sakari Ailus > --- > Documentation/admin-guide/media/ipu3.rst | 35 ++- > .../media/v4l/pixfmt-meta-intel-ipu3.rst | 2 +- > .../ipu3/include/{ => uapi}/intel-ipu3.h | 0 > drivers/staging/media/ipu3/ipu3-abi.h | 2 +- > 4 files changed, 20 insertions(+), 19 deletions(-) > rename drivers/staging/media/ipu3/include/{ => uapi}/intel-ipu3.h (100%) > > diff --git a/Documentation/admin-guide/media/ipu3.rst > b/Documentation/admin-guide/media/ipu3.rst > index f59697c7b374..d6454f637ff4 100644 > --- a/Documentation/admin-guide/media/ipu3.rst > +++ b/Documentation/admin-guide/media/ipu3.rst > @@ -234,22 +234,23 @@ The IPU3 ImgU pipelines can be configured using the > Media Controller, defined at > Running mode and firmware binary selection > -- > > -ImgU works based on firmware, currently the ImgU firmware support run 2 > pipes in > -time-sharing with single input frame data. Each pipe can run at certain mode > - > -"VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture, > and > -"STILL" is used for still frame capture. However, you can also select > "VIDEO" to > -capture still frames if you want to capture images with less system load and > -power. For "STILL" mode, ImgU will try to use smaller BDS factor and output > -larger bayer frame for further YUV processing than "VIDEO" mode to get high > -quality images. Besides, "STILL" mode need XNR3 to do noise reduction, hence > -"STILL" mode will need more power and memory bandwidth than "VIDEO" mode. TNR > -will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is > running at > -“VIDEO” mode by default, the user can use v4l2 control > V4L2_CID_INTEL_IPU3_MODE > -(currently defined in drivers/staging/media/ipu3/include/intel-ipu3.h) to > query > -and set the running mode. For user, there is no difference for buffer > queueing > -between the "VIDEO" and "STILL" mode, mandatory input and main output node > -should be enabled and buffers need be queued, the statistics and the > view-finder > -queues are optional. > +ImgU works based on firmware, currently the ImgU firmware support run 2 pipes > +in time-sharing with single input frame data. Each pipe can run at certain > mode > +- "VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture, > +and "STILL" is used for still frame capture. However, you can also select > +"VIDEO" to capture still frames if you want to capture images with less > system > +load and power. For "STILL" mode, ImgU will try to use smaller BDS factor and > +output larger bayer frame for further YUV processing than "VIDEO" mode to get > +high quality images. Besides, "STILL" mode need XNR3 to do noise reduction, > +hence "STILL" mode will need more power and memory bandwidth than "VIDEO" > mode. > +TNR will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is > +running at “VIDEO” mode by default, the user can use v4l2 control > +V4L2_CID_INTEL_IPU3_MODE (currently defined in > +drivers/staging/media/ipu3/include/uapi/intel-ipu3.h) to query and set the > +running mode. For user, there is no difference for buffer queueing between > the > +"VIDEO" and "STILL" mode, mandatory input and main output node should be > +enabled and buffers need be queued, the statistics and the view-finder queues > +are optional. > > The firmware binary will be selected according to current running mode, such > log > "using binary if_to_osys_striped " or "using binary > if_to_osys_primary_striped" > @@ -586,7 +587,7 @@ preserved. > References > == > > -.. [#f5] drivers/staging/media/ipu3/include/intel-ipu3.h > +.. [#f5] drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > > .. [#f1] https://github.com/intel/nvt > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > b/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > index 5f33d35532ef..84d81dd7a7b5 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > @@ -78,4 +78,4 @@ hardware and algorithm details. > Intel IPU3 ImgU uAPI data types > === > > -.. kernel-doc:: drivers/staging/media/ipu3/include/intel-ipu3.h > +.. kernel-doc:: drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > similarity index 100% > rename from drivers/staging/media/ipu3/include/intel-ipu3.h > rename to drivers/staging/media/ipu3/include/uapi/intel-ipu3.h > diff --git
Re: [PATCH] Drivers: hv: vmbus: remove unused including
On Tue, Apr 13, 2021 at 05:49:18PM +0800, Yang Li wrote: > Fix the following versioncheck warning: > ./drivers/hv/hv.c: 16 linux/version.h not needed. > > Reported-by: Abaci Robot > Signed-off-by: Yang Li Thanks for the patch. This has also been reported by Huawei's kernel bot and fixed in hyperv-next. Wei.
Re: [PATCH v5 0/3] staging: rtl8192e: Cleanup patchset for style issues in rtl819x_HTProc.c
On Tue, Apr 13, 2021 at 04:15:03PM +0530, Mitali Borkar wrote: > On Tue, Apr 13, 2021 at 09:52:48AM +0200, Greg KH wrote: > > On Tue, Apr 13, 2021 at 08:55:03AM +0530, Mitali Borkar wrote: > > > Changes from v4:- > > > [PATCH v4 1/3]:- No changes. > > > [PATCH v4 2/3]:- No changes. > > > [PATCH V4 3/3]:- Removed casts and parentheses. > > > > This series does not apply cleanly, please rebase and resend. > > > Resend as v6? Does not apply cleanly as in? Were mails not threaded > properly? Yes, send a v6, after rebasing on my tree and the staging-testing branch and resend. They were threaded properly, it looks like others have made changes to the same files that you were, which is quite common when doing small cleanup changes like this. thanks, greg k-h
Re: [PATCH v8 4/8] dt-bindings: pwm: Support new PWM_USAGE_POWER flag
On Mon, Apr 12, 2021 at 06:27:23PM +0200, Uwe Kleine-König wrote: > On Mon, Apr 12, 2021 at 03:27:41PM +0200, Clemens Gruber wrote: > > Add the flag and corresponding documentation for PWM_USAGE_POWER. > > My concern here in the previous round was that PWM_USAGE_POWER isn't a > name that intuitively suggests its semantic. Do you disagree? I suggested PWM_USAGE_POWER because I think it accurately captures what we want here. > > Cc: Rob Herring > > Signed-off-by: Clemens Gruber > > --- > > Documentation/devicetree/bindings/pwm/pwm.txt | 3 +++ > > include/dt-bindings/pwm/pwm.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt > > b/Documentation/devicetree/bindings/pwm/pwm.txt > > index 084886bd721e..fe3a28f887c0 100644 > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt > > @@ -46,6 +46,9 @@ period in nanoseconds. > > Optionally, the pwm-specifier can encode a number of flags (defined in > > ) in a third cell: > > - PWM_POLARITY_INVERTED: invert the PWM signal polarity > > +- PWM_USAGE_POWER: Only care about the power output of the signal. This > > + allows drivers (if supported) to optimize the signals, for example to > > + improve EMI and reduce current spikes. > > IMHO there are too many open questions about which freedom this gives to > the lowlevel driver. If the consumer requests .duty_cycle = 25ns + > .period = 100ns, can the driver provide .duty_cycle = 25s + .period = > 100s which nominally has the same power output? Let's not introduce more > ambiguity than there already is. The freedom given to the driver should be to adjust the signal within reasonable bounds. Changing the time unit by a factor of 10 is not within reason, and I doubt anyone would interpret it that way, even if we didn't document this at all. To be frank I think that quest of yours to try and rid the PWM API of all ambiguity is futile. I've been trying to be lenient because you seem motivated, but I think you're taking this too far. There are always going to be cases that aren't completely clear-cut and where drivers need the flexibility to cheat in order to be useful at all. If we get to a point where everything needs to be 100% accurate, the majority of the PWM controllers won't be usable at all. Don't let perfect be the enemy of good. Thierry signature.asc Description: PGP signature
Re: [PATCH v8 1/8] pwm: pca9685: Switch to atomic API
On Tue, Apr 13, 2021 at 02:11:38PM +0200, Clemens Gruber wrote: > Hi Uwe, > > On Mon, Apr 12, 2021 at 10:10:19PM +0200, Uwe Kleine-König wrote: > > Hello Clemens, > > > > On Mon, Apr 12, 2021 at 06:39:28PM +0200, Clemens Gruber wrote: > > > On Mon, Apr 12, 2021 at 06:18:08PM +0200, Uwe Kleine-König wrote: > > > > On Mon, Apr 12, 2021 at 03:27:38PM +0200, Clemens Gruber wrote: > > > > > The switch to the atomic API goes hand in hand with a few fixes to > > > > > previously experienced issues: > > > > > - The duty cycle is no longer lost after disable/enable (previously > > > > > the > > > > > OFF registers were cleared in disable and the user was required to > > > > > call config to restore the duty cycle settings) > > > > > - If one sets a period resulting in the same prescale register value, > > > > > the sleep and write to the register is now skipped > > > > > - Previously, only the full ON bit was toggled in GPIO mode (and full > > > > > OFF cleared if set to high), which could result in both full OFF and > > > > > full ON not being set and on=0, off=0, which is not allowed > > > > > according > > > > > to the datasheet > > > > > - The OFF registers were reset to 0 in probe, which could lead to the > > > > > forbidden on=0, off=0. Fixed by resetting to POR default (full OFF) > > > > > > > > > > Signed-off-by: Clemens Gruber > > > > > --- > > > > > Changes since v7: > > > > > - Moved check for !state->enabled before prescaler configuration > > > > > - Removed unnecessary cast > > > > > - Use DIV_ROUND_DOWN in .apply > > > > > > > > > > Changes since v6: > > > > > - Order of a comparison switched for improved readability > > > > > > > > > > Changes since v5: > > > > > - Function documentation for set_duty > > > > > - Variable initializations > > > > > - Print warning if all LEDs channel > > > > > - Changed EOPNOTSUPP to EINVAL > > > > > - Improved error messages > > > > > - Register reset corrections moved to this patch > > > > > > > > > > Changes since v4: > > > > > - Patches split up > > > > > - Use a single set_duty function > > > > > - Improve readability / new macros > > > > > - Added a patch to restrict prescale changes to the first user > > > > > > > > > > Changes since v3: > > > > > - Refactoring: Extracted common functions > > > > > - Read prescale register value instead of caching it > > > > > - Return all zeros and disabled for "all LEDs" channel state > > > > > - Improved duty calculation / mapping to 0..4096 > > > > > > > > > > Changes since v2: > > > > > - Always set default prescale value in probe > > > > > - Simplified probe code > > > > > - Inlined functions with one callsite > > > > > > > > > > Changes since v1: > > > > > - Fixed a logic error > > > > > - Impoved PM runtime handling and fixed !CONFIG_PM > > > > > - Write default prescale reg value if invalid in probe > > > > > - Reuse full_off/_on functions throughout driver > > > > > - Use cached prescale value whenever possible > > > > > > > > > > drivers/pwm/pwm-pca9685.c | 259 > > > > > +- > > > > > 1 file changed, 89 insertions(+), 170 deletions(-) > > > > > > > > > > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c > > > > > index 4a55dc18656c..827b57ced3c2 100644 > > > > > --- a/drivers/pwm/pwm-pca9685.c > > > > > +++ b/drivers/pwm/pwm-pca9685.c > > > > > @@ -51,7 +51,6 @@ > > > > > #define PCA9685_PRESCALE_MAX 0xFF/* => min. frequency of 24 Hz */ > > > > > > > > > > #define PCA9685_COUNTER_RANGE4096 > > > > > -#define PCA9685_DEFAULT_PERIOD 500 /* Default period_ns = > > > > > 1/200 Hz */ > > > > > #define PCA9685_OSC_CLOCK_MHZ25 /* Internal oscillator > > > > > with 25 MHz */ > > > > > > > > > > #define PCA9685_NUMREGS 0xFF > > > > > @@ -71,10 +70,14 @@ > > > > > #define LED_N_OFF_H(N) (PCA9685_LEDX_OFF_H + (4 * (N))) > > > > > #define LED_N_OFF_L(N) (PCA9685_LEDX_OFF_L + (4 * (N))) > > > > > > > > > > +#define REG_ON_H(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_ON_H > > > > > : LED_N_ON_H((C))) > > > > > +#define REG_ON_L(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_ON_L > > > > > : LED_N_ON_L((C))) > > > > > +#define REG_OFF_H(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_OFF_H > > > > > : LED_N_OFF_H((C))) > > > > > +#define REG_OFF_L(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_OFF_L > > > > > : LED_N_OFF_L((C))) > > > > > > > > I'd like to see these named PCA9685_REG_ON_H etc. > > > > > > I did not use the prefix because the existing LED_N_ON/OFF_H/L also do > > > not have a prefix. If the prefix is mandatory, I think LED_N_.. should > > > also be prefixed, right? > > > > I'd like to seem the prefixed (and assume that Thierry doesn't agree). > > IMHO it's good style and even though it yields longer name usually it > > yields easier understandable code. (But this seems to be subjective.) > > I am not sure I want to also rename the existing LED_N_OFF stuff
Re: [PATCH v8 1/8] pwm: pca9685: Switch to atomic API
On Tue, Apr 13, 2021 at 02:11:38PM +0200, Clemens Gruber wrote: > Hi Uwe, > > On Mon, Apr 12, 2021 at 10:10:19PM +0200, Uwe Kleine-König wrote: > > Hello Clemens, > > > > On Mon, Apr 12, 2021 at 06:39:28PM +0200, Clemens Gruber wrote: > > > On Mon, Apr 12, 2021 at 06:18:08PM +0200, Uwe Kleine-König wrote: > > > > On Mon, Apr 12, 2021 at 03:27:38PM +0200, Clemens Gruber wrote: > > > > > The switch to the atomic API goes hand in hand with a few fixes to > > > > > previously experienced issues: > > > > > - The duty cycle is no longer lost after disable/enable (previously > > > > > the > > > > > OFF registers were cleared in disable and the user was required to > > > > > call config to restore the duty cycle settings) > > > > > - If one sets a period resulting in the same prescale register value, > > > > > the sleep and write to the register is now skipped > > > > > - Previously, only the full ON bit was toggled in GPIO mode (and full > > > > > OFF cleared if set to high), which could result in both full OFF and > > > > > full ON not being set and on=0, off=0, which is not allowed > > > > > according > > > > > to the datasheet > > > > > - The OFF registers were reset to 0 in probe, which could lead to the > > > > > forbidden on=0, off=0. Fixed by resetting to POR default (full OFF) > > > > > > > > > > Signed-off-by: Clemens Gruber > > > > > --- > > > > > Changes since v7: > > > > > - Moved check for !state->enabled before prescaler configuration > > > > > - Removed unnecessary cast > > > > > - Use DIV_ROUND_DOWN in .apply > > > > > > > > > > Changes since v6: > > > > > - Order of a comparison switched for improved readability > > > > > > > > > > Changes since v5: > > > > > - Function documentation for set_duty > > > > > - Variable initializations > > > > > - Print warning if all LEDs channel > > > > > - Changed EOPNOTSUPP to EINVAL > > > > > - Improved error messages > > > > > - Register reset corrections moved to this patch > > > > > > > > > > Changes since v4: > > > > > - Patches split up > > > > > - Use a single set_duty function > > > > > - Improve readability / new macros > > > > > - Added a patch to restrict prescale changes to the first user > > > > > > > > > > Changes since v3: > > > > > - Refactoring: Extracted common functions > > > > > - Read prescale register value instead of caching it > > > > > - Return all zeros and disabled for "all LEDs" channel state > > > > > - Improved duty calculation / mapping to 0..4096 > > > > > > > > > > Changes since v2: > > > > > - Always set default prescale value in probe > > > > > - Simplified probe code > > > > > - Inlined functions with one callsite > > > > > > > > > > Changes since v1: > > > > > - Fixed a logic error > > > > > - Impoved PM runtime handling and fixed !CONFIG_PM > > > > > - Write default prescale reg value if invalid in probe > > > > > - Reuse full_off/_on functions throughout driver > > > > > - Use cached prescale value whenever possible > > > > > > > > > > drivers/pwm/pwm-pca9685.c | 259 > > > > > +- > > > > > 1 file changed, 89 insertions(+), 170 deletions(-) > > > > > > > > > > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c > > > > > index 4a55dc18656c..827b57ced3c2 100644 > > > > > --- a/drivers/pwm/pwm-pca9685.c > > > > > +++ b/drivers/pwm/pwm-pca9685.c > > > > > @@ -51,7 +51,6 @@ > > > > > #define PCA9685_PRESCALE_MAX 0xFF/* => min. frequency of 24 Hz */ > > > > > > > > > > #define PCA9685_COUNTER_RANGE4096 > > > > > -#define PCA9685_DEFAULT_PERIOD 500 /* Default period_ns = > > > > > 1/200 Hz */ > > > > > #define PCA9685_OSC_CLOCK_MHZ25 /* Internal oscillator > > > > > with 25 MHz */ > > > > > > > > > > #define PCA9685_NUMREGS 0xFF > > > > > @@ -71,10 +70,14 @@ > > > > > #define LED_N_OFF_H(N) (PCA9685_LEDX_OFF_H + (4 * (N))) > > > > > #define LED_N_OFF_L(N) (PCA9685_LEDX_OFF_L + (4 * (N))) > > > > > > > > > > +#define REG_ON_H(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_ON_H > > > > > : LED_N_ON_H((C))) > > > > > +#define REG_ON_L(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_ON_L > > > > > : LED_N_ON_L((C))) > > > > > +#define REG_OFF_H(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_OFF_H > > > > > : LED_N_OFF_H((C))) > > > > > +#define REG_OFF_L(C) ((C) >= PCA9685_MAXCHAN ? PCA9685_ALL_LED_OFF_L > > > > > : LED_N_OFF_L((C))) > > > > > > > > I'd like to see these named PCA9685_REG_ON_H etc. > > > > > > I did not use the prefix because the existing LED_N_ON/OFF_H/L also do > > > not have a prefix. If the prefix is mandatory, I think LED_N_.. should > > > also be prefixed, right? > > > > I'd like to seem the prefixed (and assume that Thierry doesn't agree). > > IMHO it's good style and even though it yields longer name usually it > > yields easier understandable code. (But this seems to be subjective.) > > I am not sure I want to also rename the existing LED_N_OFF stuff
Re: [PATCH v3 12/27] perf parse-events: Support no alias assigned event inside hybrid PMU
On Mon, Apr 12, 2021 at 10:51:14AM +0800, Jin, Yao wrote: SNIP > > Do you suggest we just use string comparison for doing the direct check? > > e.g. > > if (strstr(term->config, "L1-dcache")) > ... > > Of course, we can define a string array first and use a loop for string > comparison. > > > > + if (!parse_state->fake_pmu && head_config && !found && > > > + perf_pmu__is_hybrid(name)) { > > > + struct parse_events_term *term; > > > + int ret; > > > + > > > + list_for_each_entry(term, head_config, list) { > > > + if (!term->config) > > > + continue; > > > + > > > + ret = parse_events__with_hybrid_pmu(parse_state, > > > + term->config, > > > + name, , > > > + list); > > > > do we need to call the parsing again? could we just call > > parse_events__add_cache_hybrid? > > > > jirka > > > > > > If we do the direct check for cache events, I think we don't need the parsing > again. > > As I mentioned above, we need to define a string array and compare with > term->config one by one. > maybe another way is to find a way to run just the lexer (without parser) and check that it returns PE_NAME_CACHE_OP_RESULT jirka
Re: [PATCH 1/2] dt-binding: bcm43xx-fmac: add optional brcm,ccode-map
On 09-04-2021 20:46, Rob Herring wrote: On Thu, Apr 08, 2021 at 07:30:21PM +0800, Shawn Guo wrote: Add optional brcm,ccode-map property to support translation from ISO3166 country code to brcmfmac firmware country code and revision. Signed-off-by: Shawn Guo --- .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 7 +++ 1 file changed, 7 insertions(+) Can you convert this to schema first. Hi Rob, You mean to change it to YAML, right? You already applied a patch for that a few weeks ago: https://lore.kernel.org/linux-devicetree/20210324174737.ga3319...@robh.at.kernel.org/ Regards, Arend -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
[RFC PATCH v8 06/19] af_vsock: rest of SEQPACKET support
This does rest of SOCK_SEQPACKET support: 1) Adds socket ops for SEQPACKET type. 2) Allows to create socket with SEQPACKET type. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- include/net/af_vsock.h | 1 + net/vmw_vsock/af_vsock.c | 36 +++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index 5860027d5173..53d3f33dbdbf 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -140,6 +140,7 @@ struct vsock_transport { int flags, bool *msg_ready); int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg, size_t len); + bool (*seqpacket_allow)(void); /* Notification. */ int (*notify_poll_in)(struct vsock_sock *, size_t, bool *); diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 78f703b3ad7f..e7693f9a6cad 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -452,6 +452,7 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk) new_transport = transport_dgram; break; case SOCK_STREAM: + case SOCK_SEQPACKET: if (vsock_use_local_transport(remote_cid)) new_transport = transport_local; else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g || @@ -484,6 +485,14 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk) if (!new_transport || !try_module_get(new_transport->module)) return -ENODEV; + if (sk->sk_type == SOCK_SEQPACKET) { + if (!new_transport->seqpacket_allow || + !new_transport->seqpacket_allow()) { + module_put(new_transport->module); + return -ESOCKTNOSUPPORT; + } + } + ret = new_transport->init(vsk, psk); if (ret) { module_put(new_transport->module); @@ -684,6 +693,7 @@ static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr) switch (sk->sk_socket->type) { case SOCK_STREAM: + case SOCK_SEQPACKET: spin_lock_bh(_table_lock); retval = __vsock_bind_connectible(vsk, addr); spin_unlock_bh(_table_lock); @@ -770,7 +780,7 @@ static struct sock *__vsock_create(struct net *net, static bool sock_type_connectible(u16 type) { - return type == SOCK_STREAM; + return (type == SOCK_STREAM) || (type == SOCK_SEQPACKET); } static void __vsock_release(struct sock *sk, int level) @@ -2131,6 +2141,27 @@ static const struct proto_ops vsock_stream_ops = { .sendpage = sock_no_sendpage, }; +static const struct proto_ops vsock_seqpacket_ops = { + .family = PF_VSOCK, + .owner = THIS_MODULE, + .release = vsock_release, + .bind = vsock_bind, + .connect = vsock_connect, + .socketpair = sock_no_socketpair, + .accept = vsock_accept, + .getname = vsock_getname, + .poll = vsock_poll, + .ioctl = sock_no_ioctl, + .listen = vsock_listen, + .shutdown = vsock_shutdown, + .setsockopt = vsock_connectible_setsockopt, + .getsockopt = vsock_connectible_getsockopt, + .sendmsg = vsock_connectible_sendmsg, + .recvmsg = vsock_connectible_recvmsg, + .mmap = sock_no_mmap, + .sendpage = sock_no_sendpage, +}; + static int vsock_create(struct net *net, struct socket *sock, int protocol, int kern) { @@ -2151,6 +2182,9 @@ static int vsock_create(struct net *net, struct socket *sock, case SOCK_STREAM: sock->ops = _stream_ops; break; + case SOCK_SEQPACKET: + sock->ops = _seqpacket_ops; + break; default: return -ESOCKTNOSUPPORT; } -- 2.25.1
[RFC PATCH v8 05/19] af_vsock: implement send logic for SEQPACKET
This adds some logic to current stream enqueue function for SEQPACKET support: 1) Use transport's seqpacket enqueue callback. 2) Return value from enqueue function is whole record length or error for SOCK_SEQPACKET. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- include/net/af_vsock.h | 2 ++ net/vmw_vsock/af_vsock.c | 20 +++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index 5175f5a52ce1..5860027d5173 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -138,6 +138,8 @@ struct vsock_transport { /* SEQ_PACKET. */ ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, int flags, bool *msg_ready); + int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg, +size_t len); /* Notification. */ int (*notify_poll_in)(struct vsock_sock *, size_t, bool *); diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index d9fb4f9a3063..78f703b3ad7f 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1808,9 +1808,13 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, * responsibility to check how many bytes we were able to send. */ - written = transport->stream_enqueue( - vsk, msg, - len - total_written); + if (sk->sk_type == SOCK_SEQPACKET) { + written = transport->seqpacket_enqueue(vsk, + msg, len - total_written); + } else { + written = transport->stream_enqueue(vsk, + msg, len - total_written); + } if (written < 0) { err = -ENOMEM; goto out_err; @@ -1826,8 +1830,14 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, } out_err: - if (total_written > 0) - err = total_written; + if (total_written > 0) { + /* Return number of written bytes only if: +* 1) SOCK_STREAM socket. +* 2) SOCK_SEQPACKET socket when whole buffer is sent. +*/ + if (sk->sk_type == SOCK_STREAM || total_written == len) + err = total_written; + } out: release_sock(sk); return err; -- 2.25.1
Re: [PATCH] ASoC: cs35l35: Fix an error handling path in 'cs35l35_i2c_probe()'
On Sun, Apr 11, 2021 at 02:51:06PM +0200, Christophe JAILLET wrote: > If 'devm_regmap_init_i2c()' fails, there is no need to goto err. We should > return directly as already done by the surrounding error handling paths. These are stylistic improvements rather than bug fixes so it's probably better not to call them fixes. signature.asc Description: PGP signature
[RFC PATCH v8 07/19] af_vsock: update comments for stream sockets
This replaces 'stream' to 'connection oriented' in comments as SEQPACKET is also connection oriented. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- net/vmw_vsock/af_vsock.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index e7693f9a6cad..54bee7e643f4 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -415,8 +415,8 @@ static void vsock_deassign_transport(struct vsock_sock *vsk) /* Assign a transport to a socket and call the .init transport callback. * - * Note: for stream socket this must be called when vsk->remote_addr is set - * (e.g. during the connect() or when a connection request on a listener + * Note: for connection oriented socket this must be called when vsk->remote_addr + * is set (e.g. during the connect() or when a connection request on a listener * socket is received). * The vsk->remote_addr is used to decide which transport to use: * - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or VMADDR_CID_HOST if @@ -470,10 +470,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk) return 0; /* transport->release() must be called with sock lock acquired. -* This path can only be taken during vsock_stream_connect(), -* where we have already held the sock lock. -* In the other cases, this function is called on a new socket -* which is not assigned to any transport. +* This path can only be taken during vsock_connect(), where we +* have already held the sock lock. In the other cases, this +* function is called on a new socket which is not assigned to +* any transport. */ vsk->transport->release(vsk); vsock_deassign_transport(vsk); @@ -658,9 +658,10 @@ static int __vsock_bind_connectible(struct vsock_sock *vsk, vsock_addr_init(>local_addr, new_addr.svm_cid, new_addr.svm_port); - /* Remove stream sockets from the unbound list and add them to the hash -* table for easy lookup by its address. The unbound list is simply an -* extra entry at the end of the hash table, a trick used by AF_UNIX. + /* Remove connection oriented sockets from the unbound list and add them +* to the hash table for easy lookup by its address. The unbound list +* is simply an extra entry at the end of the hash table, a trick used +* by AF_UNIX. */ __vsock_remove_bound(vsk); __vsock_insert_bound(vsock_bound_sockets(>local_addr), vsk); @@ -952,10 +953,10 @@ static int vsock_shutdown(struct socket *sock, int mode) if ((mode & ~SHUTDOWN_MASK) || !mode) return -EINVAL; - /* If this is a STREAM socket and it is not connected then bail out -* immediately. If it is a DGRAM socket then we must first kick the -* socket so that it wakes up from any sleeping calls, for example -* recv(), and then afterwards return the error. + /* If this is a connection oriented socket and it is not connected then +* bail out immediately. If it is a DGRAM socket then we must first +* kick the socket so that it wakes up from any sleeping calls, for +* example recv(), and then afterwards return the error. */ sk = sock->sk; @@ -1727,7 +1728,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, transport = vsk->transport; - /* Callers should not provide a destination with stream sockets. */ + /* Callers should not provide a destination with connection oriented +* sockets. +*/ if (msg->msg_namelen) { err = sk->sk_state == TCP_ESTABLISHED ? -EISCONN : -EOPNOTSUPP; goto out; -- 2.25.1
[RFC PATCH v8 09/19] virtio/vsock: simplify credit update function API
This function is static and 'hdr' arg was always NULL. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- net/vmw_vsock/virtio_transport_common.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index f69993d67f89..833104b71a1c 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -271,8 +271,7 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit) } EXPORT_SYMBOL_GPL(virtio_transport_put_credit); -static int virtio_transport_send_credit_update(struct vsock_sock *vsk, - struct virtio_vsock_hdr *hdr) +static int virtio_transport_send_credit_update(struct vsock_sock *vsk) { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE, @@ -384,7 +383,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, * with different values. */ if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) - virtio_transport_send_credit_update(vsk, NULL); + virtio_transport_send_credit_update(vsk); return total; @@ -493,7 +492,7 @@ void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val) vvs->buf_alloc = *val; - virtio_transport_send_credit_update(vsk, NULL); + virtio_transport_send_credit_update(vsk); } EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size); -- 2.25.1
[RFC PATCH v8 08/19] virtio/vsock: set packet's type in virtio_transport_send_pkt_info()
This moves passing type of packet from 'info' structure to 'virtio_ transport_send_pkt_info()' function. There is no need to set type of packet which differs from type of socket. Since at current time only stream type is supported, set it directly in 'virtio_transport_send_ pkt_info()', so callers don't need to set it. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- net/vmw_vsock/virtio_transport_common.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index e4370b1b7494..f69993d67f89 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -179,6 +179,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, struct virtio_vsock_pkt *pkt; u32 pkt_len = info->pkt_len; + info->type = VIRTIO_VSOCK_TYPE_STREAM; + t_ops = virtio_transport_get_ops(vsk); if (unlikely(!t_ops)) return -EFAULT; @@ -270,12 +272,10 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit) EXPORT_SYMBOL_GPL(virtio_transport_put_credit); static int virtio_transport_send_credit_update(struct vsock_sock *vsk, - int type, struct virtio_vsock_hdr *hdr) { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE, - .type = type, .vsk = vsk, }; @@ -383,11 +383,8 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, * messages, we set the limit to a high value. TODO: experiment * with different values. */ - if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { - virtio_transport_send_credit_update(vsk, - VIRTIO_VSOCK_TYPE_STREAM, - NULL); - } + if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) + virtio_transport_send_credit_update(vsk, NULL); return total; @@ -496,8 +493,7 @@ void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val) vvs->buf_alloc = *val; - virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM, - NULL); + virtio_transport_send_credit_update(vsk, NULL); } EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size); @@ -624,7 +620,6 @@ int virtio_transport_connect(struct vsock_sock *vsk) { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_REQUEST, - .type = VIRTIO_VSOCK_TYPE_STREAM, .vsk = vsk, }; @@ -636,7 +631,6 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode) { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_SHUTDOWN, - .type = VIRTIO_VSOCK_TYPE_STREAM, .flags = (mode & RCV_SHUTDOWN ? VIRTIO_VSOCK_SHUTDOWN_RCV : 0) | (mode & SEND_SHUTDOWN ? @@ -665,7 +659,6 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk, { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_RW, - .type = VIRTIO_VSOCK_TYPE_STREAM, .msg = msg, .pkt_len = len, .vsk = vsk, @@ -688,7 +681,6 @@ static int virtio_transport_reset(struct vsock_sock *vsk, { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_RST, - .type = VIRTIO_VSOCK_TYPE_STREAM, .reply = !!pkt, .vsk = vsk, }; @@ -990,7 +982,6 @@ virtio_transport_send_response(struct vsock_sock *vsk, { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_RESPONSE, - .type = VIRTIO_VSOCK_TYPE_STREAM, .remote_cid = le64_to_cpu(pkt->hdr.src_cid), .remote_port = le32_to_cpu(pkt->hdr.src_port), .reply = true, -- 2.25.1
Re: [Outreachy kernel] Re: [PATCH v2 3/4] staging: media: intel-ipu3: reduce length of line
On Tuesday, April 13, 2021 12:56:36 PM CEST Mitali Borkar wrote: > On Tue, Apr 13, 2021 at 01:44:32PM +0300, Sakari Ailus wrote: > > On Tue, Apr 13, 2021 at 04:13:04PM +0530, Mitali Borkar wrote: > > > On Tue, Apr 13, 2021 at 01:01:34PM +0300, Sakari Ailus wrote: > > > > Hi Mitali, > > > > > > > > Thanks for the update. > > > > > > > > On Tue, Apr 13, 2021 at 10:46:06AM +0530, Mitali Borkar wrote: > > > > > Reduced length of the line under 80 characters to meet > > > > > linux-kernel > > > > > coding style. > > > > > > > > > > Signed-off-by: Mitali Borkar > > > > > --- > > > > > > > > > > Changes from v1:- Reduced length of the line under 80 characters > > > > > > > > > > drivers/staging/media/ipu3/include/intel-ipu3.h | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > > b/drivers/staging/media/ipu3/include/intel-ipu3.h index > > > > > 6a72c81d2b67..52dcc6cdcffc 100644 > > > > > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > > @@ -247,7 +247,8 @@ struct ipu3_uapi_ae_ccm { > > > > > > > > > > */ > > > > > > > > > > struct ipu3_uapi_ae_config { > > > > > > > > > > struct ipu3_uapi_ae_grid_config grid_cfg __aligned(32); > > > > > > > > > > - struct ipu3_uapi_ae_weight_elem weights[IPU3_UAPI_AE_WEIGHTS] > > > > > __aligned(32); + struct ipu3_uapi_ae_weight_elem > > > > > weights[IPU3_UAPI_AE_WEIGHTS] + __aligned(32); > > > > > > > > Do you still have the other two patches in your tree? This doesn't > > > > apply > > > > here due to the different attribute syntax. > > > > > > I have patch 1/6 and 2/6 in my tree which you asked me to drop. > > > > Could you drop them and then submit v3? > > I am extremely sorry Sir, but I am still learning to use git, drop them > means to delete those commits? Even if I delete those, this patch was > made after those, so the changes I made then will remain as it is, so > what to do now? > > > Thanks. > I think that this document can help: https://kernelnewbies.org/GitTips Fabio
[RFC PATCH v8 02/19] af_vsock: separate wait data loop
This moves wait loop for data to dedicated function, because later it will be used by SEQPACKET data receive loop. While moving the code around, let's update an old comment. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- net/vmw_vsock/af_vsock.c | 156 +-- 1 file changed, 84 insertions(+), 72 deletions(-) diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 35fcab370885..ba5ec37a4e12 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1833,6 +1833,69 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, return err; } +static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait, + long timeout, + struct vsock_transport_recv_notify_data *recv_data, + size_t target) +{ + const struct vsock_transport *transport; + struct vsock_sock *vsk; + s64 data; + int err; + + vsk = vsock_sk(sk); + err = 0; + transport = vsk->transport; + + while ((data = vsock_stream_has_data(vsk)) == 0) { + prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE); + + if (sk->sk_err != 0 || + (sk->sk_shutdown & RCV_SHUTDOWN) || + (vsk->peer_shutdown & SEND_SHUTDOWN)) { + break; + } + + /* Don't wait for non-blocking sockets. */ + if (timeout == 0) { + err = -EAGAIN; + break; + } + + if (recv_data) { + err = transport->notify_recv_pre_block(vsk, target, recv_data); + if (err < 0) + break; + } + + release_sock(sk); + timeout = schedule_timeout(timeout); + lock_sock(sk); + + if (signal_pending(current)) { + err = sock_intr_errno(timeout); + break; + } else if (timeout == 0) { + err = -EAGAIN; + break; + } + } + + finish_wait(sk_sleep(sk), wait); + + if (err) + return err; + + /* Internal transport error when checking for available +* data. XXX This should be changed to a connection +* reset in a later change. +*/ + if (data < 0) + return -ENOMEM; + + return data; +} + static int vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, int flags) @@ -1912,85 +1975,34 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, while (1) { - s64 ready; + ssize_t read; - prepare_to_wait(sk_sleep(sk), , TASK_INTERRUPTIBLE); - ready = vsock_stream_has_data(vsk); + err = vsock_wait_data(sk, , timeout, _data, target); + if (err <= 0) + break; - if (ready == 0) { - if (sk->sk_err != 0 || - (sk->sk_shutdown & RCV_SHUTDOWN) || - (vsk->peer_shutdown & SEND_SHUTDOWN)) { - finish_wait(sk_sleep(sk), ); - break; - } - /* Don't wait for non-blocking sockets. */ - if (timeout == 0) { - err = -EAGAIN; - finish_wait(sk_sleep(sk), ); - break; - } - - err = transport->notify_recv_pre_block( - vsk, target, _data); - if (err < 0) { - finish_wait(sk_sleep(sk), ); - break; - } - release_sock(sk); - timeout = schedule_timeout(timeout); - lock_sock(sk); - - if (signal_pending(current)) { - err = sock_intr_errno(timeout); - finish_wait(sk_sleep(sk), ); - break; - } else if (timeout == 0) { - err = -EAGAIN; - finish_wait(sk_sleep(sk), ); - break; - } - } else { - ssize_t read; - - finish_wait(sk_sleep(sk), ); - - if (ready < 0) { - /* Invalid queue pair content. XXX This should - * be changed to a connection reset in a later -
[RFC PATCH v8 03/19] af_vsock: separate receive data loop
Move STREAM specific data receive logic to '__vsock_stream_recvmsg()' dedicated function, while checks, that will be same for both STREAM and SEQPACKET sockets, stays in 'vsock_connectible_recvmsg()' shared functions. Signed-off-by: Arseny Krasnov Reviewed-by: Stefano Garzarella --- net/vmw_vsock/af_vsock.c | 116 ++- 1 file changed, 67 insertions(+), 49 deletions(-) diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index ba5ec37a4e12..c4f6bfa1e381 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1896,65 +1896,22 @@ static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait, return data; } -static int -vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, - int flags) +static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg, + size_t len, int flags) { - struct sock *sk; - struct vsock_sock *vsk; + struct vsock_transport_recv_notify_data recv_data; const struct vsock_transport *transport; - int err; - size_t target; + struct vsock_sock *vsk; ssize_t copied; + size_t target; long timeout; - struct vsock_transport_recv_notify_data recv_data; + int err; DEFINE_WAIT(wait); - sk = sock->sk; vsk = vsock_sk(sk); - err = 0; - - lock_sock(sk); - transport = vsk->transport; - if (!transport || sk->sk_state != TCP_ESTABLISHED) { - /* Recvmsg is supposed to return 0 if a peer performs an -* orderly shutdown. Differentiate between that case and when a -* peer has not connected or a local shutdown occured with the -* SOCK_DONE flag. -*/ - if (sock_flag(sk, SOCK_DONE)) - err = 0; - else - err = -ENOTCONN; - - goto out; - } - - if (flags & MSG_OOB) { - err = -EOPNOTSUPP; - goto out; - } - - /* We don't check peer_shutdown flag here since peer may actually shut -* down, but there can be data in the queue that a local socket can -* receive. -*/ - if (sk->sk_shutdown & RCV_SHUTDOWN) { - err = 0; - goto out; - } - - /* It is valid on Linux to pass in a zero-length receive buffer. This -* is not an error. We may as well bail out now. -*/ - if (!len) { - err = 0; - goto out; - } - /* We must not copy less than target bytes into the user's buffer * before returning successfully, so we wait for the consume queue to * have that much data to consume before dequeueing. Note that this @@ -2013,6 +1970,67 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, if (copied > 0) err = copied; +out: + return err; +} + +static int +vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, + int flags) +{ + struct sock *sk; + struct vsock_sock *vsk; + const struct vsock_transport *transport; + int err; + + DEFINE_WAIT(wait); + + sk = sock->sk; + vsk = vsock_sk(sk); + err = 0; + + lock_sock(sk); + + transport = vsk->transport; + + if (!transport || sk->sk_state != TCP_ESTABLISHED) { + /* Recvmsg is supposed to return 0 if a peer performs an +* orderly shutdown. Differentiate between that case and when a +* peer has not connected or a local shutdown occurred with the +* SOCK_DONE flag. +*/ + if (sock_flag(sk, SOCK_DONE)) + err = 0; + else + err = -ENOTCONN; + + goto out; + } + + if (flags & MSG_OOB) { + err = -EOPNOTSUPP; + goto out; + } + + /* We don't check peer_shutdown flag here since peer may actually shut +* down, but there can be data in the queue that a local socket can +* receive. +*/ + if (sk->sk_shutdown & RCV_SHUTDOWN) { + err = 0; + goto out; + } + + /* It is valid on Linux to pass in a zero-length receive buffer. This +* is not an error. We may as well bail out now. +*/ + if (!len) { + err = 0; + goto out; + } + + err = __vsock_stream_recvmsg(sk, msg, len, flags); + out: release_sock(sk); return err; -- 2.25.1
[RFC PATCH v8 04/19] af_vsock: implement SEQPACKET receive loop
This adds receive loop for SEQPACKET. It looks like receive loop for STREAM, but there is a little bit difference: 1) It doesn't call notify callbacks. 2) It doesn't care about 'SO_SNDLOWAT' and 'SO_RCVLOWAT' values, because there is no sense for these values in SEQPACKET case. 3) It waits until whole record is received or error is found during receiving. 4) It processes and sets 'MSG_TRUNC' flag. So to avoid extra conditions for two types of socket inside one loop, two independent functions were created. Signed-off-by: Arseny Krasnov --- v7 -> v8: - Length of message is now not returned by callback, it returns only length of data read by each call. - Previous case, when EAGAIN is return and dequeue loop restarted now removed(in this simplified version we consider that message could not be corrupted). - MSG_TRUNC in input flags is now handled by callback. include/net/af_vsock.h | 4 +++ net/vmw_vsock/af_vsock.c | 66 +++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index b1c717286993..5175f5a52ce1 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -135,6 +135,10 @@ struct vsock_transport { bool (*stream_is_active)(struct vsock_sock *); bool (*stream_allow)(u32 cid, u32 port); + /* SEQ_PACKET. */ + ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, +int flags, bool *msg_ready); + /* Notification. */ int (*notify_poll_in)(struct vsock_sock *, size_t, bool *); int (*notify_poll_out)(struct vsock_sock *, size_t, bool *); diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index c4f6bfa1e381..d9fb4f9a3063 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1974,6 +1974,67 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg, return err; } +static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg, +size_t len, int flags) +{ + const struct vsock_transport *transport; + bool msg_ready; + struct vsock_sock *vsk; + ssize_t record_len; + long timeout; + int err = 0; + DEFINE_WAIT(wait); + + vsk = vsock_sk(sk); + transport = vsk->transport; + + timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT); + msg_ready = false; + record_len = 0; + + while (1) { + ssize_t tmp_record_len; + + if (vsock_wait_data(sk, , timeout, NULL, 0) <= 0) { + /* In case of any loop break(timeout, signal +* interrupt or shutdown), we report user that +* nothing was copied. +*/ + err = 0; + break; + } + + tmp_record_len = transport->seqpacket_dequeue(vsk, msg, flags, _ready); + + if (tmp_record_len < 0) { + err = -ENOMEM; + break; + } + + record_len += tmp_record_len; + + if (msg_ready) + break; + } + + if (sk->sk_err) + err = -sk->sk_err; + else if (sk->sk_shutdown & RCV_SHUTDOWN) + err = 0; + + if (msg_ready && err == 0) { + err = record_len; + + /* Always set MSG_TRUNC if real length of packet is +* bigger than user's buffer. +*/ + if (record_len > len) + msg->msg_flags |= MSG_TRUNC; + } + + return err; +} + static int vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, int flags) @@ -2029,7 +2090,10 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, goto out; } - err = __vsock_stream_recvmsg(sk, msg, len, flags); + if (sk->sk_type == SOCK_STREAM) + err = __vsock_stream_recvmsg(sk, msg, len, flags); + else + err = __vsock_seqpacket_recvmsg(sk, msg, len, flags); out: release_sock(sk); -- 2.25.1
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Jacob, I love your patch! Yet something to improve: [auto build test ERROR on vkoul-dmaengine/next] [also build test ERROR on char-misc/char-misc-testing v5.12-rc7] [cannot apply to iommu/next next-20210413] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Jacob-Pan/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 base: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next config: x86_64-randconfig-a016-20200531 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/3b009446e2f451c401cff7a6d4766424acbcc890 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jacob-Pan/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 git checkout 3b009446e2f451c401cff7a6d4766424acbcc890 # save the attached .config to linux build tree make W=1 ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/dma/idxd/init.c: In function 'idxd_enable_system_pasid': >> drivers/dma/idxd/init.c:307:10: error: 'IOMMU_SVA_BIND_SUPERVISOR' >> undeclared (first use in this function) 307 | flags = IOMMU_SVA_BIND_SUPERVISOR; | ^ drivers/dma/idxd/init.c:307:10: note: each undeclared identifier is reported only once for each function it appears in vim +/IOMMU_SVA_BIND_SUPERVISOR +307 drivers/dma/idxd/init.c 300 301 static int idxd_enable_system_pasid(struct idxd_device *idxd) 302 { 303 unsigned int flags; 304 unsigned int pasid; 305 struct iommu_sva *sva; 306 > 307 flags = IOMMU_SVA_BIND_SUPERVISOR; 308 309 sva = iommu_sva_bind_device(>pdev->dev, NULL, flags); 310 if (IS_ERR(sva)) { 311 dev_warn(>pdev->dev, 312 "iommu sva bind failed: %ld\n", PTR_ERR(sva)); 313 return PTR_ERR(sva); 314 } 315 316 pasid = iommu_sva_get_pasid(sva); 317 if (pasid == IOMMU_PASID_INVALID) { 318 iommu_sva_unbind_device(sva); 319 return -ENODEV; 320 } 321 322 idxd->sva = sva; 323 idxd->pasid = pasid; 324 dev_dbg(>pdev->dev, "system pasid: %u\n", pasid); 325 return 0; 326 } 327 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH net-next 1/2] net: phy: marvell-88x2222: check that link is operational
Hi Andrew, On Tue, Apr 13, 2021 at 03:12:05PM +0200, Andrew Lunn wrote: > Is there something like this in the marvell10 driver? No, it doesn't seem to be necessary there - I haven't seen spontaneous link-ups with the 88x3310 there. Even if we did, that would cause other issues beyond a nusience link-up event, as the PHY selects the first media that has link between copper and fiber (and both are present on Macchiatobin platforms.) If the fiber indicates link up, it would prevent a copper connection being established. > Also, do you know when there is an SFP cage? Do we need a standardised > DT property for this? phydev->sfp_bus being non-NULL? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] riscv: locks: introduce ticket-based spinlock implementation
On Tue, Apr 13, 2021 at 6:25 PM Christoph Müllner wrote: > > On Tue, Apr 13, 2021 at 11:37 AM Peter Zijlstra wrote: > > > > On Tue, Apr 13, 2021 at 11:22:40AM +0200, Christoph Müllner wrote: > > > > > > For ticket locks you really only needs atomic_fetch_add() and > > > > smp_store_release() and an architectural guarantees that the > > > > atomic_fetch_add() has fwd progress under contention and that a sub-word > > > > store (through smp_store_release()) will fail the SC. > > > > > > > > Then you can do something like: > > > > > > > > void lock(atomic_t *lock) > > > > { > > > > u32 val = atomic_fetch_add(1<<16, lock); /* SC, gives us RCsc */ > > > > u16 ticket = val >> 16; > > > > > > > > for (;;) { > > > > if (ticket == (u16)val) > > > > break; > > > > cpu_relax(); > > > > val = atomic_read_acquire(lock); > > > > } > > > > } > > > > > > > > void unlock(atomic_t *lock) > > > > { > > > > u16 *ptr = (u16 *)lock + (!!__BIG_ENDIAN__); > > > > u32 val = atomic_read(lock); > > > > > > > > smp_store_release(ptr, (u16)val + 1); > > > > } > > > > > > > > That's _almost_ as simple as a test-and-set :-) It isn't quite optimal > > > > on x86 for not being allowed to use a memop on unlock, since its being > > > > forced into a load-store because of all the volatile, but whatever. > > > > > > What about trylock()? > > > I.e. one could implement trylock() without a loop, by letting > > > trylock() fail if the SC fails. > > > That looks safe on first view, but nobody does this right now. > > > > Generic code has to use cmpxchg(), and then you get something like this: > > > > bool trylock(atomic_t *lock) > > { > > u32 old = atomic_read(lock); > > > > if ((old >> 16) != (old & 0x)) > > return false; > > > > return atomic_try_cmpxchg(lock, , old + (1<<16)); /* SC, for > > RCsc */ > > } > > This approach requires two loads (atomic_read() and cmpxchg()), which > is not required. > Detecting this pattern and optimizing it in a compiler is quite unlikely. > > A bit less generic solution would be to wrap the LL/SC (would be > mandatory in this case) > instructions and do something like this: > > uint32_t __smp_load_acquire_reserved(void*); > int __smp_store_release_conditional(void*, uint32_t); > > typedef union { > uint32_t v32; > struct { > uint16_t owner; > uint16_t next; > }; > } arch_spinlock_t; > > int trylock(arch_spinlock_t *lock) > { > arch_spinlock_t l; > int success; > do { > l.v32 = __smp_load_acquire_reserved(lock); > if (l.owner != l.next) > return 0; > l.next++; > success = __smp_store_release_conditional(lock, l.v32); It's a new semantics v.s cmpxchg, and cmpxchg is come from CAS instruction to solve some complex scenario. The primitive of cmpxchg has been widely used in Linux, so I don't think introducing a new semantics (reserved_conditional) is a good idea. Also, from this point: It seems CAS instruction is more suitable for software compatibility. Although riscv privilege spec chose the LR/SC and list some bad sides of CAS. > } while (!success); > return success; > } > > But here we can't tell the compiler to optimize the code between LL and SC... > > > > > That will try and do the full LL/SC loop, because it wants to complete > > the cmpxchg, but in generic code we have no other option. > > > > (Is this what C11's weak cmpxchg is for?) -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/
[PATCH 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick
From: jun qian If it has been determined that the current cpu need resched in the early stage of for_each_sched_entity, then there is no need to check preempt in the subsequent se->parent entity_tick. Signed-off-by: jun qian --- kernel/sched/fair.c | 5 + 1 file changed, 5 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 1a68a0536add..c0d135100d54 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4352,8 +4352,13 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) { unsigned long ideal_runtime, delta_exec; struct sched_entity *se; + struct rq *rq = rq_of(cfs_rq); s64 delta; + /* If the TIF_NEED_RESCHED has been set, it is no need to check again */ + if (test_tsk_need_resched(rq->curr)) + return; + ideal_runtime = sched_slice(cfs_rq, curr); delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime; if (delta_exec > ideal_runtime) { -- 2.18.2
[PATCH v2 3/3] nvmem: eeprom: add documentation for FRAM
Added dt binding documentation. Signed-off-by: Jiri Prchal --- -v2: fixed dt_binding_check warnings thanks to Rob Herring --- .../devicetree/bindings/eeprom/at25.yaml | 31 +++ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/eeprom/at25.yaml b/Documentation/devicetree/bindings/eeprom/at25.yaml index 121a601db22e..840ee7a83a14 100644 --- a/Documentation/devicetree/bindings/eeprom/at25.yaml +++ b/Documentation/devicetree/bindings/eeprom/at25.yaml @@ -4,14 +4,16 @@ $id: "http://devicetree.org/schemas/eeprom/at25.yaml#; $schema: "http://devicetree.org/meta-schemas/core.yaml#; -title: SPI EEPROMs compatible with Atmel's AT25 +title: SPI EEPROMs or FRAMs compatible with Atmel's AT25 maintainers: - Christian Eggers properties: $nodename: -pattern: "^eeprom@[0-9a-f]{1,2}$" +anyOf: + - pattern: "^eeprom@[0-9a-f]{1,2}$" + - pattern: "^fram@[0-9a-f]{1,2}$" # There are multiple known vendors who manufacture EEPROM chips compatible # with Atmel's AT25. The compatible string requires two items where the @@ -31,6 +33,7 @@ properties: - microchip,25lc040 - st,m95m02 - st,m95256 + - cypress,fm25 - const: atmel,at25 @@ -48,7 +51,7 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32 enum: [1, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072] description: - Size of the eeprom page. + Size of the eeprom page. FRAMs don't have pages. size: $ref: /schemas/types.yaml#/definitions/uint32 @@ -101,9 +104,19 @@ required: - compatible - reg - spi-max-frequency - - pagesize - - size - - address-width + +allOf: + - if: + properties: +compatible: + not: +contains: + const: cypress,fm25 +then: + required: +- pagesize +- size +- address-width additionalProperties: false @@ -126,4 +139,10 @@ examples: size = <32768>; address-width = <16>; }; + +fram@1 { +compatible = "cypress,fm25", "atmel,at25"; +reg = <1>; +spi-max-frequency = <4000>; +}; }; -- 2.25.1
Re: [PATCH v7 5/7] mm: Make alloc_contig_range handle free hugetlb pages
On Tue 13-04-21 12:47:45, Oscar Salvador wrote: > alloc_contig_range will fail if it ever sees a HugeTLB page within the > range we are trying to allocate, even when that page is free and can be > easily reallocated. > This has proved to be problematic for some users of alloc_contic_range, > e.g: CMA and virtio-mem, where those would fail the call even when those > pages lay in ZONE_MOVABLE and are free. > > We can do better by trying to replace such page. > > Free hugepages are tricky to handle so as to no userspace application > notices disruption, we need to replace the current free hugepage with > a new one. > > In order to do that, a new function called alloc_and_dissolve_huge_page > is introduced. > This function will first try to get a new fresh hugepage, and if it > succeeds, it will replace the old one in the free hugepage pool. > > The free page replacement is done under hugetlb_lock, so no external > users of hugetlb will notice the change. > To allocate the new huge page, we use alloc_buddy_huge_page(), so we > do not have to deal with any counters, and prep_new_huge_page() is not > called. This is valulable because in case we need to free the new page, > we only need to call __free_pages(). > > Once we know that the page to be replaced is a genuine 0-refcounted > huge page, we remove the old page from the freelist by remove_hugetlb_page(). > Then, we can call __prep_new_huge_page() and __prep_account_new_huge_page() > for the new huge page to properly initialize it and increment the > hstate->nr_huge_pages counter (previously decremented by > remove_hugetlb_page()). > Once done, the page is enqueued by enqueue_huge_page() and it is ready > to be used. > > There is one tricky case when > page's refcount is 0 because it is in the process of being released. > A missing PageHugeFreed bit will tell us that freeing is in flight so > we retry after dropping the hugetlb_lock. The race window should be > small and the next retry should make a forward progress. > > E.g: > > CPU0 CPU1 > free_huge_page() isolate_or_dissolve_huge_page > PageHuge() == T > alloc_and_dissolve_huge_page > alloc_buddy_huge_page() > spin_lock_irq(hugetlb_lock) > // PageHuge() && !PageHugeFreed && > // !PageCount() > spin_unlock_irq(hugetlb_lock) > spin_lock_irq(hugetlb_lock) > 1) update_and_free_page >PageHuge() == F >__free_pages() > 2) enqueue_huge_page >SetPageHugeFreed() > spin_unlock(_lock) > spin_lock_irq(hugetlb_lock) >1) PageHuge() == F (freed by case#1 from > CPU0) > 2) PageHuge() == T >PageHugeFreed() == T >- proceed with replacing the page > > In the case above we retry as the window race is quite small and we have high > chances to succeed next time. > > With regard to the allocation, we restrict it to the node the page belongs > to with __GFP_THISNODE, meaning we do not fallback on other node's zones. > > Note that gigantic hugetlb pages are fenced off since there is a cyclic > dependency between them and alloc_contig_range. > > Signed-off-by: Oscar Salvador Acked-by: Michal Hocko One minor nit below [...] > + /* > + * Ok, old_page is still a genuine free hugepage. Remove it from > + * the freelist and decrease the counters. These will be > + * incremented again when calling __prep_account_new_huge_page() > + * and enqueue_huge_page() for new_page. The counters will > remain > + * stable since this happens under the lock. > + */ > + remove_hugetlb_page(h, old_page, false); > + > + /* > + * Call __prep_new_huge_page() to construct the hugetlb page, > and > + * enqueue it then to place it in the freelists. After this, > + * counters are back on track. Free hugepages have a refcount > of 0, > + * so we need to decrease new_page's count as well. > + */ > + __prep_new_huge_page(new_page); > + __prep_account_new_huge_page(h, nid); I think it would help to put something like the following into the comment above this really strange construct. /* * new_page needs to be initialized with the standard * hugetlb state. This is normally done by * prep_new_huge_page but that takes hugetlb_lock which * is already held so we need to open code it here. * Reference count trick is needed because allocator * gives us
Re: [PATCH v3 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers
On Fri, Apr 09, 2021 at 10:13:29AM +0700, Quan Nguyen wrote: > Adds device tree bindings for SMPro drivers found on the Mt.Jade hardware > reference platform with Ampere's Altra Processor family. > > Signed-off-by: Quan Nguyen > --- > .../bindings/hwmon/ampere,ac01-hwmon.yaml | 28 + > .../devicetree/bindings/mfd/ampere,smpro.yaml | 105 ++ > 2 files changed, 133 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > create mode 100644 Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > new file mode 100644 > index ..fbf7ec754160 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > @@ -0,0 +1,28 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/ampere,ac01-hwmon.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Hardware monitoring driver for the Ampere Altra SMPro > + > +maintainers: > + - Quan Nguyen > + > +description: | > + This module is part of the Ampere Altra SMPro multi-function device. For > more > + details see ../mfd/ampere,smpro.yaml. > + > +properties: > + compatible: > +enum: > + - ampere,ac01-hwmon > + > + reg: > +maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > diff --git a/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > new file mode 100644 > index ..5613c420869e > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > @@ -0,0 +1,105 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/ampere,smpro.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Ampere Altra SMPro firmware driver > + > +maintainers: > + - Quan Nguyen > + > +description: | > + Ampere Altra SMPro firmware may contain different blocks like hardware > + monitoring, error monitoring and other miscellaneous features. > + > +properties: > + compatible: > +enum: > + - ampere,smpro Again, not very specific. There's only 1 version of 'smpro' h/w or firmware? Are the firmware version and features discoverable? If not, you need to be more specific (or better yet, make them discoverable). > + > + reg: > +description: > + I2C device address. > +maxItems: 1 > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > +patternProperties: > + "^hwmon(@[0-9a-f]+)?$": > +$ref: ../hwmon/ampere,ac01-hwmon.yaml > + > + "^misc(@[0-9a-f]+)?$": You don't need these child nodes in DT if there are no resources associated with them. The parent driver can instantiate all the sub-functions. > +type: object > +description: | > + This module is part of the Ampere Altra SMPro multi-function device > + to support miscellaneous features > +properties: > + compatible: > +enum: > + - ampere,ac01-misc > + reg: > +maxItems: 1 > + > +required: > + - compatible > + - reg > + > + "^errmon(@[0-9a-f]+)?$": > +type: object > +description: | > + This module is part of the Ampere Altra SMPro multi-function device > + that supports error monitoring feature. > + > +properties: > + compatible: > +enum: > + - ampere,ac01-errmon > + reg: > +maxItems: 1 > + > +required: > + - compatible > + - reg > + > +required: > + - "#address-cells" > + - "#size-cells" > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > +i2c { > +#address-cells = <1>; > +#size-cells = <0>; > + > +smpro@4f { > +compatible = "ampere,smpro"; > +reg = <0x4f>; > +#address-cells = <1>; > +#size-cells = <0>; > + > +hwmon@10 { > +compatible = "ampere,ac01-hwmon"; > +reg = <0x10>; > +}; > + > +misc@b0 { > +compatible = "ampere,ac01-misc"; > +reg = <0xb0>; > +}; > + > +errmon@80 { > +compatible = "ampere,ac01-errmon"; > +reg = <0x80>; > +}; > + > +}; > +}; > -- > 2.28.0 >
Re: [PATCH] ASoC: cs35l35: Fix an error handling path in 'cs35l35_i2c_probe()'
Le 13/04/2021 à 14:43, Mark Brown a écrit : On Sun, Apr 11, 2021 at 02:51:06PM +0200, Christophe JAILLET wrote: If 'devm_regmap_init_i2c()' fails, there is no need to goto err. We should return directly as already done by the surrounding error handling paths. These are stylistic improvements rather than bug fixes so it's probably better not to call them fixes. Ok, agreed. The error handling path is a no-op in such a case. What do you prefer: - you fix the subject? - I send a v2 with a new subject? - we leave it as-is as this patch is a no-op in the real world? So it doesn't really mater. CJ
Re: [PATCH RFC v2 1/4] virtio: fix up virtio_disable_cb
On Tue, Apr 13, 2021 at 1:47 AM Michael S. Tsirkin wrote: > > virtio_disable_cb is currently a nop for split ring with event index. > This is because it used to be always called from a callback when we know > device won't trigger more events until we update the index. However, > now that we run with interrupts enabled a lot we also poll without a > callback so that is different: disabling callbacks will help reduce the > number of spurious interrupts. The device may poll for transmit completions as a result of an interrupt from virtnet_poll_tx. As well as asynchronously to this transmit interrupt, from start_xmit or from virtnet_poll_cleantx as a result of a receive interrupt. As of napi-tx, transmit interrupts are left enabled to operate in standard napi mode. While previously they would be left disabled for most of the time, enabling only when the queue as low on descriptors. (in practice, for the at the time common case of split ring with event index, little changed, as that mode does not actually enable/disable the interrupt, but looks at the consumer index in the ring to decide whether to interrupt) Combined, this may cause the following: 1. device sends a packet and fires transmit interrupt 2. driver cleans interrupts using virtnet_poll_cleantx 3. driver handles transmit interrupt using vring_interrupt, detects that the vring is empty: !more_used(vq), and records a spurious interrupt. I don't quite follow how suppressing interrupt suppression, i.e., skipping disable_cb, helps avoid this. I'm probably missing something. Is this solving a subtly different problem from the one as I understand it? > Further, if using event index with a packed ring, and if being called > from a callback, we actually do disable interrupts which is unnecessary. > > Fix both issues by tracking whenever we get a callback. If that is > the case disabling interrupts with event index can be a nop. > If not the case disable interrupts. Note: with a split ring > there's no explicit "no interrupts" value. For now we write > a fixed value so our chance of triggering an interupt > is 1/ring size. It's probably better to write something > related to the last used index there to reduce the chance > even further. For now I'm keeping it simple. > > Signed-off-by: Michael S. Tsirkin
[PATCH v2 4/8] KVM: Introduce memslots hva tree
From: "Maciej S. Szmigiero" The current memslots implementation only allows quick binary search by gfn, quick lookup by hva is not possible - the implementation has to do a linear scan of the whole memslots array, even though the operation being performed might apply just to a single memslot. This significantly hurts performance of per-hva operations with higher memslot counts. Since hva ranges can overlap between memslots an interval tree is needed for tracking them. Signed-off-by: Maciej S. Szmigiero --- arch/arm64/kvm/Kconfig | 1 + arch/arm64/kvm/mmu.c| 10 +++--- arch/mips/kvm/Kconfig | 1 + arch/mips/kvm/mmu.c | 10 +++--- arch/powerpc/kvm/Kconfig| 1 + arch/powerpc/kvm/book3s_64_mmu_hv.c | 10 +++--- arch/powerpc/kvm/book3s_pr.c| 10 +++--- arch/s390/kvm/Kconfig | 1 + arch/x86/kvm/Kconfig| 1 + arch/x86/kvm/mmu/mmu.c | 11 +++--- arch/x86/kvm/mmu/tdp_mmu.c | 13 +--- include/linux/kvm_host.h| 8 virt/kvm/kvm_main.c | 31 + 13 files changed, 86 insertions(+), 22 deletions(-) diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig index 3964acf5451e..f075e9939a2a 100644 --- a/arch/arm64/kvm/Kconfig +++ b/arch/arm64/kvm/Kconfig @@ -40,6 +40,7 @@ menuconfig KVM select HAVE_KVM_VCPU_RUN_PID_CHANGE select TASKSTATS select TASK_DELAY_ACCT + select INTERVAL_TREE help Support hosting virtualized guest machines. diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 4b7e1e327337..4b0ac98a5a53 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -1073,21 +1073,25 @@ static int handle_hva_to_gpa(struct kvm *kvm, void *data) { struct kvm_memslots *slots; + struct interval_tree_node *node; struct kvm_memory_slot *memslot; int ret = 0; + if (end == start || WARN_ON(end < start)) + return 0; + slots = kvm_memslots(kvm); /* we only care about the pages that the guest sees */ - kvm_for_each_memslot(memslot, slots) { + kvm_for_each_hva_range_memslot(node, slots, start, end - 1) { unsigned long hva_start, hva_end; gfn_t gpa; + memslot = container_of(node, struct kvm_memory_slot, + hva_node); hva_start = max(start, memslot->userspace_addr); hva_end = min(end, memslot->userspace_addr + (memslot->npages << PAGE_SHIFT)); - if (hva_start >= hva_end) - continue; gpa = hva_to_gfn_memslot(hva_start, memslot) << PAGE_SHIFT; ret |= handler(kvm, gpa, (u64)(hva_end - hva_start), data); diff --git a/arch/mips/kvm/Kconfig b/arch/mips/kvm/Kconfig index 032b3fca6cbb..5ba260f38e75 100644 --- a/arch/mips/kvm/Kconfig +++ b/arch/mips/kvm/Kconfig @@ -27,6 +27,7 @@ config KVM select KVM_MMIO select MMU_NOTIFIER select SRCU + select INTERVAL_TREE help Support for hosting Guest kernels. diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c index 3dabeda82458..0c7c48f08ec2 100644 --- a/arch/mips/kvm/mmu.c +++ b/arch/mips/kvm/mmu.c @@ -449,21 +449,25 @@ static int handle_hva_to_gpa(struct kvm *kvm, void *data) { struct kvm_memslots *slots; + struct interval_tree_node *node; struct kvm_memory_slot *memslot; int ret = 0; + if (end == start || WARN_ON(end < start)) + return 0; + slots = kvm_memslots(kvm); /* we only care about the pages that the guest sees */ - kvm_for_each_memslot(memslot, slots) { + kvm_for_each_hva_range_memslot(node, slots, start, end - 1) { unsigned long hva_start, hva_end; gfn_t gfn, gfn_end; + memslot = container_of(node, struct kvm_memory_slot, + hva_node); hva_start = max(start, memslot->userspace_addr); hva_end = min(end, memslot->userspace_addr + (memslot->npages << PAGE_SHIFT)); - if (hva_start >= hva_end) - continue; /* * {gfn(page) | page intersects with [hva_start, hva_end)} = diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig index e45644657d49..519d6d3642a5 100644 --- a/arch/powerpc/kvm/Kconfig +++ b/arch/powerpc/kvm/Kconfig @@ -26,6 +26,7 @@ config KVM select KVM_VFIO select IRQ_BYPASS_MANAGER select HAVE_KVM_IRQ_BYPASS + select INTERVAL_TREE config KVM_BOOK3S_HANDLER bool diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
[PATCH v2 3/8] KVM: Resolve memslot ID via a hash table instead of via a static array
From: "Maciej S. Szmigiero" Memslot ID to the corresponding memslot mappings are currently kept as indices in static id_to_index array. The size of this array depends on the maximum allowed memslot count (regardless of the number of memslots actually in use). This has become especially problematic recently, when memslot count cap was removed, so the maximum count is now full 32k memslots - the maximum allowed by the current KVM API. Keeping these IDs in a hash table (instead of an array) avoids this problem. Resolving a memslot ID to the actual memslot (instead of its index) will also enable transitioning away from an array-based implementation of the whole memslots structure in a later commit. Signed-off-by: Maciej S. Szmigiero --- include/linux/kvm_host.h | 16 +-- virt/kvm/kvm_main.c | 58 ++-- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 6664aa10db93..e4aadb9875e9 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -350,6 +351,7 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu) #define KVM_MEM_MAX_NR_PAGES ((1UL << 31) - 1) struct kvm_memory_slot { + struct hlist_node id_node; gfn_t base_gfn; unsigned long npages; unsigned long *dirty_bitmap; @@ -452,7 +454,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu) struct kvm_memslots { u64 generation; /* The mapping table from slot id to the index in memslots[]. */ - short id_to_index[KVM_MEM_SLOTS_NUM]; + DECLARE_HASHTABLE(id_hash, 7); atomic_t lru_slot; int used_slots; struct kvm_memory_slot memslots[]; @@ -674,16 +676,14 @@ static inline struct kvm_memslots *kvm_vcpu_memslots(struct kvm_vcpu *vcpu) static inline struct kvm_memory_slot *id_to_memslot(struct kvm_memslots *slots, int id) { - int index = slots->id_to_index[id]; struct kvm_memory_slot *slot; - if (index < 0) - return NULL; - - slot = >memslots[index]; + hash_for_each_possible(slots->id_hash, slot, id_node, id) { + if (slot->id == id) + return slot; + } - WARN_ON(slot->id != id); - return slot; + return NULL; } /* diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 0a481e7780f0..8f0b49d937d1 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -656,15 +656,13 @@ static int kvm_init_mmu_notifier(struct kvm *kvm) static struct kvm_memslots *kvm_alloc_memslots(void) { - int i; struct kvm_memslots *slots; slots = kvzalloc(sizeof(struct kvm_memslots), GFP_KERNEL_ACCOUNT); if (!slots) return NULL; - for (i = 0; i < KVM_MEM_SLOTS_NUM; i++) - slots->id_to_index[i] = -1; + hash_init(slots->id_hash); return slots; } @@ -972,14 +970,16 @@ static int kvm_alloc_dirty_bitmap(struct kvm_memory_slot *memslot) /* * Delete a memslot by decrementing the number of used slots and shifting all * other entries in the array forward one spot. + * @memslot is a detached dummy struct with just .id and .as_id filled. */ static inline void kvm_memslot_delete(struct kvm_memslots *slots, struct kvm_memory_slot *memslot) { struct kvm_memory_slot *mslots = slots->memslots; + struct kvm_memory_slot *dmemslot = id_to_memslot(slots, memslot->id); int i; - if (WARN_ON(slots->id_to_index[memslot->id] == -1)) + if (WARN_ON(!dmemslot)) return; slots->used_slots--; @@ -987,12 +987,13 @@ static inline void kvm_memslot_delete(struct kvm_memslots *slots, if (atomic_read(>lru_slot) >= slots->used_slots) atomic_set(>lru_slot, 0); - for (i = slots->id_to_index[memslot->id]; i < slots->used_slots; i++) { + for (i = dmemslot - mslots; i < slots->used_slots; i++) { + hash_del([i].id_node); mslots[i] = mslots[i + 1]; - slots->id_to_index[mslots[i].id] = i; + hash_add(slots->id_hash, [i].id_node, mslots[i].id); } + hash_del([i].id_node); mslots[i] = *memslot; - slots->id_to_index[memslot->id] = -1; } /* @@ -1010,31 +1011,41 @@ static inline int kvm_memslot_insert_back(struct kvm_memslots *slots) * itself is not preserved in the array, i.e. not swapped at this time, only * its new index into the array is tracked. Returns the changed memslot's * current index into the memslots array. + * The memslot at the returned index will not be in @slots->id_hash by then. + * @memslot is a detached struct with desired final data of the changed slot. */ static inline int kvm_memslot_move_backward(struct kvm_memslots *slots,
[PATCH v2 5/8] KVM: s390: Introduce kvm_s390_get_gfn_end()
From: "Maciej S. Szmigiero" And use it where s390 code would just access the memslot with the highest gfn directly. No functional change intended. Signed-off-by: Maciej S. Szmigiero --- arch/s390/kvm/kvm-s390.c | 2 +- arch/s390/kvm/kvm-s390.h | 12 arch/s390/kvm/pv.c | 4 +--- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 62491ec003d8..4445c99eb00e 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1977,7 +1977,7 @@ static int kvm_s390_get_cmma(struct kvm *kvm, struct kvm_s390_cmma_log *args, if (!ms) return 0; next_gfn = kvm_s390_next_dirty_cmma(slots, cur_gfn + 1); - mem_end = slots->memslots[0].base_gfn + slots->memslots[0].npages; + mem_end = kvm_s390_get_gfn_end(slots); while (args->count < bufsize) { hva = gfn_to_hva(kvm, cur_gfn); diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h index 79dcd647b378..96537370ce8c 100644 --- a/arch/s390/kvm/kvm-s390.h +++ b/arch/s390/kvm/kvm-s390.h @@ -208,6 +208,18 @@ static inline int kvm_s390_user_cpu_state_ctrl(struct kvm *kvm) return kvm->arch.user_cpu_state_ctrl != 0; } +/* get the end gfn of the last (highest gfn) memslot */ +static inline unsigned long kvm_s390_get_gfn_end(struct kvm_memslots *slots) +{ + struct kvm_memory_slot *ms; + + if (WARN_ON(!slots->used_slots)) + return 0; + + ms = slots->memslots; + return ms->base_gfn + ms->npages; +} + /* implemented in pv.c */ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc); int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc); diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index 813b6e93dc83..6bf42cdf4013 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -117,7 +117,6 @@ static int kvm_s390_pv_alloc_vm(struct kvm *kvm) unsigned long base = uv_info.guest_base_stor_len; unsigned long virt = uv_info.guest_virt_var_stor_len; unsigned long npages = 0, vlen = 0; - struct kvm_memory_slot *memslot; kvm->arch.pv.stor_var = NULL; kvm->arch.pv.stor_base = __get_free_pages(GFP_KERNEL_ACCOUNT, get_order(base)); @@ -131,8 +130,7 @@ static int kvm_s390_pv_alloc_vm(struct kvm *kvm) * Slots are sorted by GFN */ mutex_lock(>slots_lock); - memslot = kvm_memslots(kvm)->memslots; - npages = memslot->base_gfn + memslot->npages; + npages = kvm_s390_get_gfn_end(kvm_memslots(kvm)); mutex_unlock(>slots_lock); kvm->arch.pv.guest_len = npages * PAGE_SIZE;
Re: [PATCH 3/3] rseq: optimise for 64bit arches
- On Apr 13, 2021, at 6:36 AM, David Laight david.lai...@aculab.com wrote: > From: Peter Zijlstra >> Sent: 13 April 2021 10:10 >> >> On Tue, Apr 13, 2021 at 12:36:57AM -0700, Eric Dumazet wrote: >> > From: Eric Dumazet >> > >> > Commit ec9c82e03a74 ("rseq: uapi: Declare rseq_cs field as union, >> > update includes") added regressions for our servers. >> > >> > Using copy_from_user() and clear_user() for 64bit values >> > on 64bit arches is suboptimal. >> > >> > We might revisit this patch once all 32bit arches support >> > get_user() and/or put_user() for 8 bytes values. >> >> Argh, what a mess :/ afaict only nios32 lacks put_user_8, but get_user_8 >> is missing in a fair number of archs. >> >> That said; 32bit archs never have to actually set the top bits in that >> word, so they _could_ only set the low 32 bits. That works provided >> userspace itself keeps the high bits clear. > > Does that work for 32bit BE ? Yes, because uapi/linux/rseq.h defines the ptr32 as: #if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined(__BIG_ENDIAN) __u32 padding; /* Initialized to zero. */ __u32 ptr32; #else /* LITTLE */ __u32 ptr32; __u32 padding; /* Initialized to zero. */ #endif /* ENDIAN */ which takes care of BE vs LE. > > David > >> So I suppose that if we're going to #ifdef this, we might as well do the >> whole thing. >> >> Mathieu; did I forget a reason why this cannot work? The only difference it brings on 32-bit is that the truncation of high bits will be done before the following validation: if (!ptr) { memset(rseq_cs, 0, sizeof(*rseq_cs)); return 0; } if (ptr >= TASK_SIZE) return -EINVAL; The question is whether we really want to issue a segmentation fault if 32-bit user-space has set non-zero high bits, or if silently ignoring those high bits is acceptable. Nits below: >> >> diff --git a/kernel/rseq.c b/kernel/rseq.c >> index a4f86a9d6937..94006190b8eb 100644 >> --- a/kernel/rseq.c >> +++ b/kernel/rseq.c >> @@ -115,20 +115,25 @@ static int rseq_reset_rseq_cpu_id(struct task_struct >> *t) >> static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs) >> { >> struct rseq_cs __user *urseq_cs; >> -u64 ptr; >> +unsigned long ptr; I am always reluctant to use long/unsigned long type as type for the get/put_user (x) argument, because it hides the cast deep within architecture-specific macros. I understand that in this specific case it happens that on 64-bit archs we end up casting a u64 to unsigned long (same size), and on 32-bit archs we end up casting a u32 to unsigned long (also same size), so there is no practical concern about type promotion and sign-extension, but I think it would be better to have something explicit, e.g.: #ifdef CONFIG_64BIT static int rseq_get_cs_ptr(struct rseq_cs __user **uptrp, struct rseq_cs *rseq_cs) { u64 ptr; if (get_user(ptr, _cs->ptr64)) return -EFAULT; *ptrp = (struct rseq_cs __user *)ptr; return 0; } #else static int rseq_get_cs_ptr(struct rseq_cs __user **uptrp, struct rseq_cs *rseq_cs) { u32 ptr; if (get_user(ptr, _cs->ptr.ptr32)) return -EFAULT; *ptrp = (struct rseq_cs __user *)ptr; return 0; } #endif And use those helpers to get the ptr value. >> u32 __user *usig; >> u32 sig; >> int ret; >> >> -if (copy_from_user(, >rseq->rseq_cs.ptr64, sizeof(ptr))) >> +#ifdef CONFIG_64BIT >> +if (get_user(ptr, >rseq->rseq_cs.ptr64)) >> return -EFAULT; >> +#else >> +if (get_user(ptr, >rseq->rseq_cs.ptr32)) Note that this is also not right. It should be >rseq->rseq_cs.ptr.ptr32. Thanks, Mathieu >> +return -EFAULT; >> +#endif >> if (!ptr) { >> memset(rseq_cs, 0, sizeof(*rseq_cs)); >> return 0; >> } >> if (ptr >= TASK_SIZE) >> return -EINVAL; >> -urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr; >> +urseq_cs = (struct rseq_cs __user *)ptr; >> if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs))) >> return -EFAULT; >> > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, > UK > Registration No: 1397386 (Wales) -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com