Re: linux-next: build warning after merge of the sound-asoc tree
On 7/26/2018 7:49 AM, Stephen Rothwell wrote:
> Hi all,
>
> After merging the sound-asoc tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
>
> sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
> sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>dev_err(>dev, "Failed to register regulator: %d\n",
>^
> ret);
>
>
> Introduced by commit
>
> 7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and
> ADAU7002")
>
> This is a real bug.
>
Posted the fix for the above warning message:
https://patchwork.kernel.org/patch/10545235/
Thanks,
Akshu
Re: linux-next: build warning after merge of the sound-asoc tree
On 7/26/2018 7:49 AM, Stephen Rothwell wrote:
> Hi all,
>
> After merging the sound-asoc tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
>
> sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
> sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>dev_err(>dev, "Failed to register regulator: %d\n",
>^
> ret);
>
>
> Introduced by commit
>
> 7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and
> ADAU7002")
>
> This is a real bug.
>
Posted the fix for the above warning message:
https://patchwork.kernel.org/patch/10545235/
Thanks,
Akshu
[PATCH] ASoC: AMD: Fix build warning
Fixes
sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may
be used uninitialized in this function [-Wmaybe-uninitialized]
dev_err(>dev, "Failed to register regulator: %d\n",
ret);
Signed-off-by: Akshu Agrawal
---
sound/soc/amd/acp-da7219-max98357a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/amd/acp-da7219-max98357a.c
b/sound/soc/amd/acp-da7219-max98357a.c
index cd3cf6e..48c1d94 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -365,7 +365,7 @@ static int cz_probe(struct platform_device *pdev)
_da7219_cfg);
if (IS_ERR(rdev)) {
dev_err(>dev, "Failed to register regulator: %d\n",
- ret);
+ (int)PTR_ERR(rdev);
return -EINVAL;
}
--
1.9.1
[PATCH] ASoC: AMD: Fix build warning
Fixes
sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may
be used uninitialized in this function [-Wmaybe-uninitialized]
dev_err(>dev, "Failed to register regulator: %d\n",
ret);
Signed-off-by: Akshu Agrawal
---
sound/soc/amd/acp-da7219-max98357a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/amd/acp-da7219-max98357a.c
b/sound/soc/amd/acp-da7219-max98357a.c
index cd3cf6e..48c1d94 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -365,7 +365,7 @@ static int cz_probe(struct platform_device *pdev)
_da7219_cfg);
if (IS_ERR(rdev)) {
dev_err(>dev, "Failed to register regulator: %d\n",
- ret);
+ (int)PTR_ERR(rdev);
return -EINVAL;
}
--
1.9.1
[PATCH v2 3/3] selftests/ftrace: Fix kprobe string testcase to not probe notrace function
Fix kprobe string argument testcase to not probe notrace
function. Instead, it probes tracefs function which must
be available with ftrace.
Signed-off-by: Masami Hiramatsu
---
.../ftrace/test.d/kprobe/kprobe_args_string.tc | 30
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
index a0002563e9ee..1ad70cdaf442 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
@@ -9,28 +9,22 @@ echo > kprobe_events
case `uname -m` in
x86_64)
- ARG2=%si
- OFFS=8
+ ARG1=%di
;;
i[3456]86)
- ARG2=%cx
- OFFS=4
+ ARG1=%ax
;;
aarch64)
- ARG2=%x1
- OFFS=8
+ ARG1=%x0
;;
arm*)
- ARG2=%r1
- OFFS=4
+ ARG1=%r0
;;
ppc64*)
- ARG2=%r4
- OFFS=8
+ ARG1=%r3
;;
ppc*)
- ARG2=%r4
- OFFS=4
+ ARG1=%r3
;;
*)
echo "Please implement other architecture here"
@@ -38,17 +32,17 @@ ppc*)
esac
: "Test get argument (1)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string" >
kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events
echo 1 > events/kprobes/testprobe/enable
-! echo test >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\"" trace
echo 0 > events/kprobes/testprobe/enable
: "Test get argument (2)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string
arg2=+0(+${OFFS}(${ARG2})):string" > kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string
arg2=+0(${ARG1}):string" > kprobe_events
echo 1 > events/kprobes/testprobe/enable
-! echo test1 test2 >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test1\" arg2=\"test2\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace
echo 0 > events/enable
echo > kprobe_events
[PATCH v2 3/3] selftests/ftrace: Fix kprobe string testcase to not probe notrace function
Fix kprobe string argument testcase to not probe notrace
function. Instead, it probes tracefs function which must
be available with ftrace.
Signed-off-by: Masami Hiramatsu
---
.../ftrace/test.d/kprobe/kprobe_args_string.tc | 30
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
index a0002563e9ee..1ad70cdaf442 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
@@ -9,28 +9,22 @@ echo > kprobe_events
case `uname -m` in
x86_64)
- ARG2=%si
- OFFS=8
+ ARG1=%di
;;
i[3456]86)
- ARG2=%cx
- OFFS=4
+ ARG1=%ax
;;
aarch64)
- ARG2=%x1
- OFFS=8
+ ARG1=%x0
;;
arm*)
- ARG2=%r1
- OFFS=4
+ ARG1=%r0
;;
ppc64*)
- ARG2=%r4
- OFFS=8
+ ARG1=%r3
;;
ppc*)
- ARG2=%r4
- OFFS=4
+ ARG1=%r3
;;
*)
echo "Please implement other architecture here"
@@ -38,17 +32,17 @@ ppc*)
esac
: "Test get argument (1)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string" >
kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events
echo 1 > events/kprobes/testprobe/enable
-! echo test >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\"" trace
echo 0 > events/kprobes/testprobe/enable
: "Test get argument (2)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string
arg2=+0(+${OFFS}(${ARG2})):string" > kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string
arg2=+0(${ARG1}):string" > kprobe_events
echo 1 > events/kprobes/testprobe/enable
-! echo test1 test2 >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test1\" arg2=\"test2\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace
echo 0 > events/enable
echo > kprobe_events
[PATCH v2 2/3] selftest/ftrace: Move kprobe selftest function to separate compile unit
From: Francis Deslauriers
Move selftest function to its own compile unit so it can be compiled
with the ftrace cflags (CC_FLAGS_FTRACE) allowing it to be probed
during the ftrace startup tests.
Signed-off-by: Francis Deslauriers
Acked-by: Masami Hiramatsu
---
kernel/trace/Makefile|5 +
kernel/trace/trace_kprobe.c | 12 +---
kernel/trace/trace_kprobe_selftest.c | 10 ++
kernel/trace/trace_kprobe_selftest.h |7 +++
4 files changed, 23 insertions(+), 11 deletions(-)
create mode 100644 kernel/trace/trace_kprobe_selftest.c
create mode 100644 kernel/trace/trace_kprobe_selftest.h
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index e2538c7638d4..e38771eccb2f 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -13,6 +13,11 @@ obj-y += trace_selftest_dynamic.o
endif
endif
+ifdef CONFIG_FTRACE_STARTUP_TEST
+CFLAGS_trace_kprobe_selftest.o = $(CC_FLAGS_FTRACE)
+obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe_selftest.o
+endif
+
# If unlikely tracing is enabled, do not trace these files
ifdef CONFIG_TRACING_BRANCHES
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 1f1b4d712a7e..859f7c310a00 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -23,6 +23,7 @@
#include
#include
+#include "trace_kprobe_selftest.h"
#include "trace_probe.h"
#define KPROBE_EVENT_SYSTEM "kprobes"
@@ -1573,17 +1574,6 @@ fs_initcall(init_kprobe_trace);
#ifdef CONFIG_FTRACE_STARTUP_TEST
-/*
- * The "__used" keeps gcc from removing the function symbol
- * from the kallsyms table. 'noinline' makes sure that there
- * isn't an inlined version used by the test method below
- */
-static __used __init noinline int
-kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6)
-{
- return a1 + a2 + a3 + a4 + a5 + a6;
-}
-
static __init struct trace_event_file *
find_trace_probe_file(struct trace_kprobe *tk, struct trace_array *tr)
{
diff --git a/kernel/trace/trace_kprobe_selftest.c
b/kernel/trace/trace_kprobe_selftest.c
new file mode 100644
index ..16548ee4c8c6
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int
a6)
+{
+ return a1 + a2 + a3 + a4 + a5 + a6;
+}
diff --git a/kernel/trace/trace_kprobe_selftest.h
b/kernel/trace/trace_kprobe_selftest.h
new file mode 100644
index ..4e10ec41c013
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int
a6);
[PATCH v2 2/3] selftest/ftrace: Move kprobe selftest function to separate compile unit
From: Francis Deslauriers
Move selftest function to its own compile unit so it can be compiled
with the ftrace cflags (CC_FLAGS_FTRACE) allowing it to be probed
during the ftrace startup tests.
Signed-off-by: Francis Deslauriers
Acked-by: Masami Hiramatsu
---
kernel/trace/Makefile|5 +
kernel/trace/trace_kprobe.c | 12 +---
kernel/trace/trace_kprobe_selftest.c | 10 ++
kernel/trace/trace_kprobe_selftest.h |7 +++
4 files changed, 23 insertions(+), 11 deletions(-)
create mode 100644 kernel/trace/trace_kprobe_selftest.c
create mode 100644 kernel/trace/trace_kprobe_selftest.h
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index e2538c7638d4..e38771eccb2f 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -13,6 +13,11 @@ obj-y += trace_selftest_dynamic.o
endif
endif
+ifdef CONFIG_FTRACE_STARTUP_TEST
+CFLAGS_trace_kprobe_selftest.o = $(CC_FLAGS_FTRACE)
+obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe_selftest.o
+endif
+
# If unlikely tracing is enabled, do not trace these files
ifdef CONFIG_TRACING_BRANCHES
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 1f1b4d712a7e..859f7c310a00 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -23,6 +23,7 @@
#include
#include
+#include "trace_kprobe_selftest.h"
#include "trace_probe.h"
#define KPROBE_EVENT_SYSTEM "kprobes"
@@ -1573,17 +1574,6 @@ fs_initcall(init_kprobe_trace);
#ifdef CONFIG_FTRACE_STARTUP_TEST
-/*
- * The "__used" keeps gcc from removing the function symbol
- * from the kallsyms table. 'noinline' makes sure that there
- * isn't an inlined version used by the test method below
- */
-static __used __init noinline int
-kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6)
-{
- return a1 + a2 + a3 + a4 + a5 + a6;
-}
-
static __init struct trace_event_file *
find_trace_probe_file(struct trace_kprobe *tk, struct trace_array *tr)
{
diff --git a/kernel/trace/trace_kprobe_selftest.c
b/kernel/trace/trace_kprobe_selftest.c
new file mode 100644
index ..16548ee4c8c6
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int
a6)
+{
+ return a1 + a2 + a3 + a4 + a5 + a6;
+}
diff --git a/kernel/trace/trace_kprobe_selftest.h
b/kernel/trace/trace_kprobe_selftest.h
new file mode 100644
index ..4e10ec41c013
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int
a6);
[PATCH v2 1/3] tracing: kprobes: Prohibit probing on notrace function
Prohibit kprobe-events probing on notrace function.
Since probing on the notrace function can cause recursive
event call. In most case those are just skipped, but
in some case it falls into infinit recursive call.
This protection can be disabled by the kconfig
CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
recommended to keep it "n" for normal kernel.
Signed-off-by: Masami Hiramatsu
Tested-by: Francis Deslauriers
---
Changes from v1
- Add CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig for knocking down
the protection.
---
kernel/trace/Kconfig| 18 ++
kernel/trace/trace_kprobe.c | 23 +++
2 files changed, 41 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index dcc0166d1997..24d5a58467a3 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -456,6 +456,24 @@ config KPROBE_EVENTS
This option is also required by perf-probe subcommand of perf tools.
If you want to use perf tools, this option is strongly recommended.
+config KPROBE_EVENTS_ON_NOTRACE
+ bool "Do NOT protect notrace function from kprobe events"
+ depends on KPROBE_EVENTS
+ default n
+ help
+ This is only for the developers who want to debug ftrace itself
+ using kprobe events.
+
+ Usually, ftrace related functions are protected from kprobe-events
+ to prevent an infinit recursion or any unexpected execution path
+ which leads to a kernel crash.
+
+ This option disables such protection and allows you to put kprobe
+ events on ftrace functions for debugging ftrace by itself.
+ Note that this might let you shoot yourself in the foot.
+
+ If unsure, say N.
+
config UPROBE_EVENTS
bool "Enable uprobes-based dynamic events"
depends on ARCH_SUPPORTS_UPROBES
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 27ace4513c43..1f1b4d712a7e 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -496,6 +496,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct
trace_event_file *file)
return ret;
}
+#ifdef CONFIG_KPROBE_EVENTS_ON_NOTRACE
+#define within_notrace_func(tk)(false)
+#else
+static bool within_notrace_func(struct trace_kprobe *tk)
+{
+ unsigned long offset, size, addr;
+
+ addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
+ addr += trace_kprobe_offset(tk);
+
+ if (!kallsyms_lookup_size_offset(addr, , ))
+ return true;/* Out of range. */
+
+ return !ftrace_location_range(addr - offset, addr - offset + size);
+}
+#endif
+
/* Internal register function - just handle k*probes and flags */
static int __register_trace_kprobe(struct trace_kprobe *tk)
{
@@ -504,6 +521,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
if (trace_probe_is_registered(>tp))
return -EINVAL;
+ if (within_notrace_func(tk)) {
+ pr_warn("Could not probe notrace function %s\n",
+ trace_kprobe_symbol(tk));
+ return -EINVAL;
+ }
+
for (i = 0; i < tk->tp.nr_args; i++)
traceprobe_update_arg(>tp.args[i]);
[PATCH v2 1/3] tracing: kprobes: Prohibit probing on notrace function
Prohibit kprobe-events probing on notrace function.
Since probing on the notrace function can cause recursive
event call. In most case those are just skipped, but
in some case it falls into infinit recursive call.
This protection can be disabled by the kconfig
CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
recommended to keep it "n" for normal kernel.
Signed-off-by: Masami Hiramatsu
Tested-by: Francis Deslauriers
---
Changes from v1
- Add CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig for knocking down
the protection.
---
kernel/trace/Kconfig| 18 ++
kernel/trace/trace_kprobe.c | 23 +++
2 files changed, 41 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index dcc0166d1997..24d5a58467a3 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -456,6 +456,24 @@ config KPROBE_EVENTS
This option is also required by perf-probe subcommand of perf tools.
If you want to use perf tools, this option is strongly recommended.
+config KPROBE_EVENTS_ON_NOTRACE
+ bool "Do NOT protect notrace function from kprobe events"
+ depends on KPROBE_EVENTS
+ default n
+ help
+ This is only for the developers who want to debug ftrace itself
+ using kprobe events.
+
+ Usually, ftrace related functions are protected from kprobe-events
+ to prevent an infinit recursion or any unexpected execution path
+ which leads to a kernel crash.
+
+ This option disables such protection and allows you to put kprobe
+ events on ftrace functions for debugging ftrace by itself.
+ Note that this might let you shoot yourself in the foot.
+
+ If unsure, say N.
+
config UPROBE_EVENTS
bool "Enable uprobes-based dynamic events"
depends on ARCH_SUPPORTS_UPROBES
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 27ace4513c43..1f1b4d712a7e 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -496,6 +496,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct
trace_event_file *file)
return ret;
}
+#ifdef CONFIG_KPROBE_EVENTS_ON_NOTRACE
+#define within_notrace_func(tk)(false)
+#else
+static bool within_notrace_func(struct trace_kprobe *tk)
+{
+ unsigned long offset, size, addr;
+
+ addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
+ addr += trace_kprobe_offset(tk);
+
+ if (!kallsyms_lookup_size_offset(addr, , ))
+ return true;/* Out of range. */
+
+ return !ftrace_location_range(addr - offset, addr - offset + size);
+}
+#endif
+
/* Internal register function - just handle k*probes and flags */
static int __register_trace_kprobe(struct trace_kprobe *tk)
{
@@ -504,6 +521,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
if (trace_probe_is_registered(>tp))
return -EINVAL;
+ if (within_notrace_func(tk)) {
+ pr_warn("Could not probe notrace function %s\n",
+ trace_kprobe_symbol(tk));
+ return -EINVAL;
+ }
+
for (i = 0; i < tk->tp.nr_args; i++)
traceprobe_update_arg(>tp.args[i]);
BUG: soft lockup in snd_virmidi_output_trigger
Reporting the crash: BUG: soft lockup in snd_virmidi_output_trigger This crash has been found in v4.18-rc3 using RaceFuzzer (a modified version of Syzkaller), which we describe more at the end of this report. Note that this bug is previously reported by Syzkaller a few month ago. (https://syzkaller.appspot.com/bug?id=642c927972318775e0ea1b4779ccd8624ce9e6f5) Nonetheless, the crash still can be detected. We guess that the crash has not fixed yet. We report the crash log and the attached reproducer with a hope that they are helpful to diagnose the crash and to fix a bug. C Reproducer: https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-dab082.c Kernel config: https://kiwi.cs.purdue.edu/static/race-fuzzer/config-dab082 (Please disregard all code related to "should_hypercall" in the C repro, as this is only for our debugging purposes using our own hypervisor.) Crash log: == watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [syz-executor0:24261] Modules linked in: irq event stamp: 6692 hardirqs last enabled at (6691): [] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] hardirqs last enabled at (6691): [] _raw_spin_unlock_irqrestore+0x62/0x90 kernel/locking/spinlock.c:184 hardirqs last disabled at (6692): [] interrupt_entry+0xb5/0xf0 arch/x86/entry/entry_64.S:625 softirqs last enabled at (1468): [] __do_softirq+0x5ff/0x7f4 kernel/softirq.c:314 softirqs last disabled at (1455): [] invoke_softirq kernel/softirq.c:368 [inline] softirqs last disabled at (1455): [] irq_exit+0x126/0x130 kernel/softirq.c:408 CPU: 1 PID: 24261 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline] RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] RIP: 0010:_raw_spin_unlock_irqrestore+0x7d/0x90 kernel/locking/spinlock.c:184 Code: 0d c8 ce 16 7b 5b 41 5c 5d c3 e8 3e 0f 52 fc 48 c7 c7 68 b0 71 86 e8 62 4c 8d fc 48 83 3d 42 91 86 01 00 74 0e 48 89 df 57 9d <0f> 1f 44 00 00 eb cd 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 55 48 89 RSP: 0018:8801b8e4f7e8 EFLAGS: 0282 ORIG_RAX: ff13 RAX: RBX: 0282 RCX: 84eb1f1e RDX: dc00 RSI: dc00 RDI: 0282 RBP: 8801b8e4f7f8 R08: ed002573a06a R09: R10: R11: R12: 88012b9d0348 R13: 8801e6417580 R14: 8801b8e4f868 R15: 8801e64175a8 FS: 7f3447a49700() GS:8801f6d0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f3447a48db8 CR3: 0001ee58d000 CR4: 06e0 Call Trace: spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] snd_virmidi_output_trigger+0x37d/0x420 sound/core/seq/seq_virmidi.c:205 snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [inline] snd_rawmidi_kernel_write1+0x383/0x410 sound/core/rawmidi.c:1288 snd_rawmidi_write+0x249/0xa50 sound/core/rawmidi.c:1338 __vfs_write+0xf9/0x5d0 fs/read_write.c:485 vfs_write+0x195/0x380 fs/read_write.c:549 ksys_write+0xdb/0x1d0 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x43/0x50 fs/read_write.c:607 do_syscall_64+0x182/0x540 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x44b939 Code: 8d 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 5b 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f3447a48b48 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0071bfa0 RCX: 0044b939 RDX: 210b RSI: 2100 RDI: 0013 RBP: b7a8 R08: R09: R10: R11: 0246 R12: 7f3447a496d4 R13: R14: 006f7848 R15: = About RaceFuzzer RaceFuzzer is a customized version of Syzkaller, specifically tailored to find race condition bugs in the Linux kernel. While we leverage many different technique, the notable feature of RaceFuzzer is in leveraging a custom hypervisor (QEMU/KVM) to interleave the scheduling. In particular, we modified the hypervisor to intentionally stall a per-core execution, which is similar to supporting per-core breakpoint functionality. This allows RaceFuzzer to force the kernel to deterministically trigger racy condition (which may rarely happen in practice due to randomness in scheduling).
BUG: soft lockup in snd_virmidi_output_trigger
Reporting the crash: BUG: soft lockup in snd_virmidi_output_trigger This crash has been found in v4.18-rc3 using RaceFuzzer (a modified version of Syzkaller), which we describe more at the end of this report. Note that this bug is previously reported by Syzkaller a few month ago. (https://syzkaller.appspot.com/bug?id=642c927972318775e0ea1b4779ccd8624ce9e6f5) Nonetheless, the crash still can be detected. We guess that the crash has not fixed yet. We report the crash log and the attached reproducer with a hope that they are helpful to diagnose the crash and to fix a bug. C Reproducer: https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-dab082.c Kernel config: https://kiwi.cs.purdue.edu/static/race-fuzzer/config-dab082 (Please disregard all code related to "should_hypercall" in the C repro, as this is only for our debugging purposes using our own hypervisor.) Crash log: == watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [syz-executor0:24261] Modules linked in: irq event stamp: 6692 hardirqs last enabled at (6691): [] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] hardirqs last enabled at (6691): [] _raw_spin_unlock_irqrestore+0x62/0x90 kernel/locking/spinlock.c:184 hardirqs last disabled at (6692): [] interrupt_entry+0xb5/0xf0 arch/x86/entry/entry_64.S:625 softirqs last enabled at (1468): [] __do_softirq+0x5ff/0x7f4 kernel/softirq.c:314 softirqs last disabled at (1455): [] invoke_softirq kernel/softirq.c:368 [inline] softirqs last disabled at (1455): [] irq_exit+0x126/0x130 kernel/softirq.c:408 CPU: 1 PID: 24261 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline] RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] RIP: 0010:_raw_spin_unlock_irqrestore+0x7d/0x90 kernel/locking/spinlock.c:184 Code: 0d c8 ce 16 7b 5b 41 5c 5d c3 e8 3e 0f 52 fc 48 c7 c7 68 b0 71 86 e8 62 4c 8d fc 48 83 3d 42 91 86 01 00 74 0e 48 89 df 57 9d <0f> 1f 44 00 00 eb cd 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 55 48 89 RSP: 0018:8801b8e4f7e8 EFLAGS: 0282 ORIG_RAX: ff13 RAX: RBX: 0282 RCX: 84eb1f1e RDX: dc00 RSI: dc00 RDI: 0282 RBP: 8801b8e4f7f8 R08: ed002573a06a R09: R10: R11: R12: 88012b9d0348 R13: 8801e6417580 R14: 8801b8e4f868 R15: 8801e64175a8 FS: 7f3447a49700() GS:8801f6d0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f3447a48db8 CR3: 0001ee58d000 CR4: 06e0 Call Trace: spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] snd_virmidi_output_trigger+0x37d/0x420 sound/core/seq/seq_virmidi.c:205 snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [inline] snd_rawmidi_kernel_write1+0x383/0x410 sound/core/rawmidi.c:1288 snd_rawmidi_write+0x249/0xa50 sound/core/rawmidi.c:1338 __vfs_write+0xf9/0x5d0 fs/read_write.c:485 vfs_write+0x195/0x380 fs/read_write.c:549 ksys_write+0xdb/0x1d0 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x43/0x50 fs/read_write.c:607 do_syscall_64+0x182/0x540 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x44b939 Code: 8d 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 5b 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f3447a48b48 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0071bfa0 RCX: 0044b939 RDX: 210b RSI: 2100 RDI: 0013 RBP: b7a8 R08: R09: R10: R11: 0246 R12: 7f3447a496d4 R13: R14: 006f7848 R15: = About RaceFuzzer RaceFuzzer is a customized version of Syzkaller, specifically tailored to find race condition bugs in the Linux kernel. While we leverage many different technique, the notable feature of RaceFuzzer is in leveraging a custom hypervisor (QEMU/KVM) to interleave the scheduling. In particular, we modified the hypervisor to intentionally stall a per-core execution, which is similar to supporting per-core breakpoint functionality. This allows RaceFuzzer to force the kernel to deterministically trigger racy condition (which may rarely happen in practice due to randomness in scheduling).
[PATCH v2 0/3] tracing: kprobes: Prohibit probing on notrace functions
Hello, this is the 2nd version of the series to prohibit kprobe on notrace functions which Francis sent before. Here is v1 series: https://lkml.org/lkml/2018/7/12/678 In this version, I've add a kconfig option to remove notrace protection just for debugging, and also fix ftracetest testcase to not probe notrace function. Thank you, --- Francis Deslauriers (1): selftest/ftrace: Move kprobe selftest function to separate compile unit Masami Hiramatsu (2): tracing: kprobes: Prohibit probing on notrace function selftests/ftrace: Fix kprobe string testcase to not probe notrace function kernel/trace/Kconfig | 18 ++ kernel/trace/Makefile |5 +++ kernel/trace/trace_kprobe.c| 35 ++-- kernel/trace/trace_kprobe_selftest.c | 10 ++ kernel/trace/trace_kprobe_selftest.h |7 .../ftrace/test.d/kprobe/kprobe_args_string.tc | 30 +++-- 6 files changed, 76 insertions(+), 29 deletions(-) create mode 100644 kernel/trace/trace_kprobe_selftest.c create mode 100644 kernel/trace/trace_kprobe_selftest.h -- Masami Hiramatsu (Linaro)
[PATCH v2 0/3] tracing: kprobes: Prohibit probing on notrace functions
Hello, this is the 2nd version of the series to prohibit kprobe on notrace functions which Francis sent before. Here is v1 series: https://lkml.org/lkml/2018/7/12/678 In this version, I've add a kconfig option to remove notrace protection just for debugging, and also fix ftracetest testcase to not probe notrace function. Thank you, --- Francis Deslauriers (1): selftest/ftrace: Move kprobe selftest function to separate compile unit Masami Hiramatsu (2): tracing: kprobes: Prohibit probing on notrace function selftests/ftrace: Fix kprobe string testcase to not probe notrace function kernel/trace/Kconfig | 18 ++ kernel/trace/Makefile |5 +++ kernel/trace/trace_kprobe.c| 35 ++-- kernel/trace/trace_kprobe_selftest.c | 10 ++ kernel/trace/trace_kprobe_selftest.h |7 .../ftrace/test.d/kprobe/kprobe_args_string.tc | 30 +++-- 6 files changed, 76 insertions(+), 29 deletions(-) create mode 100644 kernel/trace/trace_kprobe_selftest.c create mode 100644 kernel/trace/trace_kprobe_selftest.h -- Masami Hiramatsu (Linaro)
Re: [PATCH 2/2] drivers: clk: st: address sparse warnings
On Wed, Jul 25, 2018 at 01:40:53PM -0700, Stephen Boyd wrote:
> Quoting Nicholas Mc Guire (2018-07-15 03:18:24)
> > Refactoring of code to make it more readable and at the same time make
> > sparse happy again.
> >
> > Signed-off-by: Nicholas Mc Guire
> > ---
> >
> > sparse complained about:
> > drivers/clk/st/clkgen-pll.c:225:12: warning: context imbalance in
> > 'clkgen_pll_enable' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:267:9: warning: context imbalance in
> > 'clkgen_pll_disable' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:413:9: warning: context imbalance in
> > 'set_rate_stm_pll3200c32' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:570:9: warning: context imbalance in
> > 'set_rate_stm_pll4600c28' - different lock contexts for basic block
> >
> > Which are technically false positives as the pll->lock which is being
> > checked is determined at configuration time, thus the locks are balanced.
> > Never the less the refactored code seems more readable and was
> > commented to make it clear.
>
> This stuff above should go into the commit text.
ok - then I´ll resend that with that moved into the commit message.
>
> >
> > Patch was compile tested with: multi_v7_defconfig (implies
> > CONFIG_ARCH_STI=y)
> >
> > Patch is against 4.18-rc4 (localversion-next is next-20180713)
> >
> > drivers/clk/st/clkgen-pll.c | 51
> > +
> > 1 file changed, 28 insertions(+), 23 deletions(-)
>
> I believe this driver is in stasis mode. Not sure anyone is testing this
> right now. Please Cc people who have worked on this driver, like Gabriel
> Fernandez.
>
> >
> > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
> > index 7a7106dc..cbb5184 100644
> > --- a/drivers/clk/st/clkgen-pll.c
> > +++ b/drivers/clk/st/clkgen-pll.c
> > @@ -228,13 +228,13 @@ static int clkgen_pll_enable(struct clk_hw *hw)
> > unsigned long flags = 0;
> > int ret = 0;
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih418 and stih407 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - ret = __clkgen_pll_enable(hw);
> > -
> > - if (pll->lock)
> > + ret = __clkgen_pll_enable(hw);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else
> > + ret = __clkgen_pll_enable(hw);
> >
> > return ret;
> > }
> > @@ -259,13 +259,13 @@ static void clkgen_pll_disable(struct clk_hw *hw)
> > struct clkgen_pll *pll = to_clkgen_pll(hw);
> > unsigned long flags = 0;
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih418 and stih407 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - __clkgen_pll_disable(hw);
> > -
> > - if (pll->lock)
> > + __clkgen_pll_disable(hw);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else
> > + __clkgen_pll_disable(hw);
> > }
> >
> > static int clk_pll3200c32_get_params(unsigned long input, unsigned long
> > output,
> > @@ -400,15 +400,18 @@ static int set_rate_stm_pll3200c32(struct clk_hw *hw,
> > unsigned long rate,
> >
> > __clkgen_pll_disable(hw);
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih407 and stih418 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > - CLKGEN_WRITE(pll, idf, pll->idf);
> > - CLKGEN_WRITE(pll, cp, pll->cp);
> > -
> > - if (pll->lock)
> > + CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > + CLKGEN_WRITE(pll, idf, pll->idf);
> > + CLKGEN_WRITE(pll, cp, pll->cp);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else {
> > + CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > + CLKGEN_WRITE(pll, idf, pll->idf);
> > + CLKGEN_WRITE(pll, cp, pll->cp);
> > + }
>
> Please don't duplicate this code. The sparse warnings can be fixed by
> adding a fake lock acquire to the else of the if condition. We do this
> in drivers/clk/clk.c so you should be able to copy it from there.
the duplication is not a mater of the sparse warning only - the inetnt was
to improve readability - atleast for the one-line cases it seems more
readable to have it this way than to have the two lock checks - notably
as you can then comment what the difference here really is.
I agree that for this case with the three lines in the body it is not
that reasonable - this was simply a matter of consistency as the other lock
checks were moved into a if-else construct for readability and commenting.
thx!
hofrat
Re: [PATCH 2/2] drivers: clk: st: address sparse warnings
On Wed, Jul 25, 2018 at 01:40:53PM -0700, Stephen Boyd wrote:
> Quoting Nicholas Mc Guire (2018-07-15 03:18:24)
> > Refactoring of code to make it more readable and at the same time make
> > sparse happy again.
> >
> > Signed-off-by: Nicholas Mc Guire
> > ---
> >
> > sparse complained about:
> > drivers/clk/st/clkgen-pll.c:225:12: warning: context imbalance in
> > 'clkgen_pll_enable' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:267:9: warning: context imbalance in
> > 'clkgen_pll_disable' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:413:9: warning: context imbalance in
> > 'set_rate_stm_pll3200c32' - different lock contexts for basic block
> > drivers/clk/st/clkgen-pll.c:570:9: warning: context imbalance in
> > 'set_rate_stm_pll4600c28' - different lock contexts for basic block
> >
> > Which are technically false positives as the pll->lock which is being
> > checked is determined at configuration time, thus the locks are balanced.
> > Never the less the refactored code seems more readable and was
> > commented to make it clear.
>
> This stuff above should go into the commit text.
ok - then I´ll resend that with that moved into the commit message.
>
> >
> > Patch was compile tested with: multi_v7_defconfig (implies
> > CONFIG_ARCH_STI=y)
> >
> > Patch is against 4.18-rc4 (localversion-next is next-20180713)
> >
> > drivers/clk/st/clkgen-pll.c | 51
> > +
> > 1 file changed, 28 insertions(+), 23 deletions(-)
>
> I believe this driver is in stasis mode. Not sure anyone is testing this
> right now. Please Cc people who have worked on this driver, like Gabriel
> Fernandez.
>
> >
> > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
> > index 7a7106dc..cbb5184 100644
> > --- a/drivers/clk/st/clkgen-pll.c
> > +++ b/drivers/clk/st/clkgen-pll.c
> > @@ -228,13 +228,13 @@ static int clkgen_pll_enable(struct clk_hw *hw)
> > unsigned long flags = 0;
> > int ret = 0;
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih418 and stih407 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - ret = __clkgen_pll_enable(hw);
> > -
> > - if (pll->lock)
> > + ret = __clkgen_pll_enable(hw);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else
> > + ret = __clkgen_pll_enable(hw);
> >
> > return ret;
> > }
> > @@ -259,13 +259,13 @@ static void clkgen_pll_disable(struct clk_hw *hw)
> > struct clkgen_pll *pll = to_clkgen_pll(hw);
> > unsigned long flags = 0;
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih418 and stih407 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - __clkgen_pll_disable(hw);
> > -
> > - if (pll->lock)
> > + __clkgen_pll_disable(hw);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else
> > + __clkgen_pll_disable(hw);
> > }
> >
> > static int clk_pll3200c32_get_params(unsigned long input, unsigned long
> > output,
> > @@ -400,15 +400,18 @@ static int set_rate_stm_pll3200c32(struct clk_hw *hw,
> > unsigned long rate,
> >
> > __clkgen_pll_disable(hw);
> >
> > - if (pll->lock)
> > + if (pll->lock) {
> > + /* stih407 and stih418 */
> > spin_lock_irqsave(pll->lock, flags);
> > -
> > - CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > - CLKGEN_WRITE(pll, idf, pll->idf);
> > - CLKGEN_WRITE(pll, cp, pll->cp);
> > -
> > - if (pll->lock)
> > + CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > + CLKGEN_WRITE(pll, idf, pll->idf);
> > + CLKGEN_WRITE(pll, cp, pll->cp);
> > spin_unlock_irqrestore(pll->lock, flags);
> > + } else {
> > + CLKGEN_WRITE(pll, ndiv, pll->ndiv);
> > + CLKGEN_WRITE(pll, idf, pll->idf);
> > + CLKGEN_WRITE(pll, cp, pll->cp);
> > + }
>
> Please don't duplicate this code. The sparse warnings can be fixed by
> adding a fake lock acquire to the else of the if condition. We do this
> in drivers/clk/clk.c so you should be able to copy it from there.
the duplication is not a mater of the sparse warning only - the inetnt was
to improve readability - atleast for the one-line cases it seems more
readable to have it this way than to have the two lock checks - notably
as you can then comment what the difference here really is.
I agree that for this case with the three lines in the body it is not
that reasonable - this was simply a matter of consistency as the other lock
checks were moved into a if-else construct for readability and commenting.
thx!
hofrat
Re: [PATCH 1/2] drivers: clk: st: warn on iomap failure
On Wed, Jul 25, 2018 at 01:41:38PM -0700, Stephen Boyd wrote: > Quoting Nicholas Mc Guire (2018-07-15 03:18:23) > > While the return value of clkgen_get_register_base() is being checked > > at the call site, there is no indication of failure cause thus making > > diagnosis of the issues hard. The WARN_ON() allows to determine the > > cause of failure. > > > > Signed-off-by: Nicholas Mc Guire > > --- > > > > Problem found by an experimental coccinelle script > > > > Patch was compile tested with: multi_v7_defconfig (implies > > CONFIG_ARCH_STI=y) > > > > Patch is against 4.18-rc4 (localversion-next is next-20180713) > > > > drivers/clk/st/clkgen-pll.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c > > index cbb5184..aeb30ab 100644 > > --- a/drivers/clk/st/clkgen-pll.c > > +++ b/drivers/clk/st/clkgen-pll.c > > @@ -647,6 +647,7 @@ static void __iomem * __init clkgen_get_register_base( > > return NULL; > > > > reg = of_iomap(pnode, 0); > > + WARN_ON(!reg); > > > > of_node_put(pnode); > > return reg; > > Shouldn't the caller blow up on NULL pointer access? This patch doesn't > seem useful, sorry. > if you look at the call chain then there is a check for !NULL along the way - but never any information - no pr_*/printk or the like so ultimately you would get a failure but not know where that failure came from - the intent of the WARN_ON() is to allow you to locate the trigger event. Blowing up with a BUG_ON() is not necessary as the call chain does check for !NULL along the way. thx! hofrat
Re: [PATCH 1/2] drivers: clk: st: warn on iomap failure
On Wed, Jul 25, 2018 at 01:41:38PM -0700, Stephen Boyd wrote: > Quoting Nicholas Mc Guire (2018-07-15 03:18:23) > > While the return value of clkgen_get_register_base() is being checked > > at the call site, there is no indication of failure cause thus making > > diagnosis of the issues hard. The WARN_ON() allows to determine the > > cause of failure. > > > > Signed-off-by: Nicholas Mc Guire > > --- > > > > Problem found by an experimental coccinelle script > > > > Patch was compile tested with: multi_v7_defconfig (implies > > CONFIG_ARCH_STI=y) > > > > Patch is against 4.18-rc4 (localversion-next is next-20180713) > > > > drivers/clk/st/clkgen-pll.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c > > index cbb5184..aeb30ab 100644 > > --- a/drivers/clk/st/clkgen-pll.c > > +++ b/drivers/clk/st/clkgen-pll.c > > @@ -647,6 +647,7 @@ static void __iomem * __init clkgen_get_register_base( > > return NULL; > > > > reg = of_iomap(pnode, 0); > > + WARN_ON(!reg); > > > > of_node_put(pnode); > > return reg; > > Shouldn't the caller blow up on NULL pointer access? This patch doesn't > seem useful, sorry. > if you look at the call chain then there is a check for !NULL along the way - but never any information - no pr_*/printk or the like so ultimately you would get a failure but not know where that failure came from - the intent of the WARN_ON() is to allow you to locate the trigger event. Blowing up with a BUG_ON() is not necessary as the call chain does check for !NULL along the way. thx! hofrat
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter,
Thank you.. I sent next version patch with modification.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:39 AM, Guenter Roeck wrote:
> On 07/25/2018 09:47 PM, Srinath Mannam wrote:
>>
>> Hi Guenter,
>>
>> It was my mistake sorry for that. I thought to add as Rivewed-by your
>> name..
>> Many changes are made based on your review comments and suggestions.
>> So, Can I add your name in Reviewed list?
>
>
> You did that already below, and I am ok with that.
>
> Guenter
>
>> I will modify and send next patchset.
>>
>> thank you.
>>
>> Regards,
>> Srinath.
>>
>> On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck
>> wrote:
>>>
>>> On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
>>>
>>>
>>>
>>> I just noticed this. This is completely inappropriate. It is
>>> the equivalent of forging a signature on a check.
>>>
>>> _Never_ add a Signed-off-by: statement for anyone but yourself.
>>>
>>> Guenter
>>>
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of
first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
watchdog_device *wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
struct amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency
property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
>>>
>>
>
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter,
Thank you.. I sent next version patch with modification.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:39 AM, Guenter Roeck wrote:
> On 07/25/2018 09:47 PM, Srinath Mannam wrote:
>>
>> Hi Guenter,
>>
>> It was my mistake sorry for that. I thought to add as Rivewed-by your
>> name..
>> Many changes are made based on your review comments and suggestions.
>> So, Can I add your name in Reviewed list?
>
>
> You did that already below, and I am ok with that.
>
> Guenter
>
>> I will modify and send next patchset.
>>
>> thank you.
>>
>> Regards,
>> Srinath.
>>
>> On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck
>> wrote:
>>>
>>> On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
>>>
>>>
>>>
>>> I just noticed this. This is completely inappropriate. It is
>>> the equivalent of forging a signature on a check.
>>>
>>> _Never_ add a Signed-off-by: statement for anyone but yourself.
>>>
>>> Guenter
>>>
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of
first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
watchdog_device *wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
struct amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency
property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
>>>
>>
>
RE: [PATCH v3 2/2] fpga: zynq-fpga: Add support for readback
Hi Alan, Thanks for the review... > > In Zynq Case it supports two types of the readback (Configuration registers, > Configuration data(fpga image)) which may not be the same case for other > vendors. > > Since I need to support both the use cases I have differentiated them using > module param in the zynq-fpga driver. > > > > As you said we shouldn't change the attribute's function, So in the > > zynq-fpga driver, I am planning to add another debugfs attribute for > reading back of configuration registers as it is vendor specific readback > type. > > (Configuration registers Usage: > /sys/kernel/debug/fpga/zynq-fpga/cfgreg_summary) > > Is it called '...summary' because it is not all the registers? Could just be > "cfg_regs"? Sure will make it cfg_regs... Are you ok with the above proposal?? if you are ok will make changes accordingly and will post v4... > > > (Configuration data(fpga image) Usage: > /sys/kernel/debug/fpga/fpga0/image) > > Please let me know your thoughts for the same... > > > >> > >> > One other option is sysfs. Do you want me to implement sysfs path??? > >> > Any other suggestions??? > >> > >> You could implement another sysfs attribute for reading FPGA registers > >> with a separate ops callback. Please clarify what it is doing (not > >> all FPGAs have this function) and what that will look like when the > >> user reads the from the sysfs > > > > AFAIK we shouldn't add another sysfs/debugfs attribute for vendor-specific > readback type in the FPGA manager ops. > > Please correct me if my understanding is wrong. > > You are not wrong :) Thanks Regards, Kedar. > > > > > Regards, > > Kedar. > > > >> > >> Alan
RE: [PATCH v3 2/2] fpga: zynq-fpga: Add support for readback
Hi Alan, Thanks for the review... > > In Zynq Case it supports two types of the readback (Configuration registers, > Configuration data(fpga image)) which may not be the same case for other > vendors. > > Since I need to support both the use cases I have differentiated them using > module param in the zynq-fpga driver. > > > > As you said we shouldn't change the attribute's function, So in the > > zynq-fpga driver, I am planning to add another debugfs attribute for > reading back of configuration registers as it is vendor specific readback > type. > > (Configuration registers Usage: > /sys/kernel/debug/fpga/zynq-fpga/cfgreg_summary) > > Is it called '...summary' because it is not all the registers? Could just be > "cfg_regs"? Sure will make it cfg_regs... Are you ok with the above proposal?? if you are ok will make changes accordingly and will post v4... > > > (Configuration data(fpga image) Usage: > /sys/kernel/debug/fpga/fpga0/image) > > Please let me know your thoughts for the same... > > > >> > >> > One other option is sysfs. Do you want me to implement sysfs path??? > >> > Any other suggestions??? > >> > >> You could implement another sysfs attribute for reading FPGA registers > >> with a separate ops callback. Please clarify what it is doing (not > >> all FPGAs have this function) and what that will look like when the > >> user reads the from the sysfs > > > > AFAIK we shouldn't add another sysfs/debugfs attribute for vendor-specific > readback type in the FPGA manager ops. > > Please correct me if my understanding is wrong. > > You are not wrong :) Thanks Regards, Kedar. > > > > > Regards, > > Kedar. > > > >> > >> Alan
Re: linux-next: build warning after merge of the rdma tree
On Wed, 2018-07-25 at 21:05 -0600, Jason Gunthorpe wrote:
> On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the rdma tree, today's linux-next build (powerpc
> > ppc64_defconfig) produced this warning:
> >
> > net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt':
> > net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
> > if (bad_wr != first_wr)
> > ^
>
> Huh. I'm quite surprised 0-day build service didn't warn on this.
>
> > Introduced by commit
> >
> > ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)()
> > calls")
> >
> > This is an actual problem.
>
> Yes, for sure. Bart?
Thanks Stephen for having reported this. I propose to revert the changes in
net/sunrpc/xprtrdma/svc_rdma_rw.c. Jason, do you want me to submit the below
as a formal patch?
Thanks,
Bart.
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c
b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 80975427f523..ce3ea8419704 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -329,7 +329,7 @@ static int svc_rdma_post_chunk_ctxt(struct
svc_rdma_chunk_ctxt *cc)
do {
if (atomic_sub_return(cc->cc_sqecount,
>sc_sq_avail) > 0) {
- ret = ib_post_send(rdma->sc_qp, first_wr, NULL);
+ ret = ib_post_send(rdma->sc_qp, first_wr, _wr);
trace_svcrdma_post_rw(>cc_cqe,
cc->cc_sqecount, ret);
if (ret)
Re: linux-next: build warning after merge of the rdma tree
On Wed, 2018-07-25 at 21:05 -0600, Jason Gunthorpe wrote:
> On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the rdma tree, today's linux-next build (powerpc
> > ppc64_defconfig) produced this warning:
> >
> > net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt':
> > net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
> > if (bad_wr != first_wr)
> > ^
>
> Huh. I'm quite surprised 0-day build service didn't warn on this.
>
> > Introduced by commit
> >
> > ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)()
> > calls")
> >
> > This is an actual problem.
>
> Yes, for sure. Bart?
Thanks Stephen for having reported this. I propose to revert the changes in
net/sunrpc/xprtrdma/svc_rdma_rw.c. Jason, do you want me to submit the below
as a formal patch?
Thanks,
Bart.
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c
b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 80975427f523..ce3ea8419704 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -329,7 +329,7 @@ static int svc_rdma_post_chunk_ctxt(struct
svc_rdma_chunk_ctxt *cc)
do {
if (atomic_sub_return(cc->cc_sqecount,
>sc_sq_avail) > 0) {
- ret = ib_post_send(rdma->sc_qp, first_wr, NULL);
+ ret = ib_post_send(rdma->sc_qp, first_wr, _wr);
trace_svcrdma_post_rw(>cc_cqe,
cc->cc_sqecount, ret);
if (ret)
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/25/2018 09:47 PM, Srinath Mannam wrote:
Hi Guenter,
It was my mistake sorry for that. I thought to add as Rivewed-by your name..
Many changes are made based on your review comments and suggestions.
So, Can I add your name in Reviewed list?
You did that already below, and I am ok with that.
Guenter
I will modify and send next patchset.
thank you.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote:
On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
I just noticed this. This is completely inappropriate. It is
the equivalent of forging a signature on a check.
_Never_ add a Signed-off-by: statement for anyone but yourself.
Guenter
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of
first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
watchdog_device *wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
struct amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency
property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/25/2018 09:47 PM, Srinath Mannam wrote:
Hi Guenter,
It was my mistake sorry for that. I thought to add as Rivewed-by your name..
Many changes are made based on your review comments and suggestions.
So, Can I add your name in Reviewed list?
You did that already below, and I am ok with that.
Guenter
I will modify and send next patchset.
thank you.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote:
On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
I just noticed this. This is completely inappropriate. It is
the equivalent of forging a signature on a check.
_Never_ add a Signed-off-by: statement for anyone but yourself.
Guenter
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of
first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
watchdog_device *wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
struct amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency
property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
[PATCH v3 4/4] MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver
Add entry for Actions Semi Owl SoCs DMA driver under ARM/ACTIONS. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 09b54e9ebc6f..56d9c7715c2a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1145,12 +1145,14 @@ F: arch/arm/boot/dts/owl-* F: arch/arm64/boot/dts/actions/ F: drivers/clk/actions/ F: drivers/clocksource/owl-* +F: drivers/dma/owl-dma.c F: drivers/pinctrl/actions/* F: drivers/soc/actions/ F: include/dt-bindings/power/owl-* F: include/linux/soc/actions/ F: Documentation/devicetree/bindings/arm/actions.txt F: Documentation/devicetree/bindings/clock/actions,s900-cmu.txt +F: Documentation/devicetree/bindings/dma/owl-dma.txt F: Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt F: Documentation/devicetree/bindings/power/actions,owl-sps.txt F: Documentation/devicetree/bindings/timer/actions,owl-timer.txt -- 2.17.1
[PATCH v3 4/4] MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver
Add entry for Actions Semi Owl SoCs DMA driver under ARM/ACTIONS. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 09b54e9ebc6f..56d9c7715c2a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1145,12 +1145,14 @@ F: arch/arm/boot/dts/owl-* F: arch/arm64/boot/dts/actions/ F: drivers/clk/actions/ F: drivers/clocksource/owl-* +F: drivers/dma/owl-dma.c F: drivers/pinctrl/actions/* F: drivers/soc/actions/ F: include/dt-bindings/power/owl-* F: include/linux/soc/actions/ F: Documentation/devicetree/bindings/arm/actions.txt F: Documentation/devicetree/bindings/clock/actions,s900-cmu.txt +F: Documentation/devicetree/bindings/dma/owl-dma.txt F: Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt F: Documentation/devicetree/bindings/power/actions,owl-sps.txt F: Documentation/devicetree/bindings/timer/actions,owl-timer.txt -- 2.17.1
[PATCH v3 0/4] Add Actions Semi Owl family S900 DMA Controller support
This patchset adds DMA controller support for Actions Semi Owl family S900 SoC. This driver has been structured in a way such that there will be only one controller driver for the whole Owl family series (S500, S700 and S900 SoCs). There are 12 physical channels and 46 logical channels supported by the DMA controller. The DMA controller also supports 4 software configurable interrupt lines for the priority based DMA usecase. But the DMA driver supports only 1 interrupt for simplification. By default, the driver uses Linked list mode for all transfers. Right now only MEMCPY support has been added and the rest (SLAVE, CYCLIC) will be added in upcoming patches. The driver has been tested using dmatest utility on the Bubblegum-96 board. The DTS patches in this series depends on the pinctrl DTS patches submitted [1], which is yet to be merged by the platform maintainer Andreas. Since the DMA driver goes through DMA tree and the relevant DTS patches goes through ARM-SoC tree, Andreas will pick it up once it has been reviewed. For the reference, I have queued up all reviewed dts patches so far in my repo [2] from which Andreas is picking them for 4.19. Thanks, Mani [1] https://patchwork.kernel.org/patch/10322937/ [2] https://git.linaro.org/people/manivannan.sadhasivam/linux.git/log/?h=s900-for-next Changes in v3: As per Vinod's review: * Removed unused header and API's * Used GENMASK for defines * Removed per member comment for structs * Modified pchan* and dma* API's to use corresponding containers as arguments * Removed error messages regarding the no free channels * Added devm_free_irq in dma_remove * Used dmaengine instead of dma for commit titles Changes in v2: * Fixed up the multi-line alignments according to `checkpatch --strict` Manivannan Sadhasivam (4): dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs arm64: dts: actions: Add Actions Semi S900 DMA Controller dmaengine: Add Actions Semi Owl family S900 DMA driver MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver .../devicetree/bindings/dma/owl-dma.txt | 47 + MAINTAINERS | 2 + arch/arm64/boot/dts/actions/s900.dtsi | 13 + drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 6 files changed, 1042 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt create mode 100644 drivers/dma/owl-dma.c -- 2.17.1
[PATCH v3 3/4] dmaengine: Add Actions Semi Owl family S900 DMA driver
Add Actions Semi Owl family S900 DMA driver. Signed-off-by: Manivannan Sadhasivam --- drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 3 files changed, 980 insertions(+) create mode 100644 drivers/dma/owl-dma.c diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index ca1680afa20a..92a278e6618c 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -413,6 +413,14 @@ config NBPFAXI_DMA help Support for "Type-AXI" NBPF DMA IPs from Renesas +config OWL_DMA + tristate "Actions Semi Owl SoCs DMA support" + depends on ARCH_ACTIONS + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Enable support for the Actions Semi Owl SoCs DMA controller. + config PCH_DMA tristate "Intel EG20T PCH / LAPIS Semicon IOH(ML7213/ML7223/ML7831) DMA" depends on PCI && (X86_32 || COMPILE_TEST) diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 203a99d68315..c91702d88b95 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_MV_XOR_V2) += mv_xor_v2.o obj-$(CONFIG_MXS_DMA) += mxs-dma.o obj-$(CONFIG_MX3_IPU) += ipu/ obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o +obj-$(CONFIG_OWL_DMA) += owl-dma.o obj-$(CONFIG_PCH_DMA) += pch_dma.o obj-$(CONFIG_PL330_DMA) += pl330.o obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/ diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c new file mode 100644 index ..7812a6338acd --- /dev/null +++ b/drivers/dma/owl-dma.c @@ -0,0 +1,971 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Actions Semi Owl SoCs DMA driver +// +// Copyright (c) 2014 Actions Semi Inc. +// Author: David Liu +// +// Copyright (c) 2018 Linaro Ltd. +// Author: Manivannan Sadhasivam + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "virt-dma.h" + +#define OWL_DMA_FRAME_MAX_LENGTH 0xf + +/* Global DMA Controller Registers */ +#define OWL_DMA_IRQ_PD00x00 +#define OWL_DMA_IRQ_PD10x04 +#define OWL_DMA_IRQ_PD20x08 +#define OWL_DMA_IRQ_PD30x0C +#define OWL_DMA_IRQ_EN00x10 +#define OWL_DMA_IRQ_EN10x14 +#define OWL_DMA_IRQ_EN20x18 +#define OWL_DMA_IRQ_EN30x1C +#define OWL_DMA_SECURE_ACCESS_CTL 0x20 +#define OWL_DMA_NIC_QOS0x24 +#define OWL_DMA_DBGSEL 0x28 +#define OWL_DMA_IDLE_STAT 0x2C + +/* Channel Registers */ +#define OWL_DMA_CHAN_BASE(i) (0x100 + (i) * 0x100) +#define OWL_DMAX_MODE 0x00 +#define OWL_DMAX_SOURCE0x04 +#define OWL_DMAX_DESTINATION 0x08 +#define OWL_DMAX_FRAME_LEN 0x0C +#define OWL_DMAX_FRAME_CNT 0x10 +#define OWL_DMAX_REMAIN_FRAME_CNT 0x14 +#define OWL_DMAX_REMAIN_CNT0x18 +#define OWL_DMAX_SOURCE_STRIDE 0x1C +#define OWL_DMAX_DESTINATION_STRIDE0x20 +#define OWL_DMAX_START 0x24 +#define OWL_DMAX_PAUSE 0x28 +#define OWL_DMAX_CHAINED_CTL 0x2C +#define OWL_DMAX_CONSTANT 0x30 +#define OWL_DMAX_LINKLIST_CTL 0x34 +#define OWL_DMAX_NEXT_DESCRIPTOR 0x38 +#define OWL_DMAX_CURRENT_DESCRIPTOR_NUM0x3C +#define OWL_DMAX_INT_CTL 0x40 +#define OWL_DMAX_INT_STATUS0x44 +#define OWL_DMAX_CURRENT_SOURCE_POINTER0x48 +#define OWL_DMAX_CURRENT_DESTINATION_POINTER 0x4C + +/* OWL_DMAX_MODE Bits */ +#define OWL_DMA_MODE_TS(x) (((x) & GENMASK(5, 0)) << 0) +#define OWL_DMA_MODE_ST(x) (((x) & GENMASK(1, 0)) << 8) +#defineOWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0) +#defineOWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2) +#defineOWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3) +#define OWL_DMA_MODE_DT(x) (((x) & GENMASK(1, 0)) << 10) +#defineOWL_DMA_MODE_DT_DEV OWL_DMA_MODE_DT(0) +#defineOWL_DMA_MODE_DT_DCU OWL_DMA_MODE_DT(2) +#defineOWL_DMA_MODE_DT_SRAMOWL_DMA_MODE_DT(3) +#define OWL_DMA_MODE_SAM(x)(((x) & GENMASK(1, 0)) << 16) +#defineOWL_DMA_MODE_SAM_CONST OWL_DMA_MODE_SAM(0) +#defineOWL_DMA_MODE_SAM_INCOWL_DMA_MODE_SAM(1) +#defineOWL_DMA_MODE_SAM_STRIDE
[PATCH v3 0/4] Add Actions Semi Owl family S900 DMA Controller support
This patchset adds DMA controller support for Actions Semi Owl family S900 SoC. This driver has been structured in a way such that there will be only one controller driver for the whole Owl family series (S500, S700 and S900 SoCs). There are 12 physical channels and 46 logical channels supported by the DMA controller. The DMA controller also supports 4 software configurable interrupt lines for the priority based DMA usecase. But the DMA driver supports only 1 interrupt for simplification. By default, the driver uses Linked list mode for all transfers. Right now only MEMCPY support has been added and the rest (SLAVE, CYCLIC) will be added in upcoming patches. The driver has been tested using dmatest utility on the Bubblegum-96 board. The DTS patches in this series depends on the pinctrl DTS patches submitted [1], which is yet to be merged by the platform maintainer Andreas. Since the DMA driver goes through DMA tree and the relevant DTS patches goes through ARM-SoC tree, Andreas will pick it up once it has been reviewed. For the reference, I have queued up all reviewed dts patches so far in my repo [2] from which Andreas is picking them for 4.19. Thanks, Mani [1] https://patchwork.kernel.org/patch/10322937/ [2] https://git.linaro.org/people/manivannan.sadhasivam/linux.git/log/?h=s900-for-next Changes in v3: As per Vinod's review: * Removed unused header and API's * Used GENMASK for defines * Removed per member comment for structs * Modified pchan* and dma* API's to use corresponding containers as arguments * Removed error messages regarding the no free channels * Added devm_free_irq in dma_remove * Used dmaengine instead of dma for commit titles Changes in v2: * Fixed up the multi-line alignments according to `checkpatch --strict` Manivannan Sadhasivam (4): dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs arm64: dts: actions: Add Actions Semi S900 DMA Controller dmaengine: Add Actions Semi Owl family S900 DMA driver MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver .../devicetree/bindings/dma/owl-dma.txt | 47 + MAINTAINERS | 2 + arch/arm64/boot/dts/actions/s900.dtsi | 13 + drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 6 files changed, 1042 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt create mode 100644 drivers/dma/owl-dma.c -- 2.17.1
[PATCH v3 3/4] dmaengine: Add Actions Semi Owl family S900 DMA driver
Add Actions Semi Owl family S900 DMA driver. Signed-off-by: Manivannan Sadhasivam --- drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 3 files changed, 980 insertions(+) create mode 100644 drivers/dma/owl-dma.c diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index ca1680afa20a..92a278e6618c 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -413,6 +413,14 @@ config NBPFAXI_DMA help Support for "Type-AXI" NBPF DMA IPs from Renesas +config OWL_DMA + tristate "Actions Semi Owl SoCs DMA support" + depends on ARCH_ACTIONS + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Enable support for the Actions Semi Owl SoCs DMA controller. + config PCH_DMA tristate "Intel EG20T PCH / LAPIS Semicon IOH(ML7213/ML7223/ML7831) DMA" depends on PCI && (X86_32 || COMPILE_TEST) diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 203a99d68315..c91702d88b95 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_MV_XOR_V2) += mv_xor_v2.o obj-$(CONFIG_MXS_DMA) += mxs-dma.o obj-$(CONFIG_MX3_IPU) += ipu/ obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o +obj-$(CONFIG_OWL_DMA) += owl-dma.o obj-$(CONFIG_PCH_DMA) += pch_dma.o obj-$(CONFIG_PL330_DMA) += pl330.o obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/ diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c new file mode 100644 index ..7812a6338acd --- /dev/null +++ b/drivers/dma/owl-dma.c @@ -0,0 +1,971 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Actions Semi Owl SoCs DMA driver +// +// Copyright (c) 2014 Actions Semi Inc. +// Author: David Liu +// +// Copyright (c) 2018 Linaro Ltd. +// Author: Manivannan Sadhasivam + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "virt-dma.h" + +#define OWL_DMA_FRAME_MAX_LENGTH 0xf + +/* Global DMA Controller Registers */ +#define OWL_DMA_IRQ_PD00x00 +#define OWL_DMA_IRQ_PD10x04 +#define OWL_DMA_IRQ_PD20x08 +#define OWL_DMA_IRQ_PD30x0C +#define OWL_DMA_IRQ_EN00x10 +#define OWL_DMA_IRQ_EN10x14 +#define OWL_DMA_IRQ_EN20x18 +#define OWL_DMA_IRQ_EN30x1C +#define OWL_DMA_SECURE_ACCESS_CTL 0x20 +#define OWL_DMA_NIC_QOS0x24 +#define OWL_DMA_DBGSEL 0x28 +#define OWL_DMA_IDLE_STAT 0x2C + +/* Channel Registers */ +#define OWL_DMA_CHAN_BASE(i) (0x100 + (i) * 0x100) +#define OWL_DMAX_MODE 0x00 +#define OWL_DMAX_SOURCE0x04 +#define OWL_DMAX_DESTINATION 0x08 +#define OWL_DMAX_FRAME_LEN 0x0C +#define OWL_DMAX_FRAME_CNT 0x10 +#define OWL_DMAX_REMAIN_FRAME_CNT 0x14 +#define OWL_DMAX_REMAIN_CNT0x18 +#define OWL_DMAX_SOURCE_STRIDE 0x1C +#define OWL_DMAX_DESTINATION_STRIDE0x20 +#define OWL_DMAX_START 0x24 +#define OWL_DMAX_PAUSE 0x28 +#define OWL_DMAX_CHAINED_CTL 0x2C +#define OWL_DMAX_CONSTANT 0x30 +#define OWL_DMAX_LINKLIST_CTL 0x34 +#define OWL_DMAX_NEXT_DESCRIPTOR 0x38 +#define OWL_DMAX_CURRENT_DESCRIPTOR_NUM0x3C +#define OWL_DMAX_INT_CTL 0x40 +#define OWL_DMAX_INT_STATUS0x44 +#define OWL_DMAX_CURRENT_SOURCE_POINTER0x48 +#define OWL_DMAX_CURRENT_DESTINATION_POINTER 0x4C + +/* OWL_DMAX_MODE Bits */ +#define OWL_DMA_MODE_TS(x) (((x) & GENMASK(5, 0)) << 0) +#define OWL_DMA_MODE_ST(x) (((x) & GENMASK(1, 0)) << 8) +#defineOWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0) +#defineOWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2) +#defineOWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3) +#define OWL_DMA_MODE_DT(x) (((x) & GENMASK(1, 0)) << 10) +#defineOWL_DMA_MODE_DT_DEV OWL_DMA_MODE_DT(0) +#defineOWL_DMA_MODE_DT_DCU OWL_DMA_MODE_DT(2) +#defineOWL_DMA_MODE_DT_SRAMOWL_DMA_MODE_DT(3) +#define OWL_DMA_MODE_SAM(x)(((x) & GENMASK(1, 0)) << 16) +#defineOWL_DMA_MODE_SAM_CONST OWL_DMA_MODE_SAM(0) +#defineOWL_DMA_MODE_SAM_INCOWL_DMA_MODE_SAM(1) +#defineOWL_DMA_MODE_SAM_STRIDE
[PATCH v3 1/4] dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs
Add devicetree binding for Actions Semi Owl SoCs DMA controller.
Signed-off-by: Manivannan Sadhasivam
---
.../devicetree/bindings/dma/owl-dma.txt | 47 +++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/owl-dma.txt
b/Documentation/devicetree/bindings/dma/owl-dma.txt
new file mode 100644
index ..03e9bb12b75f
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/owl-dma.txt
@@ -0,0 +1,47 @@
+* Actions Semi Owl SoCs DMA controller
+
+This binding follows the generic DMA bindings defined in dma.txt.
+
+Required properties:
+- compatible: Should be "actions,s900-dma".
+- reg: Should contain DMA registers location and length.
+- interrupts: Should contain 4 interrupts shared by all channel.
+- #dma-cells: Must be <1>. Used to represent the number of integer
+ cells in the dmas property of client device.
+- dma-channels: Physical channels supported.
+- dma-requests: Number of DMA request signals supported by the controller.
+Refer to Documentation/devicetree/bindings/dma/dma.txt
+- clocks: Phandle and Specifier of the clock feeding the DMA controller.
+
+Example:
+
+Controller:
+dma: dma-controller@e026 {
+compatible = "actions,s900-dma";
+reg = <0x0 0xe026 0x0 0x1000>;
+interrupts = ,
+ ,
+ ,
+ ;
+#dma-cells = <1>;
+dma-channels = <12>;
+dma-requests = <46>;
+clocks = < CLK_DMAC>;
+};
+
+Client:
+
+DMA clients connected to the Actions Semi Owl SoCs DMA controller must
+use the format described in the dma.txt file, using a two-cell specifier
+for each channel.
+
+The two cells in order are:
+1. A phandle pointing to the DMA controller.
+2. The channel id.
+
+uart5: serial@e012a000 {
+...
+dma-names = "tx", "rx";
+dmas = < 26>, < 27>;
+...
+};
--
2.17.1
[PATCH v3 2/4] arm64: dts: actions: Add Actions Semi S900 DMA Controller
Add DMA controller node for Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s900.dtsi | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index 7ae8b931f000..2e8178e50832 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -191,6 +191,19 @@
;
};
+ dma: dma-controller@e026 {
+ compatible = "actions,s900-dma";
+ reg = <0x0 0xe026 0x0 0x1000>;
+ interrupts = ,
+,
+,
+;
+ #dma-cells = <1>;
+ dma-channels = <12>;
+ dma-requests = <46>;
+ clocks = < CLK_DMAC>;
+ };
+
timer: timer@e0228000 {
compatible = "actions,s900-timer";
reg = <0x0 0xe0228000 0x0 0x8000>;
--
2.17.1
[PATCH v3 1/4] dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs
Add devicetree binding for Actions Semi Owl SoCs DMA controller.
Signed-off-by: Manivannan Sadhasivam
---
.../devicetree/bindings/dma/owl-dma.txt | 47 +++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/owl-dma.txt
b/Documentation/devicetree/bindings/dma/owl-dma.txt
new file mode 100644
index ..03e9bb12b75f
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/owl-dma.txt
@@ -0,0 +1,47 @@
+* Actions Semi Owl SoCs DMA controller
+
+This binding follows the generic DMA bindings defined in dma.txt.
+
+Required properties:
+- compatible: Should be "actions,s900-dma".
+- reg: Should contain DMA registers location and length.
+- interrupts: Should contain 4 interrupts shared by all channel.
+- #dma-cells: Must be <1>. Used to represent the number of integer
+ cells in the dmas property of client device.
+- dma-channels: Physical channels supported.
+- dma-requests: Number of DMA request signals supported by the controller.
+Refer to Documentation/devicetree/bindings/dma/dma.txt
+- clocks: Phandle and Specifier of the clock feeding the DMA controller.
+
+Example:
+
+Controller:
+dma: dma-controller@e026 {
+compatible = "actions,s900-dma";
+reg = <0x0 0xe026 0x0 0x1000>;
+interrupts = ,
+ ,
+ ,
+ ;
+#dma-cells = <1>;
+dma-channels = <12>;
+dma-requests = <46>;
+clocks = < CLK_DMAC>;
+};
+
+Client:
+
+DMA clients connected to the Actions Semi Owl SoCs DMA controller must
+use the format described in the dma.txt file, using a two-cell specifier
+for each channel.
+
+The two cells in order are:
+1. A phandle pointing to the DMA controller.
+2. The channel id.
+
+uart5: serial@e012a000 {
+...
+dma-names = "tx", "rx";
+dmas = < 26>, < 27>;
+...
+};
--
2.17.1
[PATCH v3 2/4] arm64: dts: actions: Add Actions Semi S900 DMA Controller
Add DMA controller node for Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s900.dtsi | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index 7ae8b931f000..2e8178e50832 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -191,6 +191,19 @@
;
};
+ dma: dma-controller@e026 {
+ compatible = "actions,s900-dma";
+ reg = <0x0 0xe026 0x0 0x1000>;
+ interrupts = ,
+,
+,
+;
+ #dma-cells = <1>;
+ dma-channels = <12>;
+ dma-requests = <46>;
+ clocks = < CLK_DMAC>;
+ };
+
timer: timer@e0228000 {
compatible = "actions,s900-timer";
reg = <0x0 0xe0228000 0x0 0x8000>;
--
2.17.1
Re: [PATCH] mmc: host: iproc: Add ACPI support to IPROC SDHCI
Hi Ulf Hansson,
Kindly review this patch.
Thank you,
Regards,
Srinath.
On Tue, Jul 17, 2018 at 10:15 PM, Srinath Mannam
wrote:
> Add ACPI support to all IPROC SDHCI varients
>
> Signed-off-by: Srinath Mannam
> Reviewed-by: Ray Jui
> Reviewed-by: Scott Branden
> Reviewed-by: Vladimir Olovyannikov
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/sdhci-iproc.c | 187
> -
> 2 files changed, 128 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 0581c19..bc6702e 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -334,6 +334,7 @@ config MMC_SDHCI_IPROC
> tristate "SDHCI support for the BCM2835 & iProc SD/MMC Controller"
> depends on ARCH_BCM2835 || ARCH_BCM_IPROC || COMPILE_TEST
> depends on MMC_SDHCI_PLTFM
> + depends on OF || ACPI
> default ARCH_BCM_IPROC
> select MMC_SDHCI_IO_ACCESSORS
> help
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index d0e83db..7c5c923 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -15,6 +15,7 @@
> * iProc SDHCI platform driver
> */
>
> +#include
> #include
> #include
> #include
> @@ -162,9 +163,19 @@ static void sdhci_iproc_writeb(struct sdhci_host *host,
> u8 val, int reg)
> sdhci_iproc_writel(host, newval, reg & ~3);
> }
>
> +static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> + if (pltfm_host->clk)
> + return sdhci_pltfm_clk_get_max_clock(host);
> + else
> + return pltfm_host->clock;
> +}
> +
> static const struct sdhci_ops sdhci_iproc_ops = {
> .set_clock = sdhci_set_clock,
> - .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .get_max_clock = sdhci_iproc_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> @@ -178,34 +189,24 @@ static const struct sdhci_ops sdhci_iproc_32only_ops = {
> .write_w = sdhci_iproc_writew,
> .write_b = sdhci_iproc_writeb,
> .set_clock = sdhci_set_clock,
> - .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .get_max_clock = sdhci_iproc_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
>
> +enum sdhci_pltfm_type {
> + SDHCI_PLTFM_IPROC_CYGNUS,
> + SDHCI_PLTFM_IPROC,
> + SDHCI_PLTFM_BCM2835,
> +};
> +
> static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN | SDHCI_QUIRK2_HOST_OFF_CARD_ON,
> .ops = _iproc_32only_ops,
> };
>
> -static const struct sdhci_iproc_data iproc_cygnus_data = {
> - .pdata = _iproc_cygnus_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> - SDHCI_CAN_VDD_330 |
> - SDHCI_CAN_VDD_180 |
> - SDHCI_CAN_DO_SUSPEND |
> - SDHCI_CAN_DO_HISPD |
> - SDHCI_CAN_DO_ADMA2 |
> - SDHCI_CAN_DO_SDMA,
> - .caps1 = SDHCI_DRIVER_TYPE_C |
> -SDHCI_DRIVER_TYPE_D |
> -SDHCI_SUPPORT_DDR50,
> - .mmc_caps = MMC_CAP_1_8V_DDR,
> -};
> -
> static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> @@ -213,21 +214,6 @@ static const struct sdhci_pltfm_data
> sdhci_iproc_pltfm_data = {
> .ops = _iproc_ops,
> };
>
> -static const struct sdhci_iproc_data iproc_data = {
> - .pdata = _iproc_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> - SDHCI_CAN_VDD_330 |
> - SDHCI_CAN_VDD_180 |
> - SDHCI_CAN_DO_SUSPEND |
> - SDHCI_CAN_DO_HISPD |
> - SDHCI_CAN_DO_ADMA2 |
> - SDHCI_CAN_DO_SDMA,
> - .caps1 = SDHCI_DRIVER_TYPE_C |
> -SDHCI_DRIVER_TYPE_D |
> -SDHCI_SUPPORT_DDR50,
> -};
> -
> static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
> .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -237,38 +223,104 @@ static const struct sdhci_pltfm_data
> sdhci_bcm2835_pltfm_data = {
> .ops = _iproc_32only_ops,
> };
>
> -static const struct sdhci_iproc_data bcm2835_data = {
> - .pdata = _bcm2835_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> -
Re: [PATCH] mmc: host: iproc: Add ACPI support to IPROC SDHCI
Hi Ulf Hansson,
Kindly review this patch.
Thank you,
Regards,
Srinath.
On Tue, Jul 17, 2018 at 10:15 PM, Srinath Mannam
wrote:
> Add ACPI support to all IPROC SDHCI varients
>
> Signed-off-by: Srinath Mannam
> Reviewed-by: Ray Jui
> Reviewed-by: Scott Branden
> Reviewed-by: Vladimir Olovyannikov
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/sdhci-iproc.c | 187
> -
> 2 files changed, 128 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 0581c19..bc6702e 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -334,6 +334,7 @@ config MMC_SDHCI_IPROC
> tristate "SDHCI support for the BCM2835 & iProc SD/MMC Controller"
> depends on ARCH_BCM2835 || ARCH_BCM_IPROC || COMPILE_TEST
> depends on MMC_SDHCI_PLTFM
> + depends on OF || ACPI
> default ARCH_BCM_IPROC
> select MMC_SDHCI_IO_ACCESSORS
> help
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index d0e83db..7c5c923 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -15,6 +15,7 @@
> * iProc SDHCI platform driver
> */
>
> +#include
> #include
> #include
> #include
> @@ -162,9 +163,19 @@ static void sdhci_iproc_writeb(struct sdhci_host *host,
> u8 val, int reg)
> sdhci_iproc_writel(host, newval, reg & ~3);
> }
>
> +static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> + if (pltfm_host->clk)
> + return sdhci_pltfm_clk_get_max_clock(host);
> + else
> + return pltfm_host->clock;
> +}
> +
> static const struct sdhci_ops sdhci_iproc_ops = {
> .set_clock = sdhci_set_clock,
> - .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .get_max_clock = sdhci_iproc_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> @@ -178,34 +189,24 @@ static const struct sdhci_ops sdhci_iproc_32only_ops = {
> .write_w = sdhci_iproc_writew,
> .write_b = sdhci_iproc_writeb,
> .set_clock = sdhci_set_clock,
> - .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .get_max_clock = sdhci_iproc_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .reset = sdhci_reset,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
>
> +enum sdhci_pltfm_type {
> + SDHCI_PLTFM_IPROC_CYGNUS,
> + SDHCI_PLTFM_IPROC,
> + SDHCI_PLTFM_BCM2835,
> +};
> +
> static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN | SDHCI_QUIRK2_HOST_OFF_CARD_ON,
> .ops = _iproc_32only_ops,
> };
>
> -static const struct sdhci_iproc_data iproc_cygnus_data = {
> - .pdata = _iproc_cygnus_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> - SDHCI_CAN_VDD_330 |
> - SDHCI_CAN_VDD_180 |
> - SDHCI_CAN_DO_SUSPEND |
> - SDHCI_CAN_DO_HISPD |
> - SDHCI_CAN_DO_ADMA2 |
> - SDHCI_CAN_DO_SDMA,
> - .caps1 = SDHCI_DRIVER_TYPE_C |
> -SDHCI_DRIVER_TYPE_D |
> -SDHCI_SUPPORT_DDR50,
> - .mmc_caps = MMC_CAP_1_8V_DDR,
> -};
> -
> static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> @@ -213,21 +214,6 @@ static const struct sdhci_pltfm_data
> sdhci_iproc_pltfm_data = {
> .ops = _iproc_ops,
> };
>
> -static const struct sdhci_iproc_data iproc_data = {
> - .pdata = _iproc_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> - SDHCI_CAN_VDD_330 |
> - SDHCI_CAN_VDD_180 |
> - SDHCI_CAN_DO_SUSPEND |
> - SDHCI_CAN_DO_HISPD |
> - SDHCI_CAN_DO_ADMA2 |
> - SDHCI_CAN_DO_SDMA,
> - .caps1 = SDHCI_DRIVER_TYPE_C |
> -SDHCI_DRIVER_TYPE_D |
> -SDHCI_SUPPORT_DDR50,
> -};
> -
> static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
> .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> @@ -237,38 +223,104 @@ static const struct sdhci_pltfm_data
> sdhci_bcm2835_pltfm_data = {
> .ops = _iproc_32only_ops,
> };
>
> -static const struct sdhci_iproc_data bcm2835_data = {
> - .pdata = _bcm2835_pltfm_data,
> - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> - & SDHCI_MAX_BLOCK_MASK) |
> -
[PATCH v2] watchdog: sp805: Add clock-frequency property
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned
int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device
*wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct
amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
--
2.7.4
[PATCH v2] watchdog: sp805: Add clock-frequency property
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned
int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device
*wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct
amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
--
2.7.4
Re: [PATCH] net/rds/Kconfig: RDS should depend on IPV6
On 07/26/2018 06:36 AM, Santosh Shilimkar wrote:
On 7/25/2018 3:20 PM, Anders Roxell wrote:
Build error, implicit declaration of function __inet6_ehashfn shows up
When RDS is enabled but not IPV6.
net/rds/connection.c: In function ‘rds_conn_bucket’:
net/rds/connection.c:67:9: error: implicit declaration of function
‘__inet6_ehashfn’; did you mean ‘__inet_ehashfn’?
[-Werror=implicit-function-declaration]
hash = __inet6_ehashfn(lhash, 0, fhash, 0, rds_hash_secret);
^~~
__inet_ehashfn
Current code adds IPV6 as a depends on in config RDS.
Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation
to struct in6_addr")
Signed-off-by: Anders Roxell
---
net/rds/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/Kconfig b/net/rds/Kconfig
index 41f75563b54b..607128f10bcd 100644
--- a/net/rds/Kconfig
+++ b/net/rds/Kconfig
@@ -1,7 +1,7 @@
config RDS
tristate "The RDS Protocol"
- depends on INET
+ depends on INET && CONFIG_IPV6
This should build without CONFIG_IPV6 too.
Hi Ka-cheong,
Can you please loot at it ? I know you modified
lookup function to take always in6_addr now, but
probably hashing with '__inet_ehashfn' should
work too for non IPV6 address(s).
I guess for now, let's add this dependency first. I
will do a follow up patch to remove this dependency.
Thanks.
--
K. Poon
ka-cheong.p...@oracle.com
Re: [PATCH] net/rds/Kconfig: RDS should depend on IPV6
On 07/26/2018 06:36 AM, Santosh Shilimkar wrote:
On 7/25/2018 3:20 PM, Anders Roxell wrote:
Build error, implicit declaration of function __inet6_ehashfn shows up
When RDS is enabled but not IPV6.
net/rds/connection.c: In function ‘rds_conn_bucket’:
net/rds/connection.c:67:9: error: implicit declaration of function
‘__inet6_ehashfn’; did you mean ‘__inet_ehashfn’?
[-Werror=implicit-function-declaration]
hash = __inet6_ehashfn(lhash, 0, fhash, 0, rds_hash_secret);
^~~
__inet_ehashfn
Current code adds IPV6 as a depends on in config RDS.
Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation
to struct in6_addr")
Signed-off-by: Anders Roxell
---
net/rds/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/Kconfig b/net/rds/Kconfig
index 41f75563b54b..607128f10bcd 100644
--- a/net/rds/Kconfig
+++ b/net/rds/Kconfig
@@ -1,7 +1,7 @@
config RDS
tristate "The RDS Protocol"
- depends on INET
+ depends on INET && CONFIG_IPV6
This should build without CONFIG_IPV6 too.
Hi Ka-cheong,
Can you please loot at it ? I know you modified
lookup function to take always in6_addr now, but
probably hashing with '__inet_ehashfn' should
work too for non IPV6 address(s).
I guess for now, let's add this dependency first. I
will do a follow up patch to remove this dependency.
Thanks.
--
K. Poon
ka-cheong.p...@oracle.com
linux-next: build failure after merge of the block tree
Hi all,
After merging the block tree, today's linux-next build (x86_64
allmodconfig) failed like this:
drivers/nvme/target/rdma.c: In function 'nvmet_rdma_find_get_device':
drivers/nvme/target/rdma.c:894:26: error: 'struct ib_device_attr' has no member
named 'max_sge'; did you mean 'max_cqe'?
cm_id->device->attrs.max_sge) - 1;
^
drivers/nvme/target/rdma.c:893:21: note: in expansion of macro 'max'
inline_sge_count = max(cm_id->device->attrs.max_sge_rd,
^~~
In file included from include/linux/list.h:9:0,
from include/linux/module.h:9,
from drivers/nvme/host/rdma.c:15:
drivers/nvme/host/rdma.c: In function 'nvme_rdma_find_get_device':
drivers/nvme/host/rdma.c:381:23: error: 'struct ib_device_attr' has no member
named 'max_sge'; did you mean 'max_cqe'?
ndev->dev->attrs.max_sge - 1);
^
drivers/nvme/host/rdma.c:380:30: note: in expansion of macro 'min'
ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS,
^~~
Caused by commits
64a741c1eaa8 ("nvme-rdma: support up to 4 segments of inline data")
0d5ee2b2ab4f ("nvmet-rdma: support max(16KB, PAGE_SIZE) inline data")
interacting with commit
33023fb85a42 ("IB/core: add max_send_sge and max_recv_sge attributes")
from the rdma tree.
I have applied the following merge fix patch for today:
From: Stephen Rothwell
Date: Thu, 26 Jul 2018 14:32:15 +1000
Subject: [PATCH] nvme-dma: merge fix up for replacement of max_sge
Signed-off-by: Stephen Rothwell
---
drivers/nvme/host/rdma.c | 2 +-
drivers/nvme/target/rdma.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index cfa0319fcd1c..368fe5ac0c6b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -378,7 +378,7 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
}
ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS,
- ndev->dev->attrs.max_sge - 1);
+ ndev->dev->attrs.max_send_sge - 1);
list_add(>entry, _list);
out_unlock:
mutex_unlock(_list_mutex);
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 86121a7a19b2..8c30ac7d8078 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -891,7 +891,7 @@ nvmet_rdma_find_get_device(struct rdma_cm_id *cm_id)
inline_page_count = num_pages(port->inline_data_size);
inline_sge_count = max(cm_id->device->attrs.max_sge_rd,
- cm_id->device->attrs.max_sge) - 1;
+ cm_id->device->attrs.max_send_sge) - 1;
if (inline_page_count > inline_sge_count) {
pr_warn("inline_data_size %d cannot be supported by device %s.
Reducing to %lu.\n",
port->inline_data_size, cm_id->device->name,
--
2.18.0
--
Cheers,
Stephen Rothwell
pgpcIpnrcEv73.pgp
Description: OpenPGP digital signature
linux-next: build failure after merge of the block tree
Hi all,
After merging the block tree, today's linux-next build (x86_64
allmodconfig) failed like this:
drivers/nvme/target/rdma.c: In function 'nvmet_rdma_find_get_device':
drivers/nvme/target/rdma.c:894:26: error: 'struct ib_device_attr' has no member
named 'max_sge'; did you mean 'max_cqe'?
cm_id->device->attrs.max_sge) - 1;
^
drivers/nvme/target/rdma.c:893:21: note: in expansion of macro 'max'
inline_sge_count = max(cm_id->device->attrs.max_sge_rd,
^~~
In file included from include/linux/list.h:9:0,
from include/linux/module.h:9,
from drivers/nvme/host/rdma.c:15:
drivers/nvme/host/rdma.c: In function 'nvme_rdma_find_get_device':
drivers/nvme/host/rdma.c:381:23: error: 'struct ib_device_attr' has no member
named 'max_sge'; did you mean 'max_cqe'?
ndev->dev->attrs.max_sge - 1);
^
drivers/nvme/host/rdma.c:380:30: note: in expansion of macro 'min'
ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS,
^~~
Caused by commits
64a741c1eaa8 ("nvme-rdma: support up to 4 segments of inline data")
0d5ee2b2ab4f ("nvmet-rdma: support max(16KB, PAGE_SIZE) inline data")
interacting with commit
33023fb85a42 ("IB/core: add max_send_sge and max_recv_sge attributes")
from the rdma tree.
I have applied the following merge fix patch for today:
From: Stephen Rothwell
Date: Thu, 26 Jul 2018 14:32:15 +1000
Subject: [PATCH] nvme-dma: merge fix up for replacement of max_sge
Signed-off-by: Stephen Rothwell
---
drivers/nvme/host/rdma.c | 2 +-
drivers/nvme/target/rdma.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index cfa0319fcd1c..368fe5ac0c6b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -378,7 +378,7 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id)
}
ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS,
- ndev->dev->attrs.max_sge - 1);
+ ndev->dev->attrs.max_send_sge - 1);
list_add(>entry, _list);
out_unlock:
mutex_unlock(_list_mutex);
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 86121a7a19b2..8c30ac7d8078 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -891,7 +891,7 @@ nvmet_rdma_find_get_device(struct rdma_cm_id *cm_id)
inline_page_count = num_pages(port->inline_data_size);
inline_sge_count = max(cm_id->device->attrs.max_sge_rd,
- cm_id->device->attrs.max_sge) - 1;
+ cm_id->device->attrs.max_send_sge) - 1;
if (inline_page_count > inline_sge_count) {
pr_warn("inline_data_size %d cannot be supported by device %s.
Reducing to %lu.\n",
port->inline_data_size, cm_id->device->name,
--
2.18.0
--
Cheers,
Stephen Rothwell
pgpcIpnrcEv73.pgp
Description: OpenPGP digital signature
Re: [PATCH v2 3/4] dma: Add Actions Semi Owl family S900 DMA driver
Hi Vinod,
On Tue, Jul 24, 2018 at 06:39:43PM +0530, Vinod wrote:
> somehow this got stuck so sending again...
>
> On 24-07-18, 18:16, Vinod wrote:
> > On 23-07-18, 09:47, Manivannan Sadhasivam wrote:
> >
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> >
> > do you need this?
> >
Not now ;-) will remove this.
> > > +/* OWL_DMAX_MODE Bits */
> > > +#define OWL_DMA_MODE_TS(x) (((x) & 0x3f) << 0)
> > > +#define OWL_DMA_MODE_ST(x) (((x) & 0x3) << 8)
> > > +#define OWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0)
> > > +#define OWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2)
> > > +#define OWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3)
> >
> > what are you trying to do with this? Generally we would define register
> > bits using BIT and GENMASK here..
> >
Okay. Not sure about BIT() but I can use GENMASK() here.
> > > +/* Extract the bit field to new shift */
> > > +#define BIT_FIELD(val, width, shift, newshift) \
> > > + val) >> (shift)) & ((BIT(width)) - 1)) << (newshift))
> >
> > why new shift? I guess you want to extract bits from a register here and
> > use those, right?
> >
No. Here we are trying to pack two bit fields in a single word. So, the
`shift` is for the first Bit field and the `newshift` is for the second
one. Will modify the comment accordingly!
> > > +struct owl_dma_lli_hw {
> > > + u32 next_lli; /* physical address of the next link list */
> > > + u32 saddr; /* source physical address */
> > > + u32 daddr; /* destination physical address */
> > > + u32 flen:20;/* frame length */
> > > + u32 fcnt:12;/* frame count */
> > > + u32 src_stride; /* source stride */
> > > + u32 dst_stride; /* destination stride */
> > > + u32 ctrla; /* dma_mode and linklist ctrl */
> > > + u32 ctrlb; /* interrupt control */
> > > + u32 const_num; /* data for constant fill */
> >
> > i think you can skip comment here or kernel-doc style, please pick one
> > and not both
> >
Ack. Will remove the per member comment.
> > > +struct owl_dma_txd {
> > > + struct virt_dma_descvd;
> > > + struct list_headlli_list;
> >
> > why do you need this list. vd has its own list as well!
> >
Yes, but vd's list is named as node and that will create ambiguity since we
will be using it as a list. So, I guess we would need lli_list.
> > > +static void pchan_update(void __iomem *reg, u32 val, bool state)
> >
> > why does this not use pchan as arg as the name of API implies (you did
> > that on the other two)
> >
I wanted to just update the reg without using too many arguments.
Anyway, I can modify it to use pchan as the argument.
> > > +static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan,
> > > + struct owl_dma_lli *lli,
> > > + dma_addr_t src, dma_addr_t dst,
> > > + u32 len, enum dma_transfer_direction dir)
> > > +{
> > > + struct owl_dma_lli_hw *hw = >hw;
> > > + u32 mode;
> > > +
> > > + mode = OWL_DMA_MODE_PW(0);
> > > +
> > > + switch (dir) {
> > > + case DMA_MEM_TO_MEM:
> > > + mode |= OWL_DMA_MODE_TS(0) | OWL_DMA_MODE_ST_DCU |
> > > + OWL_DMA_MODE_DT_DCU | OWL_DMA_MODE_SAM_INC |
> > > + OWL_DMA_MODE_DAM_INC;
> > > +
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + hw->next_lli = 0; /* One link list by default */
> > > + hw->saddr = src;
> > > + hw->daddr = dst;
> > > +
> > > + hw->fcnt = 1; /* Frame count fixed as 1 */
> > > + hw->flen = len; /* Max frame length is 1MB */
> >
> > are you checking that somewhere?
> >
No need to check since we allow only max size in the caller. The
following line does the job:
bytes = min_t(size_t, (len - offset), OWL_DMA_FRAME_MAX_LENGTH);
> > > +static struct owl_dma_pchan *owl_dma_get_pchan(struct owl_dma *od,
> > > +struct owl_dma_vchan *vchan)
> > > +{
> > > + struct owl_dma_pchan *pchan;
> > > + unsigned long flags;
> > > + int i;
> > > +
> > > + for (i = 0; i < od->nr_pchans; i++) {
> > > + pchan = >pchans[i];
> > > +
> > > + spin_lock_irqsave(>lock, flags);
> > > + if (!pchan->vchan) {
> > > + pchan->vchan = vchan;
> > > + spin_unlock_irqrestore(>lock, flags);
> > > + break;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(>lock, flags);
> > > + }
> > > +
> > > + if (i == od->nr_pchans) {
> > > + /* No physical channel available, cope with it */
> > > + dev_dbg(od->dma.dev, "no physical channel available\n");
> >
> > not sure about this. The
Re: [PATCH v2 3/4] dma: Add Actions Semi Owl family S900 DMA driver
Hi Vinod,
On Tue, Jul 24, 2018 at 06:39:43PM +0530, Vinod wrote:
> somehow this got stuck so sending again...
>
> On 24-07-18, 18:16, Vinod wrote:
> > On 23-07-18, 09:47, Manivannan Sadhasivam wrote:
> >
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> >
> > do you need this?
> >
Not now ;-) will remove this.
> > > +/* OWL_DMAX_MODE Bits */
> > > +#define OWL_DMA_MODE_TS(x) (((x) & 0x3f) << 0)
> > > +#define OWL_DMA_MODE_ST(x) (((x) & 0x3) << 8)
> > > +#define OWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0)
> > > +#define OWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2)
> > > +#define OWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3)
> >
> > what are you trying to do with this? Generally we would define register
> > bits using BIT and GENMASK here..
> >
Okay. Not sure about BIT() but I can use GENMASK() here.
> > > +/* Extract the bit field to new shift */
> > > +#define BIT_FIELD(val, width, shift, newshift) \
> > > + val) >> (shift)) & ((BIT(width)) - 1)) << (newshift))
> >
> > why new shift? I guess you want to extract bits from a register here and
> > use those, right?
> >
No. Here we are trying to pack two bit fields in a single word. So, the
`shift` is for the first Bit field and the `newshift` is for the second
one. Will modify the comment accordingly!
> > > +struct owl_dma_lli_hw {
> > > + u32 next_lli; /* physical address of the next link list */
> > > + u32 saddr; /* source physical address */
> > > + u32 daddr; /* destination physical address */
> > > + u32 flen:20;/* frame length */
> > > + u32 fcnt:12;/* frame count */
> > > + u32 src_stride; /* source stride */
> > > + u32 dst_stride; /* destination stride */
> > > + u32 ctrla; /* dma_mode and linklist ctrl */
> > > + u32 ctrlb; /* interrupt control */
> > > + u32 const_num; /* data for constant fill */
> >
> > i think you can skip comment here or kernel-doc style, please pick one
> > and not both
> >
Ack. Will remove the per member comment.
> > > +struct owl_dma_txd {
> > > + struct virt_dma_descvd;
> > > + struct list_headlli_list;
> >
> > why do you need this list. vd has its own list as well!
> >
Yes, but vd's list is named as node and that will create ambiguity since we
will be using it as a list. So, I guess we would need lli_list.
> > > +static void pchan_update(void __iomem *reg, u32 val, bool state)
> >
> > why does this not use pchan as arg as the name of API implies (you did
> > that on the other two)
> >
I wanted to just update the reg without using too many arguments.
Anyway, I can modify it to use pchan as the argument.
> > > +static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan,
> > > + struct owl_dma_lli *lli,
> > > + dma_addr_t src, dma_addr_t dst,
> > > + u32 len, enum dma_transfer_direction dir)
> > > +{
> > > + struct owl_dma_lli_hw *hw = >hw;
> > > + u32 mode;
> > > +
> > > + mode = OWL_DMA_MODE_PW(0);
> > > +
> > > + switch (dir) {
> > > + case DMA_MEM_TO_MEM:
> > > + mode |= OWL_DMA_MODE_TS(0) | OWL_DMA_MODE_ST_DCU |
> > > + OWL_DMA_MODE_DT_DCU | OWL_DMA_MODE_SAM_INC |
> > > + OWL_DMA_MODE_DAM_INC;
> > > +
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + hw->next_lli = 0; /* One link list by default */
> > > + hw->saddr = src;
> > > + hw->daddr = dst;
> > > +
> > > + hw->fcnt = 1; /* Frame count fixed as 1 */
> > > + hw->flen = len; /* Max frame length is 1MB */
> >
> > are you checking that somewhere?
> >
No need to check since we allow only max size in the caller. The
following line does the job:
bytes = min_t(size_t, (len - offset), OWL_DMA_FRAME_MAX_LENGTH);
> > > +static struct owl_dma_pchan *owl_dma_get_pchan(struct owl_dma *od,
> > > +struct owl_dma_vchan *vchan)
> > > +{
> > > + struct owl_dma_pchan *pchan;
> > > + unsigned long flags;
> > > + int i;
> > > +
> > > + for (i = 0; i < od->nr_pchans; i++) {
> > > + pchan = >pchans[i];
> > > +
> > > + spin_lock_irqsave(>lock, flags);
> > > + if (!pchan->vchan) {
> > > + pchan->vchan = vchan;
> > > + spin_unlock_irqrestore(>lock, flags);
> > > + break;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(>lock, flags);
> > > + }
> > > +
> > > + if (i == od->nr_pchans) {
> > > + /* No physical channel available, cope with it */
> > > + dev_dbg(od->dma.dev, "no physical channel available\n");
> >
> > not sure about this. The
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter,
It was my mistake sorry for that. I thought to add as Rivewed-by your name..
Many changes are made based on your review comments and suggestions.
So, Can I add your name in Reviewed list?
I will modify and send next patchset.
thank you.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote:
> On 07/23/2018 01:37 AM, Srinath Mannam wrote:
>>
>> Use clock-frequency property given in _DSD object
>> of ACPI device to calculate Watchdog rate as binding
>> clock devices are not available as device tree.
>>
>> Note: There is no formal review process for _DSD
>> properties
>>
>> Signed-off-by: Srinath Mannam
>> Signed-off-by: Guenter Roeck
>
>
> I just noticed this. This is completely inappropriate. It is
> the equivalent of forging a signature on a check.
>
> _Never_ add a Signed-off-by: statement for anyone but yourself.
>
> Guenter
>
>> Reviewed-by: Guenter Roeck
>> Reviewed-by: Ray Jui
>> Reviewed-by: Scott Branden
>> ---
>> drivers/watchdog/sp805_wdt.c | 35 +--
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 9849db0..a896b1c 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -11,6 +11,7 @@
>>* warranty of any kind, whether express or implied.
>>*/
>> +#include
>> #include
>> #include
>> #include
>> @@ -22,6 +23,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -65,6 +67,7 @@ struct sp805_wdt {
>> spinlock_t lock;
>> void __iomem*base;
>> struct clk *clk;
>> + u64 rate;
>> struct amba_device *adev;
>> unsigned intload_val;
>> };
>> @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
>> unsigned int timeout)
>> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> u64 load, rate;
>> - rate = clk_get_rate(wdt->clk);
>> + rate = wdt->rate;
>> /*
>> * sp805 runs counter with given value twice, after the end of
>> first
>> @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
>> unsigned int timeout)
>> static unsigned int wdt_timeleft(struct watchdog_device *wdd)
>> {
>> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> - u64 load, rate;
>> -
>> - rate = clk_get_rate(wdt->clk);
>> + u64 load;
>> spin_lock(>lock);
>> load = readl_relaxed(wdt->base + WDTVALUE);
>> @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
>> watchdog_device *wdd)
>> load += wdt->load_val + 1;
>> spin_unlock(>lock);
>> - return div_u64(load, rate);
>> + return div_u64(load, wdt->rate);
>> }
>> static int
>> @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
>> struct amba_id *id)
>> if (IS_ERR(wdt->base))
>> return PTR_ERR(wdt->base);
>> - wdt->clk = devm_clk_get(>dev, NULL);
>> - if (IS_ERR(wdt->clk)) {
>> - dev_warn(>dev, "Clock not found\n");
>> - ret = PTR_ERR(wdt->clk);
>> - goto err;
>> + if (adev->dev.of_node) {
>> + wdt->clk = devm_clk_get(>dev, NULL);
>> + if (IS_ERR(wdt->clk)) {
>> + dev_err(>dev, "Clock not found\n");
>> + return PTR_ERR(wdt->clk);
>> + }
>> + wdt->rate = clk_get_rate(wdt->clk);
>> + } else if (has_acpi_companion(>dev)) {
>> + /*
>> +* When Driver probe with ACPI device, clock devices
>> +* are not available, so watchdog rate get from
>> +* clock-frequency property given in _DSD object.
>> +*/
>> + device_property_read_u64(>dev, "clock-frequency",
>> +>rate);
>> + if (!wdt->rate) {
>> + dev_err(>dev, "no clock-frequency
>> property\n");
>> + return -ENODEV;
>> + }
>> }
>> wdt->adev = adev;
>>
>
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter,
It was my mistake sorry for that. I thought to add as Rivewed-by your name..
Many changes are made based on your review comments and suggestions.
So, Can I add your name in Reviewed list?
I will modify and send next patchset.
thank you.
Regards,
Srinath.
On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote:
> On 07/23/2018 01:37 AM, Srinath Mannam wrote:
>>
>> Use clock-frequency property given in _DSD object
>> of ACPI device to calculate Watchdog rate as binding
>> clock devices are not available as device tree.
>>
>> Note: There is no formal review process for _DSD
>> properties
>>
>> Signed-off-by: Srinath Mannam
>> Signed-off-by: Guenter Roeck
>
>
> I just noticed this. This is completely inappropriate. It is
> the equivalent of forging a signature on a check.
>
> _Never_ add a Signed-off-by: statement for anyone but yourself.
>
> Guenter
>
>> Reviewed-by: Guenter Roeck
>> Reviewed-by: Ray Jui
>> Reviewed-by: Scott Branden
>> ---
>> drivers/watchdog/sp805_wdt.c | 35 +--
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 9849db0..a896b1c 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -11,6 +11,7 @@
>>* warranty of any kind, whether express or implied.
>>*/
>> +#include
>> #include
>> #include
>> #include
>> @@ -22,6 +23,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -65,6 +67,7 @@ struct sp805_wdt {
>> spinlock_t lock;
>> void __iomem*base;
>> struct clk *clk;
>> + u64 rate;
>> struct amba_device *adev;
>> unsigned intload_val;
>> };
>> @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd,
>> unsigned int timeout)
>> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> u64 load, rate;
>> - rate = clk_get_rate(wdt->clk);
>> + rate = wdt->rate;
>> /*
>> * sp805 runs counter with given value twice, after the end of
>> first
>> @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
>> unsigned int timeout)
>> static unsigned int wdt_timeleft(struct watchdog_device *wdd)
>> {
>> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> - u64 load, rate;
>> -
>> - rate = clk_get_rate(wdt->clk);
>> + u64 load;
>> spin_lock(>lock);
>> load = readl_relaxed(wdt->base + WDTVALUE);
>> @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct
>> watchdog_device *wdd)
>> load += wdt->load_val + 1;
>> spin_unlock(>lock);
>> - return div_u64(load, rate);
>> + return div_u64(load, wdt->rate);
>> }
>> static int
>> @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const
>> struct amba_id *id)
>> if (IS_ERR(wdt->base))
>> return PTR_ERR(wdt->base);
>> - wdt->clk = devm_clk_get(>dev, NULL);
>> - if (IS_ERR(wdt->clk)) {
>> - dev_warn(>dev, "Clock not found\n");
>> - ret = PTR_ERR(wdt->clk);
>> - goto err;
>> + if (adev->dev.of_node) {
>> + wdt->clk = devm_clk_get(>dev, NULL);
>> + if (IS_ERR(wdt->clk)) {
>> + dev_err(>dev, "Clock not found\n");
>> + return PTR_ERR(wdt->clk);
>> + }
>> + wdt->rate = clk_get_rate(wdt->clk);
>> + } else if (has_acpi_companion(>dev)) {
>> + /*
>> +* When Driver probe with ACPI device, clock devices
>> +* are not available, so watchdog rate get from
>> +* clock-frequency property given in _DSD object.
>> +*/
>> + device_property_read_u64(>dev, "clock-frequency",
>> +>rate);
>> + if (!wdt->rate) {
>> + dev_err(>dev, "no clock-frequency
>> property\n");
>> + return -ENODEV;
>> + }
>> }
>> wdt->adev = adev;
>>
>
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
I just noticed this. This is completely inappropriate. It is
the equivalent of forging a signature on a check.
_Never_ add a Signed-off-by: statement for anyone but yourself.
Guenter
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned
int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device
*wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct
amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/23/2018 01:37 AM, Srinath Mannam wrote:
Use clock-frequency property given in _DSD object
of ACPI device to calculate Watchdog rate as binding
clock devices are not available as device tree.
Note: There is no formal review process for _DSD
properties
Signed-off-by: Srinath Mannam
Signed-off-by: Guenter Roeck
I just noticed this. This is completely inappropriate. It is
the equivalent of forging a signature on a check.
_Never_ add a Signed-off-by: statement for anyone but yourself.
Guenter
Reviewed-by: Guenter Roeck
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/watchdog/sp805_wdt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 9849db0..a896b1c 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -11,6 +11,7 @@
* warranty of any kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,6 +67,7 @@ struct sp805_wdt {
spinlock_t lock;
void __iomem*base;
struct clk *clk;
+ u64 rate;
struct amba_device *adev;
unsigned intload_val;
};
@@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned
int timeout)
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
u64 load, rate;
- rate = clk_get_rate(wdt->clk);
+ rate = wdt->rate;
/*
* sp805 runs counter with given value twice, after the end of first
@@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd,
unsigned int timeout)
static unsigned int wdt_timeleft(struct watchdog_device *wdd)
{
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
- u64 load, rate;
-
- rate = clk_get_rate(wdt->clk);
+ u64 load;
spin_lock(>lock);
load = readl_relaxed(wdt->base + WDTVALUE);
@@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device
*wdd)
load += wdt->load_val + 1;
spin_unlock(>lock);
- return div_u64(load, rate);
+ return div_u64(load, wdt->rate);
}
static int
@@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct
amba_id *id)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(wdt->clk)) {
- dev_warn(>dev, "Clock not found\n");
- ret = PTR_ERR(wdt->clk);
- goto err;
+ if (adev->dev.of_node) {
+ wdt->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(wdt->clk)) {
+ dev_err(>dev, "Clock not found\n");
+ return PTR_ERR(wdt->clk);
+ }
+ wdt->rate = clk_get_rate(wdt->clk);
+ } else if (has_acpi_companion(>dev)) {
+ /*
+* When Driver probe with ACPI device, clock devices
+* are not available, so watchdog rate get from
+* clock-frequency property given in _DSD object.
+*/
+ device_property_read_u64(>dev, "clock-frequency",
+>rate);
+ if (!wdt->rate) {
+ dev_err(>dev, "no clock-frequency property\n");
+ return -ENODEV;
+ }
}
wdt->adev = adev;
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:44:57PM -0500, Richard Kuo wrote: > On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > > This patch adds registration of the system memory with memblock, eliminates > > bootmem initialization and converts early memory reservations from bootmem > > to memblock. > > > > Signed-off-by: Mike Rapoport > > --- > > v2: fix calculation of the reserved memory size > > > > arch/hexagon/Kconfig | 3 +++ > > arch/hexagon/mm/init.c | 20 > > 2 files changed, 11 insertions(+), 12 deletions(-) > > > > Looks good, I can take this through my tree. Thanks. The hexagon tree seems the most appropriate :) > Acked-by: Richard Kuo > -- Sincerely yours, Mike.
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:44:57PM -0500, Richard Kuo wrote: > On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > > This patch adds registration of the system memory with memblock, eliminates > > bootmem initialization and converts early memory reservations from bootmem > > to memblock. > > > > Signed-off-by: Mike Rapoport > > --- > > v2: fix calculation of the reserved memory size > > > > arch/hexagon/Kconfig | 3 +++ > > arch/hexagon/mm/init.c | 20 > > 2 files changed, 11 insertions(+), 12 deletions(-) > > > > Looks good, I can take this through my tree. Thanks. The hexagon tree seems the most appropriate :) > Acked-by: Richard Kuo > -- Sincerely yours, Mike.
[PATCH 1/4] ARM: dts: mvebu: 98dx3236: Rename nand controller node
Update the 98dx3236 SoC and dependent boards to use
"nand-controller" instead of "nand".
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +-
arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 2 +-
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 8d708cc22495..eb03a5773903 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -189,7 +189,7 @@
};
};
- nand: nand@d {
+ nand_controller: nand-controller@d {
clocks = <_coredivclk 0>;
};
diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
index f42fc6118b7c..944e0a9c9dac 100644
--- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
+++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
@@ -68,7 +68,7 @@
status = "okay";
};
- {
+_controller {
status = "okay";
label = "pxa3xx_nand-0";
num-cs = <1>;
diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index 8432f517e346..d58ea48cb151 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -67,7 +67,7 @@
status = "okay";
};
- {
+_controller {
status = "okay";
label = "pxa3xx_nand-0";
num-cs = <1>;
--
2.18.0
[PATCH 1/4] ARM: dts: mvebu: 98dx3236: Rename nand controller node
Update the 98dx3236 SoC and dependent boards to use
"nand-controller" instead of "nand".
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +-
arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 2 +-
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 8d708cc22495..eb03a5773903 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -189,7 +189,7 @@
};
};
- nand: nand@d {
+ nand_controller: nand-controller@d {
clocks = <_coredivclk 0>;
};
diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
index f42fc6118b7c..944e0a9c9dac 100644
--- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
+++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
@@ -68,7 +68,7 @@
status = "okay";
};
- {
+_controller {
status = "okay";
label = "pxa3xx_nand-0";
num-cs = <1>;
diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index 8432f517e346..d58ea48cb151 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -67,7 +67,7 @@
status = "okay";
};
- {
+_controller {
status = "okay";
label = "pxa3xx_nand-0";
num-cs = <1>;
--
2.18.0
[PATCH 2/4] ARM: dts: mvebu: db-dxbc2: use new style nand binding
Update the nand flash binding to the new style.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
index 944e0a9c9dac..8a3aa616bbd0 100644
--- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
+++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
@@ -70,12 +70,16 @@
_controller {
status = "okay";
- label = "pxa3xx_nand-0";
- num-cs = <1>;
- marvell,nand-keep-config;
- nand-on-flash-bbt;
- nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ marvell,nand-keep-config;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+ };
};
{
--
2.18.0
[PATCH 3/4] ARM: dts: mvebu: db-xc3-24g4: use new style nand binding
Update the nand flash binding to the new style.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index d58ea48cb151..df048050615f 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -69,12 +69,16 @@
_controller {
status = "okay";
- label = "pxa3xx_nand-0";
- num-cs = <1>;
- marvell,nand-keep-config;
- nand-on-flash-bbt;
- nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ marvell,nand-keep-config;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+ };
};
{
--
2.18.0
[PATCH 4/4] ARM: dts: mvebu: Add device tree for db-88f6820-amc board
This board is a plugin card for some of Marvell's switch development
kits. It's similar to the non-amc board except that it has no SATA
support.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/Makefile| 1 +
.../boot/dts/armada-385-db-88f6820-amc.dts| 147 ++
2 files changed, 148 insertions(+)
create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 37a3de760d40..e4212b7b7d9e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1126,6 +1126,7 @@ dtb-$(CONFIG_MACH_ARMADA_370) += \
dtb-$(CONFIG_MACH_ARMADA_375) += \
armada-375-db.dtb
dtb-$(CONFIG_MACH_ARMADA_38X) += \
+ armada-385-db-88f6820-amc.dtb \
armada-385-db-ap.dtb \
armada-385-linksys-caiman.dtb \
armada-385-linksys-cobra.dtb \
diff --git a/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
new file mode 100644
index ..d87614057e3f
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Device Tree file for Marvell Armada 385 AMC board
+ * (DB-88F6820-AMC)
+ *
+ * Copyright (C) 2017 Allied Telesis Labs
+ */
+
+/dts-v1/;
+#include "armada-385.dtsi"
+
+#include
+
+/ {
+ model = "Marvell Armada 385 AMC";
+ compatible = "marvell,a385-db-amc", "marvell,armada385",
"marvell,armada380";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ aliases {
+ ethernet0 =
+ ethernet1 =
+ spi1 =
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x 0x8000>; /* 2GB */
+ };
+
+ soc {
+ ranges = ;
+ };
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+};
+
+ {
+ /*
+* Exported on the micro USB connector CON3
+* through an FTDI
+*/
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+};
+
+
+ {
+ pinctrl-names = "default";
+ /*
+* The Reference Clock 0 is used to provide a
+* clock to the PHY
+*/
+ pinctrl-0 = <_rgmii_pins>, <_clk0_pins>;
+ status = "okay";
+ phy = <>;
+ phy-mode = "rgmii-id";
+};
+
+ {
+ status = "okay";
+ phy = <>;
+ phy-mode = "sgmii";
+};
+
+ {
+ status = "okay";
+};
+
+
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+
+ phy0: ethernet-phy@1 {
+ reg = <1>;
+ };
+
+ phy1: ethernet-phy@0 {
+ reg = <0>;
+ };
+};
+
+_controller {
+ status = "okay";
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+
+ partition@user {
+ reg = <0x 0x4000>;
+ label = "user";
+ };
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ /* Port 0, Lane 0 */
+ status = "okay";
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+
+ spi-flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "jedec,spi-nor";
+ reg = <0>; /* Chip select 0 */
+ spi-max-frequency = <5000>;
+ m25p,fast-read;
+
+ partition@u-boot {
+ reg = <0x 0x0010>;
+ label = "u-boot";
+ };
+ partition@u-boot-env {
+ reg = <0x0010 0x0004>;
+ label = "u-boot-env";
+ };
+ };
+};
+
+ {
+ clock-frequency = <2000>;
+};
--
2.18.0
[PATCH 0/4] ARM: dts: mvebu: updates and new board
This series updates the armada-xp-98dx3236 SoC and related boards to use the new style dts bindings for nand. I've also added a new db-88f6820-amc board which is an Armada-385 based reference board from Marvell's switch team. It's a plugin card for either the db-dxbc2 or db-xc3-24g4 which can be used if you disable the internal CPU on those platforms. Chris Packham (4): ARM: dts: mvebu: 98dx3236: Rename nand controller node ARM: dts: mvebu: db-dxbc2: use new style nand binding ARM: dts: mvebu: db-xc3-24g4: use new style nand binding ARM: dts: mvebu: Add device tree for db-88f6820-amc board arch/arm/boot/dts/Makefile| 1 + .../boot/dts/armada-385-db-88f6820-amc.dts| 184 ++ arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +- arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 18 +- arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 18 +- 5 files changed, 208 insertions(+), 15 deletions(-) create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts -- 2.18.0
[PATCH 2/4] ARM: dts: mvebu: db-dxbc2: use new style nand binding
Update the nand flash binding to the new style.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
index 944e0a9c9dac..8a3aa616bbd0 100644
--- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
+++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts
@@ -70,12 +70,16 @@
_controller {
status = "okay";
- label = "pxa3xx_nand-0";
- num-cs = <1>;
- marvell,nand-keep-config;
- nand-on-flash-bbt;
- nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ marvell,nand-keep-config;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+ };
};
{
--
2.18.0
[PATCH 3/4] ARM: dts: mvebu: db-xc3-24g4: use new style nand binding
Update the nand flash binding to the new style.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index d58ea48cb151..df048050615f 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -69,12 +69,16 @@
_controller {
status = "okay";
- label = "pxa3xx_nand-0";
- num-cs = <1>;
- marvell,nand-keep-config;
- nand-on-flash-bbt;
- nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ marvell,nand-keep-config;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+ };
};
{
--
2.18.0
[PATCH 4/4] ARM: dts: mvebu: Add device tree for db-88f6820-amc board
This board is a plugin card for some of Marvell's switch development
kits. It's similar to the non-amc board except that it has no SATA
support.
Signed-off-by: Chris Packham
---
arch/arm/boot/dts/Makefile| 1 +
.../boot/dts/armada-385-db-88f6820-amc.dts| 147 ++
2 files changed, 148 insertions(+)
create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 37a3de760d40..e4212b7b7d9e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1126,6 +1126,7 @@ dtb-$(CONFIG_MACH_ARMADA_370) += \
dtb-$(CONFIG_MACH_ARMADA_375) += \
armada-375-db.dtb
dtb-$(CONFIG_MACH_ARMADA_38X) += \
+ armada-385-db-88f6820-amc.dtb \
armada-385-db-ap.dtb \
armada-385-linksys-caiman.dtb \
armada-385-linksys-cobra.dtb \
diff --git a/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
new file mode 100644
index ..d87614057e3f
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Device Tree file for Marvell Armada 385 AMC board
+ * (DB-88F6820-AMC)
+ *
+ * Copyright (C) 2017 Allied Telesis Labs
+ */
+
+/dts-v1/;
+#include "armada-385.dtsi"
+
+#include
+
+/ {
+ model = "Marvell Armada 385 AMC";
+ compatible = "marvell,a385-db-amc", "marvell,armada385",
"marvell,armada380";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ aliases {
+ ethernet0 =
+ ethernet1 =
+ spi1 =
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x 0x8000>; /* 2GB */
+ };
+
+ soc {
+ ranges = ;
+ };
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+};
+
+ {
+ /*
+* Exported on the micro USB connector CON3
+* through an FTDI
+*/
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+};
+
+
+ {
+ pinctrl-names = "default";
+ /*
+* The Reference Clock 0 is used to provide a
+* clock to the PHY
+*/
+ pinctrl-0 = <_rgmii_pins>, <_clk0_pins>;
+ status = "okay";
+ phy = <>;
+ phy-mode = "rgmii-id";
+};
+
+ {
+ status = "okay";
+ phy = <>;
+ phy-mode = "sgmii";
+};
+
+ {
+ status = "okay";
+};
+
+
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+
+ phy0: ethernet-phy@1 {
+ reg = <1>;
+ };
+
+ phy1: ethernet-phy@0 {
+ reg = <0>;
+ };
+};
+
+_controller {
+ status = "okay";
+
+ nand@0 {
+ reg = <0>;
+ label = "pxa3xx_nand-0";
+ nand-rb = <0>;
+ nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
+
+ partition@user {
+ reg = <0x 0x4000>;
+ label = "user";
+ };
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ /* Port 0, Lane 0 */
+ status = "okay";
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "okay";
+
+ spi-flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "jedec,spi-nor";
+ reg = <0>; /* Chip select 0 */
+ spi-max-frequency = <5000>;
+ m25p,fast-read;
+
+ partition@u-boot {
+ reg = <0x 0x0010>;
+ label = "u-boot";
+ };
+ partition@u-boot-env {
+ reg = <0x0010 0x0004>;
+ label = "u-boot-env";
+ };
+ };
+};
+
+ {
+ clock-frequency = <2000>;
+};
--
2.18.0
[PATCH 0/4] ARM: dts: mvebu: updates and new board
This series updates the armada-xp-98dx3236 SoC and related boards to use the new style dts bindings for nand. I've also added a new db-88f6820-amc board which is an Armada-385 based reference board from Marvell's switch team. It's a plugin card for either the db-dxbc2 or db-xc3-24g4 which can be used if you disable the internal CPU on those platforms. Chris Packham (4): ARM: dts: mvebu: 98dx3236: Rename nand controller node ARM: dts: mvebu: db-dxbc2: use new style nand binding ARM: dts: mvebu: db-xc3-24g4: use new style nand binding ARM: dts: mvebu: Add device tree for db-88f6820-amc board arch/arm/boot/dts/Makefile| 1 + .../boot/dts/armada-385-db-88f6820-amc.dts| 184 ++ arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +- arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 18 +- arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 18 +- 5 files changed, 208 insertions(+), 15 deletions(-) create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts -- 2.18.0
linux-next: manual merge of the block tree with the rdma tree
Hi all,
Today's linux-next merge of the block tree got a conflict in:
drivers/nvme/target/rdma.c
between commit:
23f96d1f15a7 ("nvmet-rdma: Simplify ib_post_(send|recv|srq_recv)() calls")
202093848cac ("nvmet-rdma: add an error flow for post_recv failures")
from the rdma tree and commits:
2fc464e2162c ("nvmet-rdma: add unlikely check in the fast path")
from the block tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/nvme/target/rdma.c
index 1a642e214a4c,e7f43d1e1779..
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@@ -382,13 -435,22 +435,21 @@@ static void nvmet_rdma_free_rsps(struc
static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev,
struct nvmet_rdma_cmd *cmd)
{
- struct ib_recv_wr *bad_wr;
+ int ret;
+
ib_dma_sync_single_for_device(ndev->device,
cmd->sge[0].addr, cmd->sge[0].length,
DMA_FROM_DEVICE);
if (ndev->srq)
- return ib_post_srq_recv(ndev->srq, >wr, NULL);
- return ib_post_recv(cmd->queue->cm_id->qp, >wr, NULL);
- ret = ib_post_srq_recv(ndev->srq, >wr, _wr);
++ ret = ib_post_srq_recv(ndev->srq, >wr, NULL);
+ else
- ret = ib_post_recv(cmd->queue->cm_id->qp, >wr, _wr);
++ ret = ib_post_recv(cmd->queue->cm_id->qp, >wr, NULL);
+
+ if (unlikely(ret))
+ pr_err("post_recv cmd failed\n");
+
+ return ret;
}
static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue)
@@@ -491,7 -553,7 +552,7 @@@ static void nvmet_rdma_queue_response(s
rsp->send_sge.addr, rsp->send_sge.length,
DMA_TO_DEVICE);
- if (ib_post_send(cm_id->qp, first_wr, NULL)) {
- if (unlikely(ib_post_send(cm_id->qp, first_wr, _wr))) {
++ if (unlikely(ib_post_send(cm_id->qp, first_wr, NULL))) {
pr_err("sending cmd response failed\n");
nvmet_rdma_release_rsp(rsp);
}
pgpCvQwUaa8BX.pgp
Description: OpenPGP digital signature
linux-next: manual merge of the block tree with the rdma tree
Hi all,
Today's linux-next merge of the block tree got a conflict in:
drivers/nvme/target/rdma.c
between commit:
23f96d1f15a7 ("nvmet-rdma: Simplify ib_post_(send|recv|srq_recv)() calls")
202093848cac ("nvmet-rdma: add an error flow for post_recv failures")
from the rdma tree and commits:
2fc464e2162c ("nvmet-rdma: add unlikely check in the fast path")
from the block tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/nvme/target/rdma.c
index 1a642e214a4c,e7f43d1e1779..
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@@ -382,13 -435,22 +435,21 @@@ static void nvmet_rdma_free_rsps(struc
static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev,
struct nvmet_rdma_cmd *cmd)
{
- struct ib_recv_wr *bad_wr;
+ int ret;
+
ib_dma_sync_single_for_device(ndev->device,
cmd->sge[0].addr, cmd->sge[0].length,
DMA_FROM_DEVICE);
if (ndev->srq)
- return ib_post_srq_recv(ndev->srq, >wr, NULL);
- return ib_post_recv(cmd->queue->cm_id->qp, >wr, NULL);
- ret = ib_post_srq_recv(ndev->srq, >wr, _wr);
++ ret = ib_post_srq_recv(ndev->srq, >wr, NULL);
+ else
- ret = ib_post_recv(cmd->queue->cm_id->qp, >wr, _wr);
++ ret = ib_post_recv(cmd->queue->cm_id->qp, >wr, NULL);
+
+ if (unlikely(ret))
+ pr_err("post_recv cmd failed\n");
+
+ return ret;
}
static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue)
@@@ -491,7 -553,7 +552,7 @@@ static void nvmet_rdma_queue_response(s
rsp->send_sge.addr, rsp->send_sge.length,
DMA_TO_DEVICE);
- if (ib_post_send(cm_id->qp, first_wr, NULL)) {
- if (unlikely(ib_post_send(cm_id->qp, first_wr, _wr))) {
++ if (unlikely(ib_post_send(cm_id->qp, first_wr, NULL))) {
pr_err("sending cmd response failed\n");
nvmet_rdma_release_rsp(rsp);
}
pgpCvQwUaa8BX.pgp
Description: OpenPGP digital signature
[PATCH v5] ARM: mvebu: use dt_fixup to provide fallback for enable-method
We need to maintain backwards compatibility with device trees that don't
define an enable method. At the same time we want the device tree to be
able to specify an enable-method and have it stick.
Previously by having smp assigned in the DT_MACHINE definition this
would be picked up by setup_arch() and override whatever
arm_dt_init_cpu_maps() had configured. Now we move the initial
assignment of default smp_ops to a dt_fixup and let
arm_dt_init_cpu_maps() override that if the device tree defines an
enable-method.
Signed-off-by: Chris Packham
---
Pervious versions
v1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300182.html
v2:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300480.html
v3:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302945.html
v4:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303899.html
Changes since v4:
- drop "RFC"
arch/arm/mach-mvebu/board-v7.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index ccca95173e17..5bbde5e5258e 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -145,6 +145,11 @@ static void __init mvebu_dt_init(void)
i2c_quirk();
}
+static void __init armada_370_xp_dt_fixup(void)
+{
+ smp_set_ops(smp_ops(armada_xp_smp_ops));
+}
+
static const char * const armada_370_xp_dt_compat[] __initconst = {
"marvell,armada-370-xp",
NULL,
@@ -153,17 +158,12 @@ static const char * const armada_370_xp_dt_compat[]
__initconst = {
DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
.l2c_aux_val= 0,
.l2c_aux_mask = ~0,
-/*
- * The following field (.smp) is still needed to ensure backward
- * compatibility with old Device Trees that were not specifying the
- * cpus enable-method property.
- */
- .smp= smp_ops(armada_xp_smp_ops),
.init_machine = mvebu_dt_init,
.init_irq = mvebu_init_irq,
.restart= mvebu_restart,
.reserve= mvebu_memblock_reserve,
.dt_compat = armada_370_xp_dt_compat,
+ .dt_fixup = armada_370_xp_dt_fixup,
MACHINE_END
static const char * const armada_375_dt_compat[] __initconst = {
--
2.18.0
[PATCH v5] ARM: mvebu: use dt_fixup to provide fallback for enable-method
We need to maintain backwards compatibility with device trees that don't
define an enable method. At the same time we want the device tree to be
able to specify an enable-method and have it stick.
Previously by having smp assigned in the DT_MACHINE definition this
would be picked up by setup_arch() and override whatever
arm_dt_init_cpu_maps() had configured. Now we move the initial
assignment of default smp_ops to a dt_fixup and let
arm_dt_init_cpu_maps() override that if the device tree defines an
enable-method.
Signed-off-by: Chris Packham
---
Pervious versions
v1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300182.html
v2:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300480.html
v3:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302945.html
v4:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303899.html
Changes since v4:
- drop "RFC"
arch/arm/mach-mvebu/board-v7.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index ccca95173e17..5bbde5e5258e 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -145,6 +145,11 @@ static void __init mvebu_dt_init(void)
i2c_quirk();
}
+static void __init armada_370_xp_dt_fixup(void)
+{
+ smp_set_ops(smp_ops(armada_xp_smp_ops));
+}
+
static const char * const armada_370_xp_dt_compat[] __initconst = {
"marvell,armada-370-xp",
NULL,
@@ -153,17 +158,12 @@ static const char * const armada_370_xp_dt_compat[]
__initconst = {
DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
.l2c_aux_val= 0,
.l2c_aux_mask = ~0,
-/*
- * The following field (.smp) is still needed to ensure backward
- * compatibility with old Device Trees that were not specifying the
- * cpus enable-method property.
- */
- .smp= smp_ops(armada_xp_smp_ops),
.init_machine = mvebu_dt_init,
.init_irq = mvebu_init_irq,
.restart= mvebu_restart,
.reserve= mvebu_memblock_reserve,
.dt_compat = armada_370_xp_dt_compat,
+ .dt_fixup = armada_370_xp_dt_fixup,
MACHINE_END
static const char * const armada_375_dt_compat[] __initconst = {
--
2.18.0
Re: [PATCH 0/3] PTI for x86-32 Fixes and Updates
On Mon, 2018-07-23 at 09:29 +0200, Joerg Roedel wrote: > Hey David, > > On Sun, Jul 22, 2018 at 11:49:00PM -0400, David H. Gutteridge wrote: > > Unfortunately, I can trigger a bug in KVM+QEMU with the Bochs VGA > > driver. (This is the same VM definition I shared with you in a PM > > back on Feb. 20th, except note that 4.18 kernels won't successfully > > boot with QEMU's IDE device, so I'm using SATA instead. That's a > > regression totally unrelated to your change sets, or to the general > > booting issue with 4.18 RC5, since it occurs in vanilla RC4 as > > well.) > > Yes, this needs the fixes in the tip/x86/mm branch as well. Can you > that branch in and test again, please? Sorry, I didn't realize I needed those changes, too. I've re-tested with those applied and haven't encountered any issues. I'm now re-testing again with your newer patch set from the 25th. No issues so far with those, either; I'll confirm in that email thread after the laptop has seen some more use. Dave
Re: [PATCH 0/3] PTI for x86-32 Fixes and Updates
On Mon, 2018-07-23 at 09:29 +0200, Joerg Roedel wrote: > Hey David, > > On Sun, Jul 22, 2018 at 11:49:00PM -0400, David H. Gutteridge wrote: > > Unfortunately, I can trigger a bug in KVM+QEMU with the Bochs VGA > > driver. (This is the same VM definition I shared with you in a PM > > back on Feb. 20th, except note that 4.18 kernels won't successfully > > boot with QEMU's IDE device, so I'm using SATA instead. That's a > > regression totally unrelated to your change sets, or to the general > > booting issue with 4.18 RC5, since it occurs in vanilla RC4 as > > well.) > > Yes, this needs the fixes in the tip/x86/mm branch as well. Can you > that branch in and test again, please? Sorry, I didn't realize I needed those changes, too. I've re-tested with those applied and haven't encountered any issues. I'm now re-testing again with your newer patch set from the 25th. No issues so far with those, either; I'll confirm in that email thread after the laptop has seen some more use. Dave
Re: [PATCH] ACPI / LPSS: Avoid PM quirks on suspend and resume from S3
Hi Rafael,
> On 2018Jul24, at 18:36, Rafael J. Wysocki wrote:
>
> On Tuesday, July 24, 2018 11:13:42 AM CEST Rafael J. Wysocki wrote:
>> On Tuesday, July 24, 2018 10:46:09 AM CEST Kai Heng Feng wrote:
>>> Hi Rafael,
>>>
On Jun 13, 2018, at 7:17 PM, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki
It is reported that commit a192aa923b66a (ACPI / LPSS: Consolidate
runtime PM and system sleep handling) introduced a system suspend
regression on some machines, but the only functional change made by
it was to cause the PM quirks in the LPSS to also be used during
system suspend and resume. While that should always work for
suspend-to-idle, it turns out to be problematic for S3
(suspend-to-RAM).
To address that issue restore the previous S3 suspend and resume
behavior of the LPSS to avoid applying PM quirks then.
>>>
>>> The users reported that this patch does fix the S3 issue, but the S4 still
>>> fails.
>>>
>>> Please refer to [1] for more for user's testing result.
>>>
>>> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1774950/comments/60
>>
>> Please ask the users to test the patch below, then, on top of the $subject
>> one.
>
> There was a mistake in the previous patch I posted, sorry about that.
>
> Please test this one instead:
The result is positive, thanks!
Kai-Heng
>
> ---
> drivers/acpi/acpi_lpss.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> Index: linux-pm/drivers/acpi/acpi_lpss.c
> ===
> --- linux-pm.orig/drivers/acpi/acpi_lpss.c
> +++ linux-pm/drivers/acpi/acpi_lpss.c
> @@ -879,6 +879,7 @@ static void acpi_lpss_dismiss(struct dev
> #define LPSS_GPIODEF0_DMA_LLP BIT(13)
>
> static DEFINE_MUTEX(lpss_iosf_mutex);
> +static bool lpss_iosf_d3_entered;
>
> static void lpss_iosf_enter_d3_state(void)
> {
> @@ -921,6 +922,9 @@ static void lpss_iosf_enter_d3_state(voi
>
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
> +
> + lpss_iosf_d3_entered = true;
> +
> exit:
> mutex_unlock(_iosf_mutex);
> }
> @@ -935,6 +939,11 @@ static void lpss_iosf_exit_d3_state(void
>
> mutex_lock(_iosf_mutex);
>
> + if (!lpss_iosf_d3_entered)
> + goto exit;
> +
> + lpss_iosf_d3_entered = false;
> +
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
>
> @@ -944,13 +953,13 @@ static void lpss_iosf_exit_d3_state(void
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE,
> LPSS_IOSF_PMCSR, value2, mask2);
>
> +exit:
> mutex_unlock(_iosf_mutex);
> }
>
> -static int acpi_lpss_suspend(struct device *dev, bool runtime)
> +static int acpi_lpss_suspend(struct device *dev, bool wakeup)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> - bool wakeup = runtime || device_may_wakeup(dev);
> int ret;
>
> if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
> @@ -963,14 +972,14 @@ static int acpi_lpss_suspend(struct devi
>* wrong status for devices being about to be powered off. See
>* lpss_iosf_enter_d3_state() for further information.
>*/
> - if ((runtime || !pm_suspend_via_firmware()) &&
> + if (acpi_target_system_state() == ACPI_STATE_S0 &&
> lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_enter_d3_state();
>
> return ret;
> }
>
> -static int acpi_lpss_resume(struct device *dev, bool runtime)
> +static int acpi_lpss_resume(struct device *dev)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> int ret;
> @@ -979,8 +988,7 @@ static int acpi_lpss_resume(struct devic
>* This call is kept first to be in symmetry with
>* acpi_lpss_runtime_suspend() one.
>*/
> - if ((runtime || !pm_resume_via_firmware()) &&
> - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_exit_d3_state();
>
> ret = acpi_dev_resume(dev);
> @@ -1004,12 +1012,12 @@ static int acpi_lpss_suspend_late(struct
> return 0;
>
> ret = pm_generic_suspend_late(dev);
> - return ret ? ret : acpi_lpss_suspend(dev, false);
> + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev));
> }
>
> static int acpi_lpss_resume_early(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, false);
> + int ret = acpi_lpss_resume(dev);
>
> return ret ? ret : pm_generic_resume_early(dev);
> }
> @@ -1024,7 +1032,7 @@ static int acpi_lpss_runtime_suspend(str
>
> static int acpi_lpss_runtime_resume(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, true);
> + int
Re: [PATCH] ACPI / LPSS: Avoid PM quirks on suspend and resume from S3
Hi Rafael,
> On 2018Jul24, at 18:36, Rafael J. Wysocki wrote:
>
> On Tuesday, July 24, 2018 11:13:42 AM CEST Rafael J. Wysocki wrote:
>> On Tuesday, July 24, 2018 10:46:09 AM CEST Kai Heng Feng wrote:
>>> Hi Rafael,
>>>
On Jun 13, 2018, at 7:17 PM, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki
It is reported that commit a192aa923b66a (ACPI / LPSS: Consolidate
runtime PM and system sleep handling) introduced a system suspend
regression on some machines, but the only functional change made by
it was to cause the PM quirks in the LPSS to also be used during
system suspend and resume. While that should always work for
suspend-to-idle, it turns out to be problematic for S3
(suspend-to-RAM).
To address that issue restore the previous S3 suspend and resume
behavior of the LPSS to avoid applying PM quirks then.
>>>
>>> The users reported that this patch does fix the S3 issue, but the S4 still
>>> fails.
>>>
>>> Please refer to [1] for more for user's testing result.
>>>
>>> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1774950/comments/60
>>
>> Please ask the users to test the patch below, then, on top of the $subject
>> one.
>
> There was a mistake in the previous patch I posted, sorry about that.
>
> Please test this one instead:
The result is positive, thanks!
Kai-Heng
>
> ---
> drivers/acpi/acpi_lpss.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> Index: linux-pm/drivers/acpi/acpi_lpss.c
> ===
> --- linux-pm.orig/drivers/acpi/acpi_lpss.c
> +++ linux-pm/drivers/acpi/acpi_lpss.c
> @@ -879,6 +879,7 @@ static void acpi_lpss_dismiss(struct dev
> #define LPSS_GPIODEF0_DMA_LLP BIT(13)
>
> static DEFINE_MUTEX(lpss_iosf_mutex);
> +static bool lpss_iosf_d3_entered;
>
> static void lpss_iosf_enter_d3_state(void)
> {
> @@ -921,6 +922,9 @@ static void lpss_iosf_enter_d3_state(voi
>
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
> +
> + lpss_iosf_d3_entered = true;
> +
> exit:
> mutex_unlock(_iosf_mutex);
> }
> @@ -935,6 +939,11 @@ static void lpss_iosf_exit_d3_state(void
>
> mutex_lock(_iosf_mutex);
>
> + if (!lpss_iosf_d3_entered)
> + goto exit;
> +
> + lpss_iosf_d3_entered = false;
> +
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
> LPSS_IOSF_GPIODEF0, value1, mask1);
>
> @@ -944,13 +953,13 @@ static void lpss_iosf_exit_d3_state(void
> iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE,
> LPSS_IOSF_PMCSR, value2, mask2);
>
> +exit:
> mutex_unlock(_iosf_mutex);
> }
>
> -static int acpi_lpss_suspend(struct device *dev, bool runtime)
> +static int acpi_lpss_suspend(struct device *dev, bool wakeup)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> - bool wakeup = runtime || device_may_wakeup(dev);
> int ret;
>
> if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
> @@ -963,14 +972,14 @@ static int acpi_lpss_suspend(struct devi
>* wrong status for devices being about to be powered off. See
>* lpss_iosf_enter_d3_state() for further information.
>*/
> - if ((runtime || !pm_suspend_via_firmware()) &&
> + if (acpi_target_system_state() == ACPI_STATE_S0 &&
> lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_enter_d3_state();
>
> return ret;
> }
>
> -static int acpi_lpss_resume(struct device *dev, bool runtime)
> +static int acpi_lpss_resume(struct device *dev)
> {
> struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> int ret;
> @@ -979,8 +988,7 @@ static int acpi_lpss_resume(struct devic
>* This call is kept first to be in symmetry with
>* acpi_lpss_runtime_suspend() one.
>*/
> - if ((runtime || !pm_resume_via_firmware()) &&
> - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
> lpss_iosf_exit_d3_state();
>
> ret = acpi_dev_resume(dev);
> @@ -1004,12 +1012,12 @@ static int acpi_lpss_suspend_late(struct
> return 0;
>
> ret = pm_generic_suspend_late(dev);
> - return ret ? ret : acpi_lpss_suspend(dev, false);
> + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev));
> }
>
> static int acpi_lpss_resume_early(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, false);
> + int ret = acpi_lpss_resume(dev);
>
> return ret ? ret : pm_generic_resume_early(dev);
> }
> @@ -1024,7 +1032,7 @@ static int acpi_lpss_runtime_suspend(str
>
> static int acpi_lpss_runtime_resume(struct device *dev)
> {
> - int ret = acpi_lpss_resume(dev, true);
> + int
Re: [PATCH 3/6] irqchip: RISC-V Local Interrupt Controller Driver
On Wed, Jul 25, 2018 at 4:54 PM, Christoph Hellwig wrote: > On Wed, Jul 25, 2018 at 12:18:39PM +0100, Marc Zyngier wrote: >> This feels odd. It means that you cannot have the following sequence: >> >> local_irq_disable(); >> enable_irq(x); // where x is owned by a remote hart >> >> as smp_call_function_single() requires interrupts to be enabled. >> >> More fundamentally, why are you trying to make these interrupts look >> global while they aren't? arm/arm64 have similar restrictions with GICv2 >> and earlier, and treats these interrupts as per-cpu. >> >> Given that the drivers that deal with drivers connected to the per-hart >> irqchip are themselves likely to be aware of the per-cpu aspect, it >> would make sense to align things (we've been through that same >> discussion about the clocksource driver a few weeks back). > > Right now the only direct consumers are said clocksource, the PLIC > driver later in this series and the RISC-V arch IPI code. None of them > is going to do a manual enable_irq, so I guess the remote case of the > code is simply dead code. I'll take a look at converting them to > per-cpu. I guess the GICv2 driver is the best template? Actually, RISCV HLIC and PLIC are very similar to RPi2 and RPi3 SOCs. On RPi2 and RPi3, we have per-CPU BCM2836 local intc and the global interrupts are managed using BCM2835 intc. You should certainly have a look a this drivers because these very simple compared to GICv2 and GICv3 drivers. Regards, Anup
Re: [PATCH 3/6] irqchip: RISC-V Local Interrupt Controller Driver
On Wed, Jul 25, 2018 at 4:54 PM, Christoph Hellwig wrote: > On Wed, Jul 25, 2018 at 12:18:39PM +0100, Marc Zyngier wrote: >> This feels odd. It means that you cannot have the following sequence: >> >> local_irq_disable(); >> enable_irq(x); // where x is owned by a remote hart >> >> as smp_call_function_single() requires interrupts to be enabled. >> >> More fundamentally, why are you trying to make these interrupts look >> global while they aren't? arm/arm64 have similar restrictions with GICv2 >> and earlier, and treats these interrupts as per-cpu. >> >> Given that the drivers that deal with drivers connected to the per-hart >> irqchip are themselves likely to be aware of the per-cpu aspect, it >> would make sense to align things (we've been through that same >> discussion about the clocksource driver a few weeks back). > > Right now the only direct consumers are said clocksource, the PLIC > driver later in this series and the RISC-V arch IPI code. None of them > is going to do a manual enable_irq, so I guess the remote case of the > code is simply dead code. I'll take a look at converting them to > per-cpu. I guess the GICv2 driver is the best template? Actually, RISCV HLIC and PLIC are very similar to RPi2 and RPi3 SOCs. On RPi2 and RPi3, we have per-CPU BCM2836 local intc and the global interrupts are managed using BCM2835 intc. You should certainly have a look a this drivers because these very simple compared to GICv2 and GICv3 drivers. Regards, Anup
Re: [PATCH mmc-next 3/3] mmc: sdhci-of-dwcmshc: solve 128MB DMA boundary limitation
On Wed, 25 Jul 2018 17:47:49 +0800 Jisheng Zhang wrote:
> When using DMA, if the DMA addr spans 128MB boundary, we have to split
> the DMA transfer into two so that each one doesn't exceed the boundary.
>
> Signed-off-by: Jisheng Zhang
> ---
> drivers/mmc/host/sdhci-of-dwcmshc.c | 41 +
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 1b7cd144fb01..01b5cb772554 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -13,16 +13,45 @@
>
> #include "sdhci-pltfm.h"
>
> +#define BOUNDARY_OK(addr, len) \
> + ((addr | (SZ_128M - 1)) == ((addr + len) | (SZ_128M - 1)))
this should be
((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
> +
> struct dwcmshc_priv {
> struct clk *bus_clk;
> };
>
> +/*
> + * if DMA addr spans 128MB boundary, we split the DMA transfer into two
> + * so that the DMA transfer doesn't exceed the boundary.
> + */
> +static unsigned int dwcmshc_adma_write_desc(struct sdhci_host *host,
> + void *desc, dma_addr_t addr,
> + int len, unsigned int cmd)
> +{
> + int tmplen, offset;
> +
> + if (BOUNDARY_OK(addr, len))
> + return _sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +
> + offset = addr & (SZ_128M - 1);
> + tmplen = SZ_128M - offset;
> + _sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
> +
> + addr += tmplen;
> + len -= tmplen;
> + desc += host->desc_sz;
> + _sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +
> + return host->desc_sz * 2;
> +}
> +
> static const struct sdhci_ops sdhci_dwcmshc_ops = {
> .set_clock = sdhci_set_clock,
> .set_bus_width = sdhci_set_bus_width,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> .reset = sdhci_reset,
> + .adma_write_desc= dwcmshc_adma_write_desc,
> };
>
> static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
> @@ -36,12 +65,24 @@ static int dwcmshc_probe(struct platform_device *pdev)
> struct sdhci_host *host;
> struct dwcmshc_priv *priv;
> int err;
> + u32 extra;
>
> host = sdhci_pltfm_init(pdev, _dwcmshc_pdata,
> sizeof(struct dwcmshc_priv));
> if (IS_ERR(host))
> return PTR_ERR(host);
>
> + /*
> + * The DMA descriptor table number is calculated as the maximum
> + * number of segments times 2, to allow for an alignment
> + * descriptor for each segment, plus 1 for a nop end descriptor,
> + * plus extra number for cross 128M boundary handling.
> + */
> + extra = totalram_pages / (SZ_128M / PAGE_SIZE) + 1;
will use DIV_ROUND_UP instead.
> + if (extra > SDHCI_MAX_SEGS)
> + extra = SDHCI_MAX_SEGS;
> + host->adma_table_num = SDHCI_MAX_SEGS * 2 + 1 + extra;
> +
> pltfm_host = sdhci_priv(host);
> priv = sdhci_pltfm_priv(pltfm_host);
>
Re: [PATCH mmc-next 3/3] mmc: sdhci-of-dwcmshc: solve 128MB DMA boundary limitation
On Wed, 25 Jul 2018 17:47:49 +0800 Jisheng Zhang wrote:
> When using DMA, if the DMA addr spans 128MB boundary, we have to split
> the DMA transfer into two so that each one doesn't exceed the boundary.
>
> Signed-off-by: Jisheng Zhang
> ---
> drivers/mmc/host/sdhci-of-dwcmshc.c | 41 +
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 1b7cd144fb01..01b5cb772554 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -13,16 +13,45 @@
>
> #include "sdhci-pltfm.h"
>
> +#define BOUNDARY_OK(addr, len) \
> + ((addr | (SZ_128M - 1)) == ((addr + len) | (SZ_128M - 1)))
this should be
((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
> +
> struct dwcmshc_priv {
> struct clk *bus_clk;
> };
>
> +/*
> + * if DMA addr spans 128MB boundary, we split the DMA transfer into two
> + * so that the DMA transfer doesn't exceed the boundary.
> + */
> +static unsigned int dwcmshc_adma_write_desc(struct sdhci_host *host,
> + void *desc, dma_addr_t addr,
> + int len, unsigned int cmd)
> +{
> + int tmplen, offset;
> +
> + if (BOUNDARY_OK(addr, len))
> + return _sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +
> + offset = addr & (SZ_128M - 1);
> + tmplen = SZ_128M - offset;
> + _sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
> +
> + addr += tmplen;
> + len -= tmplen;
> + desc += host->desc_sz;
> + _sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +
> + return host->desc_sz * 2;
> +}
> +
> static const struct sdhci_ops sdhci_dwcmshc_ops = {
> .set_clock = sdhci_set_clock,
> .set_bus_width = sdhci_set_bus_width,
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> .reset = sdhci_reset,
> + .adma_write_desc= dwcmshc_adma_write_desc,
> };
>
> static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
> @@ -36,12 +65,24 @@ static int dwcmshc_probe(struct platform_device *pdev)
> struct sdhci_host *host;
> struct dwcmshc_priv *priv;
> int err;
> + u32 extra;
>
> host = sdhci_pltfm_init(pdev, _dwcmshc_pdata,
> sizeof(struct dwcmshc_priv));
> if (IS_ERR(host))
> return PTR_ERR(host);
>
> + /*
> + * The DMA descriptor table number is calculated as the maximum
> + * number of segments times 2, to allow for an alignment
> + * descriptor for each segment, plus 1 for a nop end descriptor,
> + * plus extra number for cross 128M boundary handling.
> + */
> + extra = totalram_pages / (SZ_128M / PAGE_SIZE) + 1;
will use DIV_ROUND_UP instead.
> + if (extra > SDHCI_MAX_SEGS)
> + extra = SDHCI_MAX_SEGS;
> + host->adma_table_num = SDHCI_MAX_SEGS * 2 + 1 + extra;
> +
> pltfm_host = sdhci_priv(host);
> priv = sdhci_pltfm_priv(pltfm_host);
>
Re: [PATCH 06/11] sched/irq: add irq utilization tracking
Hi Vincent,
On Fri, 29 Jun 2018 at 03:07, Vincent Guittot
wrote:
>
> interrupt and steal time are the only remaining activities tracked by
> rt_avg. Like for sched classes, we can use PELT to track their average
> utilization of the CPU. But unlike sched class, we don't track when
> entering/leaving interrupt; Instead, we take into account the time spent
> under interrupt context when we update rqs' clock (rq_clock_task).
> This also means that we have to decay the normal context time and account
> for interrupt time during the update.
>
> That's also important to note that because
> rq_clock == rq_clock_task + interrupt time
> and rq_clock_task is used by a sched class to compute its utilization, the
> util_avg of a sched class only reflects the utilization of the time spent
> in normal context and not of the whole time of the CPU. The utilization of
> interrupt gives an more accurate level of utilization of CPU.
> The CPU utilization is :
> avg_irq + (1 - avg_irq / max capacity) * /Sum avg_rq
>
> Most of the time, avg_irq is small and neglictible so the use of the
> approximation CPU utilization = /Sum avg_rq was enough
>
> Cc: Ingo Molnar
> Cc: Peter Zijlstra
> Signed-off-by: Vincent Guittot
> ---
> kernel/sched/core.c | 4 +++-
> kernel/sched/fair.c | 13 ++---
> kernel/sched/pelt.c | 40
> kernel/sched/pelt.h | 16
> kernel/sched/sched.h | 3 +++
> 5 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 78d8fac..e5263a4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -18,6 +18,8 @@
> #include "../workqueue_internal.h"
> #include "../smpboot.h"
>
> +#include "pelt.h"
> +
> #define CREATE_TRACE_POINTS
> #include
>
> @@ -186,7 +188,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
>
> #if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> if ((irq_delta + steal) && sched_feat(NONTASK_CAPACITY))
> - sched_rt_avg_update(rq, irq_delta + steal);
> + update_irq_load_avg(rq, irq_delta + steal);
I think we should not add steal time into irq load tracking, steal
time is always 0 on native kernel which doesn't matter, what will
happen when guest disables IRQ_TIME_ACCOUNTING and enables
PARAVIRT_TIME_ACCOUNTING? Steal time is not the real irq util_avg. In
addition, we haven't exposed power management for performance which
means that e.g. schedutil governor can not cooperate with passive mode
intel_pstate driver to tune the OPP. To decay the old steal time avg
and add the new one just wastes cpu cycles.
Regards,
Wanpeng Li
> #endif
> }
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ffce4b2..d2758e3 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7289,7 +7289,7 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq
> *cfs_rq)
> return false;
> }
>
> -static inline bool others_rqs_have_blocked(struct rq *rq)
> +static inline bool others_have_blocked(struct rq *rq)
> {
> if (READ_ONCE(rq->avg_rt.util_avg))
> return true;
> @@ -7297,6 +7297,11 @@ static inline bool others_rqs_have_blocked(struct rq
> *rq)
> if (READ_ONCE(rq->avg_dl.util_avg))
> return true;
>
> +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> + if (READ_ONCE(rq->avg_irq.util_avg))
> + return true;
> +#endif
> +
> return false;
> }
>
> @@ -7361,8 +7366,9 @@ static void update_blocked_averages(int cpu)
> }
> update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
> update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
> + update_irq_load_avg(rq, 0);
> /* Don't need periodic decay once load/util_avg are null */
> - if (others_rqs_have_blocked(rq))
> + if (others_have_blocked(rq))
> done = false;
>
> #ifdef CONFIG_NO_HZ_COMMON
> @@ -7431,9 +7437,10 @@ static inline void update_blocked_averages(int cpu)
> update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq);
> update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
> update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
> + update_irq_load_avg(rq, 0);
> #ifdef CONFIG_NO_HZ_COMMON
> rq->last_blocked_load_update_tick = jiffies;
> - if (!cfs_rq_has_blocked(cfs_rq) && !others_rqs_have_blocked(rq))
> + if (!cfs_rq_has_blocked(cfs_rq) && !others_have_blocked(rq))
> rq->has_blocked_load = 0;
> #endif
> rq_unlock_irqrestore(rq, );
> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
> index 8b78b63..ead6d8b 100644
> --- a/kernel/sched/pelt.c
> +++ b/kernel/sched/pelt.c
> @@ -357,3 +357,43 @@ int update_dl_rq_load_avg(u64 now, struct rq *rq, int
> running)
>
> return 0;
> }
> +
> +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
>
Re: [PATCH 06/11] sched/irq: add irq utilization tracking
Hi Vincent,
On Fri, 29 Jun 2018 at 03:07, Vincent Guittot
wrote:
>
> interrupt and steal time are the only remaining activities tracked by
> rt_avg. Like for sched classes, we can use PELT to track their average
> utilization of the CPU. But unlike sched class, we don't track when
> entering/leaving interrupt; Instead, we take into account the time spent
> under interrupt context when we update rqs' clock (rq_clock_task).
> This also means that we have to decay the normal context time and account
> for interrupt time during the update.
>
> That's also important to note that because
> rq_clock == rq_clock_task + interrupt time
> and rq_clock_task is used by a sched class to compute its utilization, the
> util_avg of a sched class only reflects the utilization of the time spent
> in normal context and not of the whole time of the CPU. The utilization of
> interrupt gives an more accurate level of utilization of CPU.
> The CPU utilization is :
> avg_irq + (1 - avg_irq / max capacity) * /Sum avg_rq
>
> Most of the time, avg_irq is small and neglictible so the use of the
> approximation CPU utilization = /Sum avg_rq was enough
>
> Cc: Ingo Molnar
> Cc: Peter Zijlstra
> Signed-off-by: Vincent Guittot
> ---
> kernel/sched/core.c | 4 +++-
> kernel/sched/fair.c | 13 ++---
> kernel/sched/pelt.c | 40
> kernel/sched/pelt.h | 16
> kernel/sched/sched.h | 3 +++
> 5 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 78d8fac..e5263a4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -18,6 +18,8 @@
> #include "../workqueue_internal.h"
> #include "../smpboot.h"
>
> +#include "pelt.h"
> +
> #define CREATE_TRACE_POINTS
> #include
>
> @@ -186,7 +188,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
>
> #if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> if ((irq_delta + steal) && sched_feat(NONTASK_CAPACITY))
> - sched_rt_avg_update(rq, irq_delta + steal);
> + update_irq_load_avg(rq, irq_delta + steal);
I think we should not add steal time into irq load tracking, steal
time is always 0 on native kernel which doesn't matter, what will
happen when guest disables IRQ_TIME_ACCOUNTING and enables
PARAVIRT_TIME_ACCOUNTING? Steal time is not the real irq util_avg. In
addition, we haven't exposed power management for performance which
means that e.g. schedutil governor can not cooperate with passive mode
intel_pstate driver to tune the OPP. To decay the old steal time avg
and add the new one just wastes cpu cycles.
Regards,
Wanpeng Li
> #endif
> }
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ffce4b2..d2758e3 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7289,7 +7289,7 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq
> *cfs_rq)
> return false;
> }
>
> -static inline bool others_rqs_have_blocked(struct rq *rq)
> +static inline bool others_have_blocked(struct rq *rq)
> {
> if (READ_ONCE(rq->avg_rt.util_avg))
> return true;
> @@ -7297,6 +7297,11 @@ static inline bool others_rqs_have_blocked(struct rq
> *rq)
> if (READ_ONCE(rq->avg_dl.util_avg))
> return true;
>
> +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> + if (READ_ONCE(rq->avg_irq.util_avg))
> + return true;
> +#endif
> +
> return false;
> }
>
> @@ -7361,8 +7366,9 @@ static void update_blocked_averages(int cpu)
> }
> update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
> update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
> + update_irq_load_avg(rq, 0);
> /* Don't need periodic decay once load/util_avg are null */
> - if (others_rqs_have_blocked(rq))
> + if (others_have_blocked(rq))
> done = false;
>
> #ifdef CONFIG_NO_HZ_COMMON
> @@ -7431,9 +7437,10 @@ static inline void update_blocked_averages(int cpu)
> update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq);
> update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
> update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
> + update_irq_load_avg(rq, 0);
> #ifdef CONFIG_NO_HZ_COMMON
> rq->last_blocked_load_update_tick = jiffies;
> - if (!cfs_rq_has_blocked(cfs_rq) && !others_rqs_have_blocked(rq))
> + if (!cfs_rq_has_blocked(cfs_rq) && !others_have_blocked(rq))
> rq->has_blocked_load = 0;
> #endif
> rq_unlock_irqrestore(rq, );
> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
> index 8b78b63..ead6d8b 100644
> --- a/kernel/sched/pelt.c
> +++ b/kernel/sched/pelt.c
> @@ -357,3 +357,43 @@ int update_dl_rq_load_avg(u64 now, struct rq *rq, int
> running)
>
> return 0;
> }
> +
> +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
>
Re: linux-next: build warning after merge of the rdma tree
On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the rdma tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
>
> net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt':
> net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> if (bad_wr != first_wr)
> ^
Huh. I'm quite surprised 0-day build service didn't warn on this.
> Introduced by commit
>
> ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)() calls")
>
> This is an actual problem.
Yes, for sure. Bart?
Thanks,
Jason
Re: linux-next: build warning after merge of the rdma tree
On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the rdma tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
>
> net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt':
> net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> if (bad_wr != first_wr)
> ^
Huh. I'm quite surprised 0-day build service didn't warn on this.
> Introduced by commit
>
> ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)() calls")
>
> This is an actual problem.
Yes, for sure. Bart?
Thanks,
Jason
[PATCH] checkpatch: Warn when a patch doesn't have a description
Potential patches should have a commit description.
Emit a warning when there isn't one.
Suggested-by: Prakruthi Deepak Heragu
Signed-off-by: Joe Perches
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
[PATCH] checkpatch: Warn when a patch doesn't have a description
Potential patches should have a commit description.
Emit a warning when there isn't one.
Suggested-by: Prakruthi Deepak Heragu
Signed-off-by: Joe Perches
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
linux-next: build warning after merge of the sound-asoc tree
Hi all,
After merging the sound-asoc tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used
uninitialized in this function [-Wmaybe-uninitialized]
dev_err(>dev, "Failed to register regulator: %d\n",
^
ret);
Introduced by commit
7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and
ADAU7002")
This is a real bug.
--
Cheers,
Stephen Rothwell
pgpVMAoECqoIX.pgp
Description: OpenPGP digital signature
linux-next: build warning after merge of the sound-asoc tree
Hi all,
After merging the sound-asoc tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe':
sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used
uninitialized in this function [-Wmaybe-uninitialized]
dev_err(>dev, "Failed to register regulator: %d\n",
^
ret);
Introduced by commit
7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and
ADAU7002")
This is a real bug.
--
Cheers,
Stephen Rothwell
pgpVMAoECqoIX.pgp
Description: OpenPGP digital signature
Re: [PATCH 4.4 15/47] ubi: fastmap: Correctly handle interrupted erasures in EBA
On Tue, 2018-07-10 at 20:24 +0200, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Richard Weinberger > > commit 781932375ffc6411713ee0926ccae8596ed0261c upstream. > > Fastmap cannot track the LEB unmap operation, therefore it can > happen that after an interrupted erasure the mapping still looks > good from Fastmap's point of view, while reading from the PEB will > cause an ECC error and confuses the upper layer. > > Instead of teaching users of UBI how to deal with that, we read back > the VID header and check for errors. If the PEB is empty or shows ECC > errors we fixup the mapping and schedule the PEB for erasure. [...] Isn't there a risk here, that a read error leads to erasing data that might be recoverable if the read is retried? Ben. -- Ben Hutchings, Software Developer Codethink Ltd https://www.codethink.co.uk/ Dale House, 35 Dale Street Manchester, M1 2HF, United Kingdom
Re: [PATCH 4.4 15/47] ubi: fastmap: Correctly handle interrupted erasures in EBA
On Tue, 2018-07-10 at 20:24 +0200, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Richard Weinberger > > commit 781932375ffc6411713ee0926ccae8596ed0261c upstream. > > Fastmap cannot track the LEB unmap operation, therefore it can > happen that after an interrupted erasure the mapping still looks > good from Fastmap's point of view, while reading from the PEB will > cause an ECC error and confuses the upper layer. > > Instead of teaching users of UBI how to deal with that, we read back > the VID header and check for errors. If the PEB is empty or shows ECC > errors we fixup the mapping and schedule the PEB for erasure. [...] Isn't there a risk here, that a read error leads to erasing data that might be recoverable if the read is retried? Ben. -- Ben Hutchings, Software Developer Codethink Ltd https://www.codethink.co.uk/ Dale House, 35 Dale Street Manchester, M1 2HF, United Kingdom
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
Hi Tino, On Wed, Jul 25, 2018 at 05:12:13PM +0200, Tino Lehnig wrote: > Hi, > > On 07/25/2018 03:21 PM, Minchan Kim wrote: > > It would be much helpful if you could check more versions with git-bisect. > > I started bisecting today, but my results are not conclusive yet. It is > certain that the problem started with 4.15 though. I have not encountered A thing I could imagine is [0bcac06f27d75, skip swapcache for swapin of synchronous device] It was merged into v4.15. Could you check it by bisecting? With enabling writeback feature of zram, it's no more synchonous device so it could affect the unresponsive symptom you are seeing now. I should disable synchronous flag when the zram works with writeback feature. However, I should still see why the PG_uptodate WARN turning on. I guess there are some places where assume all of anonymous pages have PG_swapbacked and !PG_dirty are in swapcache so reference accounting could be corrupted. > the bug message in 4.15-rc1 so far, but the kvm processes always became > unresponsive after hitting swap and could not be killed there. I saw the > same behavior in rc2, rc3, and other builds in between, but the bad state > bug would only trigger occasionally there. The behavior in 4.15.18 is the > same as in newer kernels. > > > I also want to reproduce it. > > > > Today, I downloaded one window iso and execute it as cdrom with my owned > > compiled kernel on KVM but I couldn't reproduce. > > I also tested some heavy swap workload(kernel build with multiple CPU > > on small memory) but I failed to reproduce, too. > > > > Please could you told me your method more detail? > > I found that running Windows in KVM really is the only reliable method, > maybe because the zero pages are easily compressible. There is actually not > a lot of disk utilization on the backing device when running this test. Hmm, my testing was creating a tons of disk IO on backing device. Maybe, that's why I couldn't reproduce a lot. zero pages could never go to the writeback into the device so I don't understand how such kinds of testing reprodcue the problem. Anyway, I will try it again with zero pages with a few of random pages. > > My operating system is a minimal install of Debian 9. I took the kernel > configuration from the default Debian kernel and built my own kernel with > "make oldconfig" leaving all settings at their defaults. The only thing I > changed in the configuration was enabling the zram writeback feature. You mean you changed host kernel configuration? > > All my tests were done on bare-metal hardware with Xeon processors and lots > of RAM. I encounter the bug quite quickly, but it still takes several GBs of > swap usage. Below is my /proc/meminfo with enough KVM instances running (3 > in my case) to trigger the bug on my test machine. Aha.. you did writeback feature into your bare-metal host machine and execute kvm with window images as a guest. So, PG_uptodate warning happens on host side, not guest? Right? Thanks! > > I will also try to reproduce the problem on some different hardware next. > > -- > > MemTotal: 264033384 kB > MemFree: 1232968 kB > MemAvailable: 0 kB > Buffers:1152 kB > Cached: 5036 kB > SwapCached:49200 kB > Active: 249955744 kB > Inactive:5096148 kB > Active(anon): 249953396 kB > Inactive(anon): 5093084 kB > Active(file): 2348 kB > Inactive(file): 3064 kB > Unevictable: 0 kB > Mlocked: 0 kB > SwapTotal: 1073741820 kB > SwapFree: 938603260 kB > Dirty:68 kB > Writeback: 0 kB > AnonPages: 255007752 kB > Mapped: 4708 kB > Shmem: 1212 kB > Slab: 88500 kB > SReclaimable: 16096 kB > SUnreclaim:72404 kB > KernelStack:5040 kB > PageTables: 765560 kB > NFS_Unstable: 0 kB > Bounce:0 kB > WritebackTmp: 0 kB > CommitLimit:1205758512 kB > Committed_AS: 403586176 kB > VmallocTotal: 34359738367 kB > VmallocUsed: 0 kB > VmallocChunk: 0 kB > HardwareCorrupted: 0 kB > AnonHugePages: 254799872 kB > ShmemHugePages:0 kB > ShmemPmdMapped:0 kB > HugePages_Total: 0 > HugePages_Free:0 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > DirectMap4k: 75136 kB > DirectMap2M:10295296 kB > DirectMap1G:260046848 kB > > -- > Kind regards, > > Tino Lehnig
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
Hi Tino, On Wed, Jul 25, 2018 at 05:12:13PM +0200, Tino Lehnig wrote: > Hi, > > On 07/25/2018 03:21 PM, Minchan Kim wrote: > > It would be much helpful if you could check more versions with git-bisect. > > I started bisecting today, but my results are not conclusive yet. It is > certain that the problem started with 4.15 though. I have not encountered A thing I could imagine is [0bcac06f27d75, skip swapcache for swapin of synchronous device] It was merged into v4.15. Could you check it by bisecting? With enabling writeback feature of zram, it's no more synchonous device so it could affect the unresponsive symptom you are seeing now. I should disable synchronous flag when the zram works with writeback feature. However, I should still see why the PG_uptodate WARN turning on. I guess there are some places where assume all of anonymous pages have PG_swapbacked and !PG_dirty are in swapcache so reference accounting could be corrupted. > the bug message in 4.15-rc1 so far, but the kvm processes always became > unresponsive after hitting swap and could not be killed there. I saw the > same behavior in rc2, rc3, and other builds in between, but the bad state > bug would only trigger occasionally there. The behavior in 4.15.18 is the > same as in newer kernels. > > > I also want to reproduce it. > > > > Today, I downloaded one window iso and execute it as cdrom with my owned > > compiled kernel on KVM but I couldn't reproduce. > > I also tested some heavy swap workload(kernel build with multiple CPU > > on small memory) but I failed to reproduce, too. > > > > Please could you told me your method more detail? > > I found that running Windows in KVM really is the only reliable method, > maybe because the zero pages are easily compressible. There is actually not > a lot of disk utilization on the backing device when running this test. Hmm, my testing was creating a tons of disk IO on backing device. Maybe, that's why I couldn't reproduce a lot. zero pages could never go to the writeback into the device so I don't understand how such kinds of testing reprodcue the problem. Anyway, I will try it again with zero pages with a few of random pages. > > My operating system is a minimal install of Debian 9. I took the kernel > configuration from the default Debian kernel and built my own kernel with > "make oldconfig" leaving all settings at their defaults. The only thing I > changed in the configuration was enabling the zram writeback feature. You mean you changed host kernel configuration? > > All my tests were done on bare-metal hardware with Xeon processors and lots > of RAM. I encounter the bug quite quickly, but it still takes several GBs of > swap usage. Below is my /proc/meminfo with enough KVM instances running (3 > in my case) to trigger the bug on my test machine. Aha.. you did writeback feature into your bare-metal host machine and execute kvm with window images as a guest. So, PG_uptodate warning happens on host side, not guest? Right? Thanks! > > I will also try to reproduce the problem on some different hardware next. > > -- > > MemTotal: 264033384 kB > MemFree: 1232968 kB > MemAvailable: 0 kB > Buffers:1152 kB > Cached: 5036 kB > SwapCached:49200 kB > Active: 249955744 kB > Inactive:5096148 kB > Active(anon): 249953396 kB > Inactive(anon): 5093084 kB > Active(file): 2348 kB > Inactive(file): 3064 kB > Unevictable: 0 kB > Mlocked: 0 kB > SwapTotal: 1073741820 kB > SwapFree: 938603260 kB > Dirty:68 kB > Writeback: 0 kB > AnonPages: 255007752 kB > Mapped: 4708 kB > Shmem: 1212 kB > Slab: 88500 kB > SReclaimable: 16096 kB > SUnreclaim:72404 kB > KernelStack:5040 kB > PageTables: 765560 kB > NFS_Unstable: 0 kB > Bounce:0 kB > WritebackTmp: 0 kB > CommitLimit:1205758512 kB > Committed_AS: 403586176 kB > VmallocTotal: 34359738367 kB > VmallocUsed: 0 kB > VmallocChunk: 0 kB > HardwareCorrupted: 0 kB > AnonHugePages: 254799872 kB > ShmemHugePages:0 kB > ShmemPmdMapped:0 kB > HugePages_Total: 0 > HugePages_Free:0 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > DirectMap4k: 75136 kB > DirectMap2M:10295296 kB > DirectMap1G:260046848 kB > > -- > Kind regards, > > Tino Lehnig
RE: [PATCH V4 0/9] clk: add imx7ulp clk support
Hi Stephen, Do you have a chance to look at it? This patch series has been pending for quite a long time without much comments. Regards Dong Aisheng > -Original Message- > From: A.s. Dong > Sent: Wednesday, July 18, 2018 9:37 PM > To: linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > sb...@kernel.org; mturque...@baylibre.com; shawn...@kernel.org; > Anson Huang ; Jacky Bai ; dl- > linux-imx ; A.s. Dong > Subject: [PATCH V4 0/9] clk: add imx7ulp clk support > > This is a rebased version of below patch series against latest clk tree. > [PATCH RESEND V3 0/9] clk: add imx7ulp clk support > https://lkml.org/lkml/2018/3/16/310 > > This patch series intends to add imx7ulp clk support. > > i.MX7ULP Clock functions are under joint control of the System Clock > Generation (SCG) modules, Peripheral Clock Control (PCC) modules, and > Core Mode Controller (CMC)1 blocks > > The clocking scheme provides clear separation between M4 domain and A7 > domain. Except for a few clock sources shared between two domains, such > as the System Oscillator clock, the Slow IRC (SIRC), and and the Fast IRC > clock > (FIRCLK), clock sources and clock management are separated and contained > within each domain. > > M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules. > A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules. > > Note: this series only adds A7 clock domain support as M4 clock domain will > be handled by M4 seperately. > > Change Log: > v3->v4: > * rebased to latest kernel > * make scg and pcc separate nodes according to Rob's suggestion > > v2->v3: > * Patch 1 changed on: 1) split normal and gate ops 2) fix the possible racy >Others no changes. > > v1->v2: > * add enable/disable for the type of CLK_DIVIDER_ZERO_GATE dividers > * use clk_hw apis to register clocks > * use of_clk_add_hw_provider > * split the clocks register process into two parts: early part for possible >timers clocks registered by CLK_OF_DECLARE_DRIVER and the later part for >the left normal peripheral clocks registered by a platform driver. > > Dong Aisheng (9): > clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support > clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support > clk: imx: add pllv4 support > clk: imx: add pfdv2 support > clk: imx: add composite clk support > dt-bindings: clock: add imx7ulp clock binding doc > clk: imx: make mux parent strings const > clk: imx: implement new clk_hw based APIs > clk: imx: add imx7ulp clk driver > > .../devicetree/bindings/clock/imx7ulp-clock.txt| 87 + > drivers/clk/clk-divider.c | 152 +++ > drivers/clk/clk-fractional-divider.c | 10 + > drivers/clk/imx/Makefile | 6 +- > drivers/clk/imx/clk-busy.c | 2 +- > drivers/clk/imx/clk-composite.c| 85 + > drivers/clk/imx/clk-fixup-mux.c| 2 +- > drivers/clk/imx/clk-imx7ulp.c | 209 > + > drivers/clk/imx/clk-pfdv2.c| 201 > drivers/clk/imx/clk-pllv4.c| 182 ++ > drivers/clk/imx/clk.c | 22 +++ > drivers/clk/imx/clk.h | 92 - > include/dt-bindings/clock/imx7ulp-clock.h | 109 +++ > include/linux/clk-provider.h | 17 ++ > 14 files changed, 1166 insertions(+), 10 deletions(-) create mode 100644 > Documentation/devicetree/bindings/clock/imx7ulp-clock.txt > create mode 100644 drivers/clk/imx/clk-composite.c create mode 100644 > drivers/clk/imx/clk-imx7ulp.c create mode 100644 drivers/clk/imx/clk- > pfdv2.c create mode 100644 drivers/clk/imx/clk-pllv4.c create mode 100644 > include/dt-bindings/clock/imx7ulp-clock.h > > -- > 2.7.4
RE: [PATCH V4 0/9] clk: add imx7ulp clk support
Hi Stephen, Do you have a chance to look at it? This patch series has been pending for quite a long time without much comments. Regards Dong Aisheng > -Original Message- > From: A.s. Dong > Sent: Wednesday, July 18, 2018 9:37 PM > To: linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > sb...@kernel.org; mturque...@baylibre.com; shawn...@kernel.org; > Anson Huang ; Jacky Bai ; dl- > linux-imx ; A.s. Dong > Subject: [PATCH V4 0/9] clk: add imx7ulp clk support > > This is a rebased version of below patch series against latest clk tree. > [PATCH RESEND V3 0/9] clk: add imx7ulp clk support > https://lkml.org/lkml/2018/3/16/310 > > This patch series intends to add imx7ulp clk support. > > i.MX7ULP Clock functions are under joint control of the System Clock > Generation (SCG) modules, Peripheral Clock Control (PCC) modules, and > Core Mode Controller (CMC)1 blocks > > The clocking scheme provides clear separation between M4 domain and A7 > domain. Except for a few clock sources shared between two domains, such > as the System Oscillator clock, the Slow IRC (SIRC), and and the Fast IRC > clock > (FIRCLK), clock sources and clock management are separated and contained > within each domain. > > M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules. > A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules. > > Note: this series only adds A7 clock domain support as M4 clock domain will > be handled by M4 seperately. > > Change Log: > v3->v4: > * rebased to latest kernel > * make scg and pcc separate nodes according to Rob's suggestion > > v2->v3: > * Patch 1 changed on: 1) split normal and gate ops 2) fix the possible racy >Others no changes. > > v1->v2: > * add enable/disable for the type of CLK_DIVIDER_ZERO_GATE dividers > * use clk_hw apis to register clocks > * use of_clk_add_hw_provider > * split the clocks register process into two parts: early part for possible >timers clocks registered by CLK_OF_DECLARE_DRIVER and the later part for >the left normal peripheral clocks registered by a platform driver. > > Dong Aisheng (9): > clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support > clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support > clk: imx: add pllv4 support > clk: imx: add pfdv2 support > clk: imx: add composite clk support > dt-bindings: clock: add imx7ulp clock binding doc > clk: imx: make mux parent strings const > clk: imx: implement new clk_hw based APIs > clk: imx: add imx7ulp clk driver > > .../devicetree/bindings/clock/imx7ulp-clock.txt| 87 + > drivers/clk/clk-divider.c | 152 +++ > drivers/clk/clk-fractional-divider.c | 10 + > drivers/clk/imx/Makefile | 6 +- > drivers/clk/imx/clk-busy.c | 2 +- > drivers/clk/imx/clk-composite.c| 85 + > drivers/clk/imx/clk-fixup-mux.c| 2 +- > drivers/clk/imx/clk-imx7ulp.c | 209 > + > drivers/clk/imx/clk-pfdv2.c| 201 > drivers/clk/imx/clk-pllv4.c| 182 ++ > drivers/clk/imx/clk.c | 22 +++ > drivers/clk/imx/clk.h | 92 - > include/dt-bindings/clock/imx7ulp-clock.h | 109 +++ > include/linux/clk-provider.h | 17 ++ > 14 files changed, 1166 insertions(+), 10 deletions(-) create mode 100644 > Documentation/devicetree/bindings/clock/imx7ulp-clock.txt > create mode 100644 drivers/clk/imx/clk-composite.c create mode 100644 > drivers/clk/imx/clk-imx7ulp.c create mode 100644 drivers/clk/imx/clk- > pfdv2.c create mode 100644 drivers/clk/imx/clk-pllv4.c create mode 100644 > include/dt-bindings/clock/imx7ulp-clock.h > > -- > 2.7.4
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > This patch adds registration of the system memory with memblock, eliminates > bootmem initialization and converts early memory reservations from bootmem > to memblock. > > Signed-off-by: Mike Rapoport > --- > v2: fix calculation of the reserved memory size > > arch/hexagon/Kconfig | 3 +++ > arch/hexagon/mm/init.c | 20 > 2 files changed, 11 insertions(+), 12 deletions(-) > Looks good, I can take this through my tree. Acked-by: Richard Kuo -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > This patch adds registration of the system memory with memblock, eliminates > bootmem initialization and converts early memory reservations from bootmem > to memblock. > > Signed-off-by: Mike Rapoport > --- > v2: fix calculation of the reserved memory size > > arch/hexagon/Kconfig | 3 +++ > arch/hexagon/mm/init.c | 20 > 2 files changed, 11 insertions(+), 12 deletions(-) > Looks good, I can take this through my tree. Acked-by: Richard Kuo -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v1] checkpatch: check for #if 0/#if 1
On Wed, 2018-07-25 at 16:14 -0700, Prakruthi Deepak Heragu wrote:
> The #if 0 or #if 1 is used to toggle features. Warn if #if 0 or #if 1
> is present and suggest that they can be removed.
>
> Signed-off-by: Abhijeet Dharmapurikar
> Signed-off-by: Prakruthi Deepak Heragu
> ---
> Changes in v1:
> - Rephrase the warning message to fit in a single line without
> 80 column limit
>
> scripts/checkpatch.pl | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3394ed8..72513c2 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5383,9 +5383,14 @@ sub process {
>
> # warn about #if 0
> if ($line =~ /^.\s*\#\s*if\s+0\b/) {
> - CHK("REDUNDANT_CODE",
> - "if this code is redundant consider removing it\n" .
> - $herecurr);
> + WARN("IF_0",
> + "Consider removing the #if 0 and its #endif\n".
> $herecurr);
No, this wording is not correct at all.
The entire block, i.e.: 'this code' should be considered
to be removed, not just the #if 0 and its #endif.
For #if 1 code, then just the #if 1 and the #endif should
be removed.
> + }
> +
> +# warn about #if 1
> + if ($line =~ /^.\s*\#\s*if\s+1\b/) {
> + WARN("IF_1",
> + "Consider removing the #if 1 and its #endif\n".
> $herecurr);
> }
>
> # check for needless "if () fn()" uses
Re: [PATCH v1] checkpatch: check for #if 0/#if 1
On Wed, 2018-07-25 at 16:14 -0700, Prakruthi Deepak Heragu wrote:
> The #if 0 or #if 1 is used to toggle features. Warn if #if 0 or #if 1
> is present and suggest that they can be removed.
>
> Signed-off-by: Abhijeet Dharmapurikar
> Signed-off-by: Prakruthi Deepak Heragu
> ---
> Changes in v1:
> - Rephrase the warning message to fit in a single line without
> 80 column limit
>
> scripts/checkpatch.pl | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3394ed8..72513c2 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5383,9 +5383,14 @@ sub process {
>
> # warn about #if 0
> if ($line =~ /^.\s*\#\s*if\s+0\b/) {
> - CHK("REDUNDANT_CODE",
> - "if this code is redundant consider removing it\n" .
> - $herecurr);
> + WARN("IF_0",
> + "Consider removing the #if 0 and its #endif\n".
> $herecurr);
No, this wording is not correct at all.
The entire block, i.e.: 'this code' should be considered
to be removed, not just the #if 0 and its #endif.
For #if 1 code, then just the #if 1 and the #endif should
be removed.
> + }
> +
> +# warn about #if 1
> + if ($line =~ /^.\s*\#\s*if\s+1\b/) {
> + WARN("IF_1",
> + "Consider removing the #if 1 and its #endif\n".
> $herecurr);
> }
>
> # check for needless "if () fn()" uses
Re: [RFC][PATCH 0/5] Mount, Filesystem and Keyrings notifications
On Wed, 2018-07-25 at 08:48 -0700, Casey Schaufler wrote: > On 7/24/2018 10:39 PM, Ian Kent wrote: > > On Tue, 2018-07-24 at 11:57 -0700, Casey Schaufler wrote: > > > On 7/24/2018 9:00 AM, David Howells wrote: > > > > Casey Schaufler wrote: > > > > > > > > > > (1) Mount topology and reconfiguration change events. > > > > > > > > > > With the possibility of unprivileged mounting you're going to have to > > > > > address access control on events. If root in a user namespace mounts > > > > > a > > > > > filesystem you may have a case where the "real" user wouldn't want the > > > > > listener to receive a notification. > > > > > > > > Can you clarify who the listener is in this case? > > > > > > That would be anyone with a watchpoint set. > > > > And that process would have had the privilege to do so ... > > Which isn't the point. The access control isn't on the watchpoint, > it's on delivering the event to the watchpoint. The access control > needs to be based on the process that created the event and the > process receiving the event. > > > > > > > Note that mount topology events don't leak outside of the mount > > > > namespace > > > > they're generated in. > > > > > > > > That said, if you, a random user, put a watchpoint on "/" you can see > > > > the > > > > mount events triggered by another random user in the same mount > > > > namespace. I > > > > don't see a way to control this except by resorting to the LSM since > > > > UNIX > > > > doesn't have 'notify' permission bits. > > > > > > I would call that a write operation from the process that triggered > > > the watchpoint to the one watching it. Like a signal. Signals have a > > > rudimentary DAC policy (write only to the same UID) that could be > > > your model. > > > > I'm not sure signals are a good comparison. > > In both cases you have one process sending information > to another. If you use killpg() instead of kill() you can > send to a number of processes without knowing them individually. > A process can chose what to do with (most) signals, including > ignore them. I'm just saying that the analogy isn't quite the same. In the this case notifications can't be just sent by a process, it's entirely controlled by the VFS so the notion of some process doing something out-off-band doesn't apply. > > > They can affect a process in significant ways whereas triggering > > a notification is less invasive so the security requirements > > should take that into consideration. > > I'm looking at this from a security model viewpoint. If process > A sends information to process B that's a write operation with > A as the subject and B as the object. Perhaps, but again there's no process A deciding to send something. It's the VFS saying to itself, I see this thing has changed, someone that had appropriate privilege asked me to tell it so ... I may be wrong but there isn't a similar access control mechanism on the proc file system mount table (pseudo) files, only the usual access control on opening them and what is seen is based on the mount name space of the opening process. This is also not quite as what we have here but it is similar. > > > But there is a problem here I think. > > > > How about the case where a user name space is created or entered > > without a newly created mount name space and mounts and umounts > > are done, the user name space necessarily expects the table of > > mounts it sees to be up to date. > > > > But, if the methods here are used by user space, say libmount > > was updated to use it, to gain the efficiency of not constantly > > re-reading the proc mount table then restrictions on notifications > > would mean the mount table seen in the user name space might not > > be updated and would no longer be correct. > > Hey, I'm not the one saying that containers don't need/want kernel > manifestation. Of course you may have troubles with access consistency > if you go around changing the access control attributes with namespaces. > One of the problems with signals is that you can't chmod() your process > to allow signals from other specified users. If you don't design an > access control scheme into the watchpoint mechanism you aren't going to > be able to deal with situations like this. Don't get me wrong, I'm not criticizing your suggestion that some sort of security model is needed. I'm saying it's not straight forward to work out what's actually needed and, in thinking about it, I ended up described an additional potential problem that's not even (necessarily) security related. > > > The converse is more interesting, where the user name space does > > create or enter a new mount name space, then libmount would see ???, > > probably not the updated mount information ... unless it opens a > > new file handle to get mount update information ... a long running > > daemon that uses libmount and dispenses or uses mount information > > would very likely have a problem ... > > > > The current proc file
Re: [RFC][PATCH 0/5] Mount, Filesystem and Keyrings notifications
On Wed, 2018-07-25 at 08:48 -0700, Casey Schaufler wrote: > On 7/24/2018 10:39 PM, Ian Kent wrote: > > On Tue, 2018-07-24 at 11:57 -0700, Casey Schaufler wrote: > > > On 7/24/2018 9:00 AM, David Howells wrote: > > > > Casey Schaufler wrote: > > > > > > > > > > (1) Mount topology and reconfiguration change events. > > > > > > > > > > With the possibility of unprivileged mounting you're going to have to > > > > > address access control on events. If root in a user namespace mounts > > > > > a > > > > > filesystem you may have a case where the "real" user wouldn't want the > > > > > listener to receive a notification. > > > > > > > > Can you clarify who the listener is in this case? > > > > > > That would be anyone with a watchpoint set. > > > > And that process would have had the privilege to do so ... > > Which isn't the point. The access control isn't on the watchpoint, > it's on delivering the event to the watchpoint. The access control > needs to be based on the process that created the event and the > process receiving the event. > > > > > > > Note that mount topology events don't leak outside of the mount > > > > namespace > > > > they're generated in. > > > > > > > > That said, if you, a random user, put a watchpoint on "/" you can see > > > > the > > > > mount events triggered by another random user in the same mount > > > > namespace. I > > > > don't see a way to control this except by resorting to the LSM since > > > > UNIX > > > > doesn't have 'notify' permission bits. > > > > > > I would call that a write operation from the process that triggered > > > the watchpoint to the one watching it. Like a signal. Signals have a > > > rudimentary DAC policy (write only to the same UID) that could be > > > your model. > > > > I'm not sure signals are a good comparison. > > In both cases you have one process sending information > to another. If you use killpg() instead of kill() you can > send to a number of processes without knowing them individually. > A process can chose what to do with (most) signals, including > ignore them. I'm just saying that the analogy isn't quite the same. In the this case notifications can't be just sent by a process, it's entirely controlled by the VFS so the notion of some process doing something out-off-band doesn't apply. > > > They can affect a process in significant ways whereas triggering > > a notification is less invasive so the security requirements > > should take that into consideration. > > I'm looking at this from a security model viewpoint. If process > A sends information to process B that's a write operation with > A as the subject and B as the object. Perhaps, but again there's no process A deciding to send something. It's the VFS saying to itself, I see this thing has changed, someone that had appropriate privilege asked me to tell it so ... I may be wrong but there isn't a similar access control mechanism on the proc file system mount table (pseudo) files, only the usual access control on opening them and what is seen is based on the mount name space of the opening process. This is also not quite as what we have here but it is similar. > > > But there is a problem here I think. > > > > How about the case where a user name space is created or entered > > without a newly created mount name space and mounts and umounts > > are done, the user name space necessarily expects the table of > > mounts it sees to be up to date. > > > > But, if the methods here are used by user space, say libmount > > was updated to use it, to gain the efficiency of not constantly > > re-reading the proc mount table then restrictions on notifications > > would mean the mount table seen in the user name space might not > > be updated and would no longer be correct. > > Hey, I'm not the one saying that containers don't need/want kernel > manifestation. Of course you may have troubles with access consistency > if you go around changing the access control attributes with namespaces. > One of the problems with signals is that you can't chmod() your process > to allow signals from other specified users. If you don't design an > access control scheme into the watchpoint mechanism you aren't going to > be able to deal with situations like this. Don't get me wrong, I'm not criticizing your suggestion that some sort of security model is needed. I'm saying it's not straight forward to work out what's actually needed and, in thinking about it, I ended up described an additional potential problem that's not even (necessarily) security related. > > > The converse is more interesting, where the user name space does > > create or enter a new mount name space, then libmount would see ???, > > probably not the updated mount information ... unless it opens a > > new file handle to get mount update information ... a long running > > daemon that uses libmount and dispenses or uses mount information > > would very likely have a problem ... > > > > The current proc file
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 26 Jul 2018 09:41:06 +0900 Masami Hiramatsu wrote: > On Fri, 13 Jul 2018 08:18:03 -0400 > Steven Rostedt wrote: > > > On Fri, 13 Jul 2018 11:53:01 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > > Francis Deslauriers wrote: > > > > > > > From: Masami Hiramatsu > > > > > > > > Prohibit kprobe-events probing on notrace function. > > > > Since probing on the notrace function can cause recursive > > > > event call. In most case those are just skipped, but > > > > in some case it falls into infinite recursive call. > > > > > > BTW, I'm considering to add an option to allow putting > > > kprobes on notrace function - just for debugging > > > ftrace by kprobes. That is "developer only" option > > > so generally it should be disabled, but for debugging > > > the ftrace, we still need it. Or should I introduce > > > another kprobes module for debugging it? > > > > No, I think the former is better (to add an option to allow putting > > kprobes on notrace functions). By default we let people protect > > themselves. But if then provide a switch that lets you do things that > > might let you shoot yourself in the foot. > > I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows > kprobes on notrace function. I think we don't need to make it > online switchable, since it is only good for ftrace developers. > Works for me. Thanks! -- Steve
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 26 Jul 2018 09:41:06 +0900 Masami Hiramatsu wrote: > On Fri, 13 Jul 2018 08:18:03 -0400 > Steven Rostedt wrote: > > > On Fri, 13 Jul 2018 11:53:01 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > > Francis Deslauriers wrote: > > > > > > > From: Masami Hiramatsu > > > > > > > > Prohibit kprobe-events probing on notrace function. > > > > Since probing on the notrace function can cause recursive > > > > event call. In most case those are just skipped, but > > > > in some case it falls into infinite recursive call. > > > > > > BTW, I'm considering to add an option to allow putting > > > kprobes on notrace function - just for debugging > > > ftrace by kprobes. That is "developer only" option > > > so generally it should be disabled, but for debugging > > > the ftrace, we still need it. Or should I introduce > > > another kprobes module for debugging it? > > > > No, I think the former is better (to add an option to allow putting > > kprobes on notrace functions). By default we let people protect > > themselves. But if then provide a switch that lets you do things that > > might let you shoot yourself in the foot. > > I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows > kprobes on notrace function. I think we don't need to make it > online switchable, since it is only good for ftrace developers. > Works for me. Thanks! -- Steve
Re: [PATCH v5 1/3] thermal: qcom-spmi: Use PMIC thermal stage 2 for critical trip points
Hi Doug,
On Wed, Jul 25, 2018 at 04:19:56PM -0700, Doug Anderson wrote:
> On Tue, Jul 24, 2018 at 4:46 PM, Matthias Kaehlcke wrote:
> > +static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip,
> > +int temp)
> > +{
> > + u8 reg;
> > + bool disable_s2_shutdown = false;
> > + int ret;
> > +
> > + WARN_ON(!mutex_is_locked(>lock));
> > +
> > + /*
> > +* Default: S2 and S3 shutdown enabled, thresholds at
> > +* 105C/125C/145C, monitoring at 25Hz
> > +*/
> > + reg = SHUTDOWN_CTRL1_RATE_25HZ;
> > +
> > + if ((temp == THERMAL_TEMP_INVALID) ||
> > + (temp < STAGE2_THRESHOLD_MIN)) {
> > + chip->thresh = THRESH_MIN;
> > + goto skip;
> > + }
> > +
> > + if (temp <= STAGE2_THRESHOLD_MAX) {
> > + chip->thresh = THRESH_MAX -
> > + ((STAGE2_THRESHOLD_MAX - temp) /
> > +TEMP_THRESH_STEP);
> > + disable_s2_shutdown = true;
> > + } else {
> > + chip->thresh = THRESH_MAX;
> > +
> > + if (!IS_ERR(chip->adc))
> > + disable_s2_shutdown = true;
> > + else
> > + dev_warn(chip->dev,
> > +"No ADC is configured and critical
> > temperature is above the maximum stage 2 threshold of 140°C! Configuring
> > stage 2 shutdown at 140°C.\n");
>
> Putting a non-ASCII character (the degree symbol) in your commit
> message is one thing, but are you sure it's wise to put it in the
> kernel logs?
A few other drivers also do this
(drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c,
drivers/macintosh/windfarm_pm121.c), however that doesn't mean it's a
good idea. Will change to degC or C.
> > + }
> > +
> > +skip:
> > + reg |= chip->thresh;
> > + if (disable_s2_shutdown)
> > + reg |= SHUTDOWN_CTRL1_OVERRIDE_S2;
> > +
> > + ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ret;
>
> Simplify the above lines to:
>
> return qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
Ouch, my code is indeed dumb ...
> > @@ -313,12 +441,7 @@ static int qpnp_tm_probe(struct platform_device *pdev)
> > if (ret < 0)
> > return ret;
> >
> > - chip->tz_dev = devm_thermal_zone_of_sensor_register(>dev, 0,
> > chip,
> > -
> > _tm_sensor_ops);
> > - if (IS_ERR(chip->tz_dev)) {
> > - dev_err(>dev, "failed to register sensor\n");
> > - return PTR_ERR(chip->tz_dev);
> > - }
> > + chip->initialized = true;
>
> Should we add "thermal_zone_device_update(chip->tz_dev,
> THERMAL_EVENT_UNSPECIFIED);" here
Seems reasonable, will do.
> ...also: do we care about any type of locking for chip->initialized?
> Technically we can be running on weakly ordered memory so if
> qpnp_tm_update_temp_no_adc() is running on a different processor then
> possibly it could still keep returning the default temperature for a
> little while. We could try to analyze whether there's some sort of
> implicit barrier or we could add manual memory barriers, but generally
> I try to avoid that and just do the simple locking... What about just
> setting chip-Initialized = true at the end of qpnp_tm_init() while the
> mutex is still held?
Thanks for pointing that out. I agree that we should keep things
simple, chip->initialized to true at the end of qpnp_tm_init() sounds
good to me.
> I'd also love to hear from someone with more thermal framework
> experience to make sure it's legit to return a default value if
> someone calls us while we're initting. It seems sane to me but nice
> to confirm it's OK.
An alternative could be to return THERMAL_TEMP_INVALID, however I
don't see this handled outside of thermal_core.c, not sure if it could
throw some other code off.
Comments from thermal folks on either approach (or alternatives) are
definitely welcome :)
> Overall I like the idea of this patch so hopefully others do too.
> Thanks for sending it out!
Thanks for the review!
Matthias
Re: [PATCH v5 1/3] thermal: qcom-spmi: Use PMIC thermal stage 2 for critical trip points
Hi Doug,
On Wed, Jul 25, 2018 at 04:19:56PM -0700, Doug Anderson wrote:
> On Tue, Jul 24, 2018 at 4:46 PM, Matthias Kaehlcke wrote:
> > +static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip,
> > +int temp)
> > +{
> > + u8 reg;
> > + bool disable_s2_shutdown = false;
> > + int ret;
> > +
> > + WARN_ON(!mutex_is_locked(>lock));
> > +
> > + /*
> > +* Default: S2 and S3 shutdown enabled, thresholds at
> > +* 105C/125C/145C, monitoring at 25Hz
> > +*/
> > + reg = SHUTDOWN_CTRL1_RATE_25HZ;
> > +
> > + if ((temp == THERMAL_TEMP_INVALID) ||
> > + (temp < STAGE2_THRESHOLD_MIN)) {
> > + chip->thresh = THRESH_MIN;
> > + goto skip;
> > + }
> > +
> > + if (temp <= STAGE2_THRESHOLD_MAX) {
> > + chip->thresh = THRESH_MAX -
> > + ((STAGE2_THRESHOLD_MAX - temp) /
> > +TEMP_THRESH_STEP);
> > + disable_s2_shutdown = true;
> > + } else {
> > + chip->thresh = THRESH_MAX;
> > +
> > + if (!IS_ERR(chip->adc))
> > + disable_s2_shutdown = true;
> > + else
> > + dev_warn(chip->dev,
> > +"No ADC is configured and critical
> > temperature is above the maximum stage 2 threshold of 140°C! Configuring
> > stage 2 shutdown at 140°C.\n");
>
> Putting a non-ASCII character (the degree symbol) in your commit
> message is one thing, but are you sure it's wise to put it in the
> kernel logs?
A few other drivers also do this
(drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c,
drivers/macintosh/windfarm_pm121.c), however that doesn't mean it's a
good idea. Will change to degC or C.
> > + }
> > +
> > +skip:
> > + reg |= chip->thresh;
> > + if (disable_s2_shutdown)
> > + reg |= SHUTDOWN_CTRL1_OVERRIDE_S2;
> > +
> > + ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ret;
>
> Simplify the above lines to:
>
> return qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
Ouch, my code is indeed dumb ...
> > @@ -313,12 +441,7 @@ static int qpnp_tm_probe(struct platform_device *pdev)
> > if (ret < 0)
> > return ret;
> >
> > - chip->tz_dev = devm_thermal_zone_of_sensor_register(>dev, 0,
> > chip,
> > -
> > _tm_sensor_ops);
> > - if (IS_ERR(chip->tz_dev)) {
> > - dev_err(>dev, "failed to register sensor\n");
> > - return PTR_ERR(chip->tz_dev);
> > - }
> > + chip->initialized = true;
>
> Should we add "thermal_zone_device_update(chip->tz_dev,
> THERMAL_EVENT_UNSPECIFIED);" here
Seems reasonable, will do.
> ...also: do we care about any type of locking for chip->initialized?
> Technically we can be running on weakly ordered memory so if
> qpnp_tm_update_temp_no_adc() is running on a different processor then
> possibly it could still keep returning the default temperature for a
> little while. We could try to analyze whether there's some sort of
> implicit barrier or we could add manual memory barriers, but generally
> I try to avoid that and just do the simple locking... What about just
> setting chip-Initialized = true at the end of qpnp_tm_init() while the
> mutex is still held?
Thanks for pointing that out. I agree that we should keep things
simple, chip->initialized to true at the end of qpnp_tm_init() sounds
good to me.
> I'd also love to hear from someone with more thermal framework
> experience to make sure it's legit to return a default value if
> someone calls us while we're initting. It seems sane to me but nice
> to confirm it's OK.
An alternative could be to return THERMAL_TEMP_INVALID, however I
don't see this handled outside of thermal_core.c, not sure if it could
throw some other code off.
Comments from thermal folks on either approach (or alternatives) are
definitely welcome :)
> Overall I like the idea of this patch so hopefully others do too.
> Thanks for sending it out!
Thanks for the review!
Matthias
Re: [PATCH 0/10] psi: pressure stall information for CPU, memory, and IO v2
On 7/25/18 1:15 AM, Johannes Weiner wrote: > Hi Balbir, > > On Tue, Jul 24, 2018 at 07:14:02AM +1000, Balbir Singh wrote: >> Does the mechanism scale? I am a little concerned about how frequently >> this infrastructure is monitored/read/acted upon. > > I expect most users to poll in the frequency ballpark of the running > averages (10s, 1m, 5m). Our OOMD defaults to 5s polling of the 10s > average; we collect the 1m average once per minute from our machines > and cgroups to log the system/workload health trends in our fleet. > > Suren has been experimenting with adaptive polling down to the > millisecond range on Android. > I think this is a bad way of doing things, polling only adds to overheads, there needs to be an event driven mechanism and the selection of the events need to happen in user space. >> Why aren't existing mechanisms sufficient > > Our existing stuff gives a lot of indication when something *may* be > an issue, like the rate of page reclaim, the number of refaults, the > average number of active processes, one task waiting on a resource. > > But the real difference between an issue and a non-issue is how much > it affects your overall goal of making forward progress or reacting to > a request in time. And that's the only thing users really care > about. It doesn't matter whether my system is doing 2314 or 6723 page > refaults per minute, or scanned 8495 pages recently. I need to know > whether I'm losing 1% or 20% of my time on overcommitted memory. > > Delayacct is time-based, so it's a step in the right direction, but it > doesn't aggregate tasks and CPUs into compound productivity states to > tell you if only parts of your workload are seeing delays (which is > often tolerable for the purpose of ensuring maximum HW utilization) or > your system overall is not making forward progress. That aggregation > isn't something you can do in userspace with polled delayacct data. By aggregation you mean cgroup aggregation? > >> -- why is the avg delay calculation in the kernel? > > For one, as per above, most users will probably be using the standard > averaging windows, and we already have this highly optimizd > infrastructure from the load average. I don't see why we shouldn't use > that instead of exporting an obscure number that requires most users > to have an additional library or copy-paste the loadavg code. > > I also mentioned the OOM killer as a likely in-kernel user of the > pressure percentages to protect from memory livelocks out of the box, > in which case we have to do this calculation in the kernel anyway. > >> There is no talk about the overhead this introduces in general, may be >> the details are in the patches. I'll read through them > > I sent an email on benchmarks and overhead in one of the subthreads, I > will include that information in the cover letter in v3. > > https://lore.kernel.org/lkml/20180718215644.gb2...@cmpxchg.org/ Thanks, I'll take a look Balbir Singh.
Re: [PATCH 0/10] psi: pressure stall information for CPU, memory, and IO v2
On 7/25/18 1:15 AM, Johannes Weiner wrote: > Hi Balbir, > > On Tue, Jul 24, 2018 at 07:14:02AM +1000, Balbir Singh wrote: >> Does the mechanism scale? I am a little concerned about how frequently >> this infrastructure is monitored/read/acted upon. > > I expect most users to poll in the frequency ballpark of the running > averages (10s, 1m, 5m). Our OOMD defaults to 5s polling of the 10s > average; we collect the 1m average once per minute from our machines > and cgroups to log the system/workload health trends in our fleet. > > Suren has been experimenting with adaptive polling down to the > millisecond range on Android. > I think this is a bad way of doing things, polling only adds to overheads, there needs to be an event driven mechanism and the selection of the events need to happen in user space. >> Why aren't existing mechanisms sufficient > > Our existing stuff gives a lot of indication when something *may* be > an issue, like the rate of page reclaim, the number of refaults, the > average number of active processes, one task waiting on a resource. > > But the real difference between an issue and a non-issue is how much > it affects your overall goal of making forward progress or reacting to > a request in time. And that's the only thing users really care > about. It doesn't matter whether my system is doing 2314 or 6723 page > refaults per minute, or scanned 8495 pages recently. I need to know > whether I'm losing 1% or 20% of my time on overcommitted memory. > > Delayacct is time-based, so it's a step in the right direction, but it > doesn't aggregate tasks and CPUs into compound productivity states to > tell you if only parts of your workload are seeing delays (which is > often tolerable for the purpose of ensuring maximum HW utilization) or > your system overall is not making forward progress. That aggregation > isn't something you can do in userspace with polled delayacct data. By aggregation you mean cgroup aggregation? > >> -- why is the avg delay calculation in the kernel? > > For one, as per above, most users will probably be using the standard > averaging windows, and we already have this highly optimizd > infrastructure from the load average. I don't see why we shouldn't use > that instead of exporting an obscure number that requires most users > to have an additional library or copy-paste the loadavg code. > > I also mentioned the OOM killer as a likely in-kernel user of the > pressure percentages to protect from memory livelocks out of the box, > in which case we have to do this calculation in the kernel anyway. > >> There is no talk about the overhead this introduces in general, may be >> the details are in the patches. I'll read through them > > I sent an email on benchmarks and overhead in one of the subthreads, I > will include that information in the cover letter in v3. > > https://lore.kernel.org/lkml/20180718215644.gb2...@cmpxchg.org/ Thanks, I'll take a look Balbir Singh.
