Re: [PATCH v4 2/2] usb: phy: Add USB3 PHY support for Intel LGM SoC
Hi, On 10/7/2020 2:08 pm, Felipe Balbi wrote: Hi, "Ramuthevar,Vadivel MuruganX" writes: From: Ramuthevar Vadivel Murugan Add support for USB PHY on Intel LGM SoC. Signed-off-by: Ramuthevar Vadivel Murugan --- drivers/usb/phy/Kconfig | 11 ++ drivers/usb/phy/Makefile | 1 + drivers/usb/phy/phy-lgm-usb.c | 275 ++ new phy drivers should use drivers/phy instead. Noted, Will move to drivers/phy as per your suggestion, Thanks! Regards Vadivel
Re: [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries
On Fri, Jul 10, 2020 at 11:28 PM Song Liu wrote: > > > > > On Jul 10, 2020, at 8:53 PM, Andrii Nakryiko > > wrote: > > > > On Fri, Jul 10, 2020 at 6:30 PM Song Liu wrote: > >> > >> Calling get_perf_callchain() on perf_events from PEBS entries may cause > >> unwinder errors. To fix this issue, the callchain is fetched early. Such > >> perf_events are marked with __PERF_SAMPLE_CALLCHAIN_EARLY. > >> > >> Similarly, calling bpf_get_[stack|stackid] on perf_events from PEBS may > >> also cause unwinder errors. To fix this, block bpf_get_[stack|stackid] on > >> these perf_events. Unfortunately, bpf verifier cannot tell whether the > >> program will be attached to perf_event with PEBS entries. Therefore, > >> block such programs during ioctl(PERF_EVENT_IOC_SET_BPF). > >> > >> Signed-off-by: Song Liu > >> --- > > > > Perhaps it's a stupid question, but why bpf_get_stack/bpf_get_stackid > > can't figure out automatically that they are called from > > __PERF_SAMPLE_CALLCHAIN_EARLY perf event and use different callchain, > > if necessary? > > > > It is quite suboptimal from a user experience point of view to require > > two different BPF helpers depending on PEBS or non-PEBS perf events. > > I am not aware of an easy way to tell the difference in bpf_get_stack. > But I do agree that would be much better. > Hm... Looking a bit more how all this is tied together in the kernel, I think it's actually quite easy. So, for perf_event BPF program type: 1. return a special prototype for bpf_get_stack/bpf_get_stackid, which will have this extra bit of logic for callchain. All other program types with access to bpf_get_stack/bpf_get_stackid should use the current one, probably. 2. For that special program, just like for bpf_read_branch_records(), we know that context is actually `struct bpf_perf_event_data_kern *`, and it has pt_regs, perf_sample_data and perf_event itself. 3. With that, it seems like you'll have everything you need to automatically choose a proper callchain. All this absolutely transparently to the BPF program. Am I missing something? > Thanks, > Song
[PATCH v3 0/2] Syscall user redirection
Hi, This is the v3 of the syscall user redirection patch, applying the suggestions from Matthew and Kees. In particular, it modifies the ABI to allow passing a range of allowed addresses and introduces kselftests for the feature. RFC/v1: https://lkml.org/lkml/2020/7/8/96 v2: https://lkml.org/lkml/2020/7/9/17 Gabriel Krisman Bertazi (2): kernel: Implement selective syscall userspace redirection selftests: Add kselftest for syscall user dispatch arch/Kconfig | 20 ++ arch/x86/Kconfig | 1 + arch/x86/entry/common.c | 5 + arch/x86/include/asm/thread_info.h| 4 +- arch/x86/kernel/signal_compat.c | 2 +- fs/exec.c | 2 + include/linux/sched.h | 3 + include/linux/syscall_user_dispatch.h | 50 include/uapi/asm-generic/siginfo.h| 3 +- include/uapi/linux/prctl.h| 5 + kernel/Makefile | 1 + kernel/fork.c | 1 + kernel/sys.c | 5 + kernel/syscall_user_dispatch.c| 92 +++ tools/testing/selftests/Makefile | 1 + .../syscall_user_dispatch/.gitignore | 1 + .../selftests/syscall_user_dispatch/Makefile | 5 + .../selftests/syscall_user_dispatch/config| 1 + .../syscall_user_dispatch.c | 259 ++ 19 files changed, 458 insertions(+), 3 deletions(-) create mode 100644 include/linux/syscall_user_dispatch.h create mode 100644 kernel/syscall_user_dispatch.c create mode 100644 tools/testing/selftests/syscall_user_dispatch/.gitignore create mode 100644 tools/testing/selftests/syscall_user_dispatch/Makefile create mode 100644 tools/testing/selftests/syscall_user_dispatch/config create mode 100644 tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c -- 2.27.0
[PATCH v3 2/2] selftests: Add kselftest for syscall user dispatch
Implement functionality tests for syscall user dispatch. In order to make the test portable, refrain from open coding syscall dispatchers and calculating glibc memory ranges. Signed-off-by: Gabriel Krisman Bertazi --- tools/testing/selftests/Makefile | 1 + .../syscall_user_dispatch/.gitignore | 1 + .../selftests/syscall_user_dispatch/Makefile | 5 + .../selftests/syscall_user_dispatch/config| 1 + .../syscall_user_dispatch.c | 259 ++ 5 files changed, 267 insertions(+) create mode 100644 tools/testing/selftests/syscall_user_dispatch/.gitignore create mode 100644 tools/testing/selftests/syscall_user_dispatch/Makefile create mode 100644 tools/testing/selftests/syscall_user_dispatch/config create mode 100644 tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 1195bd85af38..31b07dd774a6 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -47,6 +47,7 @@ TARGETS += openat2 TARGETS += rseq TARGETS += rtc TARGETS += seccomp +TARGETS += syscall_user_dispatch TARGETS += sigaltstack TARGETS += size TARGETS += sparc64 diff --git a/tools/testing/selftests/syscall_user_dispatch/.gitignore b/tools/testing/selftests/syscall_user_dispatch/.gitignore new file mode 100644 index ..fadfb304c539 --- /dev/null +++ b/tools/testing/selftests/syscall_user_dispatch/.gitignore @@ -0,0 +1 @@ +syscall_user_dispatch diff --git a/tools/testing/selftests/syscall_user_dispatch/Makefile b/tools/testing/selftests/syscall_user_dispatch/Makefile new file mode 100644 index ..4785c98d4714 --- /dev/null +++ b/tools/testing/selftests/syscall_user_dispatch/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +CFLAGS += -Wall + +TEST_GEN_PROGS := syscall_user_dispatch +include ../lib.mk diff --git a/tools/testing/selftests/syscall_user_dispatch/config b/tools/testing/selftests/syscall_user_dispatch/config new file mode 100644 index ..22c4dfe167ca --- /dev/null +++ b/tools/testing/selftests/syscall_user_dispatch/config @@ -0,0 +1 @@ +CONFIG_SYSCALL_USER_DISPATCH=y diff --git a/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c b/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c new file mode 100644 index ..d713147863ef --- /dev/null +++ b/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c @@ -0,0 +1,259 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020 Collabora Ltd. + * + * Test code for syscall user dispatch + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include + +#include "../kselftest_harness.h" + +#ifndef PR_SET_SYSCALL_USER_DISPATCH +# define PR_SET_SYSCALL_USER_DISPATCH 59 +#endif + +#ifndef PR_SYS_DISPATCH_OFF +# define PR_SYS_DISPATCH_OFF 0 +#endif + +#ifndef PR_SYS_DISPATCH_ON +# define PR_SYS_DISPATCH_ON1 +#endif + +#ifndef SYS_USER_DISPATCH +# define SYS_USER_DISPATCH 2 +#endif + +#define SYSCALL_DISPATCH_ON(x) ((x) = 1) +#define SYSCALL_DISPATCH_OFF(x) ((x) = 0) + +/* Test Summary: + * + * - dispatch_trigger_sigsys: Verify if PR_SET_SYSCALL_USER_DISPATCH is + * able to trigger SIGSYS on a syscall. + * + * - bad_selector: Test that a bad selector value triggers SIGSEGV. + * + * - bad_prctl_param: Test that the API correctly rejects invalid + * parameters on prctl + * + * - dispatch_and_return: Test that a syscall is selectively dispatched + * to userspace depending on the value of selector. + * + * - disable_dispatch: Test that the PR_SYS_DISPATCH_OFF correctly + * disables the dispatcher + * + * - direct_dispatch_range: Test that a syscall within the allowed range + * can bypass the dispatcher. + */ + +TEST_SIGNAL(dispatch_trigger_sigsys, SIGSYS) +{ + char sel = 0; + struct sysinfo info; + int ret; + + ret = sysinfo(); + ASSERT_EQ(0, ret); + + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, ); + ASSERT_EQ(0, ret) { + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH"); + } + + SYSCALL_DISPATCH_ON(sel); + + sysinfo(); + + EXPECT_FALSE(true) { + TH_LOG("Unreachable!"); + } +} + +TEST_SIGNAL(bad_selector, SIGSEGV) +{ + char sel = -1; + long ret; + + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, ); + ASSERT_EQ(0, ret) { + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH"); + } + EXPECT_FALSE(true) { + TH_LOG("Unreachable!"); + } +} + +TEST(bad_prctl_param) +{ + char sel = 0; + int op; + + /* Invalid op */ + op = -1; + prctl(PR_SET_SYSCALL_USER_DISPATCH, op, 0, 0, ); + ASSERT_EQ(EINVAL, errno); + + /* PR_SYS_DISPATCH_OFF */ + op =
[PATCH v3 1/2] kernel: Implement selective syscall userspace redirection
Introduce a mechanism to quickly disable/enable syscall handling for a specific process and redirect to userspace via SIGSYS. This is useful for processes with parts that require syscall redirection and parts that don't, but who need to perform this boundary crossing really fast, without paying the cost of a system call to reconfigure syscall handling on each boundary transition. This is particularly important for Windows games running over Wine. The proposed interface looks like this: prctl(PR_SET_SYSCALL_USER_DISPATCH, , , , [selector]) The range [,] is a part of the process memory map that is allowed to by-pass the redirection code and dispatch syscalls directly, such that in fast paths a process doesn't need to disable the trap nor the kernel has to check the selector. This is essential to return from SIGSYS to a blocked area without triggering another SIGSYS from rt_sigreturn. selector is an optional pointer to a char-sized userspace memory region that has a key switch for the mechanism. This key switch is set to either PR_SYS_DISPATCH_ON, PR_SYS_DISPATCH_OFF to enable and disable the redirection without calling the kernel. The feature is meant to be set per-thread and it is disabled on fork/clone/execv. Internally, this doesn't add overhead to the syscall hot path, and it requires very little per-architecture support. I avoided using seccomp, even though it duplicates some functionality, due to previous feedback that maybe it shouldn't mix with seccomp since it is not a security mechanism. And obviously, this should never be considered a security mechanism, since any part of the program can by-pass it by using the syscall dispatcher. For the sysinfo benchmark, which measures the overhead added to executing a native syscall that doesn't require interception, the overhead using only the direct dispatcher region to issue syscalls is pretty much irrelevant. The overhead of using the selector goes around 40ns for a native (unredirected) syscall in my system, and it is (as expected) dominated by the supervisor-mode user-address access. In fact, with SMAP off, the overhead is consistently less than 5ns on my test box. Right now, it is only supported by x86_64 and x86, but it should be easily enabled for other architectures. An example code using this interface can be found at: https://gitlab.collabora.com/krisman/syscall-disable-personality Changes since v2: (Matthew Wilcox suggestions) - Drop __user on non-ptr type. - Move #define closer to similar defs - Allow a memory region that can dispatch directly (Kees Cook suggestions) - Improve kconfig summary line - Move flag cleanup on execve to begin_new_exec - Hint branch predictor in the syscall path (Me) - Convert selector to char Changes since RFC: (Kees Cook suggestions) - Don't mention personality while explaining the feature - Use syscall_get_nr - Remove header guard on several places - Convert WARN_ON to WARN_ON_ONCE - Explicit check for state values - Rename to syscall user dispatcher Cc: Matthew Wilcox Cc: Andy Lutomirski Cc: Paul Gofman Cc: Kees Cook Signed-off-by: Gabriel Krisman Bertazi --- arch/Kconfig | 20 ++ arch/x86/Kconfig | 1 + arch/x86/entry/common.c | 5 ++ arch/x86/include/asm/thread_info.h| 4 +- arch/x86/kernel/signal_compat.c | 2 +- fs/exec.c | 2 + include/linux/sched.h | 3 + include/linux/syscall_user_dispatch.h | 50 +++ include/uapi/asm-generic/siginfo.h| 3 +- include/uapi/linux/prctl.h| 5 ++ kernel/Makefile | 1 + kernel/fork.c | 1 + kernel/sys.c | 5 ++ kernel/syscall_user_dispatch.c| 92 +++ 14 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 include/linux/syscall_user_dispatch.h create mode 100644 kernel/syscall_user_dispatch.c diff --git a/arch/Kconfig b/arch/Kconfig index 8cc35dc556c7..0ebd971d0d8f 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -465,6 +465,26 @@ config SECCOMP_FILTER See Documentation/userspace-api/seccomp_filter.rst for details. +config HAVE_ARCH_SYSCALL_USER_DISPATCH + bool + help + An arch should select this symbol if it provides all of these things: + - TIF_SYSCALL_USER_DISPATCH + - syscall_get_arch + - syscall_rollback + - syscall_get_nr + - SIGSYS siginfo_t support + +config SYSCALL_USER_DISPATCH + bool "Support syscall redirection to userspace dispatcher" + depends on HAVE_ARCH_SYSCALL_USER_DISPATCH + help + Enable tasks to ask the kernel to redirect syscalls not + issued from a predefined dispatcher back to userspace, + depending on a userspace memory selector. + + This option is useful to optimize games running over Wine. + config
[PATCH v6 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml
Add documentation for the newly added DTS support in the shtc1 driver. To align with the drivers logic to have high precision by default a boolean sensirion,low_precision is used to switch to low precision. Signed-off-by: Chris Ruehl --- .../bindings/hwmon/sensirion,shtc1.yaml | 57 +++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml diff --git a/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml new file mode 100644 index ..752fd32eed25 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/sensirion,shtc1.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sensirion SHTC1 Humidity and Temperature Sensor IC + +maintainers: + - chris.ru...@gtsys.com.hk + +description: | + The SHTC1, SHTW1 and SHTC3 are digital humidity and temperature sensor + designed especially for battery-driven high-volume consumer electronics + applications. + For further information refere to Documentation/hwmon/shtc1.rst + + This binding document describes the binding for the hardware monitor + portion of the driver. + +properties: + compatible: +enum: + - sensirion,shtc1 + - sensirion,shtw1 + - sensirion,shtc3 + + reg: +const: 0x70 + + sensirion,blocking-io: +$ref: /schemas/types.yaml#definitions/flag +description: + If set, the driver hold the i2c bus until measurement is finished. + + sensirion,low-precision: +$ref: /schemas/types.yaml#definitions/flag +description: + If set, the sensor aquire data with low precision (not recommended). + The driver aquire data with high precision by default. + +required: + - compatible + - reg + +examples: + - | +i2c1 { + clock-frequency = <40>; + + shtc3@70 { +compatible = "sensirion,shtc3"; +reg = <0x70>; +sensirion,blocking-io; + }; +}; +... -- 2.20.1
[PATCH v6 1/2] hwmon: shtc1: add support for device tree bindings
Add support for DTS bindings for the sensirion shtc1,shtw1 and shtc3. Signed-off-by: Chris Ruehl Reviewed-by: Guenter Roeck --- drivers/hwmon/shtc1.c | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/shtc1.c b/drivers/hwmon/shtc1.c index a0078ccede03..7993a5ff8768 100644 --- a/drivers/hwmon/shtc1.c +++ b/drivers/hwmon/shtc1.c @@ -14,6 +14,7 @@ #include #include #include +#include /* commands (high precision mode) */ static const unsigned char shtc1_cmd_measure_blocking_hpm[]= { 0x7C, 0xA2 }; @@ -196,6 +197,7 @@ static int shtc1_probe(struct i2c_client *client, enum shtcx_chips chip = id->driver_data; struct i2c_adapter *adap = client->adapter; struct device *dev = >dev; + struct device_node *np = dev->of_node; if (!i2c_check_functionality(adap, I2C_FUNC_I2C)) { dev_err(dev, "plain i2c transactions not supported\n"); @@ -233,8 +235,14 @@ static int shtc1_probe(struct i2c_client *client, data->client = client; data->chip = chip; - if (client->dev.platform_data) - data->setup = *(struct shtc1_platform_data *)dev->platform_data; + if (np) { + data->setup.blocking_io = of_property_read_bool(np, "sensirion,blocking-io"); + data->setup.high_precision = !of_property_read_bool(np, "sensicon,low-precision"); + } else { + if (client->dev.platform_data) + data->setup = *(struct shtc1_platform_data *)dev->platform_data; + } + shtc1_select_command(data); mutex_init(>update_lock); @@ -257,8 +265,19 @@ static const struct i2c_device_id shtc1_id[] = { }; MODULE_DEVICE_TABLE(i2c, shtc1_id); +static const struct of_device_id shtc1_of_match[] = { + { .compatible = "sensirion,shtc1" }, + { .compatible = "sensirion,shtw1" }, + { .compatible = "sensirion,shtc3" }, + { } +}; +MODULE_DEVICE_TABLE(of, shtc1_of_match); + static struct i2c_driver shtc1_i2c_driver = { - .driver.name = "shtc1", + .driver = { + .name = "shtc1", + .of_match_table = shtc1_of_match, + }, .probe= shtc1_probe, .id_table = shtc1_id, }; -- 2.20.1
[PATCH v6 0/2] shtc1: add support for device tree bindings
Add support for DTS bindings to the shtc driver The patches add the compatible table and of_property_read_bool to the shtc1.c. Newly created Yaml document has been released to the Documentation/devicetree/hwmon/sensirion,shtc1.yaml Signed-off-by: Chris Ruehl --- Version 6 fix dt_binding_check: missing ';' in examples Version 5 devicetree/driver-source: name conversion sensirion,low-precision sensirion,blocking-io use const: 0x70 with the reg: Version 4 Fix errors report by make dt_binding_check (devicetree) Set maintainers for the yaml document to my own. Version 3 Fix errors report with checkpatch.pl Correct logic, add (!) when check for sensirion,low_precision Version 2 remove the #ifdef CONFIG_OF ignore platform data if dev->of_node is valid use boolean only therefor use sensirion,low_precise to fit the logic add missing driver.of_match_table entry Version 1 initial version
arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires more registers than available
Hi Jason, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 0aea6d5c5be33ce94c16f9ab2f64de1f481f424b commit: 07b586fe06625b0b610dc3d3a969c51913d143d4 crypto: x86/curve25519 - replace with formally verified implementation date: 5 months ago config: x86_64-randconfig-a003-20200712 (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8) 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 x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git checkout 07b586fe06625b0b610dc3d3a969c51913d143d4 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 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 >>): >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available " movq 0(%1), %%rdx;" /* f[0] */ ^ >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available >> arch/x86/crypto/curve25519-x86_64.c:518:3: error: inline assembly requires >> more registers than available 9 errors generated. vim +518 arch/x86/crypto/curve25519-x86_64.c 509 510 /* Computes the square of a field element: out <- f * f 511 * Uses the 8-element buffer tmp for intermediate results */ 512 static inline void fsqr(u64 *out, const u64 *f, u64 *tmp) 513 { 514 asm volatile( 515 /* Compute the raw multiplication: tmp <- f * f */ 516 517 /* Step 1: Compute all partial products */ > 518 " movq 0(%1), %%rdx;" > /* f[0] */ 519 " mulxq 8(%1), %%r8, %%r14;" " xor %%r15, %%r15;" /* f[1]*f[0] */ 520 " mulxq 16(%1), %%r9, %%r10;" " adcx %%r14, %%r9;" /* f[2]*f[0] */ 521 " mulxq 24(%1), %%rax, %%rcx;"" adcx %%rax, %%r10;"/* f[3]*f[0] */ 522 " movq 24(%1), %%rdx;" /* f[3] */ 523 " mulxq 8(%1), %%r11, %%r12;" " adcx %%rcx, %%r11;"/* f[1]*f[3] */ 524 " mulxq 16(%1), %%rax, %%r13;"" adcx %%rax, %%r12;"/* f[2]*f[3] */ 525 " movq 8(%1), %%rdx;" " adcx %%r15, %%r13;"/* f1 */ 526 " mulxq 16(%1), %%rax, %%rcx;"" mov $0, %%r14;" /* f[2]*f[1] */ 527 528 /* Step 2: Compute two parallel carry chains */ 529 " xor %%r15, %%r15;" 530 " adox %%rax, %%r10;" 531 " adcx %%r8, %%r8;" 532 " adox %%rcx, %%r11;" 533 " adcx %%r9, %%r9;" 534 " adox %%r15, %%r12;" 535 " adcx %%r10, %%r10;" 536 " adox %%r15, %%r13;" 537 " adcx %%r11, %%r11;" 538 " adox %%r15, %%r14;" 539 " adcx %%r12, %%r12;" 540 " adcx %%r13, %%r13;" 541 " adcx %%r14, %%r14;" 542 543 /* Step 3: Compute intermediate squares */ 544 " movq 0(%1), %%rdx;" " mulx %%rdx, %%rax, %%rcx;"/* f[0]^2 */ 545 " movq %%rax, 0(%0);" 546 " add %%rcx, %%r8;" " movq %%r8, 8(%0);" 547 " movq 8(%1), %%rdx;" " mulx %%rdx, %%rax, %%rcx;"/* f[1]^2 */ 548 " adcx %%rax, %%r9;" " movq %%r9, 16(%0);" 549 " adcx %%rcx, %%r10;" " movq %%r10, 24(%0);" 550 " movq 16(%1), %%rdx;"" mulx %%rdx, %%rax, %%rcx;"/* f[2]^2 */ 551 " adcx
[PATCH v3 3/3] xen/privcmd: Convert get_user_pages*() to pin_user_pages*()
In 2019, we introduced pin_user_pages*() and now we are converting get_user_pages*() to the new API as appropriate. [1] & [2] could be referred for more information. This is case 5 as per document [1]. [1] Documentation/core-api/pin_user_pages.rst [2] "Explicit pinning of user-space pages": https://lwn.net/Articles/807108/ Signed-off-by: Souptick Joarder Reviewed-by: Juergen Gross Cc: John Hubbard Cc: Boris Ostrovsky Cc: Paul Durrant --- drivers/xen/privcmd.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 079d35b..63abe6c 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -593,7 +593,7 @@ static int lock_pages( if (requested > nr_pages) return -ENOSPC; - page_count = get_user_pages_fast( + page_count = pin_user_pages_fast( (unsigned long) kbufs[i].uptr, requested, FOLL_WRITE, pages); if (page_count < 0) @@ -609,13 +609,7 @@ static int lock_pages( static void unlock_pages(struct page *pages[], unsigned int nr_pages) { - unsigned int i; - - for (i = 0; i < nr_pages; i++) { - if (!PageDirty(pages[i])) - set_page_dirty_lock(pages[i]); - put_page(pages[i]); - } + unpin_user_pages_dirty_lock(pages, nr_pages, true); } static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) -- 1.9.1
[PATCH v3 2/3] xen/privcmd: Mark pages as dirty
pages need to be marked as dirty before unpinned it in unlock_pages() which was oversight. This is fixed now. Signed-off-by: Souptick Joarder Suggested-by: John Hubbard Reviewed-by: Juergen Gross Cc: John Hubbard Cc: Boris Ostrovsky Cc: Paul Durrant --- drivers/xen/privcmd.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index b001673..079d35b 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -611,8 +611,11 @@ static void unlock_pages(struct page *pages[], unsigned int nr_pages) { unsigned int i; - for (i = 0; i < nr_pages; i++) + for (i = 0; i < nr_pages; i++) { + if (!PageDirty(pages[i])) + set_page_dirty_lock(pages[i]); put_page(pages[i]); + } } static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) -- 1.9.1
[PATCH v3 0/3] Few bug fixes and Convert to pin_user_pages*()
This series contains few clean up, minor bug fixes and Convert get_user_pages() to pin_user_pages(). I'm compile tested this, but unable to run-time test, so any testing help is much appriciated. v2: Addressed few review comments and compile issue. Patch[1/2] from v1 split into 2 in v2. v3: Address review comment. Add review tag. Cc: John Hubbard Cc: Boris Ostrovsky Cc: Paul Durrant Souptick Joarder (3): xen/privcmd: Corrected error handling path xen/privcmd: Mark pages as dirty xen/privcmd: Convert get_user_pages*() to pin_user_pages*() drivers/xen/privcmd.c | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) -- 1.9.1
[PATCH v3 1/3] xen/privcmd: Corrected error handling path
Previously, if lock_pages() end up partially mapping pages, it used to return -ERRNO due to which unlock_pages() have to go through each pages[i] till *nr_pages* to validate them. This can be avoided by passing correct number of partially mapped pages & -ERRNO separately, while returning from lock_pages() due to error. With this fix unlock_pages() doesn't need to validate pages[i] till *nr_pages* for error scenario and few condition checks can be ignored. Signed-off-by: Souptick Joarder Reviewed-by: Juergen Gross Cc: John Hubbard Cc: Boris Ostrovsky Cc: Paul Durrant --- drivers/xen/privcmd.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 5dfc59f..b001673 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -579,13 +579,13 @@ static long privcmd_ioctl_mmap_batch( static int lock_pages( struct privcmd_dm_op_buf kbufs[], unsigned int num, - struct page *pages[], unsigned int nr_pages) + struct page *pages[], unsigned int nr_pages, unsigned int *pinned) { unsigned int i; for (i = 0; i < num; i++) { unsigned int requested; - int pinned; + int page_count; requested = DIV_ROUND_UP( offset_in_page(kbufs[i].uptr) + kbufs[i].size, @@ -593,14 +593,15 @@ static int lock_pages( if (requested > nr_pages) return -ENOSPC; - pinned = get_user_pages_fast( + page_count = get_user_pages_fast( (unsigned long) kbufs[i].uptr, requested, FOLL_WRITE, pages); - if (pinned < 0) - return pinned; + if (page_count < 0) + return page_count; - nr_pages -= pinned; - pages += pinned; + *pinned += page_count; + nr_pages -= page_count; + pages += page_count; } return 0; @@ -610,13 +611,8 @@ static void unlock_pages(struct page *pages[], unsigned int nr_pages) { unsigned int i; - if (!pages) - return; - - for (i = 0; i < nr_pages; i++) { - if (pages[i]) - put_page(pages[i]); - } + for (i = 0; i < nr_pages; i++) + put_page(pages[i]); } static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) @@ -629,6 +625,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) struct xen_dm_op_buf *xbufs = NULL; unsigned int i; long rc; + unsigned int pinned = 0; if (copy_from_user(, udata, sizeof(kdata))) return -EFAULT; @@ -682,9 +679,11 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) goto out; } - rc = lock_pages(kbufs, kdata.num, pages, nr_pages); - if (rc) + rc = lock_pages(kbufs, kdata.num, pages, nr_pages, ); + if (rc < 0) { + nr_pages = pinned; goto out; + } for (i = 0; i < kdata.num; i++) { set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr); -- 1.9.1
Re: [RFC PATCH v3] scsi: ufs: Quiesce all scsi devices before shutdown
On 2020-07-06 06:22, Stanley Chu wrote: > +static void ufshcd_cleanup_queue(struct scsi_device *sdev, void *data) > +{ > + if (sdev->request_queue) > + blk_cleanup_queue(sdev->request_queue); > +} No SCSI LLD should ever call blk_cleanup_queue() directly for sdev->request_queue. Only the SCSI core should call blk_cleanup_queue() directly for that queue. > int ufshcd_shutdown(struct ufs_hba *hba) > { > int ret = 0; > + struct scsi_target *starget; > > if (!hba->is_powered) > goto out; > @@ -8612,7 +8632,25 @@ int ufshcd_shutdown(struct ufs_hba *hba) > goto out; > } > > + /* > + * Quiesce all SCSI devices to prevent any non-PM requests sending > + * from block layer during and after shutdown. > + * > + * Here we can not use blk_cleanup_queue() since PM requests > + * (with BLK_MQ_REQ_PREEMPT flag) are still required to be sent > + * through block layer. Therefore SCSI command queued after the > + * scsi_target_quiesce() call returned will block until > + * blk_cleanup_queue() is called. > + * > + * Besides, scsi_target_"un"quiesce (e.g., scsi_target_resume) can > + * be ignored since shutdown is one-way flow. > + */ > + ufshcd_scsi_for_each_sdev(ufshcd_quiece_sdev); > + > ret = ufshcd_suspend(hba, UFS_SHUTDOWN_PM); > + > + /* Set queue as dying to not block queueing commands */ > + ufshcd_scsi_for_each_sdev(ufshcd_cleanup_queue); > out: > if (ret) > dev_err(hba->dev, "%s failed, err %d\n", __func__, ret); > What is the purpose of ufshcd_shutdown()? Why does this function exist? How about removing the calls to ufshcd_shutdown() and invoking power down code from inside sd_suspend_common() instead? Thanks, Bart.
Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices
On Sat, Jul 11, 2020 at 05:08:51PM -0700, Rajat Jain wrote: > On Sat, Jul 11, 2020 at 12:53 PM Bjorn Helgaas wrote: > > On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote: > > > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote: > > > > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat Jain wrote: > > > > > > When enabling ACS, enable translation blocking for external facing > > > > > > ports > > > > > > and untrusted devices. > > > > > > > > > > > > Signed-off-by: Rajat Jain > > > > > > --- > > > > > > v4: Add braces to avoid warning from kernel robot > > > > > > print warning for only external-facing devices. > > > > > > v3: print warning if ACS_TB not supported on > > > > > > external-facing/untrusted ports. > > > > > > Minor code comments fixes. > > > > > > v2: Commit log change > > > > > > > > > > > > drivers/pci/pci.c| 8 > > > > > > drivers/pci/quirks.c | 15 +++ > > > > > > 2 files changed, 23 insertions(+) > > > > > > > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > > > > index 73a8627822140..a5a6bea7af7ce 100644 > > > > > > --- a/drivers/pci/pci.c > > > > > > +++ b/drivers/pci/pci.c > > > > > > @@ -876,6 +876,14 @@ static void pci_std_enable_acs(struct pci_dev > > > > > > *dev) > > > > > > /* Upstream Forwarding */ > > > > > > ctrl |= (cap & PCI_ACS_UF); > > > > > > > > > > > > + /* Enable Translation Blocking for external devices */ > > > > > > + if (dev->external_facing || dev->untrusted) { > > > > > > + if (cap & PCI_ACS_TB) > > > > > > + ctrl |= PCI_ACS_TB; > > > > > > + else if (dev->external_facing) > > > > > > + pci_warn(dev, "ACS: No Translation Blocking on > > > > > > external-facing dev\n"); > > > > > > + } > > > > > > > > > > IIUC, this means that external devices can *never* use ATS and > > > > > can never cache translations. > > > > > > Yes, but it already exists today (and this patch doesn't change that): > > > 521376741b2c2 "PCI/ATS: Only enable ATS for trusted devices" > > > > > > IMHO any external device trying to send ATS traffic despite having ATS > > > disabled should count as a bad intent. And this patch is trying to > > > plug that loophole, by blocking the AT traffic from devices that we do > > > not expect to see AT from anyway. > > > > Thinking about this some more, I wonder if Linux should: > > > > - Explicitly disable ATS for every device at enumeration-time, e.g., > > in pci_init_capabilities(), > > > > - Enable PCI_ACS_TB for every device (not just external-facing or > > untrusted ones), > > > > - Disable PCI_ACS_TB for the relevant devices along the path only > > when enabling ATS. > > > > One nice thing about doing that is that the "untrusted" test would be > > only in pci_enable_ats(), and we wouldn't need one in > > pci_std_enable_acs(). > > Yes, this could work. > > I think I had thought about this but I'm blanking out on why I had > given it up. I think it was because of the possibility that some > bridges may have "Translation blocking" disabled, even if not all > their descendents were trusted enough to enable ATS on them. But now > thinking about this again, as long as we retain the policy of not > enabling ATS on external devices (and thus enable TB for sure on > them), this should not be a problem. WDYT? I think I would feel better if we always enabled Translation Blocking except when we actually need it for ATS. But I'm not confident about how all the pieces of ATS work, so I could be missing something. > > It's possible BIOS gives us devices with ATS enabled, and this > > might break them, but that seems like something we'd want to find > > out about. > > Why would they break? We'd disable ATS on each device as we > enumerate them, so they'd be functional, just with ATS disabled > until it is enabled again on internal devices as needed. Which would > be WAI behavior? If BIOS handed off with ATS enabled and we somehow relied on it being already enabled, something might break if we start disabling ATS. Just a theoretical possibility, doesn't seem likely to me. Bjorn
RE: [PATCH 1/3] gpio: mxc: Support module build
Hi, Linus > Subject: Re: [PATCH 1/3] gpio: mxc: Support module build > > On Wed, Jul 8, 2020 at 1:28 AM Anson Huang > wrote: > > > subsys_initcall(gpio_mxc_init); > > + > > +MODULE_AUTHOR("Shawn Guo "); > > +MODULE_DESCRIPTION("i.MX GPIO Driver"); MODULE_LICENSE("GPL"); > > You are making this modualrizable but keeping the subsys_initcall(), which > doesn't make very much sense. It is obviously not necessary to do this probe > at subsys_initcall() time, right? > If building it as module, the subsys_initcall() will be equal to module_init(), I keep it unchanged is because I try to make it identical when built-in, since most of the config will still have it built-in, except the Android GKI support. Does it make sense? > Take this opportunity to convert the driver to use > module_platform_driver() as well. If you think it has to be or it is better to use module_platform_driver(), I will do it in V2. thanks, Anson
Re: [RFC PATCH v3] scsi: ufs: Quiesce all scsi devices before shutdown
Hi Bart, Avri, May I know if you have any suggestion for this RFC fix? Very appreciated : ) On Mon, 2020-07-06 at 21:22 +0800, Stanley Chu wrote: > Currently I/O request could be still submitted to UFS device while > UFS is working on shutdown flow. This may lead to racing as below > scenarios and finally system may crash due to unclocked register > accesses. > > To fix this kind of issues, specifically quiesce all SCSI devices > before UFS shutdown to block all I/O request sending from block > layer. > > Example of racing scenario: While UFS device is runtime-suspended > > Thread #1: Executing UFS shutdown flow, e.g., >ufshcd_suspend(UFS_SHUTDOWN_PM) > Thread #2: Executing runtime resume flow triggered by I/O request, >e.g., ufshcd_resume(UFS_RUNTIME_PM) > > This breaks the assumption that UFS PM flows can not be running > concurrently and some unexpected racing behavior may happen. > > Signed-off-by: Stanley Chu > --- > drivers/scsi/ufs/ufshcd.c | 38 ++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 59358bb75014..104173c03492 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -158,6 +158,12 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = { > {UFS_POWERDOWN_PWR_MODE, UIC_LINK_OFF_STATE}, > }; > > +#define ufshcd_scsi_for_each_sdev(fn) \ > + list_for_each_entry(starget, >host->__targets, siblings) { \ > + __starget_for_each_device(starget, NULL, \ > + fn); \ > + } > + > static inline enum ufs_dev_pwr_mode > ufs_get_pm_lvl_to_dev_pwr_mode(enum ufs_pm_level lvl) > { > @@ -8588,6 +8594,19 @@ int ufshcd_runtime_idle(struct ufs_hba *hba) > } > EXPORT_SYMBOL(ufshcd_runtime_idle); > > +static void ufshcd_cleanup_queue(struct scsi_device *sdev, void *data) > +{ > + if (sdev->request_queue) > + blk_cleanup_queue(sdev->request_queue); > +} > + > +static void ufshcd_quiece_sdev(struct scsi_device *sdev, void *data) > +{ > + /* Suspended devices are already quiecsed and shall be skipped */ > + if (!pm_runtime_suspended(>sdev_gendev)) > + scsi_device_quiesce(sdev); > +} > + > /** > * ufshcd_shutdown - shutdown routine > * @hba: per adapter instance > @@ -8599,6 +8618,7 @@ EXPORT_SYMBOL(ufshcd_runtime_idle); > int ufshcd_shutdown(struct ufs_hba *hba) > { > int ret = 0; > + struct scsi_target *starget; > > if (!hba->is_powered) > goto out; > @@ -8612,7 +8632,25 @@ int ufshcd_shutdown(struct ufs_hba *hba) > goto out; > } > > + /* > + * Quiesce all SCSI devices to prevent any non-PM requests sending > + * from block layer during and after shutdown. > + * > + * Here we can not use blk_cleanup_queue() since PM requests > + * (with BLK_MQ_REQ_PREEMPT flag) are still required to be sent > + * through block layer. Therefore SCSI command queued after the > + * scsi_target_quiesce() call returned will block until > + * blk_cleanup_queue() is called. > + * > + * Besides, scsi_target_"un"quiesce (e.g., scsi_target_resume) can > + * be ignored since shutdown is one-way flow. > + */ > + ufshcd_scsi_for_each_sdev(ufshcd_quiece_sdev); > + > ret = ufshcd_suspend(hba, UFS_SHUTDOWN_PM); > + > + /* Set queue as dying to not block queueing commands */ > + ufshcd_scsi_for_each_sdev(ufshcd_cleanup_queue); > out: > if (ret) > dev_err(hba->dev, "%s failed, err %d\n", __func__, ret);
RE: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification
Hi Avri, On Thu, 2020-07-09 at 08:31 +, Avri Altman wrote: > > > > If somehow no interrupt notification is raised for a completed request > > and its doorbell bit is cleared by host, UFS driver needs to cleanup > > its outstanding bit in ufshcd_abort(). > Theoretically, this case is already accounted for - > See line 6407: a proper error is issued and eventually outstanding req is > cleared. > > Can you go over the scenario you are attending line by line, > And explain why ufshcd_abort does not account for it? Sure. If a request using tag N is completed by UFS device without interrupt notification till timeout happens, ufshcd_abort() will be invoked. Since request completion flow is not executed, current status may be - Tag N in hba->outstanding_reqs is set - Tag N in doorbell register is not set In this case, ufshcd_abort() flow would be - This log is printed: "ufshcd_abort: cmd was completed, but without a notifying intr, tag = N" - This log is printed: "ufshcd_abort: Device abort task at tag N" - If hba->req_abort_skip is zero, QUERY_TASK command is sent - Device responds "UPIU_TASK_MANAGEMENT_FUNC_COMPL" - This log is printed: "ufshcd_abort: cmd at tag N not pending in the device." - Doorbell tells that tag N is not set, so the driver goes to label "out" with this log printed: "ufshcd_abort: cmd at tag %d successfully cleared from DB." - In label "out" section, no cleanup will be made, and then ufshcd_abort exits - This request will be re-queued to request queue by SCSI timeout handler Now, Inconsistent state shows-up: A request is "re-queued" but its corresponding resource in UFS layer is not cleared, below flow will trigger bad things, - A new request with tag M is finished - Interrupt is raised and ufshcd_transfer_req_compl() found both tag N and M can process the completion flow - The post-processing flow for tag N will be executed while its request is still alive I am sorry that below messages are only for old kernel in non-blk-mq case. However above scenario will also trigger bad thing in blk-mq case. > > > > > Otherwise, system may crash by below abnormal flow: > > > > After this request is requeued by SCSI layer with its > > outstanding bit set, the next completed request will trigger > > ufshcd_transfer_req_compl() to handle all "completed outstanding > > bits". In this time, the "abnormal outstanding bit" will be detected > > and the "requeued request" will be chosen to execute request > > post-processing flow. This is wrong and blk_finish_request() will > > BUG_ON because this request is still "alive". > > > > It is worth mentioning that before ufshcd_abort() cleans the timed-out > > request, driver need to check again if this request is really not > > handled by __ufshcd_transfer_req_compl() yet because it may be > > possible that the interrupt comes very lately before the cleaning. > What do you mean? Why checking the outstanding reqs isn't enough? > > > > > Signed-off-by: Stanley Chu > > --- > > drivers/scsi/ufs/ufshcd.c | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 8603b07045a6..f23fb14df9f6 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -6462,7 +6462,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > > /* command completed already */ > > dev_err(hba->dev, "%s: cmd at tag %d successfully > > cleared from > > DB.\n", > > __func__, tag); > > - goto out; > > + goto cleanup; > But you've arrived here only if (!(test_bit(tag, >outstanding_reqs))) - > See line 6400. > > > } else { > > dev_err(hba->dev, > > "%s: no response from device. tag = %d, err > > %d\n", > > @@ -6496,9 +6496,14 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > > goto out; > > } > > > > +cleanup: > > + spin_lock_irqsave(host->host_lock, flags); > > + if (!test_bit(tag, >outstanding_reqs)) { > > + spin_unlock_irqrestore(host->host_lock, flags); > > + goto out; > > + } > > scsi_dma_unmap(cmd); > > > > - spin_lock_irqsave(host->host_lock, flags); > > ufshcd_outstanding_req_clear(hba, tag); > > hba->lrb[tag].cmd = NULL; > > spin_unlock_irqrestore(host->host_lock, flags); > > -- > > 2.18.0
[Patch v2 0/4] tracing: trivial cleanup
Some trivial cleanup for tracing. v2: * drop patch 1 * merge patch 4 & 5 * introduce a new patch change the return value of tracing_init_dentry() Wei Yang (4): tracing: simplify the logic by defining next to be "lasst + 1" tracing: save one trace_event->type by using __TRACE_LAST_TYPE tracing: toplevel d_entry already initialized tracing: make tracing_init_dentry() returns an integer instead of a d_entry pointer kernel/trace/trace.c | 36 ++-- kernel/trace/trace.h | 2 +- kernel/trace/trace_dynevent.c| 8 +++ kernel/trace/trace_events.c | 9 ++- kernel/trace/trace_events_synth.c| 9 +++ kernel/trace/trace_functions_graph.c | 8 +++ kernel/trace/trace_hwlat.c | 8 +++ kernel/trace/trace_kprobe.c | 10 kernel/trace/trace_output.c | 14 +-- kernel/trace/trace_printk.c | 8 +++ kernel/trace/trace_stack.c | 12 +- kernel/trace/trace_stat.c| 8 +++ kernel/trace/trace_uprobe.c | 9 --- 13 files changed, 66 insertions(+), 75 deletions(-) -- 2.20.1 (Apple Git-117)
[Patch v2 1/4] tracing: simplify the logic by defining next to be "lasst + 1"
The value to be used and compared in trace_search_list() is "last + 1". Let's just define next to be "last + 1" instead of doing the addition each time. Signed-off-by: Wei Yang --- kernel/trace/trace_output.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index 73976de7f8cc..a35232d61601 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c @@ -675,11 +675,11 @@ static LIST_HEAD(ftrace_event_list); static int trace_search_list(struct list_head **list) { struct trace_event *e; - int last = __TRACE_LAST_TYPE; + int next = __TRACE_LAST_TYPE + 1; if (list_empty(_event_list)) { *list = _event_list; - return last + 1; + return next; } /* @@ -687,17 +687,17 @@ static int trace_search_list(struct list_head **list) * lets see if somebody freed one. */ list_for_each_entry(e, _event_list, list) { - if (e->type != last + 1) + if (e->type != next) break; - last++; + next++; } /* Did we used up all 65 thousand events??? */ - if ((last + 1) > TRACE_EVENT_TYPE_MAX) + if (next > TRACE_EVENT_TYPE_MAX) return 0; *list = >list; - return last + 1; + return next; } void trace_event_read_lock(void) -- 2.20.1 (Apple Git-117)
[Patch v2 3/4] tracing: toplevel d_entry already initialized
Currently we have following call flow: tracer_init_tracefs() tracing_init_dentry() event_trace_init() tracing_init_dentry() This shows tracing_init_dentry() is called twice in this flow and this is not necessary. Let's remove the second one when it is for sure be properly initialized. Signed-off-by: Wei Yang --- v2: * merged 4 & 5 --- kernel/trace/trace_events.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index f6f55682d3e2..76879b29cf33 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -3434,7 +3434,6 @@ early_initcall(event_trace_enable_again); __init int event_trace_init(void) { struct trace_array *tr; - struct dentry *d_tracer; struct dentry *entry; int ret; @@ -3442,11 +3441,7 @@ __init int event_trace_init(void) if (!tr) return -ENODEV; - d_tracer = tracing_init_dentry(); - if (IS_ERR(d_tracer)) - return 0; - - entry = tracefs_create_file("available_events", 0444, d_tracer, + entry = tracefs_create_file("available_events", 0444, NULL, tr, _avail_fops); if (!entry) pr_warn("Could not create tracefs 'available_events' entry\n"); @@ -3457,7 +3452,7 @@ __init int event_trace_init(void) if (trace_define_common_fields()) pr_warn("tracing: Failed to allocate common fields"); - ret = early_event_add_tracer(d_tracer, tr); + ret = early_event_add_tracer(NULL, tr); if (ret) return ret; -- 2.20.1 (Apple Git-117)
[Patch v2 4/4] tracing: make tracing_init_dentry() returns an integer instead of a d_entry pointer
Current tracing_init_dentry() return a d_entry pointer, while is not necessary. This function returns NULL on success or error on failure, which means there is no valid d_entry pointer return. Let's return 0 on success and negative value for error. Signed-off-by: Wei Yang --- kernel/trace/trace.c | 36 ++-- kernel/trace/trace.h | 2 +- kernel/trace/trace_dynevent.c| 8 +++ kernel/trace/trace_events_synth.c| 9 +++ kernel/trace/trace_functions_graph.c | 8 +++ kernel/trace/trace_hwlat.c | 8 +++ kernel/trace/trace_kprobe.c | 10 kernel/trace/trace_printk.c | 8 +++ kernel/trace/trace_stack.c | 12 +- kernel/trace/trace_stat.c| 8 +++ kernel/trace/trace_uprobe.c | 9 --- 11 files changed, 57 insertions(+), 61 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 547e1e01902a..41c3d75c34cc 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -8933,23 +8933,23 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore) * directory. It is called via fs_initcall() by any of the boot up code * and expects to return the dentry of the top level tracing directory. */ -struct dentry *tracing_init_dentry(void) +int tracing_init_dentry(void) { struct trace_array *tr = _trace; if (security_locked_down(LOCKDOWN_TRACEFS)) { pr_warn("Tracing disabled due to lockdown\n"); - return ERR_PTR(-EPERM); + return -EPERM; } /* The top level trace array uses NULL as parent */ if (tr->dir) - return NULL; + return 0; if (WARN_ON(!tracefs_initialized()) || (IS_ENABLED(CONFIG_DEBUG_FS) && WARN_ON(!debugfs_initialized( - return ERR_PTR(-ENODEV); + return -ENODEV; /* * As there may still be users that expect the tracing @@ -8960,7 +8960,7 @@ struct dentry *tracing_init_dentry(void) tr->dir = debugfs_create_automount("tracing", NULL, trace_automount, NULL); - return NULL; + return 0; } extern struct trace_eval_map *__start_ftrace_eval_maps[]; @@ -9047,48 +9047,48 @@ static struct notifier_block trace_module_nb = { static __init int tracer_init_tracefs(void) { - struct dentry *d_tracer; + int ret; trace_access_lock_init(); - d_tracer = tracing_init_dentry(); - if (IS_ERR(d_tracer)) + ret = tracing_init_dentry(); + if (ret) return 0; event_trace_init(); - init_tracer_tracefs(_trace, d_tracer); - ftrace_init_tracefs_toplevel(_trace, d_tracer); + init_tracer_tracefs(_trace, NULL); + ftrace_init_tracefs_toplevel(_trace, NULL); - trace_create_file("tracing_thresh", 0644, d_tracer, + trace_create_file("tracing_thresh", 0644, NULL, _trace, _thresh_fops); - trace_create_file("README", 0444, d_tracer, + trace_create_file("README", 0444, NULL, NULL, _readme_fops); - trace_create_file("saved_cmdlines", 0444, d_tracer, + trace_create_file("saved_cmdlines", 0444, NULL, NULL, _saved_cmdlines_fops); - trace_create_file("saved_cmdlines_size", 0644, d_tracer, + trace_create_file("saved_cmdlines_size", 0644, NULL, NULL, _saved_cmdlines_size_fops); - trace_create_file("saved_tgids", 0444, d_tracer, + trace_create_file("saved_tgids", 0444, NULL, NULL, _saved_tgids_fops); trace_eval_init(); - trace_create_eval_file(d_tracer); + trace_create_eval_file(NULL); #ifdef CONFIG_MODULES register_module_notifier(_module_nb); #endif #ifdef CONFIG_DYNAMIC_FTRACE - trace_create_file("dyn_ftrace_total_info", 0444, d_tracer, + trace_create_file("dyn_ftrace_total_info", 0444, NULL, NULL, _dyn_info_fops); #endif - create_trace_instances(d_tracer); + create_trace_instances(NULL); update_tracer_options(_trace); diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index def769df5bf1..afcef31ae76d 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -734,7 +734,7 @@ struct dentry *trace_create_file(const char *name, void *data, const struct file_operations *fops); -struct dentry *tracing_init_dentry(void); +int tracing_init_dentry(void); struct ring_buffer_event; diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c index 9f2e8520b748..9442a9bb080e 100644 --- a/kernel/trace/trace_dynevent.c +++ b/kernel/trace/trace_dynevent.c @@ -206,14 +206,14 @@ static const struct
[Patch v2 2/4] tracing: save one trace_event->type by using __TRACE_LAST_TYPE
Static defined trace_event->type stops at (__TRACE_LAST_TYPE - 1) and dynamic trace_event->type starts from (__TRACE_LAST_TYPE + 1). To save one trace_event->type index, let's use __TRACE_LAST_TYPE. Signed-off-by: Wei Yang --- kernel/trace/trace_output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index a35232d61601..4d1893564912 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c @@ -20,7 +20,7 @@ DECLARE_RWSEM(trace_event_sem); static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly; -static int next_event_type = __TRACE_LAST_TYPE + 1; +static int next_event_type = __TRACE_LAST_TYPE; enum print_line_t trace_print_bputs_msg_only(struct trace_iterator *iter) { @@ -675,7 +675,7 @@ static LIST_HEAD(ftrace_event_list); static int trace_search_list(struct list_head **list) { struct trace_event *e; - int next = __TRACE_LAST_TYPE + 1; + int next = __TRACE_LAST_TYPE; if (list_empty(_event_list)) { *list = _event_list; -- 2.20.1 (Apple Git-117)
Re: [RFC PATCH v2] scsi: ufs-mediatek: add inline encryption support
Hi Eric, On Thu, 2020-07-09 at 23:39 -0700, Eric Biggers wrote: > Hi Stanley, > > On Wed, Mar 04, 2020 at 10:21:02AM +0800, Stanley Chu wrote: > > Add inline encryption support to ufs-mediatek. > > > > The standards-compliant parts, such as querying the crypto capabilities > > and enabling crypto for individual UFS requests, are already handled by > > ufshcd-crypto.c, which itself is wired into the blk-crypto framework. > > > > However MediaTek UFS host requires a vendor-specific hce_enable operation > > to allow crypto-related registers being accessed normally in kernel. > > After this step, MediaTek UFS host can work as standard-compliant host > > for inline-encryption related functions. > > > > This patch is rebased to below repo and tag: > > Repo: > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git__;!!CTRNKA9wMg0ARbw!x8Kltcu8AQhdtlIDXASWjd_ANrBtcYzidIXMsu-fQloEqgvrDDBU9yD9GumtKLIcd_c$ > > > > Tag: inline-encryption-v7 > > > > Signed-off-by: Stanley Chu > > --- > > drivers/scsi/ufs/ufs-mediatek.c | 27 ++- > > drivers/scsi/ufs/ufs-mediatek.h | 1 + > > 2 files changed, 27 insertions(+), 1 deletion(-) > > Now that the ufshcd-crypto patches this depends on are in 5.9/scsi-queue, > could > you retest and resend this patch? It would be nice to have 5.9 support some > real hardware already. (I'm going to resend my patchset for ufs-qcom too.) Sure. Now this patch is resent as v3, please see https://patchwork.kernel.org/patch/11657987/ Thanks, Stanley Chu
[PATCH v3] scsi: ufs-mediatek: Add inline encryption support
Add inline encryption support to ufs-mediatek. The standards-compliant parts, such as querying the crypto capabilities and enabling crypto for individual UFS requests, are already handled by ufshcd-crypto.c, which itself is wired into the blk-crypto framework. However MediaTek UFS host requires a vendor-specific hce_enable operation to allow crypto-related registers being accessed normally in kernel. After this step, MediaTek UFS host can work as standard-compliant host for inline-encryption related functions. Signed-off-by: Stanley Chu --- drivers/scsi/ufs/ufs-mediatek.c | 22 ++ drivers/scsi/ufs/ufs-mediatek.h | 1 + 2 files changed, 23 insertions(+) diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c index ad929235c193..31af8b3d2b53 100644 --- a/drivers/scsi/ufs/ufs-mediatek.c +++ b/drivers/scsi/ufs/ufs-mediatek.c @@ -16,6 +16,7 @@ #include #include "ufshcd.h" +#include "ufshcd-crypto.h" #include "ufshcd-pltfrm.h" #include "ufs_quirks.h" #include "unipro.h" @@ -25,6 +26,9 @@ arm_smccc_smc(MTK_SIP_UFS_CONTROL, \ cmd, val, 0, 0, 0, 0, 0, &(res)) +#define ufs_mtk_crypto_ctrl(res, enable) \ + ufs_mtk_smc(UFS_MTK_SIP_CRYPTO_CTRL, enable, res) + #define ufs_mtk_ref_clk_notify(on, res) \ ufs_mtk_smc(UFS_MTK_SIP_REF_CLK_NOTIFICATION, on, res) @@ -73,6 +77,18 @@ static void ufs_mtk_cfg_unipro_cg(struct ufs_hba *hba, bool enable) } } +static void ufs_mtk_crypto_enable(struct ufs_hba *hba) +{ + struct arm_smccc_res res; + + ufs_mtk_crypto_ctrl(res, 1); + if (res.a0) { + dev_info(hba->dev, "%s: crypto enable failed, err: %lu\n", +__func__, res.a0); + hba->caps &= ~UFSHCD_CAP_CRYPTO; + } +} + static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba, enum ufs_notify_change_status status) { @@ -83,6 +99,9 @@ static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba, hba->vps->hba_enable_delay_us = 0; else hba->vps->hba_enable_delay_us = 600; + + if (hba->caps & UFSHCD_CAP_CRYPTO) + ufs_mtk_crypto_enable(hba); } return 0; @@ -317,6 +336,9 @@ static int ufs_mtk_init(struct ufs_hba *hba) /* Enable clock-gating */ hba->caps |= UFSHCD_CAP_CLK_GATING; + /* Enable inline encryption */ + hba->caps |= UFSHCD_CAP_CRYPTO; + /* Enable WriteBooster */ hba->caps |= UFSHCD_CAP_WB_EN; hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80); diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h index 6052ec105aba..8ed24d5fcff9 100644 --- a/drivers/scsi/ufs/ufs-mediatek.h +++ b/drivers/scsi/ufs/ufs-mediatek.h @@ -70,6 +70,7 @@ enum { */ #define MTK_SIP_UFS_CONTROL MTK_SIP_SMC_CMD(0x276) #define UFS_MTK_SIP_DEVICE_RESET BIT(1) +#define UFS_MTK_SIP_CRYPTO_CTRL BIT(2) #define UFS_MTK_SIP_REF_CLK_NOTIFICATION BIT(3) /* -- 2.18.0
Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices
On Sat, Jul 11, 2020 at 12:53 PM Bjorn Helgaas wrote: > > On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote: > > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote: > > > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat Jain wrote: > > > > > When enabling ACS, enable translation blocking for external facing > > > > > ports > > > > > and untrusted devices. > > > > > > > > > > Signed-off-by: Rajat Jain > > > > > --- > > > > > v4: Add braces to avoid warning from kernel robot > > > > > print warning for only external-facing devices. > > > > > v3: print warning if ACS_TB not supported on > > > > > external-facing/untrusted ports. > > > > > Minor code comments fixes. > > > > > v2: Commit log change > > > > > > > > > > drivers/pci/pci.c| 8 > > > > > drivers/pci/quirks.c | 15 +++ > > > > > 2 files changed, 23 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > > > index 73a8627822140..a5a6bea7af7ce 100644 > > > > > --- a/drivers/pci/pci.c > > > > > +++ b/drivers/pci/pci.c > > > > > @@ -876,6 +876,14 @@ static void pci_std_enable_acs(struct pci_dev > > > > > *dev) > > > > > /* Upstream Forwarding */ > > > > > ctrl |= (cap & PCI_ACS_UF); > > > > > > > > > > + /* Enable Translation Blocking for external devices */ > > > > > + if (dev->external_facing || dev->untrusted) { > > > > > + if (cap & PCI_ACS_TB) > > > > > + ctrl |= PCI_ACS_TB; > > > > > + else if (dev->external_facing) > > > > > + pci_warn(dev, "ACS: No Translation Blocking on > > > > > external-facing dev\n"); > > > > > + } > > > > > > > > IIUC, this means that external devices can *never* use ATS and > > > > can never cache translations. > > > > Yes, but it already exists today (and this patch doesn't change that): > > 521376741b2c2 "PCI/ATS: Only enable ATS for trusted devices" > > > > IMHO any external device trying to send ATS traffic despite having ATS > > disabled should count as a bad intent. And this patch is trying to > > plug that loophole, by blocking the AT traffic from devices that we do > > not expect to see AT from anyway. > > Thinking about this some more, I wonder if Linux should: > > - Explicitly disable ATS for every device at enumeration-time, e.g., > in pci_init_capabilities(), > > - Enable PCI_ACS_TB for every device (not just external-facing or > untrusted ones), > > - Disable PCI_ACS_TB for the relevant devices along the path only > when enabling ATS. > > One nice thing about doing that is that the "untrusted" test would be > only in pci_enable_ats(), and we wouldn't need one in > pci_std_enable_acs(). Yes, this could work. I think I had thought about this but I'm blanking out on why I had given it up. I think it was because of the possibility that some bridges may have "Translation blocking" disabled, even if not all their descendents were trusted enough to enable ATS on them. But now thinking about this again, as long as we retain the policy of not enabling ATS on external devices (and thus enable TB for sure on them), this should not be a problem. WDYT? > > It's possible BIOS gives us devices with ATS enabled, and this might > break them, but that seems like something we'd want to find out about. > Why would they break? We'd disable ATS on each device as we enumerate them, so they'd be functional, just with ATS disabled until it is enabled again on internal devices as needed. Which would be WAI behavior? Thanks, , Rajat > Bjorn
Re: [PATCH v5 2/2] devicetree: hwmon: shtc1: Add sensirion,shtc1.yaml
Morning, On 11/7/2020 6:05 pm, Chris Ruehl wrote: On 11/7/2020 12:31 am, Rob Herring wrote: On Fri, 10 Jul 2020 10:15:35 +0800, Chris Ruehl wrote: Add documentation for the newly added DTS support in the shtc1 driver. To align with the drivers logic to have high precision by default a boolean sensirion,low_precision is used to switch to low precision. Signed-off-by: Chris Ruehl --- .../bindings/hwmon/sensirion,shtc1.yaml | 57 +++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/sensirion,shtc1.yaml My bot found errors running 'make dt_binding_check' on your patch: Error: Documentation/devicetree/bindings/hwmon/sensirion,shtc1.example.dts:25.13-14 syntax error FATAL ERROR: Unable to parse input tree scripts/Makefile.lib:315: recipe for target 'Documentation/devicetree/bindings/hwmon/sensirion,shtc1.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/hwmon/sensirion,shtc1.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs Makefile:1347: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1326414 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit. Hi Rob, I did run the test and didn't had any Error. dt-schema 2020.06 installed from git. pip3 install -e. Can you help? Chris Solved, missing ";" behind reg = <0x70> will resend.
[PATCH] Input: break joystick limitation of maximum 80 buttons
The joystick max buttons 80 limitation comes from #define BTN_JOYSTICK 0x120 #define BTN_DEAD 0x12f #define BTN_TRIGGER_HAPPY 0x2c0 #define KEY_MAX 0x2ff include/uapi/linux/input-event-codes.h according to function hidinput_configure_usage() in file drivers/hid/hid-input.c the joystick button mapping is not a continues space generally speaking, the mapping space is from 1. BTN_JOYSTICK~BTN_DEAD 2. BTN_TRIGGER_HAPPY~KEY_MAX Finally, I got the max limitation is 80. The patch is expanding KEY_MAX from 0x2ff to 4ff and the change has been verified on 104 button USB HID device on Ubuntu Signed-off-by: Wei Shuai --- include/linux/mod_devicetable.h| 2 +- include/uapi/linux/input-event-codes.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 8d764aab29de..35eb59ae1f19 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -311,7 +311,7 @@ struct pcmcia_device_id { /* Input */ #define INPUT_DEVICE_ID_EV_MAX 0x1f #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING0x71 -#define INPUT_DEVICE_ID_KEY_MAX0x2ff +#define INPUT_DEVICE_ID_KEY_MAX0x4ff #define INPUT_DEVICE_ID_REL_MAX0x0f #define INPUT_DEVICE_ID_ABS_MAX0x3f #define INPUT_DEVICE_ID_MSC_MAX0x07 diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h index b6a835d37826..ad1b9bed3828 100644 --- a/include/uapi/linux/input-event-codes.h +++ b/include/uapi/linux/input-event-codes.h @@ -774,7 +774,7 @@ /* We avoid low common keys in module aliases so they don't get huge. */ #define KEY_MIN_INTERESTINGKEY_MUTE -#define KEY_MAX0x2ff +#define KEY_MAX0x4ff #define KEY_CNT(KEY_MAX+1) /* -- 2.17.1
linux-next: Signed-off-by missing for commit in the risc-v tree
Hi all, Commit 95ce6c73da3b ("riscv: Enable context tracking") is missing a Signed-off-by from its committer. -- Cheers, Stephen Rothwell pgpcrOHvPrWhw.pgp Description: OpenPGP digital signature
Re: WARNING: at mm/mremap.c:211 move_page_tables in i386
On Sat, Jul 11, 2020 at 11:12:58AM -0700, Linus Torvalds wrote: > On Sat, Jul 11, 2020 at 10:27 AM Naresh Kamboju > wrote: > > > > I have started bisecting this problem and found the first bad commit > > Thanks for the effort. Bisection is often a great tool to figure out > what's wrong. > > Sadly, in this case: > > > commit 9f132f7e145506efc0744426cb338b18a54afc3b > > Author: Joel Fernandes (Google) > > Date: Thu Jan 3 15:28:41 2019 -0800 > > > > mm: select HAVE_MOVE_PMD on x86 for faster mremap > > Yeah, that's just the commit that enables the code, not the commit > that introduces the fundamental problem. > > That said, this is a prime example of why I absolutely detest patch > series that do this kind of thing, and are several patches that create > new functionality, followed by one patch to enable it. > > If you can't get things working incrementally, maybe you shouldn't do > them at all. Doing a big series of "hidden work" and then enabling it > later is wrong. > > In this case, though, the real patch that did the code isn't that kind > of "big series of hidden work" patch series, it's just the (single) > previous commit 2c91bd4a4e2e ("mm: speed up mremap by 20x on large > regions"). > > So your bisection is useful, it's just that it really points to that > previous commit, and it's where this code was introduced. Right, I think I should have squashed the enabling of the config, and the introduction of the feature in the same patch, but as you pointed that probably would not have made a difference with this bisect since this a single patch. > It's also worth noting that that commit doesn't really *break* > anything, since it just falls back to the old behavior when it warns. Agreed, I am also of the opinion that the patch is likely surface an existing issue and not introducing a new one. > So to "fix" your test-case, we could just remove the WARN_ON. > > But the WARN_ON() is still worrisome, because the thing it tests for > really _should_ be true. I'll get some tracing in an emulated i386 environment going and try to figure out exactly what is going on before the warning triggers. thanks for the other debug hints in this thread! thanks, - Joel - Joel
Re: [PATCH v2 net] net: fec: fix hardware time stamping by external devices
Hi Sergey, On Sat, Jul 11, 2020 at 03:08:42PM +0300, Sergey Organov wrote: > Fix support for external PTP-aware devices such as DSA or PTP PHY: > > Make sure we never time stamp tx packets when hardware time stamping > is disabled. > > Check for PTP PHY being in use and then pass ioctls related to time > stamping of Ethernet packets to the PTP PHY rather than handle them > ourselves. In addition, disable our own hardware time stamping in this > case. > > Fixes: 6605b73 ("FEC: Add time stamping code and a PTP hardware clock") Please use a 12-character sha1sum. Try to use the "pretty" format specifier I gave you in the original thread, it saves you from counting, and also from people complaining once it gets merged: https://www.google.com/search?q=stephen+rothwell+%22fixes+tag+needs+some+work%22 > Signed-off-by: Sergey Organov > --- > > v2: > - Extracted from larger patch series > - Description/comments updated according to discussions > - Added Fixes: tag > > drivers/net/ethernet/freescale/fec.h | 1 + > drivers/net/ethernet/freescale/fec_main.c | 23 +-- > drivers/net/ethernet/freescale/fec_ptp.c | 12 > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec.h > b/drivers/net/ethernet/freescale/fec.h > index d8d76da..832a217 100644 > --- a/drivers/net/ethernet/freescale/fec.h > +++ b/drivers/net/ethernet/freescale/fec.h > @@ -590,6 +590,7 @@ struct fec_enet_private { > void fec_ptp_init(struct platform_device *pdev, int irq_idx); > void fec_ptp_stop(struct platform_device *pdev); > void fec_ptp_start_cyclecounter(struct net_device *ndev); > +void fec_ptp_disable_hwts(struct net_device *ndev); > int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); > int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 3982285..cc7fbfc 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1294,8 +1294,13 @@ fec_enet_tx_queue(struct net_device *ndev, u16 > queue_id) > ndev->stats.tx_bytes += skb->len; > } > > - if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) && > - fep->bufdesc_ex) { > + /* NOTE: SKBTX_IN_PROGRESS being set does not imply it's we who > + * are to time stamp the packet, so we still need to check time > + * stamping enabled flag. > + */ > + if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS && > + fep->hwts_tx_en) && > + fep->bufdesc_ex) { > struct skb_shared_hwtstamps shhwtstamps; > struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; > > @@ -2723,10 +2728,16 @@ static int fec_enet_ioctl(struct net_device *ndev, > struct ifreq *rq, int cmd) > return -ENODEV; > > if (fep->bufdesc_ex) { > - if (cmd == SIOCSHWTSTAMP) > - return fec_ptp_set(ndev, rq); > - if (cmd == SIOCGHWTSTAMP) > - return fec_ptp_get(ndev, rq); > + bool use_fec_hwts = !phy_has_hwtstamp(phydev); I thought we were in agreement that FEC does not support PHY timestamping at this point, and this patch would only be fixing DSA switches (even though PHYs would need this fixed too, when support is added for them)? I would definitely not introduce support (and incomplete, at that) for a new feature in a bugfix patch. But it looks like we aren't. > + > + if (cmd == SIOCSHWTSTAMP) { > + if (use_fec_hwts) > + return fec_ptp_set(ndev, rq); > + fec_ptp_disable_hwts(ndev); > + } else if (cmd == SIOCGHWTSTAMP) { > + if (use_fec_hwts) > + return fec_ptp_get(ndev, rq); > + } > } > > return phy_mii_ioctl(phydev, rq, cmd); > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c > b/drivers/net/ethernet/freescale/fec_ptp.c > index 945643c..f8a592c 100644 > --- a/drivers/net/ethernet/freescale/fec_ptp.c > +++ b/drivers/net/ethernet/freescale/fec_ptp.c > @@ -452,6 +452,18 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp, > return -EOPNOTSUPP; > } > > +/** > + * fec_ptp_disable_hwts - disable hardware time stamping > + * @ndev: pointer to net_device > + */ > +void fec_ptp_disable_hwts(struct net_device *ndev) This is not really needed, is it? - PHY ability of hwtstamping does not change across the runtime of the kernel (or do you have a "special" one where it does?) - The initial values for hwts_tx_en and hwts_rx_en are already 0 - There is no code path for which it is possible for hwts_tx_en or hwts_rx_en to have been non-zero prior to this call
Re: [PATCH] Revert "kernel/printk: add kmsg SEEK_CUR handling"
On Sun, Jun 21, 2020 at 9:02 PM Jason A. Donenfeld wrote: > This commit broke userspace. Bash uses ESPIPE to determine whether or > not the file should be read using "unbuffered I/O", which means reading > 1 byte at a time instead of 128 bytes at a time. I used to use bash to > read through kmsg in a really quite nasty way: > > while read -t 0.1 -r line 2>/dev/null || [[ $? -ne 142 ]]; do >echo "SARU $line" > done < /dev/kmsg > > This will show all lines that can fit into the 128 byte buffer, and skip > lines that don't. That's pretty awful, but at least it worked. FYI, bash finally bumped its read buffer up to 4k, which actually makes reading /dev/kmsg less awful than previously thought: http://git.savannah.gnu.org/cgit/bash.git/commit/?id=6edcd70089d71ee8c17bf3298527054b3223be9f This is probably too mundane to warrant an email, but in case somebody finds this thread in the future, voila.
Re: [PATCH v5] x86/umip: Add emulation/spoofing for SLDT and STR instructions
On July 10, 2020 3:45:25 PM PDT, Brendan Shanks wrote: >Add emulation/spoofing of SLDT and STR for both 32- and 64-bit >processes. > >Wine users have found a small number of Windows apps using SLDT that >were crashing when run on UMIP-enabled systems. > >Reported-by: Andreas Rammhold >Originally-by: Ricardo Neri >Signed-off-by: Brendan Shanks >--- > >v5: Capitalize instruction names in comments. > > arch/x86/kernel/umip.c | 40 +++- > 1 file changed, 27 insertions(+), 13 deletions(-) > >diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c >index 8d5cbe1bbb3b..2c304fd0bb1a 100644 >--- a/arch/x86/kernel/umip.c >+++ b/arch/x86/kernel/umip.c >@@ -45,11 +45,12 @@ >* value that, lies close to the top of the kernel memory. The limit for >the GDT > * and the IDT are set to zero. > * >- * Given that SLDT and STR are not commonly used in programs that run >on WineHQ >- * or DOSEMU2, they are not emulated. >- * >- * The instruction smsw is emulated to return the value that the >register CR0 >+ * The instruction SMSW is emulated to return the value that the >register CR0 > * has at boot time as set in the head_32. >+ * SLDT and STR are emulated to return the values that the kernel >programmatically >+ * assigns: >+ * - SLDT returns (GDT_ENTRY_LDT * 8) if an LDT has been set, 0 if >not. >+ * - STR returns (GDT_ENTRY_TSS * 8). > * > * Emulation is provided for both 32-bit and 64-bit processes. > * >@@ -244,16 +245,34 @@ static int emulate_umip_insn(struct insn *insn, >int umip_inst, > *data_size += UMIP_GDT_IDT_LIMIT_SIZE; > memcpy(data, _limit, UMIP_GDT_IDT_LIMIT_SIZE); > >- } else if (umip_inst == UMIP_INST_SMSW) { >- unsigned long dummy_value = CR0_STATE; >+ } else if (umip_inst == UMIP_INST_SMSW || umip_inst == UMIP_INST_SLDT >|| >+ umip_inst == UMIP_INST_STR) { >+ unsigned long dummy_value; >+ >+ if (umip_inst == UMIP_INST_SMSW) { >+ dummy_value = CR0_STATE; >+ } else if (umip_inst == UMIP_INST_STR) { >+ dummy_value = GDT_ENTRY_TSS * 8; >+ } else if (umip_inst == UMIP_INST_SLDT) { >+#ifdef CONFIG_MODIFY_LDT_SYSCALL >+ down_read(>mm->context.ldt_usr_sem); >+ if (current->mm->context.ldt) >+ dummy_value = GDT_ENTRY_LDT * 8; >+ else >+ dummy_value = 0; >+ up_read(>mm->context.ldt_usr_sem); >+#else >+ dummy_value = 0; >+#endif >+ } > > /* >- * Even though the CR0 register has 4 bytes, the number >+ * For these 3 instructions, the number >* of bytes to be copied in the result buffer is determined >* by whether the operand is a register or a memory location. >* If operand is a register, return as many bytes as the operand >* size. If operand is memory, return only the two least >- * siginificant bytes of CR0. >+ * siginificant bytes. >*/ > if (X86_MODRM_MOD(insn->modrm.value) == 3) > *data_size = insn->opnd_bytes; >@@ -261,7 +280,6 @@ static int emulate_umip_insn(struct insn *insn, int >umip_inst, > *data_size = 2; > > memcpy(data, _value, *data_size); >- /* STR and SLDT are not emulated */ > } else { > return -EINVAL; > } >@@ -383,10 +401,6 @@ bool fixup_umip_exception(struct pt_regs *regs) > umip_pr_warn(regs, "%s instruction cannot be used by applications.\n", > umip_insns[umip_inst]); > >- /* Do not emulate (spoof) SLDT or STR. */ >- if (umip_inst == UMIP_INST_STR || umip_inst == UMIP_INST_SLDT) >- return false; >- > umip_pr_warn(regs, "For now, expensive software emulation returns the >result.\n"); > > if (emulate_umip_insn(, umip_inst, dummy_data, _data_size, It's there any reason for SLDT to not *always* return a fixed value? "An LDT has been assigned" is formally a kernel internal property, separate from the property of whenever there are user space enteies in the LDT. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [PATCH v6 1/4] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.
Hi Bjorn, On 7/9/20 4:07 PM, Bjorn Helgaas wrote: On Fri, Jun 26, 2020 at 11:32:33AM -0700, sathyanarayanan.kuppusw...@linux.intel.com wrote: From: Kuppuswamy Sathyanarayanan pcie_ports_native is set only if user requests native handling of PCIe capabilities via pcie_port_setup command line option. User input takes precedence over _OSC based control negotiation result. So consider the _OSC negotiated result only if pcie_ports_native is unset. Also, since struct pci_host_bridge ->native_* members caches the ownership status of various PCIe capabilities, use them instead of distributed checks for pcie_ports_native. Signed-off-by: Kuppuswamy Sathyanarayanan --- drivers/acpi/pci_root.c | 26 ++ drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/pci-acpi.c| 3 --- drivers/pci/pcie/aer.c| 2 +- drivers/pci/pcie/portdrv_core.c | 9 +++-- drivers/pci/probe.c | 5 +++-- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index f90e841c59f5..02fab8b0118e 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -914,18 +914,20 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, goto out_release_info; host_bridge = to_pci_host_bridge(bus->bridge); - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) - host_bridge->native_pcie_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) - host_bridge->native_shpc_hotplug = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) - host_bridge->native_aer = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) - host_bridge->native_pme = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) - host_bridge->native_ltr = 0; - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) - host_bridge->native_dpc = 0; + if (!pcie_ports_native) { + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) + host_bridge->native_pcie_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) + host_bridge->native_shpc_hotplug = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) + host_bridge->native_aer = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL)) + host_bridge->native_pme = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) + host_bridge->native_ltr = 0; + if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) + host_bridge->native_dpc = 0; + } When the user boots with "pcie_ports=native", should we evaluate _OSC at all? Since pcie_ports="native" does not override OSC_PCI_SHPC_NATIVE_HP_CONTROL, I think we should still evaluate _OSC. Also firmware might still wants to know about supported features during _OSC call. It seems confusing to say "OK, Mr. Firmware, here are the features we want to use", and then turn around and use them all regardless of what the platform said. Why not just ignore the firmware completely and go ahead and use everything?| AFAIK, for PCIe features, It should functionally make no difference (evaluating _OSC vs not). But, IMO, its useful if we print some warnings if firmware denies some of the PCIe feature control. I can't remember if there's a reason we need to call negotiate_os_control() so early and then hang on to the results until we get here: acpi_pci_root_add negotiate_os_control <--- eval _OSC pci_acpi_scan_root acpi_pci_root_create pci_create_root_bus if (!(root->osc_control_set & ...)) <--- use results host_bridge->native_... = 0; I think it would be a lot simpler if we could do the _OSC negotiation right here where we need most of the results. It would be even better if we could update the host_bridge->native_... items directly inside negotiate_os_control() so we wouldn't have to hang onto the osc_control_set mask. Makes sense. I also don't see any one use _OSC negotiated results until pci_create_root_bus(). But I think this should be a seperate patch. /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index bf779f291f15..5fc999bf6f1b 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -255,7 +255,7 @@ static bool pme_is_native(struct pcie_device *dev) const struct pci_host_bridge *host; host = pci_find_host_bridge(dev->port->bus); - return pcie_ports_native || host->native_pme; +
Re: [PATCH] net: sky2: switch from 'pci_' to 'dma_' API
On Sat, 2020-07-11 at 22:49 +0200, Christophe JAILLET wrote: > The wrappers in include/linux/pci-dma-compat.h should go away. why?
Re: [PATCH] pinctrl: initialise nsp-mux earlier.
On Sat, Jul 11, 2020 at 11:09 PM Florian Fainelli wrote: > On 7/11/2020 2:07 PM, Linus Walleij wrote: > > I never got an updated patch. My last message was: > > > >>> so you mean something like this? > >>> > >>> if (err == -EPROBE_DEFER) > >>> dev_info(dev, "deferring probe\n") > >>> else > >>> dev_err(dev, "... failed to register\n") > >> > >> Yes exactly. > > > > Patches welcome :D > > Not sure how useful the dev_info(dev, "deferring probe\n") is nowadays > given that the device driver core will show which devices are on the > probe deferral list, maybe we can turn this into a dev_dbg() instead? Oh right. Yeah that sounds right, then we can see that it's the GPIO core bailing and deferring it when we turn on debug. Yours, Linus Walleij
Re: [PATCH 1/3] gpio: mxc: Support module build
On Wed, Jul 8, 2020 at 1:28 AM Anson Huang wrote: > subsys_initcall(gpio_mxc_init); > + > +MODULE_AUTHOR("Shawn Guo "); > +MODULE_DESCRIPTION("i.MX GPIO Driver"); > +MODULE_LICENSE("GPL"); You are making this modualrizable but keeping the subsys_initcall(), which doesn't make very much sense. It is obviously not necessary to do this probe at subsys_initcall() time, right? Take this opportunity to convert the driver to use module_platform_driver() as well. Yours, Linus Walleij
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3002:63: sparse: sparse: incorrect type in argument 2 (different address spaces)
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1df0d8960499e58963fd6c8ac75e544f2b417b29 commit: 670d0a4b10704667765f7d18f7592993d02783aa sparse: use identifiers to define address spaces date: 3 weeks ago config: i386-randconfig-s002-20200712 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-37-gc9676a3b-dirty git checkout 670d0a4b10704667765f7d18f7592993d02783aa # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) drivers/net/wireless/intel/iwlwifi/mvm/..//fw/file.h:330:19: sparse: sparse: mixed bitwiseness drivers/net/wireless/intel/iwlwifi/mvm/..//fw/file.h:484:19: sparse: sparse: mixed bitwiseness >> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3002:63: sparse: sparse: >> incorrect type in argument 2 (different address spaces) @@ expected >> unsigned char const [usertype] *ies @@ got unsigned char const [noderef] >> __rcu * @@ drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3002:63: sparse: expected unsigned char const [usertype] *ies >> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3002:63: sparse: got >> unsigned char const [noderef] __rcu * drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3003:38: sparse: sparse: dereference of noderef expression drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:3003:38: sparse: sparse: dereference of noderef expression vim +3002 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c 4f58121dc40a1d Ilan Peer 2019-05-19 2994 4f58121dc40a1d Ilan Peer 2019-05-19 2995 static void iwl_mvm_check_he_obss_narrow_bw_ru_iter(struct wiphy *wiphy, 4f58121dc40a1d Ilan Peer 2019-05-19 2996 struct cfg80211_bss *bss, 4f58121dc40a1d Ilan Peer 2019-05-19 2997 void *_data) 4f58121dc40a1d Ilan Peer 2019-05-19 2998 { 4f58121dc40a1d Ilan Peer 2019-05-19 2999 struct iwl_mvm_he_obss_narrow_bw_ru_data *data = _data; 4f58121dc40a1d Ilan Peer 2019-05-19 3000 const struct element *elem; 4f58121dc40a1d Ilan Peer 2019-05-19 3001 4f58121dc40a1d Ilan Peer 2019-05-19 @3002 elem = cfg80211_find_elem(WLAN_EID_EXT_CAPABILITY, bss->ies->data, 4f58121dc40a1d Ilan Peer 2019-05-19 3003 bss->ies->len); 4f58121dc40a1d Ilan Peer 2019-05-19 3004 4f58121dc40a1d Ilan Peer 2019-05-19 3005 if (!elem || elem->datalen < 10 || 4f58121dc40a1d Ilan Peer 2019-05-19 3006 !(elem->data[10] & 4f58121dc40a1d Ilan Peer 2019-05-19 3007 WLAN_EXT_CAPA10_OBSS_NARROW_BW_RU_TOLERANCE_SUPPORT)) { 4f58121dc40a1d Ilan Peer 2019-05-19 3008 data->tolerated = false; 4f58121dc40a1d Ilan Peer 2019-05-19 3009 } 4f58121dc40a1d Ilan Peer 2019-05-19 3010 } 4f58121dc40a1d Ilan Peer 2019-05-19 3011 :: The code at line 3002 was first introduced by commit :: 4f58121dc40a1d5dd2f630a5ec4dac5afa1ce3f4 iwlwifi: mvm: Block 26-tone RU OFDMA transmissions :: TO: Ilan Peer :: CC: Luca Coelho --- 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 v4 0/2] pinctrl: single: support #pinctrl-cells = 2
On Tue, Jul 7, 2020 at 1:02 PM Drew Fustini wrote: > Which repo/branch is the best for me to use if I am going to be > posting any further dts patches? Mine, pinctrl devel branch during this (v5.9) cycle I suppose. Yours, Linus Walleij
Re: [PATCH V2] pinctrl: qcom: ipq8074: route gpio interrupts to APPS
On Tue, Jul 7, 2020 at 9:40 AM Kathiravan T wrote: > set target proc as APPS to route the gpio interrupts to APPS > > Co-developed-by: Rajkumar Ayyasamy > Signed-off-by: Rajkumar Ayyasamy > Signed-off-by: Kathiravan T Patch applied. Yours, Linus Walleij
Re: [PATCH v2] pinctrl: nsp: Set irq handler based on trig type
On Fri, Jul 3, 2020 at 3:18 AM Mark Tomlinson wrote: > Rather than always using handle_simple_irq() as the gpio_irq_chip > handler, set a more appropriate handler based on the IRQ trigger type > requested. This is important for level triggered interrupts which need > to be masked during handling. Also, fix the interrupt acknowledge so > that it clears only one interrupt instead of all interrupts which are > currently active. Finally there is no need to clear the interrupt during > the interrupt handler, since the edge-triggered handler will do that for > us. > > Signed-off-by: Mark Tomlinson > --- > Changes in v2: > - Don't perform unnecessary acks. Patch applied. Yours, Linus Walleij
Re: [PATCH] pinctrl: initialise nsp-mux earlier.
On 7/11/2020 2:07 PM, Linus Walleij wrote: > On Wed, Jul 1, 2020 at 6:44 AM Florian Fainelli wrote: >> On 6/30/2020 9:37 PM, Mark Tomlinson wrote: > >>> That was one of my thoughts too. I found someone had tried that >>> earlier, but it was rejected: >>> >>> >>> https://patchwork.ozlabs.org/project/linux-gpio/patch/1516566774-1786-1-git-send-email-da...@lechnology.com/ >> >> clk or reset APIs do not complain loudly on EPROBE_DEFER, it seems to me >> that GPIO should follow here. Also, it does look like Linus was in >> agreement in the end, not sure why it was not applied though. > > I never got an updated patch. My last message was: > >>> so you mean something like this? >>> >>> if (err == -EPROBE_DEFER) >>> dev_info(dev, "deferring probe\n") >>> else >>> dev_err(dev, "... failed to register\n") >> >> Yes exactly. > > Patches welcome :D Not sure how useful the dev_info(dev, "deferring probe\n") is nowadays given that the device driver core will show which devices are on the probe deferral list, maybe we can turn this into a dev_dbg() instead? -- Florian
Re: [PATCH] pinctrl: initialise nsp-mux earlier.
On Wed, Jul 1, 2020 at 6:44 AM Florian Fainelli wrote: > On 6/30/2020 9:37 PM, Mark Tomlinson wrote: > > That was one of my thoughts too. I found someone had tried that > > earlier, but it was rejected: > > > > > > https://patchwork.ozlabs.org/project/linux-gpio/patch/1516566774-1786-1-git-send-email-da...@lechnology.com/ > > clk or reset APIs do not complain loudly on EPROBE_DEFER, it seems to me > that GPIO should follow here. Also, it does look like Linus was in > agreement in the end, not sure why it was not applied though. I never got an updated patch. My last message was: >> so you mean something like this? >> >> if (err == -EPROBE_DEFER) >> dev_info(dev, "deferring probe\n") >> else >> dev_err(dev, "... failed to register\n") > > Yes exactly. Patches welcome :D Yours, Linus Walleij
drivers/net/ethernet/netronome/nfp/crypto/tls.c:477:18: warning: variable 'ipv6h' set but not used
Hi Jakub, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1df0d8960499e58963fd6c8ac75e544f2b417b29 commit: 6a35ddc5445a8291ced6247a67977e110275acde nfp: tls: implement the stream sync RX resync date: 7 months ago config: i386-randconfig-s002-20200712 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-37-gc9676a3b-dirty git checkout 6a35ddc5445a8291ced6247a67977e110275acde # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/net/ethernet/netronome/nfp/crypto/tls.c: In function 'nfp_net_tls_rx_resync_req': >> drivers/net/ethernet/netronome/nfp/crypto/tls.c:477:18: warning: variable >> 'ipv6h' set but not used [-Wunused-but-set-variable] 477 | struct ipv6hdr *ipv6h; | ^ vim +/ipv6h +477 drivers/net/ethernet/netronome/nfp/crypto/tls.c 470 471 int nfp_net_tls_rx_resync_req(struct net_device *netdev, 472struct nfp_net_tls_resync_req *req, 473void *pkt, unsigned int pkt_len) 474 { 475 struct nfp_net *nn = netdev_priv(netdev); 476 struct nfp_net_tls_offload_ctx *ntls; > 477 struct ipv6hdr *ipv6h; 478 struct tcphdr *th; 479 struct iphdr *iph; 480 struct sock *sk; 481 __be32 tcp_seq; 482 int err; 483 484 iph = pkt + req->l3_offset; 485 ipv6h = pkt + req->l3_offset; 486 th = pkt + req->l4_offset; 487 488 if ((u8 *)[1] > (u8 *)pkt + pkt_len) { 489 netdev_warn_once(netdev, "invalid TLS RX resync request (l3_off: %hhu l4_off: %hhu pkt_len: %u)\n", 490 req->l3_offset, req->l4_offset, pkt_len); 491 err = -EINVAL; 492 goto err_cnt_ign; 493 } 494 495 switch (iph->version) { 496 case 4: 497 sk = inet_lookup_established(dev_net(netdev), _hashinfo, 498 iph->saddr, th->source, iph->daddr, 499 th->dest, netdev->ifindex); 500 break; 501 #if IS_ENABLED(CONFIG_IPV6) 502 case 6: 503 sk = __inet6_lookup_established(dev_net(netdev), _hashinfo, 504 >saddr, th->source, 505 >daddr, ntohs(th->dest), 506 netdev->ifindex, 0); 507 break; 508 #endif 509 default: 510 netdev_warn_once(netdev, "invalid TLS RX resync request (l3_off: %hhu l4_off: %hhu ipver: %u)\n", 511 req->l3_offset, req->l4_offset, iph->version); 512 err = -EINVAL; 513 goto err_cnt_ign; 514 } 515 516 err = 0; 517 if (!sk) 518 goto err_cnt_ign; 519 if (!tls_is_sk_rx_device_offloaded(sk) || 520 sk->sk_shutdown & RCV_SHUTDOWN) 521 goto err_put_sock; 522 523 ntls = tls_driver_ctx(sk, TLS_OFFLOAD_CTX_DIR_RX); 524 /* some FW versions can't report the handle and report 0s */ 525 if (memchr_inv(>fw_handle, 0, sizeof(req->fw_handle)) && 526 memcmp(>fw_handle, >fw_handle, sizeof(ntls->fw_handle))) 527 goto err_put_sock; 528 529 /* copy to ensure alignment */ 530 memcpy(_seq, >tcp_seq, sizeof(tcp_seq)); 531 tls_offload_rx_resync_request(sk, tcp_seq); 532 atomic_inc(>ktls_rx_resync_req); 533 534 sock_gen_put(sk); 535 return 0; 536 537 err_put_sock: 538 sock_gen_put(sk); 539 err_cnt_ign: 540 atomic_inc(>ktls_rx_resync_ign); 541 return err; 542 } 543 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: Linux kernel in-tree Rust support
On Fri, Jul 10, 2020 at 04:54:11PM -0700, Linus Torvalds wrote: > On Fri, Jul 10, 2020 at 3:59 PM Josh Triplett wrote: > > As I recall, Greg's biggest condition for initial introduction of this > > was to do the same kind of "turn this Kconfig option on and turn an > > option under it off" trick that LTO uses, so that neither "make > > allnoconfig" nor "make allyesconfig" would require Rust until we've had > > plenty of time to experiment with it. > > No, please make it a "is rust available" automatic config option. The > exact same way we already do the compiler versions and check for > various availability of compiler flags at config time. That sounds even better, and will definitely allow for more testing. We just need to make sure that any kernel CI infrastructure tests that right away, then, so that failures don't get introduced by a patch from someone without a Rust toolchain and not noticed until someone with a Rust toolchain tests it. - Josh
Re: [PATCH RFC] leds: Add support for per-LED device triggers
Hello Pavel, On Sat, Jul 11, 2020 at 12:04:09PM +0200, Pavel Machek wrote: > Hi! > > > Some LED controllers may come with an internal HW triggering mechanism > > for the LED and an ability to switch between user control of the LED, > > or the internal control. One such example is AXP20X PMIC, that allows > > wither for user control of the LED, or for internal control based on > > the state of the battery charger. > > > > Add support for registering per-LED device trigger. > > > > Names of private triggers need to be globally unique, but may clash > > with other private triggers. This is enforced during trigger > > registration. Developers can register private triggers just like > > the normal triggers, by setting private_led to a classdev > > of the LED the trigger is associated with. > > What about this? Should address Marek's concerns about resource use... What concerns? Marek's concerns seem to be about case where we register a trigger for (each led * self-working configuration) which I admit can be quite a lot of triggers if there are many functions. But that's not my proposal. My proposal is to only register on trigger per LED at most. So on my system that's 1 extra trigger and on Marek's system that'd be 48 new triggers. Neither seems like a meaningful problem from resource use perspective. regards, o. > Best regards, > Pavel > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > index 79e30d2cb7a5..e8333675959c 100644 > --- a/drivers/leds/led-triggers.c > +++ b/drivers/leds/led-triggers.c > @@ -27,6 +27,12 @@ LIST_HEAD(trigger_list); > > /* Used by LED Class */ > > +static inline bool > +trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig) > +{ > + return !trig->trigger_type || trig->trigger_type == > led_cdev->trigger_type; > +} > + > ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, > struct bin_attribute *bin_attr, char *buf, > loff_t pos, size_t count) > @@ -50,7 +56,8 @@ ssize_t led_trigger_write(struct file *filp, struct kobject > *kobj, > > down_read(_list_lock); > list_for_each_entry(trig, _list, next_trig) { > - if (sysfs_streq(buf, trig->name)) { > + if (sysfs_streq(buf, trig->name) && > + trigger_relevant(led_cdev, trig)) { > down_write(_cdev->trigger_lock); > led_trigger_set(led_cdev, trig); > up_write(_cdev->trigger_lock); > @@ -96,6 +103,9 @@ static int led_trigger_format(char *buf, size_t size, > bool hit = led_cdev->trigger && > !strcmp(led_cdev->trigger->name, trig->name); > > + if (!trigger_relevant(led_cdev, trig)) > + continue; > + > len += led_trigger_snprintf(buf + len, size - len, > " %s%s%s", hit ? "[" : "", > trig->name, hit ? "]" : ""); > @@ -243,7 +253,8 @@ void led_trigger_set_default(struct led_classdev > *led_cdev) > down_read(_list_lock); > down_write(_cdev->trigger_lock); > list_for_each_entry(trig, _list, next_trig) { > - if (!strcmp(led_cdev->default_trigger, trig->name)) { > + if (!strcmp(led_cdev->default_trigger, trig->name) && > + trigger_relevant(led_cdev, trig)) { > led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER; > led_trigger_set(led_cdev, trig); > break; > @@ -280,7 +291,8 @@ int led_trigger_register(struct led_trigger *trig) > down_write(_list_lock); > /* Make sure the trigger's name isn't already in use */ > list_for_each_entry(_trig, _list, next_trig) { > - if (!strcmp(_trig->name, trig->name)) { > + if (!strcmp(_trig->name, trig->name) && > + (!_trig->private_led || _trig->private_led == > trig->private_led)) { > up_write(_list_lock); > return -EEXIST; > } > diff --git a/include/linux/leds.h b/include/linux/leds.h > index 2451962d1ec5..cba52714558f 100644 > --- a/include/linux/leds.h > +++ b/include/linux/leds.h > @@ -57,6 +57,10 @@ struct led_init_data { > bool devname_mandatory; > }; > > +struct led_hw_trigger_type { > + int dummy; > +} > + > struct led_classdev { > const char *name; > enum led_brightness brightness; > @@ -150,6 +154,8 @@ struct led_classdev { > > /* Ensures consistent access to the LED Flash Class device */ > struct mutexled_access; > + > + struct led_hw_trigger_type *trigger_type; > }; > > /** > @@ -345,6 +351,9 @@ struct led_trigger { > int (*activate)(struct led_classdev *led_cdev); > void
[PATCH] net: sky2: switch from 'pci_' to 'dma_' API
The wrappers in include/linux/pci-dma-compat.h should go away. The patch has been generated with the coccinelle script below and has been hand modified to replace GPF_ with a correct flag. It has been compile tested. When memory is allocated in 'sky2_alloc_buffers()', GFP_KERNEL can be used because some other memory allocations in the same function already use this flag. When memory is allocated in 'sky2_probe()', GFP_KERNEL can be used because another memory allocations in the same function already uses this flag. @@ @@ -PCI_DMA_BIDIRECTIONAL +DMA_BIDIRECTIONAL @@ @@ -PCI_DMA_TODEVICE +DMA_TO_DEVICE @@ @@ -PCI_DMA_FROMDEVICE +DMA_FROM_DEVICE @@ @@ -PCI_DMA_NONE +DMA_NONE @@ expression e1, e2, e3; @@ -pci_alloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3; @@ -pci_zalloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3, e4; @@ -pci_free_consistent(e1, e2, e3, e4) +dma_free_coherent(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_single(e1, e2, e3, e4) +dma_map_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_single(e1, e2, e3, e4) +dma_unmap_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4, e5; @@ -pci_map_page(e1, e2, e3, e4, e5) +dma_map_page(>dev, e2, e3, e4, e5) @@ expression e1, e2, e3, e4; @@ -pci_unmap_page(e1, e2, e3, e4) +dma_unmap_page(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_sg(e1, e2, e3, e4) +dma_map_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_sg(e1, e2, e3, e4) +dma_unmap_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_cpu(e1, e2, e3, e4) +dma_sync_single_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_device(e1, e2, e3, e4) +dma_sync_single_for_device(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_cpu(e1, e2, e3, e4) +dma_sync_sg_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_device(e1, e2, e3, e4) +dma_sync_sg_for_device(>dev, e2, e3, e4) @@ expression e1, e2; @@ -pci_dma_mapping_error(e1, e2) +dma_mapping_error(>dev, e2) @@ expression e1, e2; @@ -pci_set_dma_mask(e1, e2) +dma_set_mask(>dev, e2) @@ expression e1, e2; @@ -pci_set_consistent_dma_mask(e1, e2) +dma_set_coherent_mask(>dev, e2) Signed-off-by: Christophe JAILLET --- If needed, see post from Christoph Hellwig on the kernel-janitors ML: https://marc.info/?l=kernel-janitors=158745678307186=4 --- drivers/net/ethernet/marvell/sky2.c | 89 +++-- 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c index fe54764caea9..adb1a9c19505 100644 --- a/drivers/net/ethernet/marvell/sky2.c +++ b/drivers/net/ethernet/marvell/sky2.c @@ -1209,8 +1209,9 @@ static int sky2_rx_map_skb(struct pci_dev *pdev, struct rx_ring_info *re, struct sk_buff *skb = re->skb; int i; - re->data_addr = pci_map_single(pdev, skb->data, size, PCI_DMA_FROMDEVICE); - if (pci_dma_mapping_error(pdev, re->data_addr)) + re->data_addr = dma_map_single(>dev, skb->data, size, + DMA_FROM_DEVICE); + if (dma_mapping_error(>dev, re->data_addr)) goto mapping_error; dma_unmap_len_set(re, data_size, size); @@ -1229,13 +1230,13 @@ static int sky2_rx_map_skb(struct pci_dev *pdev, struct rx_ring_info *re, map_page_error: while (--i >= 0) { - pci_unmap_page(pdev, re->frag_addr[i], + dma_unmap_page(>dev, re->frag_addr[i], skb_frag_size(_shinfo(skb)->frags[i]), - PCI_DMA_FROMDEVICE); + DMA_FROM_DEVICE); } - pci_unmap_single(pdev, re->data_addr, dma_unmap_len(re, data_size), -PCI_DMA_FROMDEVICE); + dma_unmap_single(>dev, re->data_addr, +dma_unmap_len(re, data_size), DMA_FROM_DEVICE); mapping_error: if (net_ratelimit()) @@ -1249,13 +1250,13 @@ static void sky2_rx_unmap_skb(struct pci_dev *pdev, struct rx_ring_info *re) struct sk_buff *skb = re->skb; int i; - pci_unmap_single(pdev, re->data_addr, dma_unmap_len(re, data_size), -PCI_DMA_FROMDEVICE); + dma_unmap_single(>dev, re->data_addr, +dma_unmap_len(re, data_size), DMA_FROM_DEVICE); for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) - pci_unmap_page(pdev, re->frag_addr[i], - skb_frag_size(_shinfo(skb)->frags[i]), - PCI_DMA_FROMDEVICE); + dma_unmap_page(>dev, re->frag_addr[i], +
thinkpad x60: oops in eb_relocate_dma in next-20200710
Hi! I attempted to suspend x60, but it did not work well... Machine is too messed up to pull more debug info from it :-(. Best regards, Pavel [11645.369495] wlan0: RX AssocResp from 5c:f4:ab:10:d2:bb (capab=0x411 status=0 aid=2) [11645.373180] wlan0: associated [12366.990398] BUG: unable to handle page fault for address: f8e01000 [12366.990406] #PF: supervisor write access in kernel mode [12366.990409] #PF: error_code(0x0002) - not-present page [12366.990412] *pdpt = 2a497001 *pde = [12366.990418] Oops: 0002 [#1] PREEMPT SMP PTI [12366.990424] CPU: 0 PID: 3016 Comm: Xorg Not tainted 5.8.0-rc4-next-20200710+ #129 [12366.990427] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW (2.19 ) 03/31/2011 [12366.990436] EIP: eb_relocate_vma+0xdee/0xf50 [12366.990441] Code: 85 c0 fd ff ff ed ff ff ff c7 85 c4 fd ff ff ff ff ff ff 8b 85 c0 fd ff ff e9 33 f7 ff ff 8d b6 00 00 00 00 8b 85 d0 fd ff ff 03 01 00 40 10 89 43 04 8b 85 dc fd ff ff 89 43 08 e9 2c f6 ff [12366.990445] EAX: 01246134 EBX: f8e01000 ECX: 013b9000 EDX: [12366.990448] ESI: eee57cbc EDI: eee57aa4 EBP: eee57c54 ESP: eee579ec [12366.990452] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210246 [12366.990456] CR0: 80050033 CR2: f8e01000 CR3: 3023 CR4: 06b0 [12366.990459] Call Trace: [12366.990469] ? shmem_getpage_gfp.isra.0+0x3ba/0x820 [12366.990477] i915_gem_do_execbuffer+0xa7b/0x2730 [12366.990479] ? intel_runtime_pm_put_unchecked+0xd/0x10 [12366.990479] ? i915_gem_gtt_pwrite_fast+0xf6/0x520 [12366.990479] ? __lock_acquire.isra.0+0x223/0x500 [12366.990479] ? cache_alloc_debugcheck_after+0x151/0x180 [12366.990479] ? kvmalloc_node+0x69/0x80 [12366.990479] ? __kmalloc+0x92/0x120 [12366.990479] ? kvmalloc_node+0x69/0x80 [12366.990479] i915_gem_execbuffer2_ioctl+0x1b9/0x3a0 [12366.990479] ? drm_dev_exit+0xb/0x40 [12366.990479] ? i915_gem_execbuffer_ioctl+0x2a0/0x2a0 [12366.990479] drm_ioctl_kernel+0x91/0xe0 [12366.990479] ? i915_gem_execbuffer_ioctl+0x2a0/0x2a0 [12366.990479] drm_ioctl+0x1fd/0x371 [12366.990479] ? i915_gem_execbuffer_ioctl+0x2a0/0x2a0 [12366.990479] ? posix_get_monotonic_timespec+0x1d/0x80 [12366.990479] ? drm_ioctl_kernel+0xe0/0xe0 [12366.990479] ksys_ioctl+0x143/0x7d0 [12366.990479] ? ktime_get_ts64+0x77/0x1d0 [12366.990479] ? _copy_to_user+0x21/0x30 [12366.990479] ? __prepare_exit_to_usermode+0xe5/0x110 [12366.990479] __ia32_sys_ioctl+0x10/0x12 [12366.990479] do_syscall_32_irqs_on+0x3a/0xf0 [12366.990479] do_int80_syscall_32+0x9/0x20 [12366.990479] entry_INT80_32+0x116/0x116 [12366.990479] EIP: 0xb7f94092 [12366.990479] Code: Bad RIP value. [12366.990479] EAX: ffda EBX: 000a ECX: c0406469 EDX: bf82313c [12366.990479] ESI: b7382000 EDI: c0406469 EBP: 000a ESP: bf8230b4 [12366.990479] DS: 007b ES: 007b FS: GS: 0033 SS: 007b EFLAGS: 00200296 [12366.990479] ? dev_proc_net_exit+0x10/0x40 [12366.990479] ? asm_exc_nmi+0xcc/0x2bc [12366.990479] Modules linked in: [12366.990479] CR2: f8e01000 [12366.990479] ---[ end trace d1eedfdf3b328098 ]--- [12366.990479] EIP: eb_relocate_vma+0xdee/0xf50 [12366.990479] Code: 85 c0 fd ff ff ed ff ff ff c7 85 c4 fd ff ff ff ff ff ff 8b 85 c0 fd ff ff e9 33 f7 ff ff 8d b6 00 00 00 00 8b 85 d0 fd ff ff 03 01 00 40 10 89 43 04 8b 85 dc fd ff ff 89 43 08 e9 2c f6 ff [12366.990479] EAX: 01246134 EBX: f8e01000 ECX: 013b9000 EDX: [12366.990479] ESI: eee57cbc EDI: eee57aa4 EBP: eee57c54 ESP: eee579ec [12366.990479] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210246 [12366.990479] CR0: 80050033 CR2: f8e01000 CR3: 3023 CR4: 06b0 [12366.996393] BUG: unable to handle page fault for address: f8e03038 [12366.996399] #PF: supervisor write access in kernel mode [12366.996402] #PF: error_code(0x0002) - not-present page [12366.996405] *pdpt = 339a4001 *pde = [12366.996411] Oops: 0002 [#2] PREEMPT SMP PTI [12366.996417] CPU: 0 PID: 3016 Comm: Xorg Tainted: G D 5.8.0-rc4-next-20200710+ #129 [12366.996420] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW (2.19 ) 03/31/2011 [12366.996429] EIP: n_tty_open+0x26/0x80 [12366.996434] Code: 00 00 00 90 55 89 e5 56 53 89 c3 b8 f0 22 00 00 e8 ef 68 cb ff 85 c0 74 62 89 c6 a1 00 2d 26 c5 b9 88 e7 6b c5 ba bd 9c 11 c5 <89> 46 38 8d 86 58 22 00 00 e8 9c 5e c0 ff 8d 86 a4 22 00 00 b9 80 [12366.996438] EAX: 002e07b0 EBX: f4a4bc00 ECX: c56be788 EDX: c5119cbd [12366.996441] ESI: f8e03000 EDI: EBP: eee57ee4 ESP: eee57edc [12366.996444] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210286 [12366.996448] CR0: 80050033 CR2: f8e03038 CR3: 33b98000 CR4: 06b0 [12366.996451] Call Trace: [12366.996457] tty_ldisc_open.isra.0+0x23/0x40 [12366.996461] tty_ldisc_reinit+0x99/0xe0 [12366.996465] tty_ldisc_hangup+0xc4/0x1e0 [12366.996470] __tty_hangup.part.0+0x13f/0x250 [12366.996476] tty_vhangup_session+0x11/0x20 [12366.996481]
[PATCH v2] hwmon: (drivetemp) Avoid SCT usage on Toshiba DT01ACA family drives
It has been observed that Toshiba DT01ACA family drives have WRITE FPDMA QUEUED command timeouts and sometimes just freeze until power-cycled under heavy write loads when their temperature is getting polled in SCT mode. The SMART mode seems to be fine, though. Let's make sure we don't use SCT mode for these drives then. While only the 3 TB model was actually caught exhibiting the problem let's play safe here to avoid data corruption and extend the ban to the whole family. Fixes: 5b46903d8bf3 ("hwmon: Driver for disk and solid state drives with temperature sensors") Cc: sta...@vger.kernel.org Signed-off-by: Maciej S. Szmigiero --- Notes: This behavior was observed on two different DT01ACA3 drives. Usually, a series of queued WRITE FPDMA QUEUED commands just time out, but sometimes the whole drive freezes. Merely disconnecting and reconnecting SATA interface cable then does not unfreeze the drive. One has to disconnect and reconnect the drive power connector for the drive to be detected again (suggesting the drive firmware itself has crashed). This only happens when the drive temperature is polled very often (like every second), so occasional SCT usage via smartmontools is probably safe. Changes from v1: 'SCT blacklist' -> 'SCT avoid models' drivers/hwmon/drivetemp.c | 37 + 1 file changed, 37 insertions(+) diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c index 0d4f3d97ffc6..da9cfcbecc96 100644 --- a/drivers/hwmon/drivetemp.c +++ b/drivers/hwmon/drivetemp.c @@ -285,6 +285,36 @@ static int drivetemp_get_scttemp(struct drivetemp_data *st, u32 attr, long *val) return err; } +static const char * const sct_avoid_models[] = { +/* + * These drives will have WRITE FPDMA QUEUED command timeouts and sometimes just + * freeze until power-cycled under heavy write loads when their temperature is + * getting polled in SCT mode. The SMART mode seems to be fine, though. + * + * While only the 3 TB model was actually caught exhibiting the problem + * let's play safe here to avoid data corruption and ban the whole family. + */ + "TOSHIBA DT01ACA0", + "TOSHIBA DT01ACA1", + "TOSHIBA DT01ACA2", + "TOSHIBA DT01ACA3", +}; + +static bool drivetemp_sct_avoid(struct drivetemp_data *st) +{ + struct scsi_device *sdev = st->sdev; + unsigned int ctr; + + if (!sdev->model) + return false; + + for (ctr = 0; ctr < ARRAY_SIZE(sct_avoid_models); ctr++) + if (strncmp(sdev->model, sct_avoid_models[ctr], 16) == 0) + return true; + + return false; +} + static int drivetemp_identify_sata(struct drivetemp_data *st) { struct scsi_device *sdev = st->sdev; @@ -326,6 +356,13 @@ static int drivetemp_identify_sata(struct drivetemp_data *st) /* bail out if this is not a SATA device */ if (!is_ata || !is_sata) return -ENODEV; + + if (have_sct && drivetemp_sct_avoid(st)) { + dev_notice(>sdev_gendev, + "will avoid using SCT for temperature monitoring\n"); + have_sct = false; + } + if (!have_sct) goto skip_sct;
[PATCH] net: skge: switch from 'pci_' to 'dma_' API
The wrappers in include/linux/pci-dma-compat.h should go away. The patch has been generated with the coccinelle script below and has been hand modified to replace GPF_ with a correct flag. It has been compile tested. When memory is allocated in 'skge_up()', GFP_KERNEL can be used because some other memory allocations done a few lines below in 'skge_ring_alloc()' already use this flag. @@ @@ -PCI_DMA_BIDIRECTIONAL +DMA_BIDIRECTIONAL @@ @@ -PCI_DMA_TODEVICE +DMA_TO_DEVICE @@ @@ -PCI_DMA_FROMDEVICE +DMA_FROM_DEVICE @@ @@ -PCI_DMA_NONE +DMA_NONE @@ expression e1, e2, e3; @@ -pci_alloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3; @@ -pci_zalloc_consistent(e1, e2, e3) +dma_alloc_coherent(>dev, e2, e3, GFP_) @@ expression e1, e2, e3, e4; @@ -pci_free_consistent(e1, e2, e3, e4) +dma_free_coherent(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_single(e1, e2, e3, e4) +dma_map_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_single(e1, e2, e3, e4) +dma_unmap_single(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4, e5; @@ -pci_map_page(e1, e2, e3, e4, e5) +dma_map_page(>dev, e2, e3, e4, e5) @@ expression e1, e2, e3, e4; @@ -pci_unmap_page(e1, e2, e3, e4) +dma_unmap_page(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_map_sg(e1, e2, e3, e4) +dma_map_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_unmap_sg(e1, e2, e3, e4) +dma_unmap_sg(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_cpu(e1, e2, e3, e4) +dma_sync_single_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_single_for_device(e1, e2, e3, e4) +dma_sync_single_for_device(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_cpu(e1, e2, e3, e4) +dma_sync_sg_for_cpu(>dev, e2, e3, e4) @@ expression e1, e2, e3, e4; @@ -pci_dma_sync_sg_for_device(e1, e2, e3, e4) +dma_sync_sg_for_device(>dev, e2, e3, e4) @@ expression e1, e2; @@ -pci_dma_mapping_error(e1, e2) +dma_mapping_error(>dev, e2) @@ expression e1, e2; @@ -pci_set_dma_mask(e1, e2) +dma_set_mask(>dev, e2) @@ expression e1, e2; @@ -pci_set_consistent_dma_mask(e1, e2) +dma_set_coherent_mask(>dev, e2) Signed-off-by: Christophe JAILLET --- If needed, see post from Christoph Hellwig on the kernel-janitors ML: https://marc.info/?l=kernel-janitors=158745678307186=4 --- drivers/net/ethernet/marvell/skge.c | 76 ++--- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c index 3c89206f18a7..869392867131 100644 --- a/drivers/net/ethernet/marvell/skge.c +++ b/drivers/net/ethernet/marvell/skge.c @@ -939,10 +939,10 @@ static int skge_rx_setup(struct skge_port *skge, struct skge_element *e, struct skge_rx_desc *rd = e->desc; dma_addr_t map; - map = pci_map_single(skge->hw->pdev, skb->data, bufsize, -PCI_DMA_FROMDEVICE); + map = dma_map_single(>hw->pdev->dev, skb->data, bufsize, +DMA_FROM_DEVICE); - if (pci_dma_mapping_error(skge->hw->pdev, map)) + if (dma_mapping_error(>hw->pdev->dev, map)) return -1; rd->dma_lo = lower_32_bits(map); @@ -990,10 +990,10 @@ static void skge_rx_clean(struct skge_port *skge) struct skge_rx_desc *rd = e->desc; rd->control = 0; if (e->skb) { - pci_unmap_single(hw->pdev, + dma_unmap_single(>pdev->dev, dma_unmap_addr(e, mapaddr), dma_unmap_len(e, maplen), -PCI_DMA_FROMDEVICE); +DMA_FROM_DEVICE); dev_kfree_skb(e->skb); e->skb = NULL; } @@ -2547,14 +2547,15 @@ static int skge_up(struct net_device *dev) rx_size = skge->rx_ring.count * sizeof(struct skge_rx_desc); tx_size = skge->tx_ring.count * sizeof(struct skge_tx_desc); skge->mem_size = tx_size + rx_size; - skge->mem = pci_alloc_consistent(hw->pdev, skge->mem_size, >dma); + skge->mem = dma_alloc_coherent(>pdev->dev, skge->mem_size, + >dma, GFP_KERNEL); if (!skge->mem) return -ENOMEM; BUG_ON(skge->dma & 7); if (upper_32_bits(skge->dma) != upper_32_bits(skge->dma + skge->mem_size)) { - dev_err(>pdev->dev, "pci_alloc_consistent region crosses 4G boundary\n"); + dev_err(>pdev->dev, "dma_alloc_coherent region crosses 4G boundary\n"); err = -EINVAL; goto free_pci_mem; } @@ -2625,7
Re: [Ksummit-discuss] [PATCH v3] CodingStyle: Inclusive Terminology
On Wed, Jul 08, 2020 at 11:14:27AM -0700, Dan Williams wrote: > Linux maintains a coding-style and its own idiomatic set of terminology. > Update the style guidelines to recommend replacements for the terms > master/slave and blacklist/whitelist. > > Link: > http://lore.kernel.org/r/159389297140.2210796.13590142254668787525.st...@dwillia2-desk3.amr.corp.intel.com > Acked-by: Randy Dunlap > Acked-by: Dave Airlie > Acked-by: SeongJae Park > Acked-by: Christian Brauner > Acked-by: James Bottomley > Reviewed-by: Mark Brown > Signed-off-by: Theodore Ts'o > Signed-off-by: Shuah Khan > Signed-off-by: Dan Carpenter > Signed-off-by: Kees Cook > Signed-off-by: Olof Johansson > Signed-off-by: Jonathan Corbet > Signed-off-by: Chris Mason > Signed-off-by: Greg Kroah-Hartman > Signed-off-by: Dan Williams Thank you for working on this, Dan! Reviewed-by: Josh Triplett
Re: [PATCH v29 00/16] Multicolor Framework v29
On Sat 2020-07-04 14:47:29, Pavel Machek wrote: > Hi! > > > This is the multi color LED framework. This framework presents clustered > > colored LEDs into an array and allows the user space to adjust the > > brightness > > of the cluster using a single file write. The individual colored LEDs > > intensities are controlled via a single file that is an array of LEDs > > > > Change to the LEDs Kconfig to fix dependencies on the LP55XX_COMMON. > > Added update to the u8500_defconfig > > Marek, would you be willing to look over this series? > > Dan, can we please get it in the order > > 1) fixes first > > 2) changes needed for multicolor but not depending on dt acks > > 3) dt changes > > 4) rest? Actually, one more request. I believe I won't be able to take at least some of the ARM: dts stuff... not everything is acked. Please put that last. Thank you, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH] MIPS: CI20: DTS: Correcting IW8103 Wifi binding
Hi Alexandre, > Am 06.07.2020 um 22:22 schrieb Alexandre GRIVEAUX : > > Le 06/07/2020 à 13:15, H. Nikolaus Schaller a écrit : >> Hi Alexandre, >> >>> Am 05.07.2020 um 12:32 schrieb agrive...@deutnet.info: >>> >>> From: Alexandre GRIVEAUX >>> >>> Use brcm,bcm4329-fmac instead of brcm,bcm4330-fmac. >>> >>> Signed-off-by: Alexandre GRIVEAUX >>> --- >>> arch/mips/boot/dts/ingenic/ci20.dts | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts >>> b/arch/mips/boot/dts/ingenic/ci20.dts >>> index 75f5bfbf2c37..82a1f126b778 100644 >>> --- a/arch/mips/boot/dts/ingenic/ci20.dts >>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts >>> @@ -116,8 +116,8 @@ >>> pinctrl-0 = <_mmc1>; >>> >>> brcmf: wifi@1 { >>> -/* reg = <4>;*/ >>> - compatible = "brcm,bcm4330-fmac"; >>> + reg = <1>; >>> + compatible = "brcm,bcm4329-fmac"; >>> vcc-supply = <_power>; >>> device-wakeup-gpios = < 9 GPIO_ACTIVE_HIGH>; >>> shutdown-gpios = < 7 GPIO_ACTIVE_LOW>; >> Do you have it working with a v5.8 kernel? >> >> I don't see any activity to detect the module or load firmware. >> >> Does it rely on some other patch? >> >> BR and thanks, >> Nikolaus >> > Hi Nikolaus > > > At this time the patch have been only "tested" for error will doing make: > > make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- olddefconfig && make > ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- -j8 && make ARCH=mips > CROSS_COMPILE=mipsel-linux-gnu- -j8 uImage > > > The .config come from creator-ci20 kernel 'config-3.18.3-ci20-1' > > > Even with the right DT > (Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt) > it's need some config with brcm enabled I gess. > > > I need to do some investigation will trying the uImage this week, > unfortunaly kernel developpement is not my main work, it's a hobby. No problem. For me, the CI20 is also a hobby project :) Here is some information about the CI20 WiFi and firmware: https://elinux.org/CI20_Hardware#WiFi.2FBT https://elinux.org/CI20_upstream#WiFi_firmware So to be it looks as if the compatible = "brcm,bcm4330-fmac"; is correct. But the reg = <4> or <1> is something we have to find out. BR, Nikolaus
Re: [PATCH v29 13/16] leds: lp5523: Update the lp5523 code to add multicolor brightness function
On Sat 2020-07-11 19:19:22, Jacek Anaszewski wrote: > On 7/11/20 5:57 PM, Pavel Machek wrote: > > Hi! > > > > > Add the multicolor brightness call back to support the multicolor > > > framework. This call back allows setting brightness on grouped channels > > > > Extra space before "brightness". > > And before "This". That one is intentional, I believe. https://www.independent.co.uk/life-style/gadgets-and-tech/news/one-space-or-two-spaces-after-a-full-stop-scientists-have-finally-found-the-answer-a8337646.html We are using fixed width fonts, so typewriter rules still apply here. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
[GIT PULL] RISC-V Fixes for 5.8-rc5 (ideally)
The following changes since commit dcb7fd82c75ee2d6e6f9d8cc71c52519ed52e258: Linux 5.8-rc4 (2020-07-05 16:20:22 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git tags/riscv-for-linus-5.8-rc5 for you to fetch changes up to 70ee5731a40b1f07f151e52c3c4ed27d70d4f9fe: riscv: Avoid kgdb.h including gdb_xml.h to solve unused-const-variable warning (2020-07-09 20:12:28 -0700) RISC-V Fixes for 5.8-rc5 (ideally) I have a few KGDB-related fixes that I'd like to target for 5.8-rc5. They're mostly fixes for build warnings, but there's also: * Support for the qSupported and qXfer packets, which are necessary to pass around GDB XML information which we need for the RISC-V GDB port to fully function. * Users can now select STRICT_KERNEL_RWX instead of forcing it on. I know it's a bit late for rc5, as these are not critical it's not a big deal if they don't make it in. Vincent Chen (5): kgdb: enable arch to support XML packet. riscv: enable the Kconfig prompt of STRICT_KERNEL_RWX riscv: Fix "no previous prototype" compile warning in kgdb.c file kgdb: Move the extern declaration kgdb_has_hit_break() to generic kgdb.h riscv: Avoid kgdb.h including gdb_xml.h to solve unused-const-variable warning arch/riscv/Kconfig | 2 ++ arch/riscv/include/asm/gdb_xml.h | 3 +-- arch/riscv/include/asm/kgdb.h| 5 +++-- arch/riscv/kernel/kgdb.c | 10 +- include/linux/kgdb.h | 12 kernel/debug/gdbstub.c | 13 + lib/Kconfig.kgdb | 5 + 7 files changed, 41 insertions(+), 9 deletions(-)
Re: [PATCH net-next] ptp: add debugfs support for IDT family of timing devices
On Fri, Jul 10, 2020 at 11:41:25AM -0400, min.li...@renesas.com wrote: > From: Min Li > > This patch is to add debugfs support for ptp_clockmatrix and ptp_idt82p33. > It will create a debugfs directory called idtptp{x} and x is the ptp index. > Three inerfaces are present, which are cmd, help and regs. help is read > only and will display a brief help message. regs is read only amd will show > all register values. You can get register dumps for free in debugfs if you use regmap. And i2c devices are well supported by regmap. Andrew
Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices
On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote: > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote: > > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat Jain wrote: > > > > When enabling ACS, enable translation blocking for external facing ports > > > > and untrusted devices. > > > > > > > > Signed-off-by: Rajat Jain > > > > --- > > > > v4: Add braces to avoid warning from kernel robot > > > > print warning for only external-facing devices. > > > > v3: print warning if ACS_TB not supported on external-facing/untrusted > > > > ports. > > > > Minor code comments fixes. > > > > v2: Commit log change > > > > > > > > drivers/pci/pci.c| 8 > > > > drivers/pci/quirks.c | 15 +++ > > > > 2 files changed, 23 insertions(+) > > > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > > index 73a8627822140..a5a6bea7af7ce 100644 > > > > --- a/drivers/pci/pci.c > > > > +++ b/drivers/pci/pci.c > > > > @@ -876,6 +876,14 @@ static void pci_std_enable_acs(struct pci_dev *dev) > > > > /* Upstream Forwarding */ > > > > ctrl |= (cap & PCI_ACS_UF); > > > > > > > > + /* Enable Translation Blocking for external devices */ > > > > + if (dev->external_facing || dev->untrusted) { > > > > + if (cap & PCI_ACS_TB) > > > > + ctrl |= PCI_ACS_TB; > > > > + else if (dev->external_facing) > > > > + pci_warn(dev, "ACS: No Translation Blocking on > > > > external-facing dev\n"); > > > > + } > > > > > > IIUC, this means that external devices can *never* use ATS and > > > can never cache translations. > > Yes, but it already exists today (and this patch doesn't change that): > 521376741b2c2 "PCI/ATS: Only enable ATS for trusted devices" > > IMHO any external device trying to send ATS traffic despite having ATS > disabled should count as a bad intent. And this patch is trying to > plug that loophole, by blocking the AT traffic from devices that we do > not expect to see AT from anyway. Thinking about this some more, I wonder if Linux should: - Explicitly disable ATS for every device at enumeration-time, e.g., in pci_init_capabilities(), - Enable PCI_ACS_TB for every device (not just external-facing or untrusted ones), - Disable PCI_ACS_TB for the relevant devices along the path only when enabling ATS. One nice thing about doing that is that the "untrusted" test would be only in pci_enable_ats(), and we wouldn't need one in pci_std_enable_acs(). It's possible BIOS gives us devices with ATS enabled, and this might break them, but that seems like something we'd want to find out about. Bjorn
Re: [PATCH 1/1] clk: qcom: smd: Add support for MSM8992/4 rpm clocks
On Wed 24 Jun 15:32 PDT 2020, Stephen Boyd wrote: > Quoting Konrad Dybcio (2020-06-24 08:09:18) > > I should also note that for quite some time a hack [1] > > has been needed on some platforms for the RPMCC to register. > > > > This includes 8992/94, 8956/76 and possibly many more. > > > > With that commit, RPMCC registers fine. > > > > What happens if that patch isn't applied? Does the system crash? Because > I'd rather not merge a patch in clk tree that causes the system to fail > to boot. The state machine code in the SMD implementation finds the RPM channel, but it's in a state that indicates that the remote side is still closing/cleaning up from when the bootloader had it open. The result is that we never probe the RPM driver. I merged a patch that would cause the logic here to be a little bit more aggressive/optimistic, but that had to be reverted because it prevented the modem from coming up cleanly after a crash. And I unfortunately still don't have any hardware that manifest this problem that I can debug this on myself. But I think it's fine to merge the rpmcc patch (which I see you did). Thanks, Bjorn
[ANNOUNCE] 4.14.188-rt87
Hello RT-list! I'm pleased to announce the 4.14.188-rt87 stable release. This is strictly a merge of stable kernels v4.14.187 and v4.14.188, no updates to the PREEMPT_RT series. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.14-rt Head SHA1: 92482e37651134a61920a57881c5053b1f24a3a3 Or to build 4.14.188-rt87 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.14.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.14.188.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.14/patch-4.14.188-rt87.patch.xz You can also build from 4.14.186-rt86 by applying the incremental patch: https://www.kernel.org/pub/linux/kernel/projects/rt/4.14/incr/patch-4.14.186-rt86-rt87.patch.xz Enjoy! Clark
[ANNOUNCE] 4.9.230-rt148
Hello RT-list! I'm pleased to announce the 4.9.230-rt148 stable release. Since the 4.9 RT kernel is in maintenance mode this is just a merge of the v4.9.229 and v4.9.230 stable updates. No changes to the PREEMPT_RT series. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.9-rt Head SHA1: e9ba5cf36f7695be6390a2eab626d8c787bf2fd6 Or to build 4.9.230-rt148 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.9.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.9.230.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.9/patch-4.9.230-rt148.patch.xz You can also build from 4.9.228-rt147 by applying the incremental patch: https://www.kernel.org/pub/linux/kernel/projects/rt/4.9/incr/patch-4.9.228-rt147-rt148.patch.xz Enjoy! Clark
Re: [PATCH -next] : add stub for of_get_next_parent() to fix qcom build error
On Fri 10 Jul 16:40 PDT 2020, Randy Dunlap wrote: > On 7/10/20 8:28 AM, Rob Herring wrote: > > On Mon, Jun 29, 2020 at 10:43 AM Randy Dunlap wrote: > >> > >> From: Randy Dunlap > >> > >> Fix a (COMPILE_TEST) build error when CONFIG_OF is not set/enabled > >> by adding a stub for of_get_next_parent(). > >> > >> ../drivers/soc/qcom/qcom-geni-se.c:819:11: error: implicit declaration of > >> function 'of_get_next_parent'; did you mean 'of_get_parent'? > >> [-Werror=implicit-function-declaration] > >> ../drivers/soc/qcom/qcom-geni-se.c:819:9: warning: assignment makes > >> pointer from integer without a cast [-Wint-conversion] > >> > > > > Fixes tag? > > Are linux-next hashes/tags stable? > Yes, the hashes of the Qualcomm tree are stable. > Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix > earlycon crash") > Thank you, added this to the commit and... > >> Signed-off-by: Randy Dunlap > >> Cc: Rob Herring > >> Cc: Frank Rowand > >> Cc: devicet...@vger.kernel.org > >> Cc: Andy Gross > >> Cc: Bjorn Andersson > >> Cc: linux-arm-...@vger.kernel.org > >> --- > >> include/linux/of.h |5 + > >> 1 file changed, 5 insertions(+) > > > > I'm assuming this will be applied to the tree that introduced the problem. > > > > Acked-by: Rob Herring > > > > Hi Akash, > Can you add this patch to your tree, as Rob indicated above? > ...applied it to the Qualcomm "drivers" tree for 5.9. Thanks for the patch Randy and thanks for the Ack, Rob. Regards, Bjorn > thanks. > -- > ~Randy >
Re: [PATCH 17/26] mm/riscv: Use general page fault accounting
On Fri, 26 Jun 2020 15:36:25 PDT (-0700), pet...@redhat.com wrote: Use the general page fault accounting by passing regs into handle_mm_fault(). It naturally solve the issue of multiple page fault accounting when page fault retry happened. CC: Paul Walmsley CC: Palmer Dabbelt CC: Albert Ou CC: linux-ri...@lists.infradead.org Signed-off-by: Peter Xu --- arch/riscv/mm/fault.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c index 677ee1bb11ac..e796ba02b572 100644 --- a/arch/riscv/mm/fault.c +++ b/arch/riscv/mm/fault.c @@ -110,7 +110,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs) * make sure we exit gracefully rather than endlessly redo * the fault. */ - fault = handle_mm_fault(vma, addr, flags, NULL); + fault = handle_mm_fault(vma, addr, flags, regs); /* * If we need to retry but a fatal signal is pending, handle the @@ -128,21 +128,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs) BUG(); } - /* -* Major/minor page fault accounting is only done on the -* initial attempt. If we go through a retry, it is extremely -* likely that the page will be found in page cache at that point. -*/ if (flags & FAULT_FLAG_ALLOW_RETRY) { - if (fault & VM_FAULT_MAJOR) { - tsk->maj_flt++; - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, - 1, regs, addr); - } else { - tsk->min_flt++; - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, - 1, regs, addr); - } if (fault & VM_FAULT_RETRY) { flags |= FAULT_FLAG_TRIED; This still slightly changes the accounting numbers, but I don't think it does so in a way that's meaningful enough to care about. SIGBUS is the only one that might happen frequently enough to notice, I doubt anyone cares about whether faults are accounted for during OOM. Acked-by: Palmer Dabbelt Thanks!
[GIT PULL] SCSI fixes for 5.8-rc4
Five small fixes, four in driver and one in the SCSI Parallel transport, which fixes an incredibly old bug so I suspect no-one has actually used the functionality it fixes. The patch is available here: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes The short changelog is: Bob Liu (1): scsi: iscsi: Change iSCSI workqueue max_active back to 1 Damien Le Moal (1): scsi: mpt3sas: Fix unlock imbalance Johannes Thumshirn (1): scsi: mpt3sas: Fix error returns in BRM_status_show Steve Schremmer (1): scsi: dh: Add Fujitsu device to devinfo and dh lists Tom Rix (1): scsi: scsi_transport_spi: Fix function pointer check And the diffstat: drivers/scsi/libiscsi.c | 2 +- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 12 +++- drivers/scsi/scsi_devinfo.c | 1 + drivers/scsi/scsi_dh.c | 1 + drivers/scsi/scsi_transport_iscsi.c | 2 +- drivers/scsi/scsi_transport_spi.c | 2 +- 6 files changed, 12 insertions(+), 8 deletions(-) With full diff below. James --- diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index e5a64d4f255c..49c8a1818baf 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -2629,7 +2629,7 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, "iscsi_q_%d", shost->host_no); ihost->workq = alloc_workqueue("%s", WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, - 2, ihost->workq_name); + 1, ihost->workq_name); if (!ihost->workq) goto free_host; } diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 62e552838565..983e568ff231 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -3145,19 +3145,18 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr, if (!ioc->is_warpdrive) { ioc_err(ioc, "%s: BRM attribute is only for warpdrive\n", __func__); - goto out; + return 0; } /* pci_access_mutex lock acquired by sysfs show path */ mutex_lock(>pci_access_mutex); - if (ioc->pci_error_recovery || ioc->remove_host) { - mutex_unlock(>pci_access_mutex); - return 0; - } + if (ioc->pci_error_recovery || ioc->remove_host) + goto out; /* allocate upto GPIOVal 36 entries */ sz = offsetof(Mpi2IOUnitPage3_t, GPIOVal) + (sizeof(u16) * 36); io_unit_pg3 = kzalloc(sz, GFP_KERNEL); if (!io_unit_pg3) { + rc = -ENOMEM; ioc_err(ioc, "%s: failed allocating memory for iounit_pg3: (%d) bytes\n", __func__, sz); goto out; @@ -3167,6 +3166,7 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr, 0) { ioc_err(ioc, "%s: failed reading iounit_pg3\n", __func__); + rc = -EINVAL; goto out; } @@ -3174,12 +3174,14 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr, if (ioc_status != MPI2_IOCSTATUS_SUCCESS) { ioc_err(ioc, "%s: iounit_pg3 failed with ioc_status(0x%04x)\n", __func__, ioc_status); + rc = -EINVAL; goto out; } if (io_unit_pg3->GPIOCount < 25) { ioc_err(ioc, "%s: iounit_pg3->GPIOCount less than 25 entries, detected (%d) entries\n", __func__, io_unit_pg3->GPIOCount); + rc = -EINVAL; goto out; } diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index eed31021e788..ba84244c1b4f 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -239,6 +239,7 @@ static struct { {"LSI", "Universal Xport", "*", BLIST_NO_ULD_ATTACH}, {"ENGENIO", "Universal Xport", "*", BLIST_NO_ULD_ATTACH}, {"LENOVO", "Universal Xport", "*", BLIST_NO_ULD_ATTACH}, + {"FUJITSU", "Universal Xport", "*", BLIST_NO_ULD_ATTACH}, {"SanDisk", "Cruzer Blade", NULL, BLIST_TRY_VPD_PAGES | BLIST_INQUIRY_36}, {"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN | BLIST_INQUIRY_36}, diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c index 42f0550d6b11..6f41e4b5a2b8 100644 --- a/drivers/scsi/scsi_dh.c +++ b/drivers/scsi/scsi_dh.c @@ -63,6 +63,7 @@ static const struct scsi_dh_blist scsi_dh_blist[] = { {"LSI", "INF-01-00","rdac", }, {"ENGENIO", "INF-01-00","rdac", }, {"LENOVO", "DE_Series", "rdac", }, + {"FUJITSU", "ETERNUS_AHB", "rdac", }, {NULL, NULL,NULL }, }; diff --git a/drivers/scsi/scsi_transport_iscsi.c
Re: [PATCH net-next] ptp: add debugfs support for IDT family of timing devices
On Sat, Jul 11, 2020 at 11:38:39AM -0700, Richard Cochran wrote: > On Sat, Jul 11, 2020 at 06:38:06PM +0200, Andrew Lunn wrote: > > But configuration does not belong in debugfs. It would be good to > > explain what is being configured by these parameters, then we can > > maybe make a suggestion about the correct API to use. > > Can we at least enumerate the possibilities? > > - driver specific char device > - private ioctls > - debugfs Hi Richard Since nobody has explained what is actually being configured here, the list is long, and is very likely to contain all sorts of wrong ways of doing it: A new generic parameter added to the PTP API which other PTP clock providers could use. A device tree property. A device tree clock, regulator, ... An ACPI property? A sysfs file. A module parameter A new POSIX clock? An LED class device? A netlink attribute? Andrew
Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection
> @@ -302,6 +357,48 @@ static int dp83822_config_init(struct phy_device *phydev) > } > } > > + if (dp83822->fx_enabled) { > + err = phy_modify(phydev, MII_DP83822_CTRL_2, > + DP83822_FX_ENABLE, 1); > + if (err < 0) > + return err; > + > + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, > + phydev->advertising); > + > + /* Auto neg is not supported in fiber mode */ > + bmcr = phy_read(phydev, MII_BMCR); > + if (bmcr < 0) > + return bmcr; > + > + if (bmcr & BMCR_ANENABLE) { > + err = phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0); > + if (err < 0) > + return err; > + } > + phydev->autoneg = AUTONEG_DISABLE; You should also be removing ETHTOOL_LINK_MODE_Autoneg_BIT from phydev->supported, to make it clear autoneg is not supported. Assuming genphy_read_abilities() cannot figure this out for itself. Andrew
Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection
> +#define MII_DP83822_FIBER_ADVERTISE (SUPPORTED_AUI | SUPPORTED_FIBRE | \ > + SUPPORTED_BNC | SUPPORTED_Pause | \ > + SUPPORTED_Asym_Pause | \ > + SUPPORTED_100baseT_Full) > + > + /* Setup fiber advertisement */ > + err = phy_modify_changed(phydev, MII_ADVERTISE, > + ADVERTISE_1000XFULL | > + ADVERTISE_1000XPAUSE | > + ADVERTISE_1000XPSE_ASYM, > + MII_DP83822_FIBER_ADVERTISE); That looks very odd. SUPPORTED_AUI #define has nothing to do with MII_ADVERTISE register. It is not a bit you can read/write in that register. Andrew
Re: [PATCH] xen: introduce xen_vring_use_dma
On Fri, Jul 10, 2020 at 10:23:22AM -0700, Stefano Stabellini wrote: > Sorry for the late reply -- a couple of conferences kept me busy. > > > On Wed, 1 Jul 2020, Michael S. Tsirkin wrote: > > On Wed, Jul 01, 2020 at 10:34:53AM -0700, Stefano Stabellini wrote: > > > Would you be in favor of a more flexible check along the lines of the > > > one proposed in the patch that started this thread: > > > > > > if (xen_vring_use_dma()) > > > return true; > > > > > > > > > xen_vring_use_dma would be implemented so that it returns true when > > > xen_swiotlb is required and false otherwise. > > > > Just to stress - with a patch like this virtio can *still* use DMA API > > if PLATFORM_ACCESS is set. So if DMA API is broken on some platforms > > as you seem to be saying, you guys should fix it before doing something > > like this.. > > Yes, DMA API is broken with some interfaces (specifically: rpmesg and > trusty), but for them PLATFORM_ACCESS is never set. That is why the > errors weren't reported before. Xen special case aside, there is no > problem under normal circumstances. So why not fix DMA API? Then this patch is not needed. > > If you are OK with this patch (after a little bit of clean-up), Peng, > are you OK with sending an update or do you want me to?
Re: [PATCH net-next] ptp: add debugfs support for IDT family of timing devices
On Sat, Jul 11, 2020 at 06:38:06PM +0200, Andrew Lunn wrote: > But configuration does not belong in debugfs. It would be good to > explain what is being configured by these parameters, then we can > maybe make a suggestion about the correct API to use. Can we at least enumerate the possibilities? - driver specific char device - private ioctls - debugfs What else? My understanding is that debugfs is the preferred place for this kind of thing. Of course nobody expects any kind of stable ABI for this, and so that is a non-issue IHMO. Thanks, Richard
drivers/gpu/drm/i915/i915_sw_fence.c:84:20: error: unused function 'debug_fence_init_onstack'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1df0d8960499e58963fd6c8ac75e544f2b417b29 commit: 6863f5643dd717376c2fdc85a47a00f9d738a834 kbuild: allow Clang to find unused static inline functions for W=1 build date: 10 months ago config: x86_64-randconfig-a002-20200712 (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8) 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 x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git checkout 6863f5643dd717376c2fdc85a47a00f9d738a834 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 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/gpu/drm/i915/i915_sw_fence.c:84:20: error: unused function >> 'debug_fence_init_onstack' [-Werror,-Wunused-function] static inline void debug_fence_init_onstack(struct i915_sw_fence *fence) ^ >> drivers/gpu/drm/i915/i915_sw_fence.c:105:20: error: unused function >> 'debug_fence_free' [-Werror,-Wunused-function] static inline void debug_fence_free(struct i915_sw_fence *fence) ^ 2 errors generated. vim +/debug_fence_init_onstack +84 drivers/gpu/drm/i915/i915_sw_fence.c fc1584059d6c43 Chris Wilson 2016-11-25 83 214707fc2ce08d Chris Wilson 2017-10-12 @84 static inline void debug_fence_init_onstack(struct i915_sw_fence *fence) 214707fc2ce08d Chris Wilson 2017-10-12 85 { 214707fc2ce08d Chris Wilson 2017-10-12 86 } 214707fc2ce08d Chris Wilson 2017-10-12 87 fc1584059d6c43 Chris Wilson 2016-11-25 88 static inline void debug_fence_activate(struct i915_sw_fence *fence) fc1584059d6c43 Chris Wilson 2016-11-25 89 { fc1584059d6c43 Chris Wilson 2016-11-25 90 } fc1584059d6c43 Chris Wilson 2016-11-25 91 fc1584059d6c43 Chris Wilson 2016-11-25 92 static inline void debug_fence_set_state(struct i915_sw_fence *fence, fc1584059d6c43 Chris Wilson 2016-11-25 93 int old, int new) fc1584059d6c43 Chris Wilson 2016-11-25 94 { fc1584059d6c43 Chris Wilson 2016-11-25 95 } fc1584059d6c43 Chris Wilson 2016-11-25 96 fc1584059d6c43 Chris Wilson 2016-11-25 97 static inline void debug_fence_deactivate(struct i915_sw_fence *fence) fc1584059d6c43 Chris Wilson 2016-11-25 98 { fc1584059d6c43 Chris Wilson 2016-11-25 99 } fc1584059d6c43 Chris Wilson 2016-11-25 100 fc1584059d6c43 Chris Wilson 2016-11-25 101 static inline void debug_fence_destroy(struct i915_sw_fence *fence) fc1584059d6c43 Chris Wilson 2016-11-25 102 { fc1584059d6c43 Chris Wilson 2016-11-25 103 } fc1584059d6c43 Chris Wilson 2016-11-25 104 fc1584059d6c43 Chris Wilson 2016-11-25 @105 static inline void debug_fence_free(struct i915_sw_fence *fence) fc1584059d6c43 Chris Wilson 2016-11-25 106 { fc1584059d6c43 Chris Wilson 2016-11-25 107 } fc1584059d6c43 Chris Wilson 2016-11-25 108 :: The code at line 84 was first introduced by commit :: 214707fc2ce08d09982bc4fe4b7a1c1f010e82be drm/i915/selftests: Wrap a timer into a i915_sw_fence :: TO: Chris Wilson :: CC: Chris Wilson --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [RFC 07/12] media: uapi: h264: Add DPB entry field reference flags
Le samedi 11 juillet 2020 à 10:21 +, Jonas Karlman a écrit : > On 2020-07-10 23:49, Nicolas Dufresne wrote: > > Le vendredi 10 juillet 2020 à 09:25 -0300, Ezequiel Garcia a écrit : > > > +Nicolas > > > > > > On Fri, 2020-07-10 at 14:05 +0200, Boris Brezillon wrote: > > > > On Fri, 10 Jul 2020 08:50:28 -0300 > > > > Ezequiel Garcia wrote: > > > > > > > > > On Fri, 2020-07-10 at 10:13 +0200, Boris Brezillon wrote: > > > > > > On Fri, 10 Jul 2020 01:21:07 -0300 > > > > > > Ezequiel Garcia wrote: > > > > > > > > > > > > > Hello Jonas, > > > > > > > > > > > > > > In the context of the uAPI cleanup, > > > > > > > I'm revisiting this patch. > > > > > > > > > > > > > > On Sun, 2019-09-01 at 12:45 +, Jonas Karlman wrote: > > > > > > > > Add DPB entry flags to help indicate when a reference frame is a > > > > > > > > field picture > > > > > > > > and how the DPB entry is referenced, top or bottom field or full > > > > > > > > frame. > > > > > > > > > > > > > > > > Signed-off-by: Jonas Karlman > > > > > > > > --- > > > > > > > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 12 > > > > > > > > > > > > > > > > include/media/h264-ctrls.h | 4 > > > > > > > > 2 files changed, 16 insertions(+) > > > > > > > > > > > > > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > > > > > > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > > > > > > index bc5dd8e76567..eb6c32668ad7 100644 > > > > > > > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > > > > > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > > > > > > > @@ -2022,6 +2022,18 @@ enum > > > > > > > > v4l2_mpeg_video_h264_hierarchical_coding_type - > > > > > > > > * - ``V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM`` > > > > > > > >- 0x0004 > > > > > > > >- The DPB entry is a long term reference frame > > > > > > > > +* - ``V4L2_H264_DPB_ENTRY_FLAG_FIELD_PICTURE`` > > > > > > > > + - 0x0008 > > > > > > > > + - The DPB entry is a field picture > > > > > > > > +* - ``V4L2_H264_DPB_ENTRY_FLAG_REF_TOP`` > > > > > > > > + - 0x0010 > > > > > > > > + - The DPB entry is a top field reference > > > > > > > > +* - ``V4L2_H264_DPB_ENTRY_FLAG_REF_BOTTOM`` > > > > > > > > + - 0x0020 > > > > > > > > + - The DPB entry is a bottom field reference > > > > > > > > +* - ``V4L2_H264_DPB_ENTRY_FLAG_REF_FRAME`` > > > > > > > > + - 0x0030 > > > > > > > > + - The DPB entry is a reference frame > > > > > > > > > > > > > > > > ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE (enum)`` > > > > > > > > Specifies the decoding mode to use. Currently exposes > > > > > > > > slice- > > > > > > > > based and > > > > > > > > diff --git a/include/media/h264-ctrls.h > > > > > > > > b/include/media/h264-ctrls.h > > > > > > > > index e877bf1d537c..76020ebd1e6c 100644 > > > > > > > > --- a/include/media/h264-ctrls.h > > > > > > > > +++ b/include/media/h264-ctrls.h > > > > > > > > @@ -185,6 +185,10 @@ struct v4l2_ctrl_h264_slice_params { > > > > > > > > #define V4L2_H264_DPB_ENTRY_FLAG_VALID 0x01 > > > > > > > > #define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE0x02 > > > > > > > > #define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x04 > > > > > > > > +#define V4L2_H264_DPB_ENTRY_FLAG_FIELD_PICTURE 0x08 > > > > > > > > +#define V4L2_H264_DPB_ENTRY_FLAG_REF_TOP 0x10 > > > > > > > > +#define V4L2_H264_DPB_ENTRY_FLAG_REF_BOTTOM0x20 > > > > > > > > +#define V4L2_H264_DPB_ENTRY_FLAG_REF_FRAME 0x30 > > > > > > > > > > > > > > > > > > > > > > I've been going thru the H264 spec and I'm unsure, > > > > > > > are all these flags semantically needed? > > > > > > > > > > > > > > For instance, if one of REF_BOTTOM or REF_TOP (or both) > > > > > > > are set, doesn't that indicate it's a field picture? > > > > > > > > > > > > > > Or conversely, if neither REF_BOTTOM or REF_TOP are set, > > > > > > > then it's a frame picture? > > > > > > > > > > > > I think that's what I was trying to do here [1] > > > > > > > > > > > > [1]https://patchwork.kernel.org/patch/11392095/ > > > > > > > > > > Right. Aren't we missing a DPB_ENTRY_FLAG_TOP_FIELD? > > > > > > > > > > If I understand correctly, the DPB can contain: > > > > > > > > > > * frames (FLAG_FIELD not set) > > > > > * a field pair, with a single field (FLAG_FIELD and either TOP or > > > > > BOTTOM). > > > > > * a field pair, with boths fields (FLAG_FIELD and both TOP or BOTTOM). > > > > > > > > Well, my understand is that, if the buffer contains both a TOP and > > > > BOTTOM field, it actually becomes a full frame, so you actually have > > > > those cases: > > > > > > > > * FLAG_FIELD not set: this a frame (note that a TOP/BOTTOM field > > > > decoded buffer can become of frame if it's complemented with the > > > > missing field later during the decoding) > > > > * FLAG_FIELD set + BOTTOM_FIELD not set: this is a TOP
Re: [PATCH net-next v1 5/5] net: phy: micrel: ksz886x/ksz8081: add cabletest support
On Fri, Jul 10, 2020 at 02:08:51PM +0200, Oleksij Rempel wrote: > This patch support for cable test for the ksz886x switches and the > ksz8081 PHY. > > The patch was tested on a KSZ8873RLL switch with following results: > > - port 1: > - cannot detect any distance > - provides inverted values > (Errata: DS8830A: "LinkMD does not work on Port 1", > > http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS8830A.pdf) > - Reports "short" on open or ok. > - Reports "ok" on short. > > - port 2: > - can detect distance > - can detect open on each wire of pair A (wire 1 and 2) > - can detect open only on one wire of pair B (only wire 3) > - can detect short between wires of a pair (wires 1 + 2 or 3 + 6) > - short between pairs is detected as open. > For example short between wires 2 + 3 is detected as open. > > In order to work around the errata for port 1, the ksz8795 switch driver > should be extended to provide proper device tree support for the related > PHY nodes. So we can set a DT property to mark the port 1 as affected by > the errata. > > Signed-off-by: Oleksij Rempel Reviewed-by: Andrew Lunn Andrew
Re: [PATCH net-next v1 5/5] net: phy: micrel: ksz886x/ksz8081: add cabletest support
On Fri, Jul 10, 2020 at 02:08:51PM +0200, Oleksij Rempel wrote: > This patch support for cable test for the ksz886x switches and the > ksz8081 PHY. > > The patch was tested on a KSZ8873RLL switch with following results: > > - port 1: > - cannot detect any distance > - provides inverted values > (Errata: DS8830A: "LinkMD does not work on Port 1", > > http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS8830A.pdf) > - Reports "short" on open or ok. > - Reports "ok" on short. > > - port 2: > - can detect distance > - can detect open on each wire of pair A (wire 1 and 2) > - can detect open only on one wire of pair B (only wire 3) > - can detect short between wires of a pair (wires 1 + 2 or 3 + 6) > - short between pairs is detected as open. > For example short between wires 2 + 3 is detected as open. > > In order to work around the errata for port 1, the ksz8795 switch driver > should be extended to provide proper device tree support for the related > PHY nodes. So we can set a DT property to mark the port 1 as affected by > the errata. Hi Oleksij Do the PHY register read/writes pass through the DSA driver for the 8873? I was wondering if the switch could intercept reads/writes on port1 for KSZ8081_LMD and return EOPNOTSUPP? That would be a more robust solution than DT properties, which are going to get forgotten. Andrew
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-6 tag
The pull request you sent on Sat, 11 Jul 2020 21:50:20 +1000: > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git > tags/powerpc-5.8-6 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/997c4431f04d8f0d6063bac3bcdceba8d939b879 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] xen: branch for v5.8-rc5
The pull request you sent on Sat, 11 Jul 2020 14:52:24 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git > for-linus-5.8b-rc5-tag has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0aea6d5c5be33ce94c16f9ab2f64de1f481f424b Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
[PATCH 2/2] locking/pvqspinlock: Optionally store lock holder cpu into lock
The previous patch enables native qspinlock to store lock holder cpu number into the lock word when the lock is acquired via the slowpath. Since PV qspinlock uses atomic unlock, allowing the fastpath and slowpath to put different values into the lock word will further slow down the performance. This is certainly undesirable. The only way we can do that without too much performance impact is to make fastpath and slowpath put in the same value. Still there is a slight performance overhead in the additional access to a percpu variable in the fastpath as well as the less optimized x86-64 PV qspinlock unlock path. A new config option QUEUED_SPINLOCKS_CPUINFO is now added to enable distros to decide if they want to enable lock holder cpu information in the lock itself for both native and PV qspinlocks across both fastpath and slowpath. If this option is not configureed, only native qspinlocks in the slowpath will put the lock holder cpu information in the lock word. Signed-off-by: Waiman Long --- arch/Kconfig | 12 +++ arch/x86/include/asm/qspinlock_paravirt.h | 9 +++-- include/asm-generic/qspinlock.h | 13 +-- kernel/locking/qspinlock.c| 5 ++- kernel/locking/qspinlock_paravirt.h | 41 --- 5 files changed, 53 insertions(+), 27 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 8cc35dc556c7..1e6fea12be41 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -954,6 +954,18 @@ config LOCK_EVENT_COUNTS the chance of application behavior change because of timing differences. The counts are reported via debugfs. +config QUEUED_SPINLOCKS_CPUINFO + bool "Store lock holder cpu information into queued spinlocks" + depends on QUEUED_SPINLOCKS + help + Enable queued spinlocks to encode (by adding 2) the lock holder + cpu number into the least significant 8 bits of the 32-bit + lock word as long as the cpu number is not larger than 252. + A value of 1 will be stored for larger cpu numbers. Having the + lock holder cpu information in the lock helps debugging and + crash dump analysis. There might be a very slight performance + overhead in enabling this option. + # Select if the architecture has support for applying RELR relocations. config ARCH_HAS_RELR bool diff --git a/arch/x86/include/asm/qspinlock_paravirt.h b/arch/x86/include/asm/qspinlock_paravirt.h index 159622ee0674..9897aedea660 100644 --- a/arch/x86/include/asm/qspinlock_paravirt.h +++ b/arch/x86/include/asm/qspinlock_paravirt.h @@ -7,8 +7,11 @@ * registers. For i386, however, only 1 32-bit register needs to be saved * and restored. So an optimized version of __pv_queued_spin_unlock() is * hand-coded for 64-bit, but it isn't worthwhile to do it for 32-bit. + * + * Accessing percpu variable when QUEUED_SPINLOCKS_CPUINFO is defined is + * tricky. So disable this optimization for now. */ -#ifdef CONFIG_64BIT +#if defined(CONFIG_64BIT) && !defined(CONFIG_QUEUED_SPINLOCKS_CPUINFO) PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath); #define __pv_queued_spin_unlock__pv_queued_spin_unlock @@ -26,13 +29,13 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath); * * if (likely(lockval == _Q_LOCKED_VAL)) * return; - * pv_queued_spin_unlock_slowpath(lock, lockval); + * __pv_queued_spin_unlock_slowpath(lock, lockval); * } * * For x86-64, * rdi = lock (first argument) * rsi = lockval (second argument) - * rdx = internal variable (set to 0) + * rdx = internal variable for cmpxchg */ asm(".pushsection .text;" ".globl " PV_UNLOCK ";" diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h index fde943d180e0..d5badc1e544e 100644 --- a/include/asm-generic/qspinlock.h +++ b/include/asm-generic/qspinlock.h @@ -12,6 +12,13 @@ #include +#ifdef CONFIG_QUEUED_SPINLOCKS_CPUINFO +DECLARE_PER_CPU_READ_MOSTLY(u8, pcpu_lockval); +#define QUEUED_LOCKED_VAL this_cpu_read(pcpu_lockval) +#else +#define QUEUED_LOCKED_VAL _Q_LOCKED_VAL +#endif + /** * queued_spin_is_locked - is the spinlock locked? * @lock: Pointer to queued spinlock structure @@ -20,7 +27,7 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock) { /* -* Any !0 state indicates it is locked, even if _Q_LOCKED_VAL +* Any !0 state indicates it is locked, even if QUEUED_LOCKED_VAL * isn't immediately observable. */ return atomic_read(>val); @@ -62,7 +69,7 @@ static __always_inline int queued_spin_trylock(struct qspinlock *lock) if (unlikely(val)) return 0; - return likely(atomic_try_cmpxchg_acquire(>val, , _Q_LOCKED_VAL)); + return likely(atomic_try_cmpxchg_acquire(>val, , QUEUED_LOCKED_VAL)); } extern void queued_spin_lock_slowpath(struct qspinlock
[PATCH 1/2] locking/qspinlock: Store lock holder cpu in lock if feasible
When doing crash dump analysis with lock, it is cumbersome to find out who the lock holder is. One have to look at the call traces of all the cpus to figure that out. It will be helpful if the lock itself can provide some clue about the lock holder cpu. The locking state is determined by whether the locked byte is set or not (1 or 0). There is another special value for PV qspinlock (3). The whole byte is used because of performance reason. So there is space to store additional information such as the lock holder cpu as long as the cpu number is small enough to fit into a little less than a byte which is true for most typical 1 or 2-socket systems. There may be a slight performance overhead in encoding the lock holder cpu number especially in the fastpath. Doing it in the slowpath, however, should be essentially free. This patch modifies the slowpath code to add the encoded cpu number (cpu_nr + 2) to the locked byte as long as it is less than 253. A locked byte value of 1 means the lock holder cpu is not known. It is set in the fast path or when the cpu number is just too large. The special locked byte value of 255 is reserved for PV qspinlock. Signed-off-by: Waiman Long --- include/asm-generic/qspinlock_types.h | 5 +++ kernel/locking/qspinlock.c| 47 +-- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/include/asm-generic/qspinlock_types.h b/include/asm-generic/qspinlock_types.h index 56d1309d32f8..11f0a3001daf 100644 --- a/include/asm-generic/qspinlock_types.h +++ b/include/asm-generic/qspinlock_types.h @@ -71,6 +71,11 @@ typedef struct qspinlock { * 8: pending * 9-10: tail index * 11-31: tail cpu (+1) + * + * Lock acquired via the fastpath will have a locked byte value of 1. Lock + * acquired via the slowpath may have a locked byte value of (cpu_nr + 2) + * if cpu_nr < 253. For cpu number beyond that range, a value of 1 will + * always be used. */ #define_Q_SET_MASK(type) (((1U << _Q_ ## type ## _BITS) - 1)\ << _Q_ ## type ## _OFFSET) diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c index b9515fcc9b29..13feefa85db4 100644 --- a/kernel/locking/qspinlock.c +++ b/kernel/locking/qspinlock.c @@ -106,6 +106,7 @@ struct qnode { * PV doubles the storage and uses the second cacheline for PV state. */ static DEFINE_PER_CPU_ALIGNED(struct qnode, qnodes[MAX_NODES]); +static DEFINE_PER_CPU_READ_MOSTLY(u8, pcpu_lockval) = _Q_LOCKED_VAL; /* * We must be able to distinguish between no-tail and the tail at 0:0, @@ -138,6 +139,19 @@ struct mcs_spinlock *grab_mcs_node(struct mcs_spinlock *base, int idx) #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK) +static __init int __init_pcpu_lockval(void) +{ + int cpu; + + for_each_possible_cpu(cpu) { + u8 lockval = (cpu + 2 < _Q_LOCKED_MASK - 1) ? cpu + 2 + : _Q_LOCKED_VAL; + per_cpu(pcpu_lockval, cpu) = lockval; + } + return 0; +} +early_initcall(__init_pcpu_lockval); + #if _Q_PENDING_BITS == 8 /** * clear_pending - clear the pending bit. @@ -158,9 +172,9 @@ static __always_inline void clear_pending(struct qspinlock *lock) * * Lock stealing is not allowed if this function is used. */ -static __always_inline void clear_pending_set_locked(struct qspinlock *lock) +static __always_inline void clear_pending_set_locked(struct qspinlock *lock, u8 lockval) { - WRITE_ONCE(lock->locked_pending, _Q_LOCKED_VAL); + WRITE_ONCE(lock->locked_pending, lockval); } /* @@ -202,9 +216,9 @@ static __always_inline void clear_pending(struct qspinlock *lock) * * *,1,0 -> *,0,1 */ -static __always_inline void clear_pending_set_locked(struct qspinlock *lock) +static __always_inline void clear_pending_set_locked(struct qspinlock *lock, u8 lockval) { - atomic_add(-_Q_PENDING_VAL + _Q_LOCKED_VAL, >val); + atomic_add(-_Q_PENDING_VAL + lockval, >val); } /** @@ -258,9 +272,9 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo * * *,*,0 -> *,0,1 */ -static __always_inline void set_locked(struct qspinlock *lock) +static __always_inline void set_locked(struct qspinlock *lock, u8 lockval) { - WRITE_ONCE(lock->locked, _Q_LOCKED_VAL); + WRITE_ONCE(lock->locked, lockval); } @@ -317,6 +331,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) struct mcs_spinlock *prev, *next, *node; u32 old, tail; int idx; + u8 lockval = this_cpu_read(pcpu_lockval); BUILD_BUG_ON(CONFIG_NR_CPUS >= (1U << _Q_TAIL_CPU_BITS)); @@ -386,7 +401,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) * * 0,1,0 -> 0,0,1 */ - clear_pending_set_locked(lock); + clear_pending_set_locked(lock, lockval); lockevent_inc(lock_pending);
Re: kvm crash on 5.7-rc1 and later
On Fri, Jul 03, 2020 at 11:15:31AM -0400, Woody Suwalski wrote: > I am observing a 100% reproducible kvm crash on kernels starting with > 5.7-rc1, always with the same opcode . > It happens during wake up from the host suspended state. Worked OK on 5.6 > and older. > The host is based on Debian testing, Thinkpad T440, i5 cpu. > > [ 61.576664] kernel BUG at arch/x86/kvm/x86.c:387! > [ 61.576672] invalid opcode: [#1] PREEMPT SMP NOPTI > [ 61.576678] CPU: 0 PID: 3851 Comm: qemu-system-x86 Not tainted 5.7-pingu > #0 > [ 61.576680] Hardware name: LENOVO 20B6005JUS/20B6005JUS, BIOS GJETA4WW > (2.54 ) 03/27/2020 > [ 61.576700] RIP: 0010:kvm_spurious_fault+0xa/0x10 [kvm] > > Crash results in a dead kvm and occasionally a very unstable system. > > Bisecting the problem between v5.6 and v5.7-rc1 points to > > commit 6650cdd9a8ccf00555dbbe743d58541ad8feb6a7 > Author: Peter Zijlstra (Intel) > Date: Sun Jan 26 12:05:35 2020 -0800 > > x86/split_lock: Enable split lock detection by kernel > > Reversing that patch seems to actually "cure" the issue. > > The problem is present in all kernels past 5.7-rc1, however the patch is not > reversing directly in later source trees, so can not retest the logic on > recent kernels. > > Peter, would you have idea how to debug that (or even better - would you > happen to know the fix)? > > I have attached dmesg logs from a "good" 5.6.9 kernel, and then "bad" 5.7.0 > and 5.8-rc3 I have no clue about kvm. Nor do I actually have hardware with SLD on. I've Cc'ed a bunch of folks who might have more ideas.
[PATCH 0/2] locking/qspinlock: Allow lock to store lock holder cpu number
This patchset modifies the qspinlock code to allow it to store the lock holder cpu number in the lock itself if feasible for easier debugging and crash dump analysis. This lock holder cpu information may also be useful to architectures like PowerPC that needs the lock holder cpu number for better paravirtual spinlock performance. A new config option QUEUED_SPINLOCKS_CPUINFO is added. If this config option is set, lock holder cpu number will always be stored if the number is small enough. Without this option, lock holder cpu number will only be stored in the slowpath of the native qspinlock. Waiman Long (2): locking/qspinlock: Store lock holder cpu in lock if feasible locking/pvqspinlock: Optionally store lock holder cpu into lock arch/Kconfig | 12 ++ arch/x86/include/asm/qspinlock_paravirt.h | 9 ++-- include/asm-generic/qspinlock.h | 13 -- include/asm-generic/qspinlock_types.h | 5 +++ kernel/locking/qspinlock.c| 50 +++ kernel/locking/qspinlock_paravirt.h | 41 ++- 6 files changed, 87 insertions(+), 43 deletions(-) -- 2.18.1
Re: WARNING: at mm/mremap.c:211 move_page_tables in i386
On Sat, Jul 11, 2020 at 11:12 AM Linus Torvalds wrote: > > The fact that it seems to happen with > > > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/thp/thp01.c > > makes me think it's somehow related to THP mappings, but I don't see > why those would matter. All the same pmd freeing should still have > happened, afaik. No, I think the only reason it happens with that test-case is simply because that test-case tests many different argument sizes, and as part of that it ends up then hitting the "stack is exactly one pmd in size" case when the stack alignment is right too. So the THP part is almost certainly a red herring. Maybe. Considering that I don't see what's wrong, anything is possible. Linus
Re: [PATCH 05/11] media: exynos4-is: Improve support for sensors with multiple pads
Hi Tomasz, On 2020-07-07 11:13 a.m., Tomasz Figa wrote: > Hi Jonathan, > > On Sat, Apr 25, 2020 at 07:26:44PM -0700, Jonathan Bakker wrote: >> Commit 1c9f5bd7cb8a ("[media] s5p-fimc: Add support for sensors with >> multiple pads") caught the case where a sensor with multiple pads was >> connected via CSIS, but missed the case where the sensor was directly >> connected to the FIMC. >> >> This still assumes that the last pad of a sensor is the source. >> >> Signed-off-by: Jonathan Bakker >> --- >> drivers/media/platform/exynos4-is/media-dev.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> > > Thank you for the patch. Please see my comments inline. > >> diff --git a/drivers/media/platform/exynos4-is/media-dev.c >> b/drivers/media/platform/exynos4-is/media-dev.c >> index 5c32abc7251b..b38445219c72 100644 >> --- a/drivers/media/platform/exynos4-is/media-dev.c >> +++ b/drivers/media/platform/exynos4-is/media-dev.c >> @@ -991,7 +991,8 @@ static int fimc_md_create_links(struct fimc_md *fmd) >> >> case FIMC_BUS_TYPE_ITU_601...FIMC_BUS_TYPE_ITU_656: >> source = >entity; >> -pad = 0; >> +/* Assume the last pad is the source */ >> +pad = sensor->entity.num_pads - 1; > > Is 0 really any worse than num_pads - 1? This sounds like quite an ugly > hack. > > Could you iterate over the pads of the sensor entity and explicitly find > a source pad instead? Yes, iterating would work better. This comes from when I was trying to integrate video-mux, before I realized I could connect multiple sensors. I will drop this patch from v2 as it's not necessary. > > Best regards, > Tomasz > Thanks, Jonathan
Re: WARNING: at mm/mremap.c:211 move_page_tables in i386
On Sat, Jul 11, 2020 at 10:27 AM Naresh Kamboju wrote: > > I have started bisecting this problem and found the first bad commit Thanks for the effort. Bisection is often a great tool to figure out what's wrong. Sadly, in this case: > commit 9f132f7e145506efc0744426cb338b18a54afc3b > Author: Joel Fernandes (Google) > Date: Thu Jan 3 15:28:41 2019 -0800 > > mm: select HAVE_MOVE_PMD on x86 for faster mremap Yeah, that's just the commit that enables the code, not the commit that introduces the fundamental problem. That said, this is a prime example of why I absolutely detest patch series that do this kind of thing, and are several patches that create new functionality, followed by one patch to enable it. If you can't get things working incrementally, maybe you shouldn't do them at all. Doing a big series of "hidden work" and then enabling it later is wrong. In this case, though, the real patch that did the code isn't that kind of "big series of hidden work" patch series, it's just the (single) previous commit 2c91bd4a4e2e ("mm: speed up mremap by 20x on large regions"). So your bisection is useful, it's just that it really points to that previous commit, and it's where this code was introduced. It's also worth noting that that commit doesn't really *break* anything, since it just falls back to the old behavior when it warns. So to "fix" your test-case, we could just remove the WARN_ON. But the WARN_ON() is still worrisome, because the thing it tests for really _should_ be true. Now, we actually have a known bug in this area that is talked about elsewhere: the way unmap does the pgtable_free() is /* Detach vmas from rbtree */ detach_vmas_to_be_unmapped(mm, vma, prev, end); if (downgrade) mmap_write_downgrade(mm); unmap_region(mm, vma, prev, start, end); (and unmap_region() is what does the pgtable_free() that should have cleared the PMD). And the problem with the "downgrade" is that another thread can change the beginning of the next vma when it's a grow-down region (or the end of the prev one if it's a grow-up). See commit dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap") for the source of that But that requires an _actual_ "unmap()" system call (the others set "downgrade" to false - I'm not entirely sure why), and it requires another thread to be attached to that VM in order to actually do that racy concurrent stack size change. And neither of those two cases will be true for the execve() path. It's a new VM, with just one thread attached, so no threaded munmap() going on there. The fact that it seems to happen with https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/thp/thp01.c makes me think it's somehow related to THP mappings, but I don't see why those would matter. All the same pmd freeing should still have happened, afaik. And the printout I asked for a few days back for when it triggered clearly showed a normal non-huge pmd ("val: 7d530067" is just "accessed, dirty, rw, user and present", which is a perfectly normal page directory entry for 4kB pages, and we could move the whole thing and move 2MB (or 4MB) of aligned virtual memory in one go). Some race with THP splitting and pgtable_free()? I can't see how anything would race in execve(), or how anything would have touched that address below the stack in the first place.. Kirill, Oleg, and reaction from this? Neither of you were on the original email, I think, it's this one: https://lore.kernel.org/lkml/CA+G9fYt+6OeibZMD0fP=o3nqfbcn3o4xclkjq0mpqbzj2ux...@mail.gmail.com/ and I really think it is harmless in that when the warning triggers, we just go back to the page-by-page code, but while I think the WARN_ON() should probably be downgraded to a WARN_ON_ONCE(), I do think it's showing _something_. I just can't see how this would trigger for execve(). That's just about the _simplest_ case for us: single-threaded, mappings set up purely by load_elf_binary() etc. I'm clearly missing something. Linus
Re: [PATCH] hwmon: (drivetemp) Avoid SCT usage on Toshiba DT01ACA family drives
On Fri, Jul 10, 2020 at 08:10:03PM +0200, Maciej S. Szmigiero wrote: > It has been observed that Toshiba DT01ACA family drives have > WRITE FPDMA QUEUED command timeouts and sometimes just freeze until > power-cycled under heavy write loads when their temperature is getting > polled in SCT mode. The SMART mode seems to be fine, though. > > Let's make sure we don't use SCT mode for these drives then. > > While only the 3 TB model was actually caught exhibiting the problem let's > play safe here to avoid data corruption and extend the ban to the whole > family. > > Fixes: 5b46903d8bf3 ("hwmon: Driver for disk and solid state drives with > temperature sensors") > Cc: sta...@vger.kernel.org > Signed-off-by: Maciej S. Szmigiero > --- I am out of town; more thorough review later. Quick feedback: Terms such as "blacklist" have run out of favor. Please use a different term. Thanks, Guenter > Sending again since the previous message bounced for most recipients. > > Notes: > This behavior was observed on two different DT01ACA3 drives. > > Usually, a series of queued WRITE FPDMA QUEUED commands just time out, > but sometimes the whole drive freezes. Merely disconnecting and > reconnecting SATA interface cable then does not unfreeze the drive. > > One has to disconnect and reconnect the drive power connector for the > drive to be detected again (suggesting the drive firmware itself has > crashed). > > This only happens when the drive temperature is polled very often (like > every second), so occasional SCT usage via smartmontools is probably > safe. > > drivers/hwmon/drivetemp.c | 37 + > 1 file changed, 37 insertions(+) > > diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c > index 0d4f3d97ffc6..4fd51fa8c6e3 100644 > --- a/drivers/hwmon/drivetemp.c > +++ b/drivers/hwmon/drivetemp.c > @@ -285,6 +285,36 @@ static int drivetemp_get_scttemp(struct drivetemp_data > *st, u32 attr, long *val) > return err; > } > > +static const char * const sct_blacklist_models[] = { > +/* > + * These drives will have WRITE FPDMA QUEUED command timeouts and sometimes > just > + * freeze until power-cycled under heavy write loads when their temperature > is > + * getting polled in SCT mode. The SMART mode seems to be fine, though. > + * > + * While only the 3 TB model was actually caught exhibiting the problem > + * let's play safe here to avoid data corruption and ban the whole family. > + */ > + "TOSHIBA DT01ACA0", > + "TOSHIBA DT01ACA1", > + "TOSHIBA DT01ACA2", > + "TOSHIBA DT01ACA3", > +}; > + > +static bool drivetemp_sct_blacklisted(struct drivetemp_data *st) > +{ > + struct scsi_device *sdev = st->sdev; > + unsigned int ctr; > + > + if (!sdev->model) > + return false; > + > + for (ctr = 0; ctr < ARRAY_SIZE(sct_blacklist_models); ctr++) > + if (strncmp(sdev->model, sct_blacklist_models[ctr], 16) == 0) > + return true; > + > + return false; > +} > + > static int drivetemp_identify_sata(struct drivetemp_data *st) > { > struct scsi_device *sdev = st->sdev; > @@ -326,6 +356,13 @@ static int drivetemp_identify_sata(struct drivetemp_data > *st) > /* bail out if this is not a SATA device */ > if (!is_ata || !is_sata) > return -ENODEV; > + > + if (have_sct && drivetemp_sct_blacklisted(st)) { > + dev_notice(>sdev_gendev, > +"will avoid using SCT for temperature monitoring\n"); > + have_sct = false; > + } > + > if (!have_sct) > goto skip_sct; >
Re: [PATCH 11/11] media: exynos4-is: Correct parallel port probing
Hi Sylwester, On 2020-07-08 9:15 a.m., Sylwester Nawrocki wrote: > Hi, > > On 26.04.2020 04:26, Jonathan Bakker wrote: >> According to the binding doc[1], port A should be reg = 0 >> and port B reg = 1. Unfortunately, the driver was treating 0 >> as invalid and 1 as camera port A. Match the binding doc and >> make 0=A and 1=B. >> >> [1] Documentation/devicetree/bindings/media/samsung-fimc.txt > > Thank you for correcting this. I would prefer to correct the binding > documentation instead, so it says reg=1 for A and reg=2 for B. > Then it would also match what we have in dts for Goni and > enum fimc_input in include/media/drv-intf/exynos-fimc.h No problem, that works for me. I'll drop this patch and replace it with one changing the documentation. > > Thanks, Jonathan
Re: [PATCH 10/11] media: exynos4-is: Prevent duplicate call to media_pipeline_stop
Hi Tomasz, On 2020-07-07 11:44 a.m., Tomasz Figa wrote: > Hi Jonathan, > > On Sat, Apr 25, 2020 at 07:26:49PM -0700, Jonathan Bakker wrote: >> media_pipeline_stop can be called from both release and streamoff, >> so make sure they're both protected under the streaming flag and >> not just one of them. > > First of all, thanks for the patch. > > Shouldn't it be that release calls streamoff, so that only streamoff > is supposed to have the call to media_pipeline_stop()? > I can't say that I understand the whole media subsystem enough to know :) Since media_pipeline_start is called in streamon, it makes sense that streamoff should have the media_pipeline_stop call. However, even after removing the call in fimc_capture_release I'm still getting a backtrace such as [ 73.843117] [ cut here ] [ 73.843251] WARNING: CPU: 0 PID: 1575 at drivers/media/mc/mc-entity.c:554 media_pipeline_stop+0x20/0x2c [mc] [ 73.843265] Modules linked in: s5p_fimc v4l2_fwnode exynos4_is_common videobuf2_dma_contig pvrsrvkm_s5pv210_sgx540_120 videobuf2_memops v4l2_mem2mem brcmfmac videobuf2_v4l2 videobuf2_common hci_uart sha256_generic libsha256 btbcm bluetooth cfg80211 brcmutil ecdh_generic ecc ce147 libaes s5ka3dfx videodev atmel_mxt_ts mc pwm_vibra rtc_max8998 [ 73.843471] CPU: 0 PID: 1575 Comm: v4l2-ctl Not tainted 5.7.0-14534-g2b33418b254e-dirty #669 [ 73.843487] Hardware name: Samsung S5PC110/S5PV210-based board [ 73.843562] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 73.843613] [] (show_stack) from [] (__warn+0xbc/0xd4) [ 73.843661] [] (__warn) from [] (warn_slowpath_fmt+0x60/0xb8) [ 73.843734] [] (warn_slowpath_fmt) from [] (media_pipeline_stop+0x20/0x2c [mc]) [ 73.843867] [] (media_pipeline_stop [mc]) from [] (fimc_cap_streamoff+0x38/0x48 [s5p_fimc]) [ 73.844109] [] (fimc_cap_streamoff [s5p_fimc]) from [] (__video_do_ioctl+0x220/0x448 [videodev]) [ 73.844308] [] (__video_do_ioctl [videodev]) from [] (video_usercopy+0x114/0x498 [videodev]) [ 73.844438] [] (video_usercopy [videodev]) from [] (ksys_ioctl+0x20c/0xa10) [ 73.844484] [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x54) [ 73.844505] Exception stack(0xe5083fa8 to 0xe5083ff0) [ 73.844546] 3fa0: 0049908d bef8f8c0 0003 40045613 bef8d5ac 004c1d16 [ 73.844590] 3fc0: 0049908d bef8f8c0 bef8f8c0 0036 bef8d5ac b6d6b320 bef8faf8 [ 73.844620] 3fe0: 004e3ed4 bef8c718 004990bb b6f00d0a [ 73.844642] ---[ end trace e6a4a8b2f20addd4 ]--- The command I'm using for testing is v4l2-ctl --verbose -d 1 --stream-mmap=3 --stream-skip=2 --stream-to=./test.yuv --stream-count=1 Since I noticed that the streaming flag was being checked fimc_capture_release but not in fimc_cap_streamoff, I assumed that it was simply a missed check. Comparing with other drivers, they seem to call media_pipeline_stop in their vb2_ops stop_streaming callback. I'm willing to test various options > Best regards, > Tomasz > Thanks, Jonathan
Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.
On Sat, 2020-07-11 at 20:41 +0530, Suraj Upadhyay wrote: > On Fri, Jul 10, 2020 at 07:56:43PM +0200, Sam Ravnborg wrote: > > Hi Suraj. > > > > On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote: > > > This patchset converts logging to drm_* functions in drm core. > > > > > > The following functions have been converted to their respective > > > DRM alternatives : > > > dev_info() --> drm_info() > > > dev_err() --> drm_err() > > > dev_warn() --> drm_warn() > > > dev_err_once() --> drm_err_once(). > > > > I would prefer that DRM_* logging in the same files are converted in the > > same patch. So we have one logging conversion patch for each file you > > touches and that we do not need to re-vist the files later to change > > another set of logging functions. > > Agreed. > > > If possible WARN_* should also be converted to drm_WARN_* > > If patch is too large, then split them up but again lets have all > > logging updated when we touch a file. > > > > Care to take a look at this approach? > > > > Hii, > The problem with WARN_* macros is that they are used without any > drm device context. For example [this use > here](https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_edid.c#n1667) > in drm_edid.c, > doesn't have a drm device context and only has one argument (namely > !raw_edid). > There are many more such use cases. > > And also there were cases where dev_* logging functions didn't have any > drm_device context. Perhaps change the __drm_printk macro to not dereference the drm argument when NULL. A trivial but perhaps inefficient way might be used like: drm_(NULL, fmt, ...) --- include/drm/drm_print.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 1c9417430d08..9323a8f46b3c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, /* Helper for struct drm_device based logging. */ #define __drm_printk(drm, level, type, fmt, ...) \ - dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__) - + dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt, \ + ##__VA_ARGS__) #define drm_info(drm, fmt, ...)\ __drm_printk((drm), info,, fmt, ##__VA_ARGS__)
Re: [PATCH] ALSA: trident: Fix a memory leak in snd_trident_create
On Sat, Jul 11, 2020 at 4:04 AM Takashi Iwai wrote: > > On Sat, 11 Jul 2020 09:08:30 +0200, > Navid Emamdoost wrote: > > > > In the implementation of snd_trident_create(), the allocated trident is > > leaked if snd_trident_mixer() fails. Release via snd_trident_free(). > > No, this patch would result in double-free. > > The manual release of trident object isn't needed once after it gets > added via snd_device_new(). Then it'll be automatically released at > the error path (via snd_trident_dev_free()). Thanks for the clarification. > > > thanks, > > Takashi > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: Navid Emamdoost > > --- > > sound/pci/trident/trident_main.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/sound/pci/trident/trident_main.c > > b/sound/pci/trident/trident_main.c > > index 6e50376163a2..e98c692f6aa9 100644 > > --- a/sound/pci/trident/trident_main.c > > +++ b/sound/pci/trident/trident_main.c > > @@ -3582,8 +3582,11 @@ int snd_trident_create(struct snd_card *card, > > return err; > > } > > > > - if ((err = snd_trident_mixer(trident, pcm_spdif_device)) < 0) > > + err = snd_trident_mixer(trident, pcm_spdif_device); > > + if (err < 0) { > > + snd_trident_free(trident); > > return err; > > + } > > > > /* initialise synth voices */ > > for (i = 0; i < 64; i++) { > > -- > > 2.17.1 > > -- Navid.
Re: [PATCH] xsk: ixgbe: solve the soft interrupt 100% CPU usage when xdp rx traffic congestion
On Sat, 11 Jul 2020 18:10:38 +0800 Yahui Chen wrote: > 2. If the wakeup mechanism is used, that is, use the > `XDP_UMEM_USES_NEED_WAKEUP` flag. This method takes advantage of the > interrupt delay function of ixgbe skillfully, thus solving the problem > that the si CPU is always 100%. However, it will cause other problems. > The port-level flow control will be triggered on 82599, and the pause > frame will be sent to the upstream sender. This will affect the other > packet receiving queues of the network card, resulting in the packet > receiving rate of all queues dropping to 10Kpps. To me the current behavior sounds correct.. if you don't want pause frames to be generated you have to disable them completely. The point of pause frames is to prevent drops.
Re: [RESEND PATCH v10] dt-bindings: ufs: Add bindings for Samsung ufs host
Hi Rob Can you please take this via your tree? On Thu, Jun 25, 2020 at 6:20 AM Alim Akhtar wrote: > > This patch adds DT bindings for Samsung ufs hci > > Reviewed-by: Rob Herring > Signed-off-by: Alim Akhtar > --- > > Hi Rob > This is just a rebase on your's dt/next > > This patch was part of [1] > [1] https://lkml.org/lkml/2020/5/27/1697 > > .../bindings/ufs/samsung,exynos-ufs.yaml | 89 +++ > 1 file changed, 89 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > > diff --git a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > new file mode 100644 > index ..38193975c9f1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > @@ -0,0 +1,89 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/ufs/samsung,exynos-ufs.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Samsung SoC series UFS host controller Device Tree Bindings > + > +maintainers: > + - Alim Akhtar > + > +description: | > + Each Samsung UFS host controller instance should have its own node. > + This binding define Samsung specific binding other then what is used > + in the common ufshcd bindings > + [1] Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > + > +properties: > + > + compatible: > +enum: > + - samsung,exynos7-ufs > + > + reg: > +items: > + - description: HCI register > + - description: vendor specific register > + - description: unipro register > + - description: UFS protector register > + > + reg-names: > +items: > + - const: hci > + - const: vs_hci > + - const: unipro > + - const: ufsp > + > + clocks: > +items: > + - description: ufs link core clock > + - description: unipro main clock > + > + clock-names: > +items: > + - const: core_clk > + - const: sclk_unipro_main > + > + interrupts: > +maxItems: 1 > + > + phys: > +maxItems: 1 > + > + phy-names: > +const: ufs-phy > + > +required: > + - compatible > + - reg > + - interrupts > + - phys > + - phy-names > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + - | > +#include > +#include > + > +ufs: ufs@1557 { > + compatible = "samsung,exynos7-ufs"; > + reg = <0x1557 0x100>, > + <0x15570100 0x100>, > + <0x15571000 0x200>, > + <0x15572000 0x300>; > + reg-names = "hci", "vs_hci", "unipro", "ufsp"; > + interrupts = ; > + clocks = <_fsys1 ACLK_UFS20_LINK>, > +<_fsys1 SCLK_UFSUNIPRO20_USER>; > + clock-names = "core_clk", "sclk_unipro_main"; > + pinctrl-names = "default"; > + pinctrl-0 = <_rst_n _refclk_out>; > + phys = <_phy>; > + phy-names = "ufs-phy"; > +}; > +... > > base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407 > prerequisite-patch-id: e0425bbe8f2aff3882b728a0caf0218b6b3e9b6e > prerequisite-patch-id: c8c8502c512f9d6fdaf7d30e54dde3e68c3d855b > prerequisite-patch-id: 8505df2fd70632150b50543cadc6fd7dd42d191c > prerequisite-patch-id: 1a9701ab83425940c8aacb76737edb57ab815e47 > prerequisite-patch-id: 7881e0b87f1f04f657d9e6d450fb5231ad6ffa1a > prerequisite-patch-id: 01dbc0e550e3fcad6e525e7e3183f9f0312e8496 > prerequisite-patch-id: ad801812fff960abab3f27d2c7383be9fd9aa439 > prerequisite-patch-id: 65474c9540e6dc749d30223897de1f486d6b3843 > prerequisite-patch-id: 64b58cd4c5ecfacf28fc20c31a6617092a1e1931 > prerequisite-patch-id: 9bcdd2995fd3f6361f8d5e89c56645058ac9ff96 > -- > 2.17.1 > -- Regards, Alim
drivers/pinctrl/pinctrl-ingenic.o: warning: objtool: ingenic_pinconf_set() falls through to next function ingenic_pinconf_group_set()
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1df0d8960499e58963fd6c8ac75e544f2b417b29 commit: c705cecc8431951b4f34178e6b1db51b4a504c43 objtool: Track original function across branches date: 12 months ago config: x86_64-randconfig-c002-20200711 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/pinctrl/pinctrl-ingenic.o: warning: objtool: ingenic_pinconf_set() >> falls through to next function ingenic_pinconf_group_set() --- 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 v12 2/2] phy: samsung-ufs: add UFS PHY driver for samsung SoC
Hi Vinod Gentle Reminder !! On Fri, Jul 3, 2020 at 11:02 PM Alim Akhtar wrote: > > This patch introduces Samsung UFS PHY driver. This driver > supports to deal with phy calibration and power control > according to UFS host driver's behavior. > > [Robot: -Wmissing-prototypes and -Wsometimes-uninitialized] > Reported-by: kernel test robot > Reviewed-by: Kiwoong Kim > Signed-off-by: Seungwon Jeon > Signed-off-by: Alim Akhtar > Cc: Kishon Vijay Abraham I > Cc: Vinod Koul > Tested-by: Paweł Chmiel > --- > - Changes V11 -> V12 > * Fixed kernel test robot warnings > > - Changes V10 -> V11 > * Addressed review comments from Vinod > > drivers/phy/samsung/Kconfig | 9 + > drivers/phy/samsung/Makefile | 1 + > drivers/phy/samsung/phy-exynos7-ufs.h | 86 ++ > drivers/phy/samsung/phy-samsung-ufs.c | 359 ++ > drivers/phy/samsung/phy-samsung-ufs.h | 139 ++ > 5 files changed, 594 insertions(+) > create mode 100644 drivers/phy/samsung/phy-exynos7-ufs.h > create mode 100644 drivers/phy/samsung/phy-samsung-ufs.c > create mode 100644 drivers/phy/samsung/phy-samsung-ufs.h > > diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig > index 19f2e3119343..e20d2fcc9fe7 100644 > --- a/drivers/phy/samsung/Kconfig > +++ b/drivers/phy/samsung/Kconfig > @@ -29,6 +29,15 @@ config PHY_EXYNOS_PCIE > Enable PCIe PHY support for Exynos SoC series. > This driver provides PHY interface for Exynos PCIe controller. > > +config PHY_SAMSUNG_UFS > + tristate "SAMSUNG SoC series UFS PHY driver" > + depends on OF && (ARCH_EXYNOS || COMPILE_TEST) > + select GENERIC_PHY > + help > + Enable this to support the Samsung UFS PHY driver for > + Samsung SoCs. This driver provides the interface for UFS > + host controller to do PHY related programming. > + > config PHY_SAMSUNG_USB2 > tristate "Samsung USB 2.0 PHY driver" > depends on HAS_IOMEM > diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile > index db9b1aa0de6e..3959100fe8a2 100644 > --- a/drivers/phy/samsung/Makefile > +++ b/drivers/phy/samsung/Makefile > @@ -2,6 +2,7 @@ > obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o > obj-$(CONFIG_PHY_EXYNOS_PCIE) += phy-exynos-pcie.o > +obj-$(CONFIG_PHY_SAMSUNG_UFS) += phy-samsung-ufs.o > obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o > phy-exynos-usb2-y += phy-samsung-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o > diff --git a/drivers/phy/samsung/phy-exynos7-ufs.h > b/drivers/phy/samsung/phy-exynos7-ufs.h > new file mode 100644 > index ..c4aab792d30e > --- /dev/null > +++ b/drivers/phy/samsung/phy-exynos7-ufs.h > @@ -0,0 +1,86 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * UFS PHY driver data for Samsung EXYNOS7 SoC > + * > + * Copyright (C) 2020 Samsung Electronics Co., Ltd. > + */ > +#ifndef _PHY_EXYNOS7_UFS_H_ > +#define _PHY_EXYNOS7_UFS_H_ > + > +#include "phy-samsung-ufs.h" > + > +#define EXYNOS7_EMBEDDED_COMBO_PHY_CTRL0x720 > +#define EXYNOS7_EMBEDDED_COMBO_PHY_CTRL_MASK 0x1 > +#define EXYNOS7_EMBEDDED_COMBO_PHY_CTRL_EN BIT(0) > + > +/* Calibration for phy initialization */ > +static const struct samsung_ufs_phy_cfg exynos7_pre_init_cfg[] = { > + PHY_COMN_REG_CFG(0x00f, 0xfa, PWR_MODE_ANY), > + PHY_COMN_REG_CFG(0x010, 0x82, PWR_MODE_ANY), > + PHY_COMN_REG_CFG(0x011, 0x1e, PWR_MODE_ANY), > + PHY_COMN_REG_CFG(0x017, 0x84, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x035, 0x58, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x036, 0x32, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x037, 0x40, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x03b, 0x83, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x042, 0x88, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x043, 0xa6, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x048, 0x74, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x04c, 0x5b, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x04d, 0x83, PWR_MODE_ANY), > + PHY_TRSV_REG_CFG(0x05c, 0x14, PWR_MODE_ANY), > + END_UFS_PHY_CFG > +}; > + > +static const struct samsung_ufs_phy_cfg exynos7_post_init_cfg[] = { > + END_UFS_PHY_CFG > +}; > + > +/* Calibration for HS mode series A/B */ > +static const struct samsung_ufs_phy_cfg exynos7_pre_pwr_hs_cfg[] = { > + PHY_COMN_REG_CFG(0x00f, 0xfa, PWR_MODE_HS_ANY), > + PHY_COMN_REG_CFG(0x010, 0x82, PWR_MODE_HS_ANY), > + PHY_COMN_REG_CFG(0x011, 0x1e, PWR_MODE_HS_ANY), > + /* Setting order: 1st(0x16, 2nd(0x15) */ > + PHY_COMN_REG_CFG(0x016, 0xff, PWR_MODE_HS_ANY), > + PHY_COMN_REG_CFG(0x015, 0x80, PWR_MODE_HS_ANY), > + PHY_COMN_REG_CFG(0x017, 0x94, PWR_MODE_HS_ANY), > + PHY_TRSV_REG_CFG(0x036, 0x32, PWR_MODE_HS_ANY), > + PHY_TRSV_REG_CFG(0x037, 0x43,
Re: [PATCH v5 07/13] pwm: add support for sl28cpld PWM controller
Hi Uwe, first of all, thank you for that thorough review. Am 2020-07-09 10:50, schrieb Uwe Kleine-König: On Mon, Jul 06, 2020 at 07:53:47PM +0200, Michael Walle wrote: diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c new file mode 100644 index ..8ee286b605bf --- /dev/null +++ b/drivers/pwm/pwm-sl28cpld.c @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * sl28cpld PWM driver + * + * Copyright 2020 Kontron Europe GmbH + */ Is there publically available documenation available? If so please add a link here. Unfortunately not. But it should be easy enough and I'll describe it briefly in the header. + +#include +#include +#include +#include +#include +#include +#include + +/* + * PWM timer block registers. + */ +#define PWM_CTRL 0x00 +#define PWM_ENABLE BIT(7) +#define PWM_MODE_250HZ 0 +#define PWM_MODE_500HZ 1 +#define PWM_MODE_1KHZ2 +#define PWM_MODE_2KHZ3 +#define PWM_MODE_MASKGENMASK(1, 0) +#define PWM_CYCLE 0x01 +#define PWM_CYCLE_MAX0x7f Please use a less generic prefix for your defines. Also I like having the defines for field names include register name. Something like: #define PWM_SL28CPLD_CTRL 0x00 #define PWM_SL28CPLD_CTRL_ENABLEBIT(7) #define PWM_SL28CPLD_CTRL_MODE_MASK GENMASK(1, 0) Ok. #define PWM_SL28CPLD_CTRL_MODE_250HZFIELD_PREP(PWM_SL28CPLD_CTRL_MODE_MASK, 0) Shouldn't we just "#define ..MODE_250HZ 1" use FIELD_PREP inside the code, so you can actually use the normalized enumeration values, too? Actually, I'll rename the PWM_MODE to PWM_PRESCALER, because that is more accurate. +struct sl28cpld_pwm { + struct pwm_chip pwm_chip; + struct regmap *regmap; + u32 offset; +}; + +struct sl28cpld_pwm_periods { + u8 ctrl; + unsigned long duty_cycle; +}; + +struct sl28cpld_pwm_config { + unsigned long period_ns; + u8 max_duty_cycle; +}; + +static struct sl28cpld_pwm_config sl28cpld_pwm_config[] = { const ? (Or drop as the values can be easily computed, see below.) + [PWM_MODE_250HZ] = { .period_ns = 400, .max_duty_cycle = 0x80 }, + [PWM_MODE_500HZ] = { .period_ns = 200, .max_duty_cycle = 0x40 }, + [PWM_MODE_1KHZ] = { .period_ns = 100, .max_duty_cycle = 0x20 }, + [PWM_MODE_2KHZ] = { .period_ns = 50, .max_duty_cycle = 0x10 }, +}; + +static void sl28cpld_pwm_get_state(struct pwm_chip *chip, + struct pwm_device *pwm, + struct pwm_state *state) +{ + struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev); + static struct sl28cpld_pwm_config *config; + unsigned int reg; + unsigned int mode; + + regmap_read(priv->regmap, priv->offset + PWM_CTRL, ); + + state->enabled = reg & PWM_ENABLE; Would it be more consisted to use FIELD_GET here, too? I had used FIELD_GET only for bit-fields with more than one bit, i.e. no flags. But that is just a matter of taste, I guess. I'd prefer to keep the simple "reg & PWM_ENABLE". If you insist on the FIELD_GET() I'll change it ;) + + mode = FIELD_GET(PWM_MODE_MASK, reg); + config = _pwm_config[mode]; + state->period = config->period_ns; I wonder if this could be done more effectively without the above table. Something like: state->period = 400 >> mode. The reason I introduced a lookup table here was that I need a list of the supported modes; I wasn't aware of the rounding. See also below. (with a #define for 400 of course). + regmap_read(priv->regmap, priv->offset + PWM_CYCLE, ); + pwm_set_relative_duty_cycle(state, reg, config->max_duty_cycle); Oh, what a creative idea to use pwm_set_relative_duty_cycle here. What is that helper for then? The former versions did the same calculations (i.e. DIV_ROUND_CLOSEST_ULL()) just open coded. But I guess then it was also rounding the wrong way. Unfortunately it's using the wrong rounding strategy. Please enable PWM_DEBUG which should diagnose these problems (given enough testing). Is there any written documentation on how to round, i.e. up or down? I had a look Documentation/driver-api/pwm.rst again. But couldn't find anything. A grep DIV_ROUND_CLOSEST_ULL() turns out that quite a few drivers use it, so I did the same ;) (Hmm, on second thought I'm not sure that rounding is relevant with the numbers of this hardware. Still it's wrong in general and I don't want to have others copy this.) +} + +static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct sl28cpld_pwm *priv = dev_get_drvdata(chip->dev); + struct sl28cpld_pwm_config *config; + unsigned int cycle; + int ret; +
Re: WARNING: at mm/mremap.c:211 move_page_tables in i386
On Sat, 11 Jul 2020 at 01:35, Linus Torvalds wrote: > > On Fri, Jul 10, 2020 at 10:48 AM Naresh Kamboju > wrote: I have started bisecting this problem and found the first bad commit commit 9f132f7e145506efc0744426cb338b18a54afc3b Author: Joel Fernandes (Google) Date: Thu Jan 3 15:28:41 2019 -0800 mm: select HAVE_MOVE_PMD on x86 for faster mremap Moving page-tables at the PMD-level on x86 is known to be safe. Enable this option so that we can do fast mremap when possible. Link: http://lkml.kernel.org/r/20181108181201.88826-4-joe...@google.com Signed-off-by: Joel Fernandes (Google) Suggested-by: Kirill A. Shutemov Acked-by: Kirill A. Shutemov Cc: Julia Lawall Cc: Michal Hocko Cc: William Kucharski Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index e260460210e1..6185d4f33296 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -172,6 +172,7 @@ config X86 select HAVE_MEMBLOCK_NODE_MAP select HAVE_MIXED_BREAKPOINTS_REGS select HAVE_MOD_ARCH_SPECIFIC + select HAVE_MOVE_PMD select HAVE_NMI select HAVE_OPROFILE select HAVE_OPTPROBES After reverting the above patch the reported kernel warning got fixed on Linus mainline tree 5.8.0-rc4. -- Linaro LKFT https://lkft.linaro.org
INFO: trying to register non-static key in addrconf_notify
Hello, syzbot found the following crash on: HEAD commit:7cc2a8ea Merge tag 'block-5.8-2020-07-01' of git://git.ker.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=102d01a310 kernel config: https://syzkaller.appspot.com/x/.config?x=183dd243398ba7ec dashboard link: https://syzkaller.appspot.com/bug?extid=bf9c23e0afdec81d9470 compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+bf9c23e0afdec81d9...@syzkaller.appspotmail.com INFO: trying to register non-static key. the code is fine but needs lockdep annotation. turning off the locking correctness validator. CPU: 1 PID: 317 Comm: kworker/u4:6 Not tainted 5.8.0-rc3-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1f0/0x31e lib/dump_stack.c:118 register_lock_class+0xf06/0x1520 kernel/locking/lockdep.c:893 __lock_acquire+0x102/0x2c30 kernel/locking/lockdep.c:4259 lock_acquire+0x160/0x720 kernel/locking/lockdep.c:4959 __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline] _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:319 addrconf_ifdown+0x5f8/0x1670 net/ipv6/addrconf.c:3734 addrconf_notify+0x3f9/0x3a60 net/ipv6/addrconf.c:3602 notifier_call_chain kernel/notifier.c:83 [inline] __raw_notifier_call_chain kernel/notifier.c:361 [inline] raw_notifier_call_chain+0xe7/0x170 kernel/notifier.c:368 call_netdevice_notifiers_info net/core/dev.c:2027 [inline] call_netdevice_notifiers_extack net/core/dev.c:2039 [inline] call_netdevice_notifiers net/core/dev.c:2053 [inline] rollback_registered_many+0xbe3/0x14a0 net/core/dev.c:8968 unregister_netdevice_many+0x46/0x260 net/core/dev.c:10113 ip6gre_exit_batch_net+0x435/0x460 net/ipv6/ip6_gre.c:1608 ops_exit_list net/core/net_namespace.c:189 [inline] cleanup_net+0x79c/0xba0 net/core/net_namespace.c:603 process_one_work+0x789/0xfc0 kernel/workqueue.c:2269 worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415 kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293 --- This bug 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 bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
Re: [PATCH v29 13/16] leds: lp5523: Update the lp5523 code to add multicolor brightness function
On 7/11/20 5:57 PM, Pavel Machek wrote: Hi! Add the multicolor brightness call back to support the multicolor framework. This call back allows setting brightness on grouped channels Extra space before "brightness". And before "This". -- Best regards, Jacek Anaszewski
Re: [PATCH] regulator: Use const regulator_ops when possible
Add const to declaration of struct regulator_ops where appropriate. Signed-off-by: Joe Perches --- Compiled/untested drivers/regulator/dummy.c | 2 +- drivers/regulator/fixed.c | 4 ++-- drivers/regulator/pca9450-regulator.c | 6 +++--- drivers/regulator/stw481x-vmmc.c | 2 +- drivers/regulator/tps51632-regulator.c | 2 +- drivers/regulator/tps6105x-regulator.c | 2 +- drivers/regulator/tps62360-regulator.c | 2 +- drivers/regulator/tps65086-regulator.c | 4 ++-- drivers/regulator/tps65090-regulator.c | 8 drivers/regulator/tps6586x-regulator.c | 8 drivers/regulator/tps65910-regulator.c | 10 +- drivers/regulator/tps65912-regulator.c | 4 ++-- 12 files changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/regulator/dummy.c b/drivers/regulator/dummy.c index 74de6983c61a..306e417448ec 100644 --- a/drivers/regulator/dummy.c +++ b/drivers/regulator/dummy.c @@ -27,7 +27,7 @@ static struct regulator_init_data dummy_initdata = { }, }; -static struct regulator_ops dummy_ops; +static const struct regulator_ops dummy_ops; static const struct regulator_desc dummy_desc = { .name = "regulator-dummy", diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c index d54830e48b8d..2ca84e88d34d 100644 --- a/drivers/regulator/fixed.c +++ b/drivers/regulator/fixed.c @@ -131,10 +131,10 @@ of_get_fixed_voltage_config(struct device *dev, return config; } -static struct regulator_ops fixed_voltage_ops = { +static const struct regulator_ops fixed_voltage_ops = { }; -static struct regulator_ops fixed_voltage_clkenabled_ops = { +static const struct regulator_ops fixed_voltage_clkenabled_ops = { .enable = reg_clock_enable, .disable = reg_clock_disable, .is_enabled = reg_clock_is_enabled, diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c index 02250459aa90..4747ceff99d0 100644 --- a/drivers/regulator/pca9450-regulator.c +++ b/drivers/regulator/pca9450-regulator.c @@ -90,7 +90,7 @@ static int pca9450_dvs_set_ramp_delay(struct regulator_dev *rdev, BUCK1_RAMP_MASK, ramp_value << 6); } -static struct regulator_ops pca9450_dvs_buck_regulator_ops = { +static const struct regulator_ops pca9450_dvs_buck_regulator_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, .is_enabled = regulator_is_enabled_regmap, @@ -101,7 +101,7 @@ static struct regulator_ops pca9450_dvs_buck_regulator_ops = { .set_ramp_delay = pca9450_dvs_set_ramp_delay, }; -static struct regulator_ops pca9450_buck_regulator_ops = { +static const struct regulator_ops pca9450_buck_regulator_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, .is_enabled = regulator_is_enabled_regmap, @@ -111,7 +111,7 @@ static struct regulator_ops pca9450_buck_regulator_ops = { .set_voltage_time_sel = regulator_set_voltage_time_sel, }; -static struct regulator_ops pca9450_ldo_regulator_ops = { +static const struct regulator_ops pca9450_ldo_regulator_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, .is_enabled = regulator_is_enabled_regmap, diff --git a/drivers/regulator/stw481x-vmmc.c b/drivers/regulator/stw481x-vmmc.c index 6dc2316daad3..1dd90aa1a79d 100644 --- a/drivers/regulator/stw481x-vmmc.c +++ b/drivers/regulator/stw481x-vmmc.c @@ -27,7 +27,7 @@ static const unsigned int stw481x_vmmc_voltages[] = { 330, }; -static struct regulator_ops stw481x_vmmc_ops = { +static const struct regulator_ops stw481x_vmmc_ops = { .list_voltage = regulator_list_voltage_table, .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, diff --git a/drivers/regulator/tps51632-regulator.c b/drivers/regulator/tps51632-regulator.c index c139890c1514..a15e415e61d5 100644 --- a/drivers/regulator/tps51632-regulator.c +++ b/drivers/regulator/tps51632-regulator.c @@ -108,7 +108,7 @@ static int tps51632_dcdc_set_ramp_delay(struct regulator_dev *rdev, return ret; } -static struct regulator_ops tps51632_dcdc_ops = { +static const struct regulator_ops tps51632_dcdc_ops = { .get_voltage_sel= regulator_get_voltage_sel_regmap, .set_voltage_sel= regulator_set_voltage_sel_regmap, .list_voltage = regulator_list_voltage_linear, diff --git a/drivers/regulator/tps6105x-regulator.c b/drivers/regulator/tps6105x-regulator.c index f8939af0bd2c..a6469fe05635 100644 --- a/drivers/regulator/tps6105x-regulator.c +++ b/drivers/regulator/tps6105x-regulator.c @@ -26,7 +26,7 @@ static const unsigned int tps6105x_voltages[] = { 500, /* There is an additional 5V */ }; -static struct regulator_ops tps6105x_regulator_ops = { +static const struct regulator_ops tps6105x_regulator_ops = { .enable
Re: [PATCH v5] x86/umip: Add emulation/spoofing for SLDT and STR instructions
On Fri, Jul 10, 2020 at 3:45 PM Brendan Shanks wrote: > > Add emulation/spoofing of SLDT and STR for both 32- and 64-bit > processes. > > Wine users have found a small number of Windows apps using SLDT that > were crashing when run on UMIP-enabled systems. > Acked-by: Andy Lutomirski I tested this under KVM-emulated UMIP. I don't have a real UMIP system handle to test STR. --Andy
Re: Linux kernel in-tree Rust support
On Thu, 9 Jul 2020, Nick Desaulniers wrote: Hello folks, I'm working on putting together an LLVM "Micro Conference" for the upcoming Linux Plumbers Conf (https://www.linuxplumbersconf.org/event/7/page/47-attend). It's not solidified yet, but I would really like to run a session on support for Rust "in tree." I suspect we could cover technical aspects of what that might look like (I have a prototype of that, was trivial to wire up KBuild support), but also a larger question of "should we do this?" or "how might we place limits on where this can be used?" Question to folks explicitly in To:, are you planning on attending plumbers? If so, would this be an interesting topic that you'd participate in? Like Alex - I hadn't decided yet but I'll definitely attend for this! -- Geoffrey Thomas https://ldpreload.com geo...@ldpreload.com
Re: BUG: stack guard page was hit in fixup_exception
On Sat, Jul 11, 2020 at 12:10 AM syzbot wrote: > > Hello, > > syzbot found the following crash on: > > HEAD commit:42f82040 Merge tag 'drm-fixes-2020-07-10' of git://anongit.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=12d8033310 > kernel config: https://syzkaller.appspot.com/x/.config?x=66ad203c2bb6d8b > dashboard link: https://syzkaller.appspot.com/bug?extid=3370f8260246b965fefd > compiler: gcc (GCC) 10.1.0-syz 20200507 > > Unfortunately, I don't have any reproducer for this crash yet. > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+3370f8260246b965f...@syzkaller.appspotmail.com > > Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <00> 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 I bet this is the same fbcon bug.
general protection fault in htab_elem_free_rcu
Hello, syzbot found the following crash on: HEAD commit:d31958b3 Add linux-next specific files for 20200710 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=1393585710 kernel config: https://syzkaller.appspot.com/x/.config?x=3fe4fccb94cbc1a6 dashboard link: https://syzkaller.appspot.com/bug?extid=a9db0ab6a8e0ca14351d compiler: gcc (GCC) 10.1.0-syz 20200507 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=133db22b10 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16fe6f1f10 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+a9db0ab6a8e0ca143...@syzkaller.appspotmail.com general protection fault, probably for non-canonical address 0xf33bb70012bc003b: [#1] PREEMPT SMP KASAN KASAN: maybe wild-memory-access in range [0x99ddd80095e001d8-0x99ddd80095e001df] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0-rc4-next-20200710-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:htab_elem_free kernel/bpf/hashtab.c:769 [inline] RIP: 0010:htab_elem_free_rcu+0x4a/0x110 kernel/bpf/hashtab.c:779 Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bc 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b f8 48 8d 7d 18 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 82 00 00 00 44 8b 65 18 bf 05 RSP: 0018:c9007e48 EFLAGS: 00010a03 RAX: dc00 RBX: 888084800010 RCX: 0001 RDX: 133bbb0012bc003b RSI: 8186891e RDI: 99ddd80095e001de RBP: 99ddd80095e001c6 R08: R09: 8c5b09f7 R10: fbfff18b613e R11: R12: c9007ed8 R13: 88808480 R14: R15: 89a86580 FS: () GS:8880ae60() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 004c6368 CR3: 09a79000 CR4: 001506f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: rcu_do_batch kernel/rcu/tree.c:2418 [inline] rcu_core+0x5dc/0x11d0 kernel/rcu/tree.c:2645 __do_softirq+0x34c/0xa60 kernel/softirq.c:292 asm_call_on_stack+0xf/0x20 arch/x86/entry/entry_64.S:706 __run_on_irqstack arch/x86/include/asm/irq_stack.h:22 [inline] run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:48 [inline] do_softirq_own_stack+0x111/0x170 arch/x86/kernel/irq_64.c:77 invoke_softirq kernel/softirq.c:387 [inline] __irq_exit_rcu kernel/softirq.c:417 [inline] irq_exit_rcu+0x229/0x270 kernel/softirq.c:429 sysvec_apic_timer_interrupt+0x54/0x120 arch/x86/kernel/apic/apic.c:1090 asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:587 RIP: 0010:native_safe_halt+0xe/0x10 arch/x86/include/asm/irqflags.h:61 Code: ff 4c 89 ef e8 93 66 c6 f9 e9 8e fe ff ff 48 89 df e8 86 66 c6 f9 eb 8a cc cc cc cc e9 07 00 00 00 0f 00 2d 34 9b 5b 00 fb f4 90 e9 07 00 00 00 0f 00 2d 24 9b 5b 00 f4 c3 cc cc 55 53 e8 09 RSP: 0018:89a07c70 EFLAGS: 0293 RAX: RBX: RCX: RDX: 89a86580 RSI: 87ed2968 RDI: 87ed293e RBP: 8880a6a93064 R08: R09: R10: 0001 R11: R12: 8880a6a93064 R13: 11340f98 R14: 8880a6a93065 R15: 0001 arch_safe_halt arch/x86/include/asm/paravirt.h:150 [inline] acpi_safe_halt+0x8d/0x110 drivers/acpi/processor_idle.c:111 acpi_idle_do_entry+0x15c/0x1b0 drivers/acpi/processor_idle.c:525 acpi_idle_enter+0x3f9/0xab0 drivers/acpi/processor_idle.c:651 cpuidle_enter_state+0xff/0x960 drivers/cpuidle/cpuidle.c:235 cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:346 call_cpuidle kernel/sched/idle.c:126 [inline] cpuidle_idle_call kernel/sched/idle.c:214 [inline] do_idle+0x431/0x6d0 kernel/sched/idle.c:276 cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:372 start_kernel+0x9cb/0xa06 init/main.c:1045 secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243 Modules linked in: ---[ end trace 5ce7b44eaacf6c96 ]--- RIP: 0010:htab_elem_free kernel/bpf/hashtab.c:769 [inline] RIP: 0010:htab_elem_free_rcu+0x4a/0x110 kernel/bpf/hashtab.c:779 Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bc 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b f8 48 8d 7d 18 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 82 00 00 00 44 8b 65 18 bf 05 RSP: 0018:c9007e48 EFLAGS: 00010a03 RAX: dc00 RBX: 888084800010 RCX: 0001 RDX: 133bbb0012bc003b RSI: 8186891e RDI: 99ddd80095e001de RBP: 99ddd80095e001c6 R08: R09: 8c5b09f7 R10: fbfff18b613e R11: R12: c9007ed8 R13: 88808480 R14: R15: 89a86580 FS: () GS:8880ae60() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2:
Re: [PATCH 07/11] media: exynos4-is: Add support for multiple sensors on one port
Hi Tomasz, On 2020-07-07 11:36 a.m., Tomasz Figa wrote: > On Sat, Apr 25, 2020 at 07:26:46PM -0700, Jonathan Bakker wrote: >> On some devices, there may be multiple camera sensors attached >> to the same port. Make sure we probe all of them, not just the >> first one. >> >> Signed-off-by: Jonathan Bakker >> --- >> drivers/media/platform/exynos4-is/media-dev.c | 32 --- >> 1 file changed, 21 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/media/platform/exynos4-is/media-dev.c >> b/drivers/media/platform/exynos4-is/media-dev.c >> index b38445219c72..a87ebd7913be 100644 >> --- a/drivers/media/platform/exynos4-is/media-dev.c >> +++ b/drivers/media/platform/exynos4-is/media-dev.c >> @@ -397,25 +397,28 @@ static void fimc_md_pipelines_free(struct fimc_md *fmd) >> /* Parse port node and register as a sub-device any sensor specified there. >> */ >> static int fimc_md_parse_port_node(struct fimc_md *fmd, >> struct device_node *port, >> - unsigned int index) >> + unsigned int *index) >> { >> -struct fimc_source_info *pd = >sensor[index].pdata; >> +struct fimc_source_info *pd; >> struct device_node *rem, *ep, *np; >> -struct v4l2_fwnode_endpoint endpoint = { .bus_type = 0 }; >> +struct v4l2_fwnode_endpoint endpoint; >> int ret; >> >> -/* Assume here a port node can have only one endpoint node. */ >> ep = of_get_next_child(port, NULL); >> if (!ep) >> return 0; >> >> +parse_sensor: >> +pd = >sensor[*index].pdata; >> +endpoint.bus_type = 0; >> + >> ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), ); >> if (ret) { >> of_node_put(ep); >> return ret; >> } >> >> -if (WARN_ON(endpoint.base.port == 0) || index >= FIMC_MAX_SENSORS) { >> +if (WARN_ON(endpoint.base.port == 0) || *index >= FIMC_MAX_SENSORS) { >> of_node_put(ep); >> return -EINVAL; >> } >> @@ -462,16 +465,16 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd, >> pd->fimc_bus_type = pd->sensor_bus_type; >> of_node_put(np); >> >> -if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) { >> +if (WARN_ON(*index >= ARRAY_SIZE(fmd->sensor))) { >> of_node_put(rem); >> return -EINVAL; >> } >> >> -fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_FWNODE; >> -fmd->sensor[index].asd.match.fwnode = of_fwnode_handle(rem); >> +fmd->sensor[*index].asd.match_type = V4L2_ASYNC_MATCH_FWNODE; >> +fmd->sensor[*index].asd.match.fwnode = of_fwnode_handle(rem); >> >> ret = v4l2_async_notifier_add_subdev(>subdev_notifier, >> - >sensor[index].asd); >> + >sensor[*index].asd); >> if (ret) { >> of_node_put(rem); >> return ret; >> @@ -479,6 +482,13 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd, >> >> fmd->num_sensors++; >> >> +/* Check for additional sensors on same port */ >> +ep = of_get_next_child(port, ep); >> +if (ep) { >> +(*index)++; > > Do we need this index argument at all? I can see that we already have > fmd->num_sensors and we increment it every time we discover a sensor. > Perhaps we could just use it instead? > >> +goto parse_sensor; > > As we know, goto in principle isn't the best coding pattern. There is a > number of exceptions where it is welcome, e.g. error handling, but > reimplementing a loop using goto is not very nice. > > Instead, could you separate the code that probes one sensor into > fimc_md_parse_one_endpoint() and in this one simply iterate over all child > nodes of the port using for_each_child_of_node()? > That definitely looks doable, thanks for the suggestion. I'll work on implementing and testing this. It should then also be possible to remove the index hack as well. > Best regards, > Tomasz > Thanks, Jonathan