Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Ingo Molnar]

* Byungchul Park  wrote:

> On Wed, Oct 18, 2017 at 12:09:44PM +0200, Ingo Molnar wrote:
> > BTW., have you attempted limiting the depth of the stack traces? I suspect 
> > more 
> > than 2-4 are rarely required to disambiguate the calling context.
> 
> I did it for you. Let me show you the result.
> 
> 1. No lockdep:2.756558155 seconds time 
> elapsed( +-  0.09% )
> 2. Lockdep:   2.968710420 seconds time 
> elapsed( +-  0.12% )
> 3. Lockdep + Crossrelease 5 entries:  3.153839636 seconds time 
> elapsed( +-  0.31% )
> 4. Lockdep + Crossrelease 3 entries:  3.137205534 seconds time 
> elapsed( +-  0.87% )
> 5. Lockdep + Crossrelease + This patch:   2.963669551 seconds time 
> elapsed( +-  0.11% )

I think the lockdep + crossrelease + full-stack numbers are missing?

But yeah, looks like single-entry-stacktrace crossrelease only has a +0.2% 
performance cost (with 0.1% noise), while lockdep itself has a +7.7% cost.

That's very reasonable and we can keep the single-entry cross-release feature 
enabled by default as part of CONFIG_PROVE_LOCKING=y - assuming all the crashes 
and false positives are fixed by the next merge window.

Thanks,

Ingo

Re: [PATCH] checkpatch: Introduce check for format of Fixes line in commit log

[Joe Perches]

On Thu, 2017-10-19 at 05:52 +, Parav Pandit wrote:
> Hi Joe,

Hello Parav

> > -Original Message-
> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> > ow...@vger.kernel.org] On Behalf Of Parav Pandit
> > Sent: Tuesday, October 10, 2017 5:44 PM
> > To: j...@perches.com; a...@canonical.com
> > Cc: linux-kernel@vger.kernel.org; linux-r...@vger.kernel.org; Parav Pandit
> > ; Brad Erickson 
> > Subject: [PATCH] checkpatch: Introduce check for format of Fixes line in 
> > commit
> > log
> > 
> > This patch introduces a format check for 'Fixes' line in commit log for 12
> > characters commit id and format for Fixes as ("...").

I think this doesn't handle case like:

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=58735

Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Ingo Molnar]

* Byungchul Park  wrote:

> On Wed, Oct 18, 2017 at 12:09:44PM +0200, Ingo Molnar wrote:
> > BTW., have you attempted limiting the depth of the stack traces? I suspect 
> > more 
> > than 2-4 are rarely required to disambiguate the calling context.
> 
> I did it for you. Let me show you the result.
> 
> 1. No lockdep:2.756558155 seconds time 
> elapsed( +-  0.09% )
> 2. Lockdep:   2.968710420 seconds time 
> elapsed( +-  0.12% )
> 3. Lockdep + Crossrelease 5 entries:  3.153839636 seconds time 
> elapsed( +-  0.31% )
> 4. Lockdep + Crossrelease 3 entries:  3.137205534 seconds time 
> elapsed( +-  0.87% )
> 5. Lockdep + Crossrelease + This patch:   2.963669551 seconds time 
> elapsed( +-  0.11% )

I think the lockdep + crossrelease + full-stack numbers are missing?

But yeah, looks like single-entry-stacktrace crossrelease only has a +0.2% 
performance cost (with 0.1% noise), while lockdep itself has a +7.7% cost.

That's very reasonable and we can keep the single-entry cross-release feature 
enabled by default as part of CONFIG_PROVE_LOCKING=y - assuming all the crashes 
and false positives are fixed by the next merge window.

Thanks,

Ingo

Re: [PATCH] checkpatch: Introduce check for format of Fixes line in commit log

[Joe Perches]

On Thu, 2017-10-19 at 05:52 +, Parav Pandit wrote:
> Hi Joe,

Hello Parav

> > -Original Message-
> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> > ow...@vger.kernel.org] On Behalf Of Parav Pandit
> > Sent: Tuesday, October 10, 2017 5:44 PM
> > To: j...@perches.com; a...@canonical.com
> > Cc: linux-kernel@vger.kernel.org; linux-r...@vger.kernel.org; Parav Pandit
> > ; Brad Erickson 
> > Subject: [PATCH] checkpatch: Introduce check for format of Fixes line in 
> > commit
> > log
> > 
> > This patch introduces a format check for 'Fixes' line in commit log for 12
> > characters commit id and format for Fixes as ("...").

I think this doesn't handle case like:

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=58735

[PATCH v2 3/3] lockdep: Add a kernel parameter, crossrelease_fullstack

[Byungchul Park]

Make whether to allow recording full stack, in cross-release feature,
switchable at boot time via a kernel parameter, 'crossrelease_fullstack'.
In case of a splat with no stack trace, one could just reboot and set
the kernel parameter to get the full data without having to recompile
the kernel.

Change CONFIG_CROSSRELEASE_STACK_TRACE default from N to Y, and
introduce the new kernel parameter.

Suggested-by: Thomas Gleixner 
Signed-off-by: Byungchul Park 
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 kernel/locking/lockdep.c| 18 --
 lib/Kconfig.debug   |  5 +++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index ead7f40..4107b01 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -709,6 +709,9 @@
It will be ignored when crashkernel=X,high is not used
or memory reserved is below 4G.
 
+   crossrelease_fullstack
+   [KNL] Allow to record full stack trace in cross-release
+
cryptomgr.notests
 [KNL] Disable crypto self-tests
 
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 5c2ddf2..feba887 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -76,6 +76,15 @@
 #define lock_stat 0
 #endif
 
+static int crossrelease_fullstack;
+static int __init allow_crossrelease_fullstack(char *str)
+{
+   crossrelease_fullstack = 1;
+   return 0;
+}
+
+early_param("crossrelease_fullstack", allow_crossrelease_fullstack);
+
 /*
  * lockdep_lock: protects the lockdep graph, the hashes and the
  *   class/list/hash allocators.
@@ -4864,8 +4873,13 @@ static void add_xhlock(struct held_lock *hlock)
xhlock->trace.max_entries = MAX_XHLOCK_TRACE_ENTRIES;
xhlock->trace.entries = xhlock->trace_entries;
 #ifdef CONFIG_CROSSRELEASE_STACK_TRACE
-   xhlock->trace.skip = 3;
-   save_stack_trace(>trace);
+   if (crossrelease_fullstack) {
+   xhlock->trace.skip = 3;
+   save_stack_trace(>trace);
+   } else {
+   xhlock->trace.nr_entries = 1;
+   xhlock->trace.entries[0] = hlock->acquire_ip;
+   }
 #else
xhlock->trace.nr_entries = 1;
xhlock->trace.entries[0] = hlock->acquire_ip;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index fe8fceb..132536d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1228,7 +1228,7 @@ config LOCKDEP_COMPLETIONS
 config CROSSRELEASE_STACK_TRACE
bool "Record more than one entity of stack trace in crossrelease"
depends on LOCKDEP_CROSSRELEASE
-   default n
+   default y
help
 The lockdep "cross-release" feature needs to record stack traces
 (of calling functions) for all acquisitions, for eventual later
@@ -1238,7 +1238,8 @@ config CROSSRELEASE_STACK_TRACE
 full analysis. This option turns on the saving of the full stack
 trace entries.
 
-If unsure, say N.
+To make the feature actually on, set "crossrelease_fullstack"
+kernel parameter, too.
 
 config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
-- 
1.9.1

[PATCH v2 3/3] lockdep: Add a kernel parameter, crossrelease_fullstack

[Byungchul Park]

Make whether to allow recording full stack, in cross-release feature,
switchable at boot time via a kernel parameter, 'crossrelease_fullstack'.
In case of a splat with no stack trace, one could just reboot and set
the kernel parameter to get the full data without having to recompile
the kernel.

Change CONFIG_CROSSRELEASE_STACK_TRACE default from N to Y, and
introduce the new kernel parameter.

Suggested-by: Thomas Gleixner 
Signed-off-by: Byungchul Park 
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 kernel/locking/lockdep.c| 18 --
 lib/Kconfig.debug   |  5 +++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index ead7f40..4107b01 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -709,6 +709,9 @@
It will be ignored when crashkernel=X,high is not used
or memory reserved is below 4G.
 
+   crossrelease_fullstack
+   [KNL] Allow to record full stack trace in cross-release
+
cryptomgr.notests
 [KNL] Disable crypto self-tests
 
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 5c2ddf2..feba887 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -76,6 +76,15 @@
 #define lock_stat 0
 #endif
 
+static int crossrelease_fullstack;
+static int __init allow_crossrelease_fullstack(char *str)
+{
+   crossrelease_fullstack = 1;
+   return 0;
+}
+
+early_param("crossrelease_fullstack", allow_crossrelease_fullstack);
+
 /*
  * lockdep_lock: protects the lockdep graph, the hashes and the
  *   class/list/hash allocators.
@@ -4864,8 +4873,13 @@ static void add_xhlock(struct held_lock *hlock)
xhlock->trace.max_entries = MAX_XHLOCK_TRACE_ENTRIES;
xhlock->trace.entries = xhlock->trace_entries;
 #ifdef CONFIG_CROSSRELEASE_STACK_TRACE
-   xhlock->trace.skip = 3;
-   save_stack_trace(>trace);
+   if (crossrelease_fullstack) {
+   xhlock->trace.skip = 3;
+   save_stack_trace(>trace);
+   } else {
+   xhlock->trace.nr_entries = 1;
+   xhlock->trace.entries[0] = hlock->acquire_ip;
+   }
 #else
xhlock->trace.nr_entries = 1;
xhlock->trace.entries[0] = hlock->acquire_ip;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index fe8fceb..132536d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1228,7 +1228,7 @@ config LOCKDEP_COMPLETIONS
 config CROSSRELEASE_STACK_TRACE
bool "Record more than one entity of stack trace in crossrelease"
depends on LOCKDEP_CROSSRELEASE
-   default n
+   default y
help
 The lockdep "cross-release" feature needs to record stack traces
 (of calling functions) for all acquisitions, for eventual later
@@ -1238,7 +1238,8 @@ config CROSSRELEASE_STACK_TRACE
 full analysis. This option turns on the saving of the full stack
 trace entries.
 
-If unsure, say N.
+To make the feature actually on, set "crossrelease_fullstack"
+kernel parameter, too.
 
 config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
-- 
1.9.1

[tip:locking/core] locking/static_keys: Improve uninitialized key warning

[tip-bot for Borislav Petkov]

Commit-ID:  5cdda5117e125e0dbb020425cc55a4c143c6febc
Gitweb: https://git.kernel.org/tip/5cdda5117e125e0dbb020425cc55a4c143c6febc
Author: Borislav Petkov 
AuthorDate: Wed, 18 Oct 2017 17:24:28 +0200
Committer:  Ingo Molnar 
CommitDate: Thu, 19 Oct 2017 07:49:14 +0200

locking/static_keys: Improve uninitialized key warning

Right now it says:

  static_key_disable_cpuslocked used before call to jump_label_init
  [ cut here ]
  WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:161 
static_key_disable_cpuslocked+0x68/0x70
  Modules linked in:
  CPU: 0 PID: 0 Comm: swapper Not tainted 4.14.0-rc5+ #1
  Hardware name: SGI.COM C2112-4GP3/X10DRT-P-Series, BIOS 2.0a 05/09/2016
  task: 81c0e480 task.stack: 81c0
  RIP: 0010:static_key_disable_cpuslocked+0x68/0x70
  RSP: :81c03ef0 EFLAGS: 00010096 ORIG_RAX: 
  RAX: 0041 RBX: 81c32680 RCX: 81c5cbf8
  RDX: 0001 RSI: 0092 RDI: 0002
  RBP: 88807fffd240 R08: 726f666562206465 R09: 0136
  R10:  R11: 696e695f6c656261 R12: 82158900
  R13: 8215f760 R14: 0001 R15: 0008
  FS:  () GS:883f7f40() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2: 88807000 CR3: 01c09000 CR4: 000606b0
  Call Trace:
   static_key_disable+0x16/0x20
   start_kernel+0x15a/0x45d
   ? load_ucode_intel_bsp+0x11/0x2d
   secondary_startup_64+0xa5/0xb0
  Code: 48 c7 c7 a0 15 cf 81 e9 47 53 4b 00 48 89 df e8 5f fc ff ff eb e8 48 c7 
c6 \
c0 97 83 81 48 c7 c7 d0 ff a2 81 31 c0 e8 c5 9d f5 ff <0f> ff eb a7 0f 
ff eb \
b0 e8 eb a2 4b 00 53 48 89 fb e8 42 0e f0

but it doesn't tell me which key it is. So dump the key's name too:

  static_key_disable_cpuslocked(): static key 'virt_spin_lock_key' used before 
call to jump_label_init()

And that makes pinpointing which key is causing that a lot easier.

 include/linux/jump_label.h   |   14 +++---
 include/linux/jump_label_ratelimit.h |6 +++---
 kernel/jump_label.c  |   14 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

Signed-off-by: Borislav Petkov 
Reviewed-by: Steven Rostedt (VMware) 
Cc: Boris Ostrovsky 
Cc: Hannes Frederic Sowa 
Cc: Jason Baron 
Cc: Juergen Gross 
Cc: Linus Torvalds 
Cc: Paolo Bonzini 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: http://lkml.kernel.org/r/20171018152428.ffjgak4o25f7e...@pd.tnic
Signed-off-by: Ingo Molnar 
---
 include/linux/jump_label.h   | 14 +++---
 include/linux/jump_label_ratelimit.h |  6 +++---
 kernel/jump_label.c  | 14 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index cd58616..979a2f2 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -81,9 +81,9 @@
 
 extern bool static_key_initialized;
 
-#define STATIC_KEY_CHECK_USE() WARN(!static_key_initialized, \
-   "%s used before call to jump_label_init", \
-   __func__)
+#define STATIC_KEY_CHECK_USE(key) WARN(!static_key_initialized,
  \
+   "%s(): static key '%pS' used before call to 
jump_label_init()", \
+   __func__, (key))
 
 #ifdef HAVE_JUMP_LABEL
 
@@ -211,13 +211,13 @@ static __always_inline bool static_key_true(struct 
static_key *key)
 
 static inline void static_key_slow_inc(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
atomic_inc(>enabled);
 }
 
 static inline void static_key_slow_dec(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
atomic_dec(>enabled);
 }
 
@@ -236,7 +236,7 @@ static inline int jump_label_apply_nops(struct module *mod)
 
 static inline void static_key_enable(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
 
if (atomic_read(>enabled) != 0) {
WARN_ON_ONCE(atomic_read(>enabled) != 1);
@@ -247,7 +247,7 @@ static inline void static_key_enable(struct static_key *key)
 
 static inline void static_key_disable(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
 
if (atomic_read(>enabled) != 1) {
WARN_ON_ONCE(atomic_read(>enabled) != 0);
diff --git a/include/linux/jump_label_ratelimit.h 
b/include/linux/jump_label_ratelimit.h
index 23da3af..93086df 100644
--- 

[tip:locking/core] locking/static_keys: Improve uninitialized key warning

[tip-bot for Borislav Petkov]

Commit-ID:  5cdda5117e125e0dbb020425cc55a4c143c6febc
Gitweb: https://git.kernel.org/tip/5cdda5117e125e0dbb020425cc55a4c143c6febc
Author: Borislav Petkov 
AuthorDate: Wed, 18 Oct 2017 17:24:28 +0200
Committer:  Ingo Molnar 
CommitDate: Thu, 19 Oct 2017 07:49:14 +0200

locking/static_keys: Improve uninitialized key warning

Right now it says:

  static_key_disable_cpuslocked used before call to jump_label_init
  [ cut here ]
  WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:161 
static_key_disable_cpuslocked+0x68/0x70
  Modules linked in:
  CPU: 0 PID: 0 Comm: swapper Not tainted 4.14.0-rc5+ #1
  Hardware name: SGI.COM C2112-4GP3/X10DRT-P-Series, BIOS 2.0a 05/09/2016
  task: 81c0e480 task.stack: 81c0
  RIP: 0010:static_key_disable_cpuslocked+0x68/0x70
  RSP: :81c03ef0 EFLAGS: 00010096 ORIG_RAX: 
  RAX: 0041 RBX: 81c32680 RCX: 81c5cbf8
  RDX: 0001 RSI: 0092 RDI: 0002
  RBP: 88807fffd240 R08: 726f666562206465 R09: 0136
  R10:  R11: 696e695f6c656261 R12: 82158900
  R13: 8215f760 R14: 0001 R15: 0008
  FS:  () GS:883f7f40() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2: 88807000 CR3: 01c09000 CR4: 000606b0
  Call Trace:
   static_key_disable+0x16/0x20
   start_kernel+0x15a/0x45d
   ? load_ucode_intel_bsp+0x11/0x2d
   secondary_startup_64+0xa5/0xb0
  Code: 48 c7 c7 a0 15 cf 81 e9 47 53 4b 00 48 89 df e8 5f fc ff ff eb e8 48 c7 
c6 \
c0 97 83 81 48 c7 c7 d0 ff a2 81 31 c0 e8 c5 9d f5 ff <0f> ff eb a7 0f 
ff eb \
b0 e8 eb a2 4b 00 53 48 89 fb e8 42 0e f0

but it doesn't tell me which key it is. So dump the key's name too:

  static_key_disable_cpuslocked(): static key 'virt_spin_lock_key' used before 
call to jump_label_init()

And that makes pinpointing which key is causing that a lot easier.

 include/linux/jump_label.h   |   14 +++---
 include/linux/jump_label_ratelimit.h |6 +++---
 kernel/jump_label.c  |   14 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

Signed-off-by: Borislav Petkov 
Reviewed-by: Steven Rostedt (VMware) 
Cc: Boris Ostrovsky 
Cc: Hannes Frederic Sowa 
Cc: Jason Baron 
Cc: Juergen Gross 
Cc: Linus Torvalds 
Cc: Paolo Bonzini 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: http://lkml.kernel.org/r/20171018152428.ffjgak4o25f7e...@pd.tnic
Signed-off-by: Ingo Molnar 
---
 include/linux/jump_label.h   | 14 +++---
 include/linux/jump_label_ratelimit.h |  6 +++---
 kernel/jump_label.c  | 14 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index cd58616..979a2f2 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -81,9 +81,9 @@
 
 extern bool static_key_initialized;
 
-#define STATIC_KEY_CHECK_USE() WARN(!static_key_initialized, \
-   "%s used before call to jump_label_init", \
-   __func__)
+#define STATIC_KEY_CHECK_USE(key) WARN(!static_key_initialized,
  \
+   "%s(): static key '%pS' used before call to 
jump_label_init()", \
+   __func__, (key))
 
 #ifdef HAVE_JUMP_LABEL
 
@@ -211,13 +211,13 @@ static __always_inline bool static_key_true(struct 
static_key *key)
 
 static inline void static_key_slow_inc(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
atomic_inc(>enabled);
 }
 
 static inline void static_key_slow_dec(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
atomic_dec(>enabled);
 }
 
@@ -236,7 +236,7 @@ static inline int jump_label_apply_nops(struct module *mod)
 
 static inline void static_key_enable(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
 
if (atomic_read(>enabled) != 0) {
WARN_ON_ONCE(atomic_read(>enabled) != 1);
@@ -247,7 +247,7 @@ static inline void static_key_enable(struct static_key *key)
 
 static inline void static_key_disable(struct static_key *key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);
 
if (atomic_read(>enabled) != 1) {
WARN_ON_ONCE(atomic_read(>enabled) != 0);
diff --git a/include/linux/jump_label_ratelimit.h 
b/include/linux/jump_label_ratelimit.h
index 23da3af..93086df 100644
--- a/include/linux/jump_label_ratelimit.h
+++ b/include/linux/jump_label_ratelimit.h
@@ -24,18 +24,18 @@ struct static_key_deferred {
 };
 static inline void static_key_slow_dec_deferred(struct static_key_deferred 
*key)
 {
-   STATIC_KEY_CHECK_USE();
+   STATIC_KEY_CHECK_USE(key);

[PATCH v2 0/3] crossrelease: make it not unwind by default

[Byungchul Park]

Change from v1
- Fix kconfig description as Ingo suggested
- Fix commit message writing out CONFIG_ variable
- Introduce a new kernel parameter, crossrelease_fullstack
- Replace the number with the output of *perf*

Byungchul Park (3):
  lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as
default
  lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE
  lockdep: Add a kernel parameter, crossrelease_fullstack

 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 include/linux/lockdep.h |  4 
 kernel/locking/lockdep.c| 23 +--
 lib/Kconfig.debug   | 20 ++--
 4 files changed, 46 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH v2 0/3] crossrelease: make it not unwind by default

[Byungchul Park]

Change from v1
- Fix kconfig description as Ingo suggested
- Fix commit message writing out CONFIG_ variable
- Introduce a new kernel parameter, crossrelease_fullstack
- Replace the number with the output of *perf*

Byungchul Park (3):
  lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as
default
  lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE
  lockdep: Add a kernel parameter, crossrelease_fullstack

 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 include/linux/lockdep.h |  4 
 kernel/locking/lockdep.c| 23 +--
 lib/Kconfig.debug   | 20 ++--
 4 files changed, 46 insertions(+), 4 deletions(-)

-- 
1.9.1

RE: [PATCH 0/9] Intel Processor Trace virtulization enabling

[Kang, Luwei]

>  Nested virtualization is interesting.  We would like the nested
>  hypervisor to be forced to set the "use GPA for processor tracing"
>  secondary execution control whenever "enable EPT" is set and
>  RTIT_CTL is nonzero.  There is no way to encode that in
>  IA32_VMX_PROCBASED_CTLS2, however.  It would be nice if Intel could
>  reserve a bit in IA32_VMX_EPT_VPID_CAP for KVM to express that
>  constraint.
> >>>
> >>> Do you mean if nested hypervisor get the capability of "Guest PT use
> >>> GPA" and EPT has enable. Highly recommend nested hypervisor set "
> >>> Guest PT use GPA " as well.
> >>
> >> Well, it's required more than recommended.  However, it's only required if 
> >> "enable EPT" is set and RTIT_CTL is nonzero.
> >>
> >>> If nested hypervisor is also KVM, "use GPA for processor tracing"
> >>> will be set for sure. But other hypervisor may not do that. So, we'd
> >>> better add a flag in IA32_VMX_EPT_VPID_CAP to express that constraint.
> >>
> >> Correct.  The constraint would be:
> >>
> >> * RTIT_CTL on entry is zero if EPT is disabled
> >>
> >> * RTIT_CTL on entry is zero if EPT is enabled and "Guest PT uses GPA"
> >> is zero
> >>
> >> Maybe IA32_VMX_EPT_VPID_CAP is not the best place.  I'll let Intel decide 
> >> that.
> >
> > Get it. I have feedback to hardware architect. I hope it can be applied but 
> > it may need wait a long time.
> 
> Note that the hardware need not do anything.  However it would be nice if the 
> SDM can define a bit _for the hypervisors_ to
> enforce the above constraint and fail vmentry if they are not respected.
> 

Hi Paolo,
Thanks for your response. I have a question want to ask for you. As you 
mentioned in previous mail " We would like the nested hypervisor to be forced 
to set the "use GPA for processor tracing"". 
Is there have any problem if we don't set "use GPA for processor tracing" 
in nested hypervisor? If yes, what should we do? In patch 9, I pass though PT 
MSRs ( IA32_RTIT_* ) to guest.

Thanks,
Luwei Kang

RE: [PATCH 0/9] Intel Processor Trace virtulization enabling

[Kang, Luwei]

>  Nested virtualization is interesting.  We would like the nested
>  hypervisor to be forced to set the "use GPA for processor tracing"
>  secondary execution control whenever "enable EPT" is set and
>  RTIT_CTL is nonzero.  There is no way to encode that in
>  IA32_VMX_PROCBASED_CTLS2, however.  It would be nice if Intel could
>  reserve a bit in IA32_VMX_EPT_VPID_CAP for KVM to express that
>  constraint.
> >>>
> >>> Do you mean if nested hypervisor get the capability of "Guest PT use
> >>> GPA" and EPT has enable. Highly recommend nested hypervisor set "
> >>> Guest PT use GPA " as well.
> >>
> >> Well, it's required more than recommended.  However, it's only required if 
> >> "enable EPT" is set and RTIT_CTL is nonzero.
> >>
> >>> If nested hypervisor is also KVM, "use GPA for processor tracing"
> >>> will be set for sure. But other hypervisor may not do that. So, we'd
> >>> better add a flag in IA32_VMX_EPT_VPID_CAP to express that constraint.
> >>
> >> Correct.  The constraint would be:
> >>
> >> * RTIT_CTL on entry is zero if EPT is disabled
> >>
> >> * RTIT_CTL on entry is zero if EPT is enabled and "Guest PT uses GPA"
> >> is zero
> >>
> >> Maybe IA32_VMX_EPT_VPID_CAP is not the best place.  I'll let Intel decide 
> >> that.
> >
> > Get it. I have feedback to hardware architect. I hope it can be applied but 
> > it may need wait a long time.
> 
> Note that the hardware need not do anything.  However it would be nice if the 
> SDM can define a bit _for the hypervisors_ to
> enforce the above constraint and fail vmentry if they are not respected.
> 

Hi Paolo,
Thanks for your response. I have a question want to ask for you. As you 
mentioned in previous mail " We would like the nested hypervisor to be forced 
to set the "use GPA for processor tracing"". 
Is there have any problem if we don't set "use GPA for processor tracing" 
in nested hypervisor? If yes, what should we do? In patch 9, I pass though PT 
MSRs ( IA32_RTIT_* ) to guest.

Thanks,
Luwei Kang

[PATCH v2 2/3] lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE

[Byungchul Park]

Now the performance regression was fixed, re-enable
CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS.

Signed-off-by: Byungchul Park 
---
 lib/Kconfig.debug | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 90ea784..fe8fceb 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1138,8 +1138,8 @@ config PROVE_LOCKING
select DEBUG_MUTEXES
select DEBUG_RT_MUTEXES if RT_MUTEXES
select DEBUG_LOCK_ALLOC
-   select LOCKDEP_CROSSRELEASE if BROKEN
-   select LOCKDEP_COMPLETIONS if BROKEN
+   select LOCKDEP_CROSSRELEASE
+   select LOCKDEP_COMPLETIONS
select TRACE_IRQFLAGS
default n
help
-- 
1.9.1

[PATCH v2 1/3] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Byungchul Park]

Johan Hovold reported a performance regression by crossrelease like:

> Boot time (from "Linux version" to login prompt) had in fact doubled
> since 4.13 where it took 17 seconds (with my current config) compared to
> the 35 seconds I now see with 4.14-rc4.
>
> I quick bisect pointed to lockdep and specifically the following commit:
>
>   28a903f63ec0 ("locking/lockdep: Handle non(or multi)-acquisition
>  of a crosslock")
>
> which I've verified is the commit which doubled the boot time (compared
> to 28a903f63ec0^) (added by lockdep crossrelease series [1]).

Currently crossrelease performs unwind on every acquisition. But, that
overloads systems too much. So this patch makes unwind optional and set
it to N as default. Instead, it records only acquire_ip normally. Of
course, unwind is sometimes required for full analysis. In that case, we
can set CROSSRELEASE_STACK_TRACE to Y and use it.

In my qemu ubuntu machin (x86_64, 4 cores, 512M), the regression was
fixed like, measuring booting times with 'perf stat --null --repeat 10
$QEMU', where $QEMU launchs a kernel with init=/bin/true:

1. No lockdep enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.756558155 seconds time elapsed( +-  0.09% )

2. Lockdep enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.968710420 seconds time elapsed( +-  0.12% )

3. Lockdep enabled + crossrelease enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.153839636 seconds time elapsed( +-  0.31% )

4. Lockdep enabled + crossrelease enabled + this patch applied

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.963669551 seconds time elapsed( +-  0.11% )

Signed-off-by: Byungchul Park 
---
 include/linux/lockdep.h  |  4 
 kernel/locking/lockdep.c |  5 +
 lib/Kconfig.debug| 15 +++
 3 files changed, 24 insertions(+)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index bfa8e0b..70358b5 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -278,7 +278,11 @@ struct held_lock {
 };
 
 #ifdef CONFIG_LOCKDEP_CROSSRELEASE
+#ifdef CONFIG_CROSSRELEASE_STACK_TRACE
 #define MAX_XHLOCK_TRACE_ENTRIES 5
+#else
+#define MAX_XHLOCK_TRACE_ENTRIES 1
+#endif
 
 /*
  * This is for keeping locks waiting for commit so that true dependencies
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e36e652..5c2ddf2 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4863,8 +4863,13 @@ static void add_xhlock(struct held_lock *hlock)
xhlock->trace.nr_entries = 0;
xhlock->trace.max_entries = MAX_XHLOCK_TRACE_ENTRIES;
xhlock->trace.entries = xhlock->trace_entries;
+#ifdef CONFIG_CROSSRELEASE_STACK_TRACE
xhlock->trace.skip = 3;
save_stack_trace(>trace);
+#else
+   xhlock->trace.nr_entries = 1;
+   xhlock->trace.entries[0] = hlock->acquire_ip;
+#endif
 }
 
 static inline int same_context_xhlock(struct hist_lock *xhlock)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3db9167..90ea784 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1225,6 +1225,21 @@ config LOCKDEP_COMPLETIONS
 A deadlock caused by wait_for_completion() and complete() can be
 detected by lockdep using crossrelease feature.
 
+config CROSSRELEASE_STACK_TRACE
+   bool "Record more than one entity of stack trace in crossrelease"
+   depends on LOCKDEP_CROSSRELEASE
+   default n
+   help
+The lockdep "cross-release" feature needs to record stack traces
+(of calling functions) for all acquisitions, for eventual later
+use during analysis. By default only a single caller is recorded,
+because the unwind operation can be very expensive with deeper
+stack chains. However, sometimes deeper traces are required for
+full analysis. This option turns on the saving of the full stack
+trace entries.
+
+If unsure, say N.
+
 config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
depends on DEBUG_KERNEL && LOCKDEP
-- 
1.9.1

[PATCH v2 2/3] lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE

[Byungchul Park]

Now the performance regression was fixed, re-enable
CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS.

Signed-off-by: Byungchul Park 
---
 lib/Kconfig.debug | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 90ea784..fe8fceb 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1138,8 +1138,8 @@ config PROVE_LOCKING
select DEBUG_MUTEXES
select DEBUG_RT_MUTEXES if RT_MUTEXES
select DEBUG_LOCK_ALLOC
-   select LOCKDEP_CROSSRELEASE if BROKEN
-   select LOCKDEP_COMPLETIONS if BROKEN
+   select LOCKDEP_CROSSRELEASE
+   select LOCKDEP_COMPLETIONS
select TRACE_IRQFLAGS
default n
help
-- 
1.9.1

[PATCH v2 1/3] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Byungchul Park]

Johan Hovold reported a performance regression by crossrelease like:

> Boot time (from "Linux version" to login prompt) had in fact doubled
> since 4.13 where it took 17 seconds (with my current config) compared to
> the 35 seconds I now see with 4.14-rc4.
>
> I quick bisect pointed to lockdep and specifically the following commit:
>
>   28a903f63ec0 ("locking/lockdep: Handle non(or multi)-acquisition
>  of a crosslock")
>
> which I've verified is the commit which doubled the boot time (compared
> to 28a903f63ec0^) (added by lockdep crossrelease series [1]).

Currently crossrelease performs unwind on every acquisition. But, that
overloads systems too much. So this patch makes unwind optional and set
it to N as default. Instead, it records only acquire_ip normally. Of
course, unwind is sometimes required for full analysis. In that case, we
can set CROSSRELEASE_STACK_TRACE to Y and use it.

In my qemu ubuntu machin (x86_64, 4 cores, 512M), the regression was
fixed like, measuring booting times with 'perf stat --null --repeat 10
$QEMU', where $QEMU launchs a kernel with init=/bin/true:

1. No lockdep enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.756558155 seconds time elapsed( +-  0.09% )

2. Lockdep enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.968710420 seconds time elapsed( +-  0.12% )

3. Lockdep enabled + crossrelease enabled

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.153839636 seconds time elapsed( +-  0.31% )

4. Lockdep enabled + crossrelease enabled + this patch applied

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.963669551 seconds time elapsed( +-  0.11% )

Signed-off-by: Byungchul Park 
---
 include/linux/lockdep.h  |  4 
 kernel/locking/lockdep.c |  5 +
 lib/Kconfig.debug| 15 +++
 3 files changed, 24 insertions(+)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index bfa8e0b..70358b5 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -278,7 +278,11 @@ struct held_lock {
 };
 
 #ifdef CONFIG_LOCKDEP_CROSSRELEASE
+#ifdef CONFIG_CROSSRELEASE_STACK_TRACE
 #define MAX_XHLOCK_TRACE_ENTRIES 5
+#else
+#define MAX_XHLOCK_TRACE_ENTRIES 1
+#endif
 
 /*
  * This is for keeping locks waiting for commit so that true dependencies
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e36e652..5c2ddf2 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4863,8 +4863,13 @@ static void add_xhlock(struct held_lock *hlock)
xhlock->trace.nr_entries = 0;
xhlock->trace.max_entries = MAX_XHLOCK_TRACE_ENTRIES;
xhlock->trace.entries = xhlock->trace_entries;
+#ifdef CONFIG_CROSSRELEASE_STACK_TRACE
xhlock->trace.skip = 3;
save_stack_trace(>trace);
+#else
+   xhlock->trace.nr_entries = 1;
+   xhlock->trace.entries[0] = hlock->acquire_ip;
+#endif
 }
 
 static inline int same_context_xhlock(struct hist_lock *xhlock)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3db9167..90ea784 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1225,6 +1225,21 @@ config LOCKDEP_COMPLETIONS
 A deadlock caused by wait_for_completion() and complete() can be
 detected by lockdep using crossrelease feature.
 
+config CROSSRELEASE_STACK_TRACE
+   bool "Record more than one entity of stack trace in crossrelease"
+   depends on LOCKDEP_CROSSRELEASE
+   default n
+   help
+The lockdep "cross-release" feature needs to record stack traces
+(of calling functions) for all acquisitions, for eventual later
+use during analysis. By default only a single caller is recorded,
+because the unwind operation can be very expensive with deeper
+stack chains. However, sometimes deeper traces are required for
+full analysis. This option turns on the saving of the full stack
+trace entries.
+
+If unsure, say N.
+
 config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
depends on DEBUG_KERNEL && LOCKDEP
-- 
1.9.1

Re: [PATCH v5] printk: hash addresses printed with %p

[Jason A. Donenfeld]

A small detail carried over from the other thread:

>
> but a bigger problem might the following thing:
>
> vscnprintf()
>  pointer()
>   ptr_to_id()
>initialize_ptr_secret()
> get_random_bytes()
>  _get_random_bytes()
>   extract_crng()
>_extract_crng()
> spin_lock_irqsave(>lock, flags);   <
>
>
> this, once again, can deadlock. can it? just like before:

So, actually, then, we need to do this as an initcall. Fortunately,
that simplifies things greatly. Here's a rough sketch of what that
looks like, which you'll probably need to debug and refine:



static siphash_key_t ptr_secret __ro_after_init;
static DEFINE_STATIC_KEY_TRUE(no_ptr_secret);

static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
{
if (static_branch_unlikely(_ptr_secret))
return "(pointer value)";

hashval = 

}

static void fill_random_ptr_key(struct random_ready_callback *rdy)
{
get_random_bytes(_secret, sizeof(ptr_secret));
static_branch_disable(_ptr_secret);
}

static struct random_ready_callback random_ready = {
.func = fill_random_ptr_key
};

static int __init initialize_ptr_random(void)
{
int ret = add_random_ready_callback(_ready);

if (!ret)
return 0;
else if (ret == -EALREADY) {
fill_random_ptr_key(_ready);
return 0;
}

return ret;
}
early_initcall(initialize_ptr_random);

Re: [PATCH v5] printk: hash addresses printed with %p

[Jason A. Donenfeld]

A small detail carried over from the other thread:

>
> but a bigger problem might the following thing:
>
> vscnprintf()
>  pointer()
>   ptr_to_id()
>initialize_ptr_secret()
> get_random_bytes()
>  _get_random_bytes()
>   extract_crng()
>_extract_crng()
> spin_lock_irqsave(>lock, flags);   <
>
>
> this, once again, can deadlock. can it? just like before:

So, actually, then, we need to do this as an initcall. Fortunately,
that simplifies things greatly. Here's a rough sketch of what that
looks like, which you'll probably need to debug and refine:



static siphash_key_t ptr_secret __ro_after_init;
static DEFINE_STATIC_KEY_TRUE(no_ptr_secret);

static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
{
if (static_branch_unlikely(_ptr_secret))
return "(pointer value)";

hashval = 

}

static void fill_random_ptr_key(struct random_ready_callback *rdy)
{
get_random_bytes(_secret, sizeof(ptr_secret));
static_branch_disable(_ptr_secret);
}

static struct random_ready_callback random_ready = {
.func = fill_random_ptr_key
};

static int __init initialize_ptr_random(void)
{
int ret = add_random_ready_callback(_ready);

if (!ret)
return 0;
else if (ret == -EALREADY) {
fill_random_ptr_key(_ready);
return 0;
}

return ret;
}
early_initcall(initialize_ptr_random);

RE: [PATCH] checkpatch: Introduce check for format of Fixes line in commit log

[Parav Pandit]

Hi Joe,

> -Original Message-
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> ow...@vger.kernel.org] On Behalf Of Parav Pandit
> Sent: Tuesday, October 10, 2017 5:44 PM
> To: j...@perches.com; a...@canonical.com
> Cc: linux-kernel@vger.kernel.org; linux-r...@vger.kernel.org; Parav Pandit
> ; Brad Erickson 
> Subject: [PATCH] checkpatch: Introduce check for format of Fixes line in 
> commit
> log
> 
> This patch introduces a format check for 'Fixes' line in commit log for 12
> characters commit id and format for Fixes as ("...").
> 
> Its created against linux-rdma Doug's tree [1].
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git
> 
> Signed-off-by: Parav Pandit 
> Signed-off-by: Brad Erickson 
> ---
>  scripts/checkpatch.pl | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index
> dd2c262..7d933e4 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2548,6 +2548,14 @@ sub process {
> "Remove Gerrit Change-Id's before submitting
> upstream.\n" . $herecurr);
>   }
> 
> +# Check for incorrect format for Fixes line in commit log
> + if ($in_commit_log && $line =~ /^\s*Fixes:/i) {
> + if ($line !~ /^\s*Fixes: [a-z0-9]{12} \(\".*?\"\)$/i) {
> + ERROR("FIXES_FORMAT",
> +   "Follow format of Fixes: <12 characters
> commit id> (\"...\")\n" . $herecurr);
> + }
> + }
> +
>  # Check if the commit log is in a possible stack dump
>   if ($in_commit_log && !$commit_log_possible_stack_dump &&
>   ($line =~ /^\s*(?:WARNING:|BUG:)/ ||
> --

Did you get a chance to review this minor improvement patch?
Does it look ok?
Can it be pushed to Linux-rdma tree?

RE: [PATCH] checkpatch: Introduce check for format of Fixes line in commit log

[Parav Pandit]

Hi Joe,

> -Original Message-
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> ow...@vger.kernel.org] On Behalf Of Parav Pandit
> Sent: Tuesday, October 10, 2017 5:44 PM
> To: j...@perches.com; a...@canonical.com
> Cc: linux-kernel@vger.kernel.org; linux-r...@vger.kernel.org; Parav Pandit
> ; Brad Erickson 
> Subject: [PATCH] checkpatch: Introduce check for format of Fixes line in 
> commit
> log
> 
> This patch introduces a format check for 'Fixes' line in commit log for 12
> characters commit id and format for Fixes as ("...").
> 
> Its created against linux-rdma Doug's tree [1].
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git
> 
> Signed-off-by: Parav Pandit 
> Signed-off-by: Brad Erickson 
> ---
>  scripts/checkpatch.pl | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index
> dd2c262..7d933e4 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2548,6 +2548,14 @@ sub process {
> "Remove Gerrit Change-Id's before submitting
> upstream.\n" . $herecurr);
>   }
> 
> +# Check for incorrect format for Fixes line in commit log
> + if ($in_commit_log && $line =~ /^\s*Fixes:/i) {
> + if ($line !~ /^\s*Fixes: [a-z0-9]{12} \(\".*?\"\)$/i) {
> + ERROR("FIXES_FORMAT",
> +   "Follow format of Fixes: <12 characters
> commit id> (\"...\")\n" . $herecurr);
> + }
> + }
> +
>  # Check if the commit log is in a possible stack dump
>   if ($in_commit_log && !$commit_log_possible_stack_dump &&
>   ($line =~ /^\s*(?:WARNING:|BUG:)/ ||
> --

Did you get a chance to review this minor improvement patch?
Does it look ok?
Can it be pushed to Linux-rdma tree?

Re: [PATCH] powerpc: ipic - fix status get and status clear

[Christophe LEROY]

Le 19/10/2017 à 07:06, Michael Ellerman a écrit :

Christophe Leroy  writes:


IPIC Status is provided by register IPIC_SERSR and not by IPIC_SERMR
which is the mask register.


This seems like it would be a bad bug. But I guess it hasn't mattered
for some reason?


As far as I can see, this function has been added in kernel 2.6.12 but 
has never been used in-tree.


I have discovered this error while implementing NMI watchdog on a 832x 
board, ie this function is needed to know when a machine check exception 
is generated by the watchdog timer.


Christophe



cheers


diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 16f1edd78c40..535cf1f6941c 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -846,12 +846,12 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
  
  u32 ipic_get_mcp_status(void)

  {
-   return ipic_read(primary_ipic->regs, IPIC_SERMR);
+   return ipic_read(primary_ipic->regs, IPIC_SERSR);
  }
  
  void ipic_clear_mcp_status(u32 mask)

  {
-   ipic_write(primary_ipic->regs, IPIC_SERMR, mask);
+   ipic_write(primary_ipic->regs, IPIC_SERSR, mask);
  }
  
  /* Return an interrupt vector or 0 if no interrupt is pending. */

--
2.13.3

Re: [PATCH] powerpc: ipic - fix status get and status clear

[Christophe LEROY]

Le 19/10/2017 à 07:06, Michael Ellerman a écrit :

Christophe Leroy  writes:


IPIC Status is provided by register IPIC_SERSR and not by IPIC_SERMR
which is the mask register.


This seems like it would be a bad bug. But I guess it hasn't mattered
for some reason?


As far as I can see, this function has been added in kernel 2.6.12 but 
has never been used in-tree.


I have discovered this error while implementing NMI watchdog on a 832x 
board, ie this function is needed to know when a machine check exception 
is generated by the watchdog timer.


Christophe



cheers


diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 16f1edd78c40..535cf1f6941c 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -846,12 +846,12 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
  
  u32 ipic_get_mcp_status(void)

  {
-   return ipic_read(primary_ipic->regs, IPIC_SERMR);
+   return ipic_read(primary_ipic->regs, IPIC_SERSR);
  }
  
  void ipic_clear_mcp_status(u32 mask)

  {
-   ipic_write(primary_ipic->regs, IPIC_SERMR, mask);
+   ipic_write(primary_ipic->regs, IPIC_SERSR, mask);
  }
  
  /* Return an interrupt vector or 0 if no interrupt is pending. */

--
2.13.3

[PATCH V2] bcma: Use bcma_debug and not pr_cont in MIPS driver

[Joe Perches]

Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
converted a printk(KERN_DEBUG to bcma_debug.

bcma_debug is guarded by a #define DEBUG via pr_debug.

This means that the bcma_debug will generally not be emitted
but any pr_cont following the bcma_debug will be emitted.

Correct this by removing the uses of pr_cont by using a temporary.

Signed-off-by: Joe Perches 
---

v2: Fix whitespace damage
I hear there's this checkpatch tool...

 drivers/bcma/driver_mips.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 5904ef1aa624..f040aba48d50 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
*dev, unsigned int irq)
 {
int i;
static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+   char interrupts[20];
+   char *ints = interrupts;
 
-   bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
-   for (i = 0; i <= 6; i++)
-   pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
-   pr_cont("\n");
+   for (i = 0; i < ARRAY_SIZE(irq_name); i++)
+   ints += sprintf(ints, " %s%c",
+   irq_name[i], i == irq ? '*' : ' ');
+
+   bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, interrupts);
 }
 
 static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
-- 
2.10.0.rc2.1.g053435c

[PATCH V2] bcma: Use bcma_debug and not pr_cont in MIPS driver

[Joe Perches]

Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
converted a printk(KERN_DEBUG to bcma_debug.

bcma_debug is guarded by a #define DEBUG via pr_debug.

This means that the bcma_debug will generally not be emitted
but any pr_cont following the bcma_debug will be emitted.

Correct this by removing the uses of pr_cont by using a temporary.

Signed-off-by: Joe Perches 
---

v2: Fix whitespace damage
I hear there's this checkpatch tool...

 drivers/bcma/driver_mips.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 5904ef1aa624..f040aba48d50 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
*dev, unsigned int irq)
 {
int i;
static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+   char interrupts[20];
+   char *ints = interrupts;
 
-   bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
-   for (i = 0; i <= 6; i++)
-   pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
-   pr_cont("\n");
+   for (i = 0; i < ARRAY_SIZE(irq_name); i++)
+   ints += sprintf(ints, " %s%c",
+   irq_name[i], i == irq ? '*' : ' ');
+
+   bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, interrupts);
 }
 
 static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
-- 
2.10.0.rc2.1.g053435c

Re: [PATCH] bcma: Use bcma_debug and not pr_cont in MIPS driver

[James Cameron]

On Wed, Oct 18, 2017 at 10:12:18PM -0700, Joe Perches wrote:
> Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
> converted a printk(KERN_DEBUG to bcma_debug.
> 
> bcma_debug is guarded by a #define DEBUG via pr_debug.
> 
> This means that the bcma_debug will generally not be emitted
> but any pr_cont following the bcma_debug will be emitted.
> 
> Correct this by removing the uses of pr_cont by using a temporary.
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/bcma/driver_mips.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
> index 5904ef1aa624..a929956150eb 100644
> --- a/drivers/bcma/driver_mips.c
> +++ b/drivers/bcma/driver_mips.c
> @@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
> *dev, unsigned int irq)
>  {
>   int i;
>   static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
> +char interrupts[20];
> +char *ints = interrupts;

Tabs were changed to spaces.

>  
> - bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
> - for (i = 0; i <= 6; i++)
> - pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
> - pr_cont("\n");
> +for (i = 0; i < ARRAY_SIZE(irq_name); i++)
> +ints += sprintf(ints, " %s%c",
> + irq_name[i], i == irq ? '*' : ' ');

But not on this line.

> +
> +bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, 
> interrupts);
>  }
>  
>  static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
> -- 
> 2.10.0.rc2.1.g053435c
> 

-- 
James Cameron
http://quozl.netrek.org/

Re: [PATCH] bcma: Use bcma_debug and not pr_cont in MIPS driver

[James Cameron]

On Wed, Oct 18, 2017 at 10:12:18PM -0700, Joe Perches wrote:
> Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
> converted a printk(KERN_DEBUG to bcma_debug.
> 
> bcma_debug is guarded by a #define DEBUG via pr_debug.
> 
> This means that the bcma_debug will generally not be emitted
> but any pr_cont following the bcma_debug will be emitted.
> 
> Correct this by removing the uses of pr_cont by using a temporary.
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/bcma/driver_mips.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
> index 5904ef1aa624..a929956150eb 100644
> --- a/drivers/bcma/driver_mips.c
> +++ b/drivers/bcma/driver_mips.c
> @@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
> *dev, unsigned int irq)
>  {
>   int i;
>   static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
> +char interrupts[20];
> +char *ints = interrupts;

Tabs were changed to spaces.

>  
> - bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
> - for (i = 0; i <= 6; i++)
> - pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
> - pr_cont("\n");
> +for (i = 0; i < ARRAY_SIZE(irq_name); i++)
> +ints += sprintf(ints, " %s%c",
> + irq_name[i], i == irq ? '*' : ' ');

But not on this line.

> +
> +bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, 
> interrupts);
>  }
>  
>  static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
> -- 
> 2.10.0.rc2.1.g053435c
> 

-- 
James Cameron
http://quozl.netrek.org/

Re: [PATCH] Documentation: Add myself to the enforcement statement list

[Greg Kroah-Hartman]

On Wed, Oct 18, 2017 at 04:20:47PM -0700, Laura Abbott wrote:
> I already Acked the patch, add my name to the list as well.
> 
> Signed-off-by: Laura Abbott 
> ---
>  Documentation/process/kernel-enforcement-statement.rst | 1 +
>  1 file changed, 1 insertion(+)

Thanks, now queued up.

greg k-h

Re: [PATCH] Documentation: Add myself to the enforcement statement list

[Greg Kroah-Hartman]

On Wed, Oct 18, 2017 at 04:20:47PM -0700, Laura Abbott wrote:
> I already Acked the patch, add my name to the list as well.
> 
> Signed-off-by: Laura Abbott 
> ---
>  Documentation/process/kernel-enforcement-statement.rst | 1 +
>  1 file changed, 1 insertion(+)

Thanks, now queued up.

greg k-h

Re: [PATCH v2] static_key: Improve uninizialized key warning

[Ingo Molnar]

* Steven Rostedt  wrote:

> On Wed, 18 Oct 2017 12:06:59 -0400
> Steven Rostedt  wrote:
> 
> > > Signed-off-by: Borislav Petkov 
> > > Cc: Hannes Frederic Sowa 
> > > Cc: Ingo Molnar 
> > > Cc: Juergen Gross 
> > > Cc: "Steven Rostedt (VMware)"   
> > 
> > Reviewed-by: Steven Rostedt (VMware) 
> > 
> 
> 
> Ingo, you may want to fix the typo in the subject "uninitialized".

Yeah, I also changed it to %pS to help debug arrays, or cases where somehow a 
non-data symbol ends up being passed to it. Plus I changed the message from:

  static_key_disable_cpuslocked, key virt_spin_lock_key used before call to 
jump_label_init.

to:

  static_key_disable_cpuslocked() static key 'virt_spin_lock_key' used before 
call to jump_label_init()

to make it all obviously readable. I mean, what if the symbol is named 'key' 
and 
we'd get confusing printouts like:

  static_key_disable_cpuslocked, key key used before call to jump_label_init.

Plus functions should be referred to with () to disambiguate them from other 
symbol names. Also, proper use of punctuation symbols.

Thanks,

Ingo

Re: [PATCH v2] static_key: Improve uninizialized key warning

[Ingo Molnar]

* Steven Rostedt  wrote:

> On Wed, 18 Oct 2017 12:06:59 -0400
> Steven Rostedt  wrote:
> 
> > > Signed-off-by: Borislav Petkov 
> > > Cc: Hannes Frederic Sowa 
> > > Cc: Ingo Molnar 
> > > Cc: Juergen Gross 
> > > Cc: "Steven Rostedt (VMware)"   
> > 
> > Reviewed-by: Steven Rostedt (VMware) 
> > 
> 
> 
> Ingo, you may want to fix the typo in the subject "uninitialized".

Yeah, I also changed it to %pS to help debug arrays, or cases where somehow a 
non-data symbol ends up being passed to it. Plus I changed the message from:

  static_key_disable_cpuslocked, key virt_spin_lock_key used before call to 
jump_label_init.

to:

  static_key_disable_cpuslocked() static key 'virt_spin_lock_key' used before 
call to jump_label_init()

to make it all obviously readable. I mean, what if the symbol is named 'key' 
and 
we'd get confusing printouts like:

  static_key_disable_cpuslocked, key key used before call to jump_label_init.

Plus functions should be referred to with () to disambiguate them from other 
symbol names. Also, proper use of punctuation symbols.

Thanks,

Ingo

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

[Sricharan R]

Hi Bjorn,

On 10/17/2017 12:28 AM, Bjorn Andersson wrote:
> On Mon 16 Oct 06:19 PDT 2017, Dwivedi, Avaneesh Kumar (avani) wrote:
>> On 10/12/2017 11:50 PM, Bjorn Andersson wrote:
> [..]
>>> Please fix this and add my Acked-by
>> Sure will do, just want to ask, when i am sending updated patches, should i
>> sent all the 4 patches in series or should skip those which are acked by
>> you.
> 
> Please include all the patches in each submission (unless the maintainer
> tells you that he/she applied some of them already).
> 
> Make sure to add any "Acked-by", "Reviewed-by" and "Tested-by" on your
> patches, so that people know that they are already reviewed/tested.
> 
> 
> And finally, when resending patches, add a section between "---" and the
> diff-stat describing changes since the last version - that makes it
> easier to see what changed since last time and will help the reviewer
> look at the "new" things.

 So i was having the ipq8074 q6 remoteproc support based on this series last 
time.
 With the msm8996 now getting cleared, i will rebase my patches and repost.
 Otherwise, just hope that you were ok with the approach [1].

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1481961.html


Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

[Sricharan R]

Hi Bjorn,

On 10/17/2017 12:28 AM, Bjorn Andersson wrote:
> On Mon 16 Oct 06:19 PDT 2017, Dwivedi, Avaneesh Kumar (avani) wrote:
>> On 10/12/2017 11:50 PM, Bjorn Andersson wrote:
> [..]
>>> Please fix this and add my Acked-by
>> Sure will do, just want to ask, when i am sending updated patches, should i
>> sent all the 4 patches in series or should skip those which are acked by
>> you.
> 
> Please include all the patches in each submission (unless the maintainer
> tells you that he/she applied some of them already).
> 
> Make sure to add any "Acked-by", "Reviewed-by" and "Tested-by" on your
> patches, so that people know that they are already reviewed/tested.
> 
> 
> And finally, when resending patches, add a section between "---" and the
> diff-stat describing changes since the last version - that makes it
> easier to see what changed since last time and will help the reviewer
> look at the "new" things.

 So i was having the ipq8074 q6 remoteproc support based on this series last 
time.
 With the msm8996 now getting cleared, i will rebase my patches and repost.
 Otherwise, just hope that you were ok with the approach [1].

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1481961.html


Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Guenter Roeck]

On 10/18/2017 07:28 PM, Shu Wang wrote:

From: "Guenter Roeck" 
To: "Shu Wang" 
Cc: "fenghua yu" , jdelv...@suse.com, 
linux-hw...@vger.kernel.org,
linux-kernel@vger.kernel.org, ch...@redhat.com, yiz...@redhat.com
Sent: Wednesday, October 18, 2017 9:14:39 PM
Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core 
id

On 10/17/2017 08:21 PM, Shu Wang wrote:

From: "Guenter Roeck" 
To: shuw...@redhat.com
Cc: "fenghua yu" , jdelv...@suse.com,
linux-hw...@vger.kernel.org,
linux-kernel@vger.kernel.org, ch...@redhat.com, yiz...@redhat.com
Sent: Tuesday, October 17, 2017 11:25:50 PM
Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same
core id

On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuw...@redhat.com wrote:

From: Shu Wang 

Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
when it's online.


What system/cpu is that ?

Normally I would assume that each CPU (package) instantiates
a separate instance of the driver.


The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.




- coretemp_cpu_online(cpu=0)
- create_core_data(cpu=0, attr_no=2)
 - create_core_attrs(attr_no=2)
- coretemp_cpu_online(cpu=1)
- create_core_data(cpu=1, attr_no=2)
 - create_core_attrs(attr_no=2)

$ grep -e processor -e 'core id' /proc/cpuinfo
processor   : 0
core id : 0
processor   : 1
core id : 0
processor   : 2
core id : 1
processor   : 3
core id : 1


Complete output of /proc/cpuinfo might be helpful.


$ cat /proc/cpuinfo
processor   : 0
vendor_id   : GenuineIntel
cpu family  : 6
model   : 61
model name  : Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz


This is a hyperthreading CPU, which should already be handled,


Do you mean that for my system, coretemp_cpu_online should only
be called twice instead of four times to create two core attrs?



coretemp_add_core() should only be called twice, and cpumask_intersects()
should filter out the duplicate ones.

/*
 * Check whether a thread sibling is already online. If not add the
 * interface for this CPU core.
 */
if (!cpumask_intersects(>cpumask, topology_sibling_cpumask(cpu)))
coretemp_add_core(pdev, cpu, 0);

Thomas, is it possible that something is wrong with this code ?

Guenter


and the problem would affect pretty much everyone. I'll have
to look into this more closely. Is this with the ToT kernel ?


What's a ToT kernel? The kernel I built was the latest
kernel-4.14.0_rc5+ from linus repo.



ToT => Top Of Tree. My apologies, I thought that was a commonly used acronym.

Guenter

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Guenter Roeck]

On 10/18/2017 07:28 PM, Shu Wang wrote:

From: "Guenter Roeck" 
To: "Shu Wang" 
Cc: "fenghua yu" , jdelv...@suse.com, 
linux-hw...@vger.kernel.org,
linux-kernel@vger.kernel.org, ch...@redhat.com, yiz...@redhat.com
Sent: Wednesday, October 18, 2017 9:14:39 PM
Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core 
id

On 10/17/2017 08:21 PM, Shu Wang wrote:

From: "Guenter Roeck" 
To: shuw...@redhat.com
Cc: "fenghua yu" , jdelv...@suse.com,
linux-hw...@vger.kernel.org,
linux-kernel@vger.kernel.org, ch...@redhat.com, yiz...@redhat.com
Sent: Tuesday, October 17, 2017 11:25:50 PM
Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same
core id

On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuw...@redhat.com wrote:

From: Shu Wang 

Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
when it's online.


What system/cpu is that ?

Normally I would assume that each CPU (package) instantiates
a separate instance of the driver.


The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.




- coretemp_cpu_online(cpu=0)
- create_core_data(cpu=0, attr_no=2)
 - create_core_attrs(attr_no=2)
- coretemp_cpu_online(cpu=1)
- create_core_data(cpu=1, attr_no=2)
 - create_core_attrs(attr_no=2)

$ grep -e processor -e 'core id' /proc/cpuinfo
processor   : 0
core id : 0
processor   : 1
core id : 0
processor   : 2
core id : 1
processor   : 3
core id : 1


Complete output of /proc/cpuinfo might be helpful.


$ cat /proc/cpuinfo
processor   : 0
vendor_id   : GenuineIntel
cpu family  : 6
model   : 61
model name  : Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz


This is a hyperthreading CPU, which should already be handled,


Do you mean that for my system, coretemp_cpu_online should only
be called twice instead of four times to create two core attrs?



coretemp_add_core() should only be called twice, and cpumask_intersects()
should filter out the duplicate ones.

/*
 * Check whether a thread sibling is already online. If not add the
 * interface for this CPU core.
 */
if (!cpumask_intersects(>cpumask, topology_sibling_cpumask(cpu)))
coretemp_add_core(pdev, cpu, 0);

Thomas, is it possible that something is wrong with this code ?

Guenter


and the problem would affect pretty much everyone. I'll have
to look into this more closely. Is this with the ToT kernel ?


What's a ToT kernel? The kernel I built was the latest
kernel-4.14.0_rc5+ from linus repo.



ToT => Top Of Tree. My apologies, I thought that was a commonly used acronym.

Guenter

Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

On 2017년 10월 19일 13:59, Chanwoo Choi wrote:
> Hi,
> 
> On 2017년 10월 19일 12:26, Raveendra Padasalagi wrote:
>> Add changes to capture optional dt attribute "debounce-timeout-ms"
>> provided in extcon node and used the same value if provided otherwise
>> default value of 20ms is used for id and vbus gpios debounce time.
>>
>> Signed-off-by: Raveendra Padasalagi 
>> Reviewed-by: Ray Jui 
>> Reviewed-by: Srinath Mannam 
>> ---
>>
>> Changes in v2:
>>  Rename gpio_debounce_timeout_ms to debounce_usecs
>>
>>  drivers/extcon/extcon-usb-gpio.c | 12 +---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c 
>> b/drivers/extcon/extcon-usb-gpio.c
>> index 9c925b0..76ef1da 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -41,6 +41,7 @@ struct usb_extcon_info {
>>  
>>  unsigned long debounce_jiffies;
>>  struct delayed_work wq_detcable;
>> +unsigned int debounce_usecs;
>>  };
>>  
>>  static const unsigned int usb_extcon_cable[] = {
>> @@ -133,6 +134,11 @@ static int usb_extcon_probe(struct platform_device 
>> *pdev)
>>  if (IS_ERR(info->vbus_gpiod))
>>  return PTR_ERR(info->vbus_gpiod);
>>  
>> +ret = of_property_read_u32(np, "input-debounce",
>> + >debounce_usecs);
>> +if (ret)
>> +info->debounce_usecs = USB_GPIO_DEBOUNCE_MS;
> 
> The USB_GPIO_DEBOUNCE_MS indicates 20 millisecond.
> You need to redefine it as following:
>   -#define USB_GPIO_DEBOUNCE_MS   20  /* ms */
>   +#define USB_GPIO_DEBOUNCE_USEC 2
> 
>   info->debounce_usecs = USB_GPIO_DEBOUNCE_USEC;
> 
> or
>   info->debounce_usecs = USB_GPIO_DEBOUNCE_MS * 1000;
> 
> 
>> +
>>  info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
>>  if (IS_ERR(info->edev)) {
>>  dev_err(dev, "failed to allocate extcon device\n");
>> @@ -147,13 +153,13 @@ static int usb_extcon_probe(struct platform_device 
>> *pdev)
>>  
>>  if (info->id_gpiod)
>>  ret = gpiod_set_debounce(info->id_gpiod,
>> - USB_GPIO_DEBOUNCE_MS * 1000);
>> + info->debounce_usecs * 1000);
> 
> The debounce_usecs is already microsecond, You don't need to mutiply with 
> 1000.
> 
> 
>>  if (!ret && info->vbus_gpiod)
>>  ret = gpiod_set_debounce(info->vbus_gpiod,
>> - USB_GPIO_DEBOUNCE_MS * 1000);
>> + info->debounce_usecs * 1000);

You don't need to mutiply with 1000.

>>  
>>  if (ret < 0)
>> -info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
>> +info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);

You should you the 'usecs_to_jiffies' because info->debounce_usecs indicates 
the usec.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

On 2017년 10월 19일 13:59, Chanwoo Choi wrote:
> Hi,
> 
> On 2017년 10월 19일 12:26, Raveendra Padasalagi wrote:
>> Add changes to capture optional dt attribute "debounce-timeout-ms"
>> provided in extcon node and used the same value if provided otherwise
>> default value of 20ms is used for id and vbus gpios debounce time.
>>
>> Signed-off-by: Raveendra Padasalagi 
>> Reviewed-by: Ray Jui 
>> Reviewed-by: Srinath Mannam 
>> ---
>>
>> Changes in v2:
>>  Rename gpio_debounce_timeout_ms to debounce_usecs
>>
>>  drivers/extcon/extcon-usb-gpio.c | 12 +---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c 
>> b/drivers/extcon/extcon-usb-gpio.c
>> index 9c925b0..76ef1da 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -41,6 +41,7 @@ struct usb_extcon_info {
>>  
>>  unsigned long debounce_jiffies;
>>  struct delayed_work wq_detcable;
>> +unsigned int debounce_usecs;
>>  };
>>  
>>  static const unsigned int usb_extcon_cable[] = {
>> @@ -133,6 +134,11 @@ static int usb_extcon_probe(struct platform_device 
>> *pdev)
>>  if (IS_ERR(info->vbus_gpiod))
>>  return PTR_ERR(info->vbus_gpiod);
>>  
>> +ret = of_property_read_u32(np, "input-debounce",
>> + >debounce_usecs);
>> +if (ret)
>> +info->debounce_usecs = USB_GPIO_DEBOUNCE_MS;
> 
> The USB_GPIO_DEBOUNCE_MS indicates 20 millisecond.
> You need to redefine it as following:
>   -#define USB_GPIO_DEBOUNCE_MS   20  /* ms */
>   +#define USB_GPIO_DEBOUNCE_USEC 2
> 
>   info->debounce_usecs = USB_GPIO_DEBOUNCE_USEC;
> 
> or
>   info->debounce_usecs = USB_GPIO_DEBOUNCE_MS * 1000;
> 
> 
>> +
>>  info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
>>  if (IS_ERR(info->edev)) {
>>  dev_err(dev, "failed to allocate extcon device\n");
>> @@ -147,13 +153,13 @@ static int usb_extcon_probe(struct platform_device 
>> *pdev)
>>  
>>  if (info->id_gpiod)
>>  ret = gpiod_set_debounce(info->id_gpiod,
>> - USB_GPIO_DEBOUNCE_MS * 1000);
>> + info->debounce_usecs * 1000);
> 
> The debounce_usecs is already microsecond, You don't need to mutiply with 
> 1000.
> 
> 
>>  if (!ret && info->vbus_gpiod)
>>  ret = gpiod_set_debounce(info->vbus_gpiod,
>> - USB_GPIO_DEBOUNCE_MS * 1000);
>> + info->debounce_usecs * 1000);

You don't need to mutiply with 1000.

>>  
>>  if (ret < 0)
>> -info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
>> +info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);

You should you the 'usecs_to_jiffies' because info->debounce_usecs indicates 
the usec.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v3 1/6] remoteproc: qcom: mdt_loader: Make the firmware authentication optional

[Sricharan R]

Hi Bjorn,

On 10/12/2017 11:56 PM, Bjorn Andersson wrote:
> On Wed 30 Aug 21:45 PDT 2017, Sricharan R wrote:
> 
>> qcom_mdt_load function loads the mdt type firmware and
>> initialises the secure memory as well. Make the initialisation only
>> when requested by the caller, so that the function can be used
>> by self-authenticating remoteproc as well.
>>
>> Signed-off-by: Sricharan R 
>> ---
>>  drivers/soc/qcom/mdt_loader.c   | 70 
>> +++--
>>  include/linux/soc/qcom/mdt_loader.h |  3 ++
>>  2 files changed, 62 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
>> index bd63df0..851f5d7 100644
>> --- a/drivers/soc/qcom/mdt_loader.c
>> +++ b/drivers/soc/qcom/mdt_loader.c
>> @@ -86,9 +86,9 @@ ssize_t qcom_mdt_get_size(const struct firmware *fw)
>>   *
>>   * Returns 0 on success, negative errno otherwise.
>>   */
> 
> This kerneldoc is now lacks @pas_init, but as it's just an internal
> function and you have kerneldoc on the public functions I suggest that
> you drop it.
> 

 Sure. Will change.

>> -int qcom_mdt_load(struct device *dev, const struct firmware *fw,
>> -  const char *firmware, int pas_id, void *mem_region,
>> -  phys_addr_t mem_phys, size_t mem_size)
>> +static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>> +   const char *firmware, int pas_id, void *mem_region,
>> +   phys_addr_t mem_phys, size_t mem_size, bool pas_init)
> 
> With this you have my Acked-by.
> 

 Thanks.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH v3 1/6] remoteproc: qcom: mdt_loader: Make the firmware authentication optional

[Sricharan R]

Hi Bjorn,

On 10/12/2017 11:56 PM, Bjorn Andersson wrote:
> On Wed 30 Aug 21:45 PDT 2017, Sricharan R wrote:
> 
>> qcom_mdt_load function loads the mdt type firmware and
>> initialises the secure memory as well. Make the initialisation only
>> when requested by the caller, so that the function can be used
>> by self-authenticating remoteproc as well.
>>
>> Signed-off-by: Sricharan R 
>> ---
>>  drivers/soc/qcom/mdt_loader.c   | 70 
>> +++--
>>  include/linux/soc/qcom/mdt_loader.h |  3 ++
>>  2 files changed, 62 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
>> index bd63df0..851f5d7 100644
>> --- a/drivers/soc/qcom/mdt_loader.c
>> +++ b/drivers/soc/qcom/mdt_loader.c
>> @@ -86,9 +86,9 @@ ssize_t qcom_mdt_get_size(const struct firmware *fw)
>>   *
>>   * Returns 0 on success, negative errno otherwise.
>>   */
> 
> This kerneldoc is now lacks @pas_init, but as it's just an internal
> function and you have kerneldoc on the public functions I suggest that
> you drop it.
> 

 Sure. Will change.

>> -int qcom_mdt_load(struct device *dev, const struct firmware *fw,
>> -  const char *firmware, int pas_id, void *mem_region,
>> -  phys_addr_t mem_phys, size_t mem_size)
>> +static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>> +   const char *firmware, int pas_id, void *mem_region,
>> +   phys_addr_t mem_phys, size_t mem_size, bool pas_init)
> 
> With this you have my Acked-by.
> 

 Thanks.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH v3 1/7] ACPI/PPTT: Add Processor Properties Topology Table parsing

[Tomasz Nowicki]

On 18.10.2017 19:30, Jeremy Linton wrote:

On 10/18/2017 05:24 AM, Tomasz Nowicki wrote:

On 18.10.2017 07:39, Tomasz Nowicki wrote:

Hi,

On 17.10.2017 17:22, Jeremy Linton wrote:

Hi,

On 10/17/2017 08:25 AM, Tomasz Nowicki wrote:

Hi Jeremy,

I did second round of review and have some more comments, please 
see below:


On 12.10.2017 21:48, Jeremy Linton wrote:

ACPI 6.2 adds a new table, which describes how processing units
are related to each other in tree like fashion. Caches are
also sprinkled throughout the tree and describe the properties
of the caches in relation to other caches and processing units.

Add the code to parse the cache hierarchy and report the total
number of levels of cache for a given core using
acpi_find_last_cache_level() as well as fill out the individual
cores cache information with cache_setup_acpi() once the
cpu_cacheinfo structure has been populated by the arch specific
code.

Further, report peers in the topology using setup_acpi_cpu_topology()
to report a unique ID for each processing unit at a given level
in the tree. These unique id's can then be used to match related
processing units which exist as threads, COD (clusters
on die), within a given package, etc.

Signed-off-by: Jeremy Linton 
---
  drivers/acpi/pptt.c | 485 


  1 file changed, 485 insertions(+)
  create mode 100644 drivers/acpi/pptt.c

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
new file mode 100644
index ..c86715fed4a7
--- /dev/null
+++ b/drivers/acpi/pptt.c
@@ -0,1 +1,485 @@
+/*
+ * Copyright (C) 2017, ARM
+ *
+ * This program is free software; you can redistribute it and/or 
modify it

+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but 
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
License for

+ * more details.
+ *
+ * This file implements parsing of Processor Properties Topology 
Table (PPTT)
+ * which is optionally used to describe the processor and cache 
topology.
+ * Due to the relative pointers used throughout the table, this 
doesn't

+ * leverage the existing subtable parsing in the kernel.
+ */
+#define pr_fmt(fmt) "ACPI PPTT: " fmt
+
+#include 
+#include 
+#include 
+
+/*
+ * Given the PPTT table, find and verify that the subtable entry
+ * is located within the table
+ */
+static struct acpi_subtable_header *fetch_pptt_subtable(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    struct acpi_subtable_header *entry;
+
+    /* there isn't a subtable at reference 0 */
+    if (!pptt_ref)
+    return NULL;
+
+    if (pptt_ref + sizeof(struct acpi_subtable_header) > 
table_hdr->length)

+    return NULL;
+
+    entry = (struct acpi_subtable_header *)((u8 *)table_hdr + 
pptt_ref);

+
+    if (pptt_ref + entry->length > table_hdr->length)
+    return NULL;
+
+    return entry;
+}
+
+static struct acpi_pptt_processor *fetch_pptt_node(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    return (struct acpi_pptt_processor 
*)fetch_pptt_subtable(table_hdr, pptt_ref);

+}
+
+static struct acpi_pptt_cache *fetch_pptt_cache(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    return (struct acpi_pptt_cache 
*)fetch_pptt_subtable(table_hdr, pptt_ref);

+}
+
+static struct acpi_subtable_header *acpi_get_pptt_resource(
+    struct acpi_table_header *table_hdr,
+    struct acpi_pptt_processor *node, int resource)
+{
+    u32 ref;
+
+    if (resource >= node->number_of_priv_resources)
+    return NULL;
+
+    ref = *(u32 *)((u8 *)node + sizeof(struct acpi_pptt_processor) +
+  sizeof(u32) * resource);
+
+    return fetch_pptt_subtable(table_hdr, ref);
+}
+
+/*
+ * given a pptt resource, verify that it is a cache node, then walk
+ * down each level of caches, counting how many levels are found
+ * as well as checking the cache type (icache, dcache, unified). 
If a

+ * level & type match, then we set found, and continue the search.
+ * Once the entire cache branch has been walked return its max
+ * depth.
+ */
+static int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
+    int local_level,
+    struct acpi_subtable_header *res,
+    struct acpi_pptt_cache **found,
+    int level, int type)
+{
+    struct acpi_pptt_cache *cache;
+
+    if (res->type != ACPI_PPTT_TYPE_CACHE)
+    return 0;
+
+    cache = (struct acpi_pptt_cache *) res;
+    while (cache) {
+    local_level++;
+
+    if ((local_level == level) &&
+    (cache->flags & ACPI_PPTT_CACHE_TYPE_VALID) &&
+    ((cache->attributes & ACPI_PPTT_MASK_CACHE_TYPE) == 
type)) {


Attributes have to be shifted:

(cache->attributes & ACPI_PPTT_MASK_CACHE_TYPE) >> 2

Re: [PATCH v3 1/7] ACPI/PPTT: Add Processor Properties Topology Table parsing

[Tomasz Nowicki]

On 18.10.2017 19:30, Jeremy Linton wrote:

On 10/18/2017 05:24 AM, Tomasz Nowicki wrote:

On 18.10.2017 07:39, Tomasz Nowicki wrote:

Hi,

On 17.10.2017 17:22, Jeremy Linton wrote:

Hi,

On 10/17/2017 08:25 AM, Tomasz Nowicki wrote:

Hi Jeremy,

I did second round of review and have some more comments, please 
see below:


On 12.10.2017 21:48, Jeremy Linton wrote:

ACPI 6.2 adds a new table, which describes how processing units
are related to each other in tree like fashion. Caches are
also sprinkled throughout the tree and describe the properties
of the caches in relation to other caches and processing units.

Add the code to parse the cache hierarchy and report the total
number of levels of cache for a given core using
acpi_find_last_cache_level() as well as fill out the individual
cores cache information with cache_setup_acpi() once the
cpu_cacheinfo structure has been populated by the arch specific
code.

Further, report peers in the topology using setup_acpi_cpu_topology()
to report a unique ID for each processing unit at a given level
in the tree. These unique id's can then be used to match related
processing units which exist as threads, COD (clusters
on die), within a given package, etc.

Signed-off-by: Jeremy Linton 
---
  drivers/acpi/pptt.c | 485 


  1 file changed, 485 insertions(+)
  create mode 100644 drivers/acpi/pptt.c

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
new file mode 100644
index ..c86715fed4a7
--- /dev/null
+++ b/drivers/acpi/pptt.c
@@ -0,1 +1,485 @@
+/*
+ * Copyright (C) 2017, ARM
+ *
+ * This program is free software; you can redistribute it and/or 
modify it

+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but 
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
License for

+ * more details.
+ *
+ * This file implements parsing of Processor Properties Topology 
Table (PPTT)
+ * which is optionally used to describe the processor and cache 
topology.
+ * Due to the relative pointers used throughout the table, this 
doesn't

+ * leverage the existing subtable parsing in the kernel.
+ */
+#define pr_fmt(fmt) "ACPI PPTT: " fmt
+
+#include 
+#include 
+#include 
+
+/*
+ * Given the PPTT table, find and verify that the subtable entry
+ * is located within the table
+ */
+static struct acpi_subtable_header *fetch_pptt_subtable(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    struct acpi_subtable_header *entry;
+
+    /* there isn't a subtable at reference 0 */
+    if (!pptt_ref)
+    return NULL;
+
+    if (pptt_ref + sizeof(struct acpi_subtable_header) > 
table_hdr->length)

+    return NULL;
+
+    entry = (struct acpi_subtable_header *)((u8 *)table_hdr + 
pptt_ref);

+
+    if (pptt_ref + entry->length > table_hdr->length)
+    return NULL;
+
+    return entry;
+}
+
+static struct acpi_pptt_processor *fetch_pptt_node(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    return (struct acpi_pptt_processor 
*)fetch_pptt_subtable(table_hdr, pptt_ref);

+}
+
+static struct acpi_pptt_cache *fetch_pptt_cache(
+    struct acpi_table_header *table_hdr, u32 pptt_ref)
+{
+    return (struct acpi_pptt_cache 
*)fetch_pptt_subtable(table_hdr, pptt_ref);

+}
+
+static struct acpi_subtable_header *acpi_get_pptt_resource(
+    struct acpi_table_header *table_hdr,
+    struct acpi_pptt_processor *node, int resource)
+{
+    u32 ref;
+
+    if (resource >= node->number_of_priv_resources)
+    return NULL;
+
+    ref = *(u32 *)((u8 *)node + sizeof(struct acpi_pptt_processor) +
+  sizeof(u32) * resource);
+
+    return fetch_pptt_subtable(table_hdr, ref);
+}
+
+/*
+ * given a pptt resource, verify that it is a cache node, then walk
+ * down each level of caches, counting how many levels are found
+ * as well as checking the cache type (icache, dcache, unified). 
If a

+ * level & type match, then we set found, and continue the search.
+ * Once the entire cache branch has been walked return its max
+ * depth.
+ */
+static int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
+    int local_level,
+    struct acpi_subtable_header *res,
+    struct acpi_pptt_cache **found,
+    int level, int type)
+{
+    struct acpi_pptt_cache *cache;
+
+    if (res->type != ACPI_PPTT_TYPE_CACHE)
+    return 0;
+
+    cache = (struct acpi_pptt_cache *) res;
+    while (cache) {
+    local_level++;
+
+    if ((local_level == level) &&
+    (cache->flags & ACPI_PPTT_CACHE_TYPE_VALID) &&
+    ((cache->attributes & ACPI_PPTT_MASK_CACHE_TYPE) == 
type)) {


Attributes have to be shifted:

(cache->attributes & ACPI_PPTT_MASK_CACHE_TYPE) >> 2


Hmmm, I'm not sure that 

[PATCH] bcma: Use bcma_debug and not pr_cont in MIPS driver

[Joe Perches]

Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
converted a printk(KERN_DEBUG to bcma_debug.

bcma_debug is guarded by a #define DEBUG via pr_debug.

This means that the bcma_debug will generally not be emitted
but any pr_cont following the bcma_debug will be emitted.

Correct this by removing the uses of pr_cont by using a temporary.

Signed-off-by: Joe Perches 
---
 drivers/bcma/driver_mips.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 5904ef1aa624..a929956150eb 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
*dev, unsigned int irq)
 {
int i;
static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+char interrupts[20];
+char *ints = interrupts;
 
-   bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
-   for (i = 0; i <= 6; i++)
-   pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
-   pr_cont("\n");
+for (i = 0; i < ARRAY_SIZE(irq_name); i++)
+ints += sprintf(ints, " %s%c",
+   irq_name[i], i == irq ? '*' : ' ');
+
+bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, interrupts);
 }
 
 static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
-- 
2.10.0.rc2.1.g053435c

[PATCH] bcma: Use bcma_debug and not pr_cont in MIPS driver

[Joe Perches]

Commit 66cc04424960 ("bcma: use bcma_debug and pr_cont in MIPS driver")
converted a printk(KERN_DEBUG to bcma_debug.

bcma_debug is guarded by a #define DEBUG via pr_debug.

This means that the bcma_debug will generally not be emitted
but any pr_cont following the bcma_debug will be emitted.

Correct this by removing the uses of pr_cont by using a temporary.

Signed-off-by: Joe Perches 
---
 drivers/bcma/driver_mips.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 5904ef1aa624..a929956150eb 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -184,11 +184,14 @@ static void bcma_core_mips_print_irq(struct bcma_device 
*dev, unsigned int irq)
 {
int i;
static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+char interrupts[20];
+char *ints = interrupts;
 
-   bcma_debug(dev->bus, "core 0x%04x, irq :", dev->id.id);
-   for (i = 0; i <= 6; i++)
-   pr_cont(" %s%s", irq_name[i], i == irq ? "*" : " ");
-   pr_cont("\n");
+for (i = 0; i < ARRAY_SIZE(irq_name); i++)
+ints += sprintf(ints, " %s%c",
+   irq_name[i], i == irq ? '*' : ' ');
+
+bcma_debug(dev->bus, "core 0x%04x, irq:%s\n", dev->id.id, interrupts);
 }
 
 static void bcma_core_mips_dump_irq(struct bcma_bus *bus)
-- 
2.10.0.rc2.1.g053435c

Re: [PATCH] perf test shell: trace+probe_libc_inet_pton.sh: force to use /bin/bash to load this script

[Li Zhijian]

ignore this patch please, it will broken the test description which is read 
from the first line of this script

Thanks


On 10/19/2017 11:53 AM, Li Zhijian wrote:

this script contains Array, but not all sh support Array. by default, dash 
provides sh at ubuntu/debian
which can not support Array. so force to use /bin/bash

it can fix following issue:
lizhijian@localhost:~/lkp/linux/tools/perf/tests/shell$ sudo 
./trace+probe_libc_inet_pton.sh
[sudo] password for lizhijian: PING ::1(::1) 56 data bytes
./trace+probe_libc_inet_pton.sh: 30: ./trace+probe_libc_inet_pton.sh: Bad 
substitution
./trace+probe_libc_inet_pton.sh: 32: ./trace+probe_libc_inet_pton.sh: Bad 
substitution

Signed-off-by: Li Zhijian 
---
  tools/perf/tests/shell/trace+probe_libc_inet_pton.sh | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh 
b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
index 7a84d73..df04cc1 100755
--- a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
  # probe libc's inet_pton & backtrace it with ping
  
  # Installs a probe on libc's inet_pton function, that will use uprobes,

Re: [PATCH] perf test shell: trace+probe_libc_inet_pton.sh: force to use /bin/bash to load this script

[Li Zhijian]

ignore this patch please, it will broken the test description which is read 
from the first line of this script

Thanks


On 10/19/2017 11:53 AM, Li Zhijian wrote:

this script contains Array, but not all sh support Array. by default, dash 
provides sh at ubuntu/debian
which can not support Array. so force to use /bin/bash

it can fix following issue:
lizhijian@localhost:~/lkp/linux/tools/perf/tests/shell$ sudo 
./trace+probe_libc_inet_pton.sh
[sudo] password for lizhijian: PING ::1(::1) 56 data bytes
./trace+probe_libc_inet_pton.sh: 30: ./trace+probe_libc_inet_pton.sh: Bad 
substitution
./trace+probe_libc_inet_pton.sh: 32: ./trace+probe_libc_inet_pton.sh: Bad 
substitution

Signed-off-by: Li Zhijian 
---
  tools/perf/tests/shell/trace+probe_libc_inet_pton.sh | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh 
b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
index 7a84d73..df04cc1 100755
--- a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
  # probe libc's inet_pton & backtrace it with ping
  
  # Installs a probe on libc's inet_pton function, that will use uprobes,

Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

[Sekhar Nori]

On Wednesday 18 October 2017 07:47 PM, Franklin S Cooper Jr wrote:
> 
> 
> On 10/18/2017 08:24 AM, Sekhar Nori wrote:
>> Hi Marc,
>>
>> On Wednesday 18 October 2017 06:14 PM, Marc Kleine-Budde wrote:
>>> On 09/21/2017 02:48 AM, Franklin S Cooper Jr wrote:


 On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>
>
> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>> Hi Wenyou,
>>
>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>
>>> On 2017/9/14 13:06, Sekhar Nori wrote:
 On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>> resulted in errors. Scoping the signals I noticed that only a single
>> bit
>> was being transmitted and with a bit more investigation realized the
>> actual
>> MCAN IP would go back to initialization mode automatically.
>>
>> It appears this issue is due to the MCAN needing to use the 
>> Transmitter
>> Delay Compensation Mode as defined in the MCAN User's Guide. When 
>> this
>> mode is used the User's Guide indicates that the Transmitter Delay
>> Compensation Offset register should be set. The document mentions
>> that this
>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>
>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>> indicates
>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>> Therefore, only enable this mode and only set TDCO when the data bit
>> rate
>> is above 2.5 Mbps.
>>
>> Signed-off-by: Franklin S Cooper Jr 
>> ---
>> I'm pretty surprised that this hasn't been implemented already since
>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>> supports up to 10 Mbps.
>>
>> So it will be nice to get comments from users of this driver to
>> understand
>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>> patch.
>> If they haven't what did they do to get around it if they needed 
>> higher
>> speeds.
>>
>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>> insure
>> everything still works at 5 Mbps which is the max speed of my CAN
>> transceiver.
> ping. Anyone has any thoughts on this?
 I added Dong who authored the m_can driver and Wenyou who added the 
 only
 in-kernel user of the driver for any help.
>>> I tested it on SAMA5D2 Xplained board both with and without this patch, 
>>> both work with the 4M bps data bit rate.
>> Thank you for testing this out. Its interesting that you have been able
>> to use higher speeds without this patch. What is the CAN transceiver
>> being used on the SAMA5D2 Xplained board? I tried looking at the
>> schematic but it seems the CAN signals are used on an extension board
>> which I can't find the schematic for. Also do you mind sharing your test
>> setup? Were you doing a short point to point test?
>>
>> Thank You,
>> Franklin
> Hello Franklin,
>
> your patch definitely makes sense.
>
> I forgot the TDC in my patches because it was not present in the
> previous driver versions and because I didn't encounter any
> problems when testing it myself.
>
> The error is highly dependent on the hardware (transceiver) setup.
> So it is definitely possible that some people don't encounter errors
> without your patch.

 So the Transmission Delay Compensation feature Value register is suppose
 to take into consideration the transceiver delay automatically and add
 the value of TDCO on top of that. So why would TDCO be dependent on the
 transceiver? I've heard conflicting things regarding TDC so any
 clarification on what actually impacts it would be appreciated.

 Also part of the issue I'm having is how can we properly configure TDCO?
 Configuring TDCO is essentially figuring out what Secondary Sample Point
 to use. However, it is unclear what value to set SSP to and which use
 cases a given SSP will work or doesn't work. I've seen various
 recommendations from Bosch on choosing SSP but ultimately it seems they
 suggestion "real world testing" to come up with a proper value. Not
 setting TDCO causes problems for my device and improperly setting TDCO
 causes problems for my device. So its likely any value I use could end
 up breaking something for someone else.

 Currently I leaning to a DT property that can be used for setting SSP.
 Perhaps use a generic default value and allow individuals to override it
 

Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

[Sekhar Nori]

On Wednesday 18 October 2017 07:47 PM, Franklin S Cooper Jr wrote:
> 
> 
> On 10/18/2017 08:24 AM, Sekhar Nori wrote:
>> Hi Marc,
>>
>> On Wednesday 18 October 2017 06:14 PM, Marc Kleine-Budde wrote:
>>> On 09/21/2017 02:48 AM, Franklin S Cooper Jr wrote:


 On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>
>
> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>> Hi Wenyou,
>>
>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>
>>> On 2017/9/14 13:06, Sekhar Nori wrote:
 On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>> resulted in errors. Scoping the signals I noticed that only a single
>> bit
>> was being transmitted and with a bit more investigation realized the
>> actual
>> MCAN IP would go back to initialization mode automatically.
>>
>> It appears this issue is due to the MCAN needing to use the 
>> Transmitter
>> Delay Compensation Mode as defined in the MCAN User's Guide. When 
>> this
>> mode is used the User's Guide indicates that the Transmitter Delay
>> Compensation Offset register should be set. The document mentions
>> that this
>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>
>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>> indicates
>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>> Therefore, only enable this mode and only set TDCO when the data bit
>> rate
>> is above 2.5 Mbps.
>>
>> Signed-off-by: Franklin S Cooper Jr 
>> ---
>> I'm pretty surprised that this hasn't been implemented already since
>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>> supports up to 10 Mbps.
>>
>> So it will be nice to get comments from users of this driver to
>> understand
>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>> patch.
>> If they haven't what did they do to get around it if they needed 
>> higher
>> speeds.
>>
>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>> insure
>> everything still works at 5 Mbps which is the max speed of my CAN
>> transceiver.
> ping. Anyone has any thoughts on this?
 I added Dong who authored the m_can driver and Wenyou who added the 
 only
 in-kernel user of the driver for any help.
>>> I tested it on SAMA5D2 Xplained board both with and without this patch, 
>>> both work with the 4M bps data bit rate.
>> Thank you for testing this out. Its interesting that you have been able
>> to use higher speeds without this patch. What is the CAN transceiver
>> being used on the SAMA5D2 Xplained board? I tried looking at the
>> schematic but it seems the CAN signals are used on an extension board
>> which I can't find the schematic for. Also do you mind sharing your test
>> setup? Were you doing a short point to point test?
>>
>> Thank You,
>> Franklin
> Hello Franklin,
>
> your patch definitely makes sense.
>
> I forgot the TDC in my patches because it was not present in the
> previous driver versions and because I didn't encounter any
> problems when testing it myself.
>
> The error is highly dependent on the hardware (transceiver) setup.
> So it is definitely possible that some people don't encounter errors
> without your patch.

 So the Transmission Delay Compensation feature Value register is suppose
 to take into consideration the transceiver delay automatically and add
 the value of TDCO on top of that. So why would TDCO be dependent on the
 transceiver? I've heard conflicting things regarding TDC so any
 clarification on what actually impacts it would be appreciated.

 Also part of the issue I'm having is how can we properly configure TDCO?
 Configuring TDCO is essentially figuring out what Secondary Sample Point
 to use. However, it is unclear what value to set SSP to and which use
 cases a given SSP will work or doesn't work. I've seen various
 recommendations from Bosch on choosing SSP but ultimately it seems they
 suggestion "real world testing" to come up with a proper value. Not
 setting TDCO causes problems for my device and improperly setting TDCO
 causes problems for my device. So its likely any value I use could end
 up breaking something for someone else.

 Currently I leaning to a DT property that can be used for setting SSP.
 Perhaps use a generic default value and allow individuals to override it
 via DT?
>>>
>>> 

Re: [PATCH] powerpc: ipic - fix status get and status clear

[Michael Ellerman]

Christophe Leroy  writes:

> IPIC Status is provided by register IPIC_SERSR and not by IPIC_SERMR
> which is the mask register.

This seems like it would be a bad bug. But I guess it hasn't mattered
for some reason?

cheers

> diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
> index 16f1edd78c40..535cf1f6941c 100644
> --- a/arch/powerpc/sysdev/ipic.c
> +++ b/arch/powerpc/sysdev/ipic.c
> @@ -846,12 +846,12 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
>  
>  u32 ipic_get_mcp_status(void)
>  {
> - return ipic_read(primary_ipic->regs, IPIC_SERMR);
> + return ipic_read(primary_ipic->regs, IPIC_SERSR);
>  }
>  
>  void ipic_clear_mcp_status(u32 mask)
>  {
> - ipic_write(primary_ipic->regs, IPIC_SERMR, mask);
> + ipic_write(primary_ipic->regs, IPIC_SERSR, mask);
>  }
>  
>  /* Return an interrupt vector or 0 if no interrupt is pending. */
> -- 
> 2.13.3

Re: [PATCH] powerpc: ipic - fix status get and status clear

[Michael Ellerman]

Christophe Leroy  writes:

> IPIC Status is provided by register IPIC_SERSR and not by IPIC_SERMR
> which is the mask register.

This seems like it would be a bad bug. But I guess it hasn't mattered
for some reason?

cheers

> diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
> index 16f1edd78c40..535cf1f6941c 100644
> --- a/arch/powerpc/sysdev/ipic.c
> +++ b/arch/powerpc/sysdev/ipic.c
> @@ -846,12 +846,12 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
>  
>  u32 ipic_get_mcp_status(void)
>  {
> - return ipic_read(primary_ipic->regs, IPIC_SERMR);
> + return ipic_read(primary_ipic->regs, IPIC_SERSR);
>  }
>  
>  void ipic_clear_mcp_status(u32 mask)
>  {
> - ipic_write(primary_ipic->regs, IPIC_SERMR, mask);
> + ipic_write(primary_ipic->regs, IPIC_SERSR, mask);
>  }
>  
>  /* Return an interrupt vector or 0 if no interrupt is pending. */
> -- 
> 2.13.3

Re: [PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

Hi,

On 2017년 10월 19일 13:47, Chanwoo Choi wrote:
> On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
>> Add documentation on optional dt attribute "debounce-timeout-ms"
>> in extcon node to capture user specified timeout value for id
>> and vbus gpio detection.
>>
>> Signed-off-by: Raveendra Padasalagi 
>> Reviewed-by: Ray Jui 
>> Reviewed-by: Srinath Mannam 
>> ---
>>
>> Changes in v2:
>>  Rename debounce-timeout-ms to input-debounce
>>
>>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt 
>> b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> index dfc14f7..d115900 100644
>> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both 
>> can be present as well.
>>  - id-gpio: gpio for USB ID pin. See gpio binding.
>>  - vbus-gpio: gpio for USB VBUS pin.
>>  
>> +Optional properties:
>> +- input-debounce: debounce timeout value for id and vbus gpio in 
>> microseconds.
> 
> The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
> The unit is usec instead of 'microsecond'. You need to modify the description 
> of unit.
> [1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Sorry. My mistake. I confused the 'nsec' with 'usec'. This patch is right.
Please ignore my comment.

> 
>> +
>>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>>  extcon_usb1 {
>>  compatible = "linux,extcon-usb-gpio";
>>
> 
> 

Looks good to me.
Reviewed-by: Chanwoo Choi 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

Hi,

On 2017년 10월 19일 13:47, Chanwoo Choi wrote:
> On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
>> Add documentation on optional dt attribute "debounce-timeout-ms"
>> in extcon node to capture user specified timeout value for id
>> and vbus gpio detection.
>>
>> Signed-off-by: Raveendra Padasalagi 
>> Reviewed-by: Ray Jui 
>> Reviewed-by: Srinath Mannam 
>> ---
>>
>> Changes in v2:
>>  Rename debounce-timeout-ms to input-debounce
>>
>>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt 
>> b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> index dfc14f7..d115900 100644
>> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both 
>> can be present as well.
>>  - id-gpio: gpio for USB ID pin. See gpio binding.
>>  - vbus-gpio: gpio for USB VBUS pin.
>>  
>> +Optional properties:
>> +- input-debounce: debounce timeout value for id and vbus gpio in 
>> microseconds.
> 
> The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
> The unit is usec instead of 'microsecond'. You need to modify the description 
> of unit.
> [1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Sorry. My mistake. I confused the 'nsec' with 'usec'. This patch is right.
Please ignore my comment.

> 
>> +
>>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>>  extcon_usb1 {
>>  compatible = "linux,extcon-usb-gpio";
>>
> 
> 

Looks good to me.
Reviewed-by: Chanwoo Choi 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

Hi,

On 2017년 10월 19일 12:26, Raveendra Padasalagi wrote:
> Add changes to capture optional dt attribute "debounce-timeout-ms"
> provided in extcon node and used the same value if provided otherwise
> default value of 20ms is used for id and vbus gpios debounce time.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Srinath Mannam 
> ---
> 
> Changes in v2:
>  Rename gpio_debounce_timeout_ms to debounce_usecs
> 
>  drivers/extcon/extcon-usb-gpio.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c 
> b/drivers/extcon/extcon-usb-gpio.c
> index 9c925b0..76ef1da 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -41,6 +41,7 @@ struct usb_extcon_info {
>  
>   unsigned long debounce_jiffies;
>   struct delayed_work wq_detcable;
> + unsigned int debounce_usecs;
>  };
>  
>  static const unsigned int usb_extcon_cable[] = {
> @@ -133,6 +134,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
>   if (IS_ERR(info->vbus_gpiod))
>   return PTR_ERR(info->vbus_gpiod);
>  
> + ret = of_property_read_u32(np, "input-debounce",
> +  >debounce_usecs);
> + if (ret)
> + info->debounce_usecs = USB_GPIO_DEBOUNCE_MS;

The USB_GPIO_DEBOUNCE_MS indicates 20 millisecond.
You need to redefine it as following:
-#define USB_GPIO_DEBOUNCE_MS   20  /* ms */
+#define USB_GPIO_DEBOUNCE_USEC 2

info->debounce_usecs = USB_GPIO_DEBOUNCE_USEC;

or
info->debounce_usecs = USB_GPIO_DEBOUNCE_MS * 1000;


> +
>   info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
>   if (IS_ERR(info->edev)) {
>   dev_err(dev, "failed to allocate extcon device\n");
> @@ -147,13 +153,13 @@ static int usb_extcon_probe(struct platform_device 
> *pdev)
>  
>   if (info->id_gpiod)
>   ret = gpiod_set_debounce(info->id_gpiod,
> -  USB_GPIO_DEBOUNCE_MS * 1000);
> +  info->debounce_usecs * 1000);

The debounce_usecs is already microsecond, You don't need to mutiply with 1000.


>   if (!ret && info->vbus_gpiod)
>   ret = gpiod_set_debounce(info->vbus_gpiod,
> -  USB_GPIO_DEBOUNCE_MS * 1000);
> +  info->debounce_usecs * 1000);
>  
>   if (ret < 0)
> - info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
> + info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);

ditto.

>  
>   INIT_DELAYED_WORK(>wq_detcable, usb_extcon_detect_cable);
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

Hi,

On 2017년 10월 19일 12:26, Raveendra Padasalagi wrote:
> Add changes to capture optional dt attribute "debounce-timeout-ms"
> provided in extcon node and used the same value if provided otherwise
> default value of 20ms is used for id and vbus gpios debounce time.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Srinath Mannam 
> ---
> 
> Changes in v2:
>  Rename gpio_debounce_timeout_ms to debounce_usecs
> 
>  drivers/extcon/extcon-usb-gpio.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c 
> b/drivers/extcon/extcon-usb-gpio.c
> index 9c925b0..76ef1da 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -41,6 +41,7 @@ struct usb_extcon_info {
>  
>   unsigned long debounce_jiffies;
>   struct delayed_work wq_detcable;
> + unsigned int debounce_usecs;
>  };
>  
>  static const unsigned int usb_extcon_cable[] = {
> @@ -133,6 +134,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
>   if (IS_ERR(info->vbus_gpiod))
>   return PTR_ERR(info->vbus_gpiod);
>  
> + ret = of_property_read_u32(np, "input-debounce",
> +  >debounce_usecs);
> + if (ret)
> + info->debounce_usecs = USB_GPIO_DEBOUNCE_MS;

The USB_GPIO_DEBOUNCE_MS indicates 20 millisecond.
You need to redefine it as following:
-#define USB_GPIO_DEBOUNCE_MS   20  /* ms */
+#define USB_GPIO_DEBOUNCE_USEC 2

info->debounce_usecs = USB_GPIO_DEBOUNCE_USEC;

or
info->debounce_usecs = USB_GPIO_DEBOUNCE_MS * 1000;


> +
>   info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
>   if (IS_ERR(info->edev)) {
>   dev_err(dev, "failed to allocate extcon device\n");
> @@ -147,13 +153,13 @@ static int usb_extcon_probe(struct platform_device 
> *pdev)
>  
>   if (info->id_gpiod)
>   ret = gpiod_set_debounce(info->id_gpiod,
> -  USB_GPIO_DEBOUNCE_MS * 1000);
> +  info->debounce_usecs * 1000);

The debounce_usecs is already microsecond, You don't need to mutiply with 1000.


>   if (!ret && info->vbus_gpiod)
>   ret = gpiod_set_debounce(info->vbus_gpiod,
> -  USB_GPIO_DEBOUNCE_MS * 1000);
> +  info->debounce_usecs * 1000);
>  
>   if (ret < 0)
> - info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
> + info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);

ditto.

>  
>   INIT_DELAYED_WORK(>wq_detcable, usb_extcon_detect_cable);
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
> Add documentation on optional dt attribute "debounce-timeout-ms"
> in extcon node to capture user specified timeout value for id
> and vbus gpio detection.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Srinath Mannam 
> ---
> 
> Changes in v2:
>  Rename debounce-timeout-ms to input-debounce
> 
>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> index dfc14f7..d115900 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both 
> can be present as well.
>  - id-gpio: gpio for USB ID pin. See gpio binding.
>  - vbus-gpio: gpio for USB VBUS pin.
>  
> +Optional properties:
> +- input-debounce: debounce timeout value for id and vbus gpio in 
> microseconds.

The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
The unit is usec instead of 'microsecond'. You need to modify the description 
of unit.
[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

> +
>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>   extcon_usb1 {
>   compatible = "linux,extcon-usb-gpio";
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[Chanwoo Choi]

On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
> Add documentation on optional dt attribute "debounce-timeout-ms"
> in extcon node to capture user specified timeout value for id
> and vbus gpio detection.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Srinath Mannam 
> ---
> 
> Changes in v2:
>  Rename debounce-timeout-ms to input-debounce
> 
>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> index dfc14f7..d115900 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both 
> can be present as well.
>  - id-gpio: gpio for USB ID pin. See gpio binding.
>  - vbus-gpio: gpio for USB VBUS pin.
>  
> +Optional properties:
> +- input-debounce: debounce timeout value for id and vbus gpio in 
> microseconds.

The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
The unit is usec instead of 'microsecond'. You need to modify the description 
of unit.
[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

> +
>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>   extcon_usb1 {
>   compatible = "linux,extcon-usb-gpio";
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [V14,1/4] powerpc/vphn: Update CPU topology when VPHN enabled

[Michael Ellerman]

On Fri, 2017-09-08 at 20:47:27 UTC, Michael Bringmann wrote:
> powerpc/vphn: On Power systems with shared configurations of CPUs
> and memory, there are some issues with the association of additional
> CPUs and memory to nodes when hot-adding resources.  This patch
> corrects the currently broken capability to set the topology for
> shared CPUs in LPARs.  At boot time for shared CPU lpars, the
> topology for each CPU was being set to node zero.  Now when
> numa_update_cpu_topology() is called appropriately, the Virtual
> Processor Home Node (VPHN) capabilities information provided by the
> pHyp allows the appropriate node in the shared configuration to be
> selected for the CPU.
> 
> Signed-off-by: Michael Bringmann 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/17f444c0549f2ce037646871e748cf

cheers

Re: [V14,1/4] powerpc/vphn: Update CPU topology when VPHN enabled

[Michael Ellerman]

On Fri, 2017-09-08 at 20:47:27 UTC, Michael Bringmann wrote:
> powerpc/vphn: On Power systems with shared configurations of CPUs
> and memory, there are some issues with the association of additional
> CPUs and memory to nodes when hot-adding resources.  This patch
> corrects the currently broken capability to set the topology for
> shared CPUs in LPARs.  At boot time for shared CPU lpars, the
> topology for each CPU was being set to node zero.  Now when
> numa_update_cpu_topology() is called appropriately, the Virtual
> Processor Home Node (VPHN) capabilities information provided by the
> pHyp allows the appropriate node in the shared configuration to be
> selected for the CPU.
> 
> Signed-off-by: Michael Bringmann 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/17f444c0549f2ce037646871e748cf

cheers

Re: [PATCH 01/14] Documentation: Add SoundWire summary

[Vinod Koul]

Thanks for the quick review Randy,

On Wed, Oct 18, 2017 at 08:33:08PM -0700, Randy Dunlap wrote:
> On 10/18/17 20:03, Vinod Koul wrote:

> > +SoundWire is a new interface ratified in 2015 by the MIPI Alliance.
> > +SoundWire is used for transporting data typically related to audio
> > +functions. SoundWire interface is optimized to integrate audio devices in
> > +mobile or mobile inspired systems.
> > +
> > +SoundWire is a 2-Pin multi-drop interface with data and clock line. It
> 
>   2-pin

ok

> > +The SoundWire protocol supports up to eleven Slave interfaces. All the
> > +interfaces share the common Bus containing data and clock line. Each of the
> > +Slaves can support up to 14 Data Ports. 13 Data Ports are dedicated to 
> > audio
> > +transport. Data Port0 is dedicated to transport of Bulk control 
> > information,
> > +each of the audio Data Ports (1..14) can support up to 8 Channels in
> 
> (1..13) ??

nope. 1 to 14, both inclusive, thats why 14 Data Ports

> > +Bus:
> > +Implements SoundWire Linux Bus which handles the SoundWire protocol.
> > +It programs all the MIPI defined Slave registers. It represents a SoundWire
> 
>MIPI-defined
> 
> > +Master. There can be multiple instances of Bus maybe present in a system.
> 
> eh?
>Multiple instances of Bus may be present in a system.

sounds better

> > +int sdw_add_bus_master(struct sdw_bus *bus)
> > +{
> > +if (!bus->dev)
> > +return -ENODEV;
> > +
> > +mutex_init(>lock);
> > +INIT_LIST_HEAD(>slaves);
> > +
> > +   /* Check ACPI for Slave devices */
> > +sdw_acpi_find_slaves(bus);
> > +
> > +   /* Check DT for Slave devices */
> > +   sdw_of_find_slaves(bus);
> 
> Please use same indentation as sdw_acpi_find_slaves().

ah not sure why it came like this, thanks for pointing out

> > +The MIPI specification requires each Slave interface to expose a unique
> > +48-bit identifier, stored in 6 read only dev_id registers. This dev_id
> 
>   read-only

right

> > +identifier, Bus enumerates the Slave device based on the 48-bit identifier.
> > +Slave device and driver match is done based on this 48-bit identifier. 
> > Probe
> > +of the Slave driver is called by Bus on successful match between device and
> > +driver id. A parent/child relationship is enforced between Slave and Master
> 
> maybe reverse this order. Master and Slave
> 
> to be in the "parent/child" order?  Unless I have them backwards?

right, will update this

> > +devices (the logical representation is aligned with the physical
> > +connectivity).
> > +
> > +The information on Master/Slave dependencies is stored in platform data,
> > +board-file, ACPI or DT. The MIPI Software specification defines an
> 

ok

-- 
~Vinod

Re: [PATCH 01/14] Documentation: Add SoundWire summary

[Vinod Koul]

Thanks for the quick review Randy,

On Wed, Oct 18, 2017 at 08:33:08PM -0700, Randy Dunlap wrote:
> On 10/18/17 20:03, Vinod Koul wrote:

> > +SoundWire is a new interface ratified in 2015 by the MIPI Alliance.
> > +SoundWire is used for transporting data typically related to audio
> > +functions. SoundWire interface is optimized to integrate audio devices in
> > +mobile or mobile inspired systems.
> > +
> > +SoundWire is a 2-Pin multi-drop interface with data and clock line. It
> 
>   2-pin

ok

> > +The SoundWire protocol supports up to eleven Slave interfaces. All the
> > +interfaces share the common Bus containing data and clock line. Each of the
> > +Slaves can support up to 14 Data Ports. 13 Data Ports are dedicated to 
> > audio
> > +transport. Data Port0 is dedicated to transport of Bulk control 
> > information,
> > +each of the audio Data Ports (1..14) can support up to 8 Channels in
> 
> (1..13) ??

nope. 1 to 14, both inclusive, thats why 14 Data Ports

> > +Bus:
> > +Implements SoundWire Linux Bus which handles the SoundWire protocol.
> > +It programs all the MIPI defined Slave registers. It represents a SoundWire
> 
>MIPI-defined
> 
> > +Master. There can be multiple instances of Bus maybe present in a system.
> 
> eh?
>Multiple instances of Bus may be present in a system.

sounds better

> > +int sdw_add_bus_master(struct sdw_bus *bus)
> > +{
> > +if (!bus->dev)
> > +return -ENODEV;
> > +
> > +mutex_init(>lock);
> > +INIT_LIST_HEAD(>slaves);
> > +
> > +   /* Check ACPI for Slave devices */
> > +sdw_acpi_find_slaves(bus);
> > +
> > +   /* Check DT for Slave devices */
> > +   sdw_of_find_slaves(bus);
> 
> Please use same indentation as sdw_acpi_find_slaves().

ah not sure why it came like this, thanks for pointing out

> > +The MIPI specification requires each Slave interface to expose a unique
> > +48-bit identifier, stored in 6 read only dev_id registers. This dev_id
> 
>   read-only

right

> > +identifier, Bus enumerates the Slave device based on the 48-bit identifier.
> > +Slave device and driver match is done based on this 48-bit identifier. 
> > Probe
> > +of the Slave driver is called by Bus on successful match between device and
> > +driver id. A parent/child relationship is enforced between Slave and Master
> 
> maybe reverse this order. Master and Slave
> 
> to be in the "parent/child" order?  Unless I have them backwards?

right, will update this

> > +devices (the logical representation is aligned with the physical
> > +connectivity).
> > +
> > +The information on Master/Slave dependencies is stored in platform data,
> > +board-file, ACPI or DT. The MIPI Software specification defines an
> 

ok

-- 
~Vinod

Re: [PATCH v5 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

[AKASHI Takahiro]

On Sat, Oct 14, 2017 at 07:37:04PM +0800, kbuild test robot wrote:
> Hi AKASHI,
> 
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on v4.14-rc4 next-20171013]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/AKASHI-Takahiro/arm64-kexec-add-kexec_file_load-support/20171012-003448
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
> for-next/core
> config: x86_64-randconfig-in0-10141752 (attached as .config)
> compiler: gcc-4.6 (Debian 4.6.4-7) 4.6.4
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):
> 
> >> kernel/kexec_file.o: warning: objtool: _kexec_kernel_image_load()+0x31: 
> >> unsupported stack register modification

I talked to Josh (objtool maintainer) and confirmed this is an issue
of objtool, not a bug in my patch.

-Takahiro AKASHI

> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v5 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

[AKASHI Takahiro]

On Sat, Oct 14, 2017 at 07:37:04PM +0800, kbuild test robot wrote:
> Hi AKASHI,
> 
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on v4.14-rc4 next-20171013]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/AKASHI-Takahiro/arm64-kexec-add-kexec_file_load-support/20171012-003448
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
> for-next/core
> config: x86_64-randconfig-in0-10141752 (attached as .config)
> compiler: gcc-4.6 (Debian 4.6.4-7) 4.6.4
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):
> 
> >> kernel/kexec_file.o: warning: objtool: _kexec_kernel_image_load()+0x31: 
> >> unsupported stack register modification

I talked to Josh (objtool maintainer) and confirmed this is an issue
of objtool, not a bug in my patch.

-Takahiro AKASHI

> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Byungchul Park]

On Wed, Oct 18, 2017 at 12:09:44PM +0200, Ingo Molnar wrote:
> BTW., have you attempted limiting the depth of the stack traces? I suspect 
> more 
> than 2-4 are rarely required to disambiguate the calling context.

I did it for you. Let me show you the result.

1. No lockdep

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.756558155 seconds time elapsed( +-  0.09% )

2. Lockdep

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.968710420 seconds time elapsed( +-  0.12% )

3. Lockdep + Crossrelease 5 entries

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.153839636 seconds time elapsed( +-  0.31% )

4. Lockdep + Crossrelease 3 entries

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.137205534 seconds time elapsed( +-  0.87% )

5. Lockdep + Crossrelease + This patch

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.963669551 seconds time elapsed( +-  0.11% )

And I will add the result in commit message at the next spin.

Thanks,
Byungchul

Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

[Byungchul Park]

On Wed, Oct 18, 2017 at 12:09:44PM +0200, Ingo Molnar wrote:
> BTW., have you attempted limiting the depth of the stack traces? I suspect 
> more 
> than 2-4 are rarely required to disambiguate the calling context.

I did it for you. Let me show you the result.

1. No lockdep

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.756558155 seconds time elapsed( +-  0.09% )

2. Lockdep

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.968710420 seconds time elapsed( +-  0.12% )

3. Lockdep + Crossrelease 5 entries

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.153839636 seconds time elapsed( +-  0.31% )

4. Lockdep + Crossrelease 3 entries

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   3.137205534 seconds time elapsed( +-  0.87% )

5. Lockdep + Crossrelease + This patch

 Performance counter stats for 'qemu_booting_time.sh bzImage' (10 runs):

   2.963669551 seconds time elapsed( +-  0.11% )

And I will add the result in commit message at the next spin.

Thanks,
Byungchul

Re: [PATCH] zswap: Same-filled pages handling

[Andi Kleen]

> Yes.  Every 64-bit repeating pattern is also a 32-bit repeating pattern.
> Supporting a 64-bit pattern on a 32-bit kernel is painful, but it makes
> no sense to *not* support a 64-bit pattern on a 64-bit kernel.  

But a 32bit repeating pattern is not necessarily a 64bit pattern.

>This is the same approach used in zram, fwiw.

Sounds bogus.

-Andi

Re: [PATCH] zswap: Same-filled pages handling

[Andi Kleen]

> Yes.  Every 64-bit repeating pattern is also a 32-bit repeating pattern.
> Supporting a 64-bit pattern on a 32-bit kernel is painful, but it makes
> no sense to *not* support a 64-bit pattern on a 64-bit kernel.  

But a 32bit repeating pattern is not necessarily a 64bit pattern.

>This is the same approach used in zram, fwiw.

Sounds bogus.

-Andi

Re: [PATCH v1] mm/mempolicy.c: Fix get_nodes() off-by-one error.

[Andi Kleen]

On Thu, Oct 19, 2017 at 03:48:09AM +, Sandoval Castro, Luis Felipe wrote:
> On Tue 18-10-17 10:42:34, Luis Felipe Sandoval Castro wrote:
> 
> Sorry for the delayed replay, from your feedback I don't think my
> patch has any chances of being merged... I'm wondering though,
> if a note in the man pages "range non inclusive" or something
> like that would help to avoid confusions? Thanks

Yes fixing the man pages is a good idea.

-Andi

Re: [PATCH v1] mm/mempolicy.c: Fix get_nodes() off-by-one error.

[Andi Kleen]

On Thu, Oct 19, 2017 at 03:48:09AM +, Sandoval Castro, Luis Felipe wrote:
> On Tue 18-10-17 10:42:34, Luis Felipe Sandoval Castro wrote:
> 
> Sorry for the delayed replay, from your feedback I don't think my
> patch has any chances of being merged... I'm wondering though,
> if a note in the man pages "range non inclusive" or something
> like that would help to avoid confusions? Thanks

Yes fixing the man pages is a good idea.

-Andi

[patch-rt] drivers/zram: pass num_pages to zram_meta_init_table_locks()

[Mike Galbraith]

zram cleanup forgot to adjust passed argument when changing
zram_meta_init_table_locks() to expect page count instead
of disk size.  Doing so fixes ltp inspired explosions.

Signed-off-by: Mike Galbraith 
---
 drivers/block/zram/zram_drv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -819,7 +819,7 @@ static bool zram_meta_alloc(struct zram
return false;
}
 
-   zram_meta_init_table_locks(zram, disksize);
+   zram_meta_init_table_locks(zram, num_pages);
return true;
 }
 

[patch-rt] drivers/zram: pass num_pages to zram_meta_init_table_locks()

[Mike Galbraith]

zram cleanup forgot to adjust passed argument when changing
zram_meta_init_table_locks() to expect page count instead
of disk size.  Doing so fixes ltp inspired explosions.

Signed-off-by: Mike Galbraith 
---
 drivers/block/zram/zram_drv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -819,7 +819,7 @@ static bool zram_meta_alloc(struct zram
return false;
}
 
-   zram_meta_init_table_locks(zram, disksize);
+   zram_meta_init_table_locks(zram, num_pages);
return true;
 }
 

[PATCH] perf test shell: trace+probe_libc_inet_pton.sh: force to use /bin/bash to load this script

[Li Zhijian]

this script contains Array, but not all sh support Array. by default, dash 
provides sh at ubuntu/debian
which can not support Array. so force to use /bin/bash

it can fix following issue:
lizhijian@localhost:~/lkp/linux/tools/perf/tests/shell$ sudo 
./trace+probe_libc_inet_pton.sh
[sudo] password for lizhijian: PING ::1(::1) 56 data bytes
./trace+probe_libc_inet_pton.sh: 30: ./trace+probe_libc_inet_pton.sh: Bad 
substitution
./trace+probe_libc_inet_pton.sh: 32: ./trace+probe_libc_inet_pton.sh: Bad 
substitution

Signed-off-by: Li Zhijian 
---
 tools/perf/tests/shell/trace+probe_libc_inet_pton.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh 
b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
index 7a84d73..df04cc1 100755
--- a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
 # probe libc's inet_pton & backtrace it with ping
 
 # Installs a probe on libc's inet_pton function, that will use uprobes,
-- 
2.7.4

[PATCH] perf test shell: trace+probe_libc_inet_pton.sh: force to use /bin/bash to load this script

[Li Zhijian]

this script contains Array, but not all sh support Array. by default, dash 
provides sh at ubuntu/debian
which can not support Array. so force to use /bin/bash

it can fix following issue:
lizhijian@localhost:~/lkp/linux/tools/perf/tests/shell$ sudo 
./trace+probe_libc_inet_pton.sh
[sudo] password for lizhijian: PING ::1(::1) 56 data bytes
./trace+probe_libc_inet_pton.sh: 30: ./trace+probe_libc_inet_pton.sh: Bad 
substitution
./trace+probe_libc_inet_pton.sh: 32: ./trace+probe_libc_inet_pton.sh: Bad 
substitution

Signed-off-by: Li Zhijian 
---
 tools/perf/tests/shell/trace+probe_libc_inet_pton.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh 
b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
index 7a84d73..df04cc1 100755
--- a/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/trace+probe_libc_inet_pton.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
 # probe libc's inet_pton & backtrace it with ping
 
 # Installs a probe on libc's inet_pton function, that will use uprobes,
-- 
2.7.4

Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0

[Jim Mattson]

MCi_STATUS sounds fine, or any other Intel constraints that are
guarded by checks for virtualizing an Intel CPU.

On Wed, Oct 18, 2017 at 8:04 PM, Wanpeng Li  wrote:
> On 10/19/17 10:49 AM, Jim Mattson wrote:
>
> Right. I was side-tracked by the code above yours for MCi_CTL.
> However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise
> #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the
>
>
> AMD Volume 2:
>
> 9.3.2 Error-Reporting Register Bank:
>
> Attempting to write a value other than 0 to an MCi_STATUS register will
> raise a general protection (#GP) exception.
>
> So maybe my patch can just handle MCi_STATUS or current patch is ok, what's
> your opinion?
>
>
> Regards,
> Wanpeng Li
>
>
> APM says:
> "In some implementations, the MCi_MISC0 register is used for error
> thresholding." Figure 9-8: Miscellaneous Information Register
> (Thresholding Register Format) suggests that non-zero value can, in
> fact, be written to MCi_MISC0 in such implementations.
>
> Do any of the AMD contributors want to weigh in?
>
> On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Li  wrote:
>
> Hi Jim，
> On 10/19/17 12:28 AM, Jim Mattson wrote:
>
> The AMD APM says, "For each error-reporting register bank, software
> should set the enable bits to 1 in the MCi_CTL register for the error
> types it wants the processor to report. Software can write each
> MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does
> not say that only all 1's or all 0's are allowed, and it implies that
> any value is allowed.
>
> Since this is a vendor-agnostic function, the Intel-specific
> constraints should only be applied when virtualizing Intel CPUs (in
> particular, Intel P6 family CPUs). The same comment applies to the
> existing constraints from commit 890ca9aefa78 ("KVM: Add MCE
>
> I have a discuss with the author of ("KVM: Add MCE support") face to
> face today in a kernel meeting held in our country. He told me his patch
> is against Intel architecture and not consider AMD when he introduced
> the patch. In addition, the difference which you mentioned is about
> MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC.
>
> Regards,
> Wanpeng Li
>
> support"), which were only partially relaxed by commit 114be429c8cd4
> ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL").
>
> On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li  wrote:
>
> From: Wanpeng Li 
>
> SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the
> registers are implemented, these registers can be cleared by explicitly
> writing
> 0s to these registers. Writing 1s to these registers will cause a
> general-protection exception.
>
> The mce is emulated in qemu, so just the guest attempts to write 1 to these
> registers should cause a #GP, this patch does it.
>
> Cc: Radim Krčmář 
> Cc: Jim Mattson 
> Signed-off-by: Wanpeng Li 
> ---
>   arch/x86/kvm/x86.c | 9 +++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5669af0..a8680ea 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct
> *work)
>  KVMCLOCK_SYNC_PERIOD);
>   }
>
> -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   {
>  u64 mcg_cap = vcpu->arch.mcg_cap;
>  unsigned bank_num = mcg_cap & 0xff;
> +   u32 msr = msr_info->index;
> +   u64 data = msr_info->data;
>
>  switch (msr) {
>  case MSR_IA32_MCG_STATUS:
> @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr,
> u64 data)
>  if ((offset & 0x3) == 0 &&
>  data != 0 && (data | (1 << 10)) != ~(u64)0)
>  return -1;
> +   if (!msr_info->host_initiated &&
> +   (offset & 0x3) != 0 && data != 0)
> +   return -1;
>  vcpu->arch.mce_banks[offset] = data;
>  break;
>  }
> @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct
> msr_data *msr_info)
>  case MSR_IA32_MCG_CTL:
>  case MSR_IA32_MCG_STATUS:
>  case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1:
> -   return set_msr_mce(vcpu, msr, data);
> +   return set_msr_mce(vcpu, msr_info);
>
>  case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3:
>  case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1:
> --
> 2.7.4
>
>

Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0

[Jim Mattson]

MCi_STATUS sounds fine, or any other Intel constraints that are
guarded by checks for virtualizing an Intel CPU.

On Wed, Oct 18, 2017 at 8:04 PM, Wanpeng Li  wrote:
> On 10/19/17 10:49 AM, Jim Mattson wrote:
>
> Right. I was side-tracked by the code above yours for MCi_CTL.
> However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise
> #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the
>
>
> AMD Volume 2:
>
> 9.3.2 Error-Reporting Register Bank:
>
> Attempting to write a value other than 0 to an MCi_STATUS register will
> raise a general protection (#GP) exception.
>
> So maybe my patch can just handle MCi_STATUS or current patch is ok, what's
> your opinion?
>
>
> Regards,
> Wanpeng Li
>
>
> APM says:
> "In some implementations, the MCi_MISC0 register is used for error
> thresholding." Figure 9-8: Miscellaneous Information Register
> (Thresholding Register Format) suggests that non-zero value can, in
> fact, be written to MCi_MISC0 in such implementations.
>
> Do any of the AMD contributors want to weigh in?
>
> On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Li  wrote:
>
> Hi Jim，
> On 10/19/17 12:28 AM, Jim Mattson wrote:
>
> The AMD APM says, "For each error-reporting register bank, software
> should set the enable bits to 1 in the MCi_CTL register for the error
> types it wants the processor to report. Software can write each
> MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does
> not say that only all 1's or all 0's are allowed, and it implies that
> any value is allowed.
>
> Since this is a vendor-agnostic function, the Intel-specific
> constraints should only be applied when virtualizing Intel CPUs (in
> particular, Intel P6 family CPUs). The same comment applies to the
> existing constraints from commit 890ca9aefa78 ("KVM: Add MCE
>
> I have a discuss with the author of ("KVM: Add MCE support") face to
> face today in a kernel meeting held in our country. He told me his patch
> is against Intel architecture and not consider AMD when he introduced
> the patch. In addition, the difference which you mentioned is about
> MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC.
>
> Regards,
> Wanpeng Li
>
> support"), which were only partially relaxed by commit 114be429c8cd4
> ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL").
>
> On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li  wrote:
>
> From: Wanpeng Li 
>
> SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the
> registers are implemented, these registers can be cleared by explicitly
> writing
> 0s to these registers. Writing 1s to these registers will cause a
> general-protection exception.
>
> The mce is emulated in qemu, so just the guest attempts to write 1 to these
> registers should cause a #GP, this patch does it.
>
> Cc: Radim Krčmář 
> Cc: Jim Mattson 
> Signed-off-by: Wanpeng Li 
> ---
>   arch/x86/kvm/x86.c | 9 +++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5669af0..a8680ea 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct
> *work)
>  KVMCLOCK_SYNC_PERIOD);
>   }
>
> -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   {
>  u64 mcg_cap = vcpu->arch.mcg_cap;
>  unsigned bank_num = mcg_cap & 0xff;
> +   u32 msr = msr_info->index;
> +   u64 data = msr_info->data;
>
>  switch (msr) {
>  case MSR_IA32_MCG_STATUS:
> @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr,
> u64 data)
>  if ((offset & 0x3) == 0 &&
>  data != 0 && (data | (1 << 10)) != ~(u64)0)
>  return -1;
> +   if (!msr_info->host_initiated &&
> +   (offset & 0x3) != 0 && data != 0)
> +   return -1;
>  vcpu->arch.mce_banks[offset] = data;
>  break;
>  }
> @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct
> msr_data *msr_info)
>  case MSR_IA32_MCG_CTL:
>  case MSR_IA32_MCG_STATUS:
>  case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1:
> -   return set_msr_mce(vcpu, msr, data);
> +   return set_msr_mce(vcpu, msr_info);
>
>  case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3:
>  case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1:
> --
> 2.7.4
>
>

[PATCH v6 00/10] rockchip: kevin: Enable edp display

[Jeffy Chen]

Make edp display works on chromebook kevin(at least for boot animation).

Also solve some issues i meet during the bringup.

Changes in v6:
Don't change order of rockchip_drm_psr_register().

Changes in v5:
Call the destroy hook in the error handling path like in unbind().
Call the destroy hook in the error handling path like in unbind().
Update cleanup order in unbind().
Add disable to unbind(), and inline clk_prepare_enable() with bind().

Jeffy Chen (10):
  arm64: dts: rockchip: Enable edp disaplay on kevin
  drm/rockchip: analogix_dp: Remove unnecessary init code
  drm/bridge: analogix: Do not use device's drvdata
  drm/bridge: analogix_dp: Fix connector and encoder cleanup
  drm/rockchip: analogix_dp: Add a sanity check for
rockchip_drm_psr_register()
  drm/rockchip: dw-mipi-dsi: Fix error handling path
  drm/rockchip: inno_hdmi: Fix error handling path
  drm/bridge/synopsys: dw-hdmi: Add missing bridge detach
  drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
  drm/rockchip: dw_hdmi: Fix error handling path

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts  | 29 +++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi   | 16 
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 52 +---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  | 53 ++--
 drivers/gpu/drm/exynos/exynos_dp.c | 29 ---
 drivers/gpu/drm/imx/dw_hdmi-imx.c  | 22 +++--
 drivers/gpu/drm/meson/meson_dw_hdmi.c  | 20 +++--
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 +++-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 95 +++---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 +++--
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c| 39 +
 drivers/gpu/drm/rockchip/inno_hdmi.c   | 22 +++--
 include/drm/bridge/analogix_dp.h   | 19 +++--
 include/drm/bridge/dw_hdmi.h   | 17 ++--
 14 files changed, 265 insertions(+), 183 deletions(-)

-- 
2.11.0

[PATCH v6 00/10] rockchip: kevin: Enable edp display

[Jeffy Chen]

Make edp display works on chromebook kevin(at least for boot animation).

Also solve some issues i meet during the bringup.

Changes in v6:
Don't change order of rockchip_drm_psr_register().

Changes in v5:
Call the destroy hook in the error handling path like in unbind().
Call the destroy hook in the error handling path like in unbind().
Update cleanup order in unbind().
Add disable to unbind(), and inline clk_prepare_enable() with bind().

Jeffy Chen (10):
  arm64: dts: rockchip: Enable edp disaplay on kevin
  drm/rockchip: analogix_dp: Remove unnecessary init code
  drm/bridge: analogix: Do not use device's drvdata
  drm/bridge: analogix_dp: Fix connector and encoder cleanup
  drm/rockchip: analogix_dp: Add a sanity check for
rockchip_drm_psr_register()
  drm/rockchip: dw-mipi-dsi: Fix error handling path
  drm/rockchip: inno_hdmi: Fix error handling path
  drm/bridge/synopsys: dw-hdmi: Add missing bridge detach
  drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
  drm/rockchip: dw_hdmi: Fix error handling path

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts  | 29 +++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi   | 16 
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 52 +---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  | 53 ++--
 drivers/gpu/drm/exynos/exynos_dp.c | 29 ---
 drivers/gpu/drm/imx/dw_hdmi-imx.c  | 22 +++--
 drivers/gpu/drm/meson/meson_dw_hdmi.c  | 20 +++--
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 +++-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 95 +++---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 +++--
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c| 39 +
 drivers/gpu/drm/rockchip/inno_hdmi.c   | 22 +++--
 include/drm/bridge/analogix_dp.h   | 19 +++--
 include/drm/bridge/dw_hdmi.h   | 17 ++--
 14 files changed, 265 insertions(+), 183 deletions(-)

-- 
2.11.0

[PATCH v6 01/10] arm64: dts: rockchip: Enable edp disaplay on kevin

[Jeffy Chen]

Add edp panel and enable related nodes on kevin.

Signed-off-by: Jeffy Chen 
Reviewed-by: Mark Yao 

---

Changes in v6: None
Changes in v5: None

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 29 +++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 16 +
 2 files changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index a3d3cea7dc4f..bc67b19f0af5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -93,6 +93,18 @@
pwm-delay-us = <1>;
};
 
+   edp_panel: edp-panel {
+   compatible = "sharp,lq123p1jx31", "simple-panel";
+   backlight = <>;
+   power-supply = <_disp>;
+
+   ports {
+   panel_in_edp: endpoint {
+   remote-endpoint = <_out_panel>;
+   };
+   };
+   };
+
thermistor_ppvar_bigcpu: thermistor-ppvar-bigcpu {
compatible = "murata,ncp15wb473";
pullup-uv = <180>;
@@ -264,6 +276,23 @@ ap_i2c_dig:  {
};
 };
 
+ {
+   status = "okay";
+
+   ports {
+   edp_out: port@1 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   edp_out_panel: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_in_edp>;
+   };
+   };
+   };
+};
+
 _bigcpu_pwm {
regulator-min-microvolt = <798674>;
regulator-max-microvolt = <1302172>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 5772c52fbfd3..470105d651c2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -927,6 +927,22 @@ ap_i2c_audio:  {
dr_mode = "host";
 };
 
+ {
+   status = "okay";
+};
+
+_mmu {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+_mmu {
+   status = "okay";
+};
+
 #include 
 #include 
 
-- 
2.11.0

[PATCH v6 01/10] arm64: dts: rockchip: Enable edp disaplay on kevin

[Jeffy Chen]

Add edp panel and enable related nodes on kevin.

Signed-off-by: Jeffy Chen 
Reviewed-by: Mark Yao 

---

Changes in v6: None
Changes in v5: None

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 29 +++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 16 +
 2 files changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index a3d3cea7dc4f..bc67b19f0af5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -93,6 +93,18 @@
pwm-delay-us = <1>;
};
 
+   edp_panel: edp-panel {
+   compatible = "sharp,lq123p1jx31", "simple-panel";
+   backlight = <>;
+   power-supply = <_disp>;
+
+   ports {
+   panel_in_edp: endpoint {
+   remote-endpoint = <_out_panel>;
+   };
+   };
+   };
+
thermistor_ppvar_bigcpu: thermistor-ppvar-bigcpu {
compatible = "murata,ncp15wb473";
pullup-uv = <180>;
@@ -264,6 +276,23 @@ ap_i2c_dig:  {
};
 };
 
+ {
+   status = "okay";
+
+   ports {
+   edp_out: port@1 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   edp_out_panel: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = <_in_edp>;
+   };
+   };
+   };
+};
+
 _bigcpu_pwm {
regulator-min-microvolt = <798674>;
regulator-max-microvolt = <1302172>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 5772c52fbfd3..470105d651c2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -927,6 +927,22 @@ ap_i2c_audio:  {
dr_mode = "host";
 };
 
+ {
+   status = "okay";
+};
+
+_mmu {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+_mmu {
+   status = "okay";
+};
+
 #include 
 #include 
 
-- 
2.11.0

[PATCH v6 04/10] drm/bridge: analogix_dp: Fix connector and encoder cleanup

[Jeffy Chen]

Since we are initing connector in the core driver and encoder in the
plat driver, let's clean them up in the right places.

Signed-off-by: Jeffy Chen 
Reviewed-by: Andrzej Hajda 
---

Changes in v6:
Don't change order of rockchip_drm_psr_register().

Changes in v5: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  2 --
 drivers/gpu/drm/exynos/exynos_dp.c |  7 +--
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 12 +---
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 74d274b6d31d..3f910ab36ff6 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1409,7 +1409,6 @@ analogix_dp_bind(struct device *dev, struct drm_device 
*drm_dev,
ret = analogix_dp_create_bridge(drm_dev, dp);
if (ret) {
DRM_ERROR("failed to create bridge (%d)\n", ret);
-   drm_encoder_cleanup(dp->encoder);
goto err_disable_pm_runtime;
}
 
@@ -1432,7 +1431,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
 {
analogix_dp_bridge_disable(dp->bridge);
dp->connector.funcs->destroy(>connector);
-   dp->encoder->funcs->destroy(dp->encoder);
 
if (dp->plat_data->panel) {
if (drm_panel_unprepare(dp->plat_data->panel))
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c 
b/drivers/gpu/drm/exynos/exynos_dp.c
index f7e5b2c405ed..33319a858f3a 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -185,8 +185,10 @@ static int exynos_dp_bind(struct device *dev, struct 
device *master, void *data)
dp->plat_data.encoder = encoder;
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
-   if (IS_ERR(dp->adp))
+   if (IS_ERR(dp->adp)) {
+   dp->encoder.funcs->destroy(>encoder);
return PTR_ERR(dp->adp);
+   }
 
return 0;
 }
@@ -196,7 +198,8 @@ static void exynos_dp_unbind(struct device *dev, struct 
device *master,
 {
struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
-   return analogix_dp_unbind(dp->adp);
+   analogix_dp_unbind(dp->adp);
+   dp->encoder.funcs->destroy(>encoder);
 }
 
 static const struct component_ops exynos_dp_ops = {
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index fa0365de31d2..117585df73e1 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -261,13 +261,8 @@ static struct drm_encoder_helper_funcs 
rockchip_dp_encoder_helper_funcs = {
.atomic_check = rockchip_dp_drm_encoder_atomic_check,
 };
 
-static void rockchip_dp_drm_encoder_destroy(struct drm_encoder *encoder)
-{
-   drm_encoder_cleanup(encoder);
-}
-
 static struct drm_encoder_funcs rockchip_dp_encoder_funcs = {
-   .destroy = rockchip_dp_drm_encoder_destroy,
+   .destroy = drm_encoder_cleanup,
 };
 
 static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
@@ -364,8 +359,10 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
-   if (IS_ERR(dp->adp))
+   if (IS_ERR(dp->adp)) {
+   dp->encoder.funcs->destroy(>encoder);
return PTR_ERR(dp->adp);
+   }
 
return 0;
 }
@@ -377,6 +374,7 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
 
rockchip_drm_psr_unregister(>encoder);
analogix_dp_unbind(dp->adp);
+   dp->encoder.funcs->destroy(>encoder);
 }
 
 static const struct component_ops rockchip_dp_component_ops = {
-- 
2.11.0

[PATCH v6 04/10] drm/bridge: analogix_dp: Fix connector and encoder cleanup

[Jeffy Chen]

Since we are initing connector in the core driver and encoder in the
plat driver, let's clean them up in the right places.

Signed-off-by: Jeffy Chen 
Reviewed-by: Andrzej Hajda 
---

Changes in v6:
Don't change order of rockchip_drm_psr_register().

Changes in v5: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  2 --
 drivers/gpu/drm/exynos/exynos_dp.c |  7 +--
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 12 +---
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 74d274b6d31d..3f910ab36ff6 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1409,7 +1409,6 @@ analogix_dp_bind(struct device *dev, struct drm_device 
*drm_dev,
ret = analogix_dp_create_bridge(drm_dev, dp);
if (ret) {
DRM_ERROR("failed to create bridge (%d)\n", ret);
-   drm_encoder_cleanup(dp->encoder);
goto err_disable_pm_runtime;
}
 
@@ -1432,7 +1431,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
 {
analogix_dp_bridge_disable(dp->bridge);
dp->connector.funcs->destroy(>connector);
-   dp->encoder->funcs->destroy(dp->encoder);
 
if (dp->plat_data->panel) {
if (drm_panel_unprepare(dp->plat_data->panel))
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c 
b/drivers/gpu/drm/exynos/exynos_dp.c
index f7e5b2c405ed..33319a858f3a 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -185,8 +185,10 @@ static int exynos_dp_bind(struct device *dev, struct 
device *master, void *data)
dp->plat_data.encoder = encoder;
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
-   if (IS_ERR(dp->adp))
+   if (IS_ERR(dp->adp)) {
+   dp->encoder.funcs->destroy(>encoder);
return PTR_ERR(dp->adp);
+   }
 
return 0;
 }
@@ -196,7 +198,8 @@ static void exynos_dp_unbind(struct device *dev, struct 
device *master,
 {
struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
-   return analogix_dp_unbind(dp->adp);
+   analogix_dp_unbind(dp->adp);
+   dp->encoder.funcs->destroy(>encoder);
 }
 
 static const struct component_ops exynos_dp_ops = {
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index fa0365de31d2..117585df73e1 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -261,13 +261,8 @@ static struct drm_encoder_helper_funcs 
rockchip_dp_encoder_helper_funcs = {
.atomic_check = rockchip_dp_drm_encoder_atomic_check,
 };
 
-static void rockchip_dp_drm_encoder_destroy(struct drm_encoder *encoder)
-{
-   drm_encoder_cleanup(encoder);
-}
-
 static struct drm_encoder_funcs rockchip_dp_encoder_funcs = {
-   .destroy = rockchip_dp_drm_encoder_destroy,
+   .destroy = drm_encoder_cleanup,
 };
 
 static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
@@ -364,8 +359,10 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
-   if (IS_ERR(dp->adp))
+   if (IS_ERR(dp->adp)) {
+   dp->encoder.funcs->destroy(>encoder);
return PTR_ERR(dp->adp);
+   }
 
return 0;
 }
@@ -377,6 +374,7 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
 
rockchip_drm_psr_unregister(>encoder);
analogix_dp_unbind(dp->adp);
+   dp->encoder.funcs->destroy(>encoder);
 }
 
 static const struct component_ops rockchip_dp_component_ops = {
-- 
2.11.0

Re: [PATCH] KVM: PPC: Book3S HV: Delete an error message for a failed memory allocation in kvmppc_allocate_hpt()

[Paul Mackerras]

On Thu, Oct 05, 2017 at 01:23:30PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Thu, 5 Oct 2017 13:16:51 +0200
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 

Thanks, applied to my kvm-ppc-next branch.

Paul.

Re: [PATCH] KVM: PPC: Book3S HV: Delete an error message for a failed memory allocation in kvmppc_allocate_hpt()

[Paul Mackerras]

On Thu, Oct 05, 2017 at 01:23:30PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Thu, 5 Oct 2017 13:16:51 +0200
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 

Thanks, applied to my kvm-ppc-next branch.

Paul.

[PATCH v6 06/10] drm/rockchip: dw-mipi-dsi: Fix error handling path

[Jeffy Chen]

Add missing pm_runtime_disable() in bind()'s error handling path.

Also cleanup encoder & connector in unbind().

Fixes: 80a9a059d4e4 ("drm/rockchip/dsi: add dw-mipi power domain support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Call the destroy hook in the error handling path like in unbind().

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index b15755b6129c..e72d4e2b61aa 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -1282,7 +1282,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
ret = dw_mipi_dsi_register(drm, dsi);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to register mipi_dsi: %d\n", ret);
-   goto err_pllref;
+   goto err_disable_pllref;
}
 
pm_runtime_enable(dev);
@@ -1292,23 +1292,24 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
ret = mipi_dsi_host_register(>dsi_host);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to register MIPI host: %d\n", ret);
-   goto err_cleanup;
+   goto err_disable_pm_runtime;
}
 
if (!dsi->panel) {
ret = -EPROBE_DEFER;
-   goto err_mipi_dsi_host;
+   goto err_unreg_mipi_dsi_host;
}
 
dev_set_drvdata(dev, dsi);
return 0;
 
-err_mipi_dsi_host:
+err_unreg_mipi_dsi_host:
mipi_dsi_host_unregister(>dsi_host);
-err_cleanup:
-   drm_encoder_cleanup(>encoder);
-   drm_connector_cleanup(>connector);
-err_pllref:
+err_disable_pm_runtime:
+   pm_runtime_disable(dev);
+   dsi->connector.funcs->destroy(>connector);
+   dsi->encoder.funcs->destroy(>encoder);
+err_disable_pllref:
clk_disable_unprepare(dsi->pllref_clk);
return ret;
 }
@@ -1320,6 +1321,10 @@ static void dw_mipi_dsi_unbind(struct device *dev, 
struct device *master,
 
mipi_dsi_host_unregister(>dsi_host);
pm_runtime_disable(dev);
+
+   dsi->connector.funcs->destroy(>connector);
+   dsi->encoder.funcs->destroy(>encoder);
+
clk_disable_unprepare(dsi->pllref_clk);
 }
 
-- 
2.11.0

Re: [PATCH 1/10] KVM: PPC: Book3S HV: Use ARRAY_SIZE macro

[Paul Mackerras]

On Sun, Sep 03, 2017 at 02:19:31PM +0200, Thomas Meyer wrote:
> Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> yourself.
> Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> /ARRAY_SIZE(\1)/g' and manual check/verification.
> 
> Signed-off-by: Thomas Meyer 

Thanks, applied to my kvm-ppc-next branch.

Paul.

[PATCH v6 06/10] drm/rockchip: dw-mipi-dsi: Fix error handling path

[Jeffy Chen]

Add missing pm_runtime_disable() in bind()'s error handling path.

Also cleanup encoder & connector in unbind().

Fixes: 80a9a059d4e4 ("drm/rockchip/dsi: add dw-mipi power domain support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Call the destroy hook in the error handling path like in unbind().

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index b15755b6129c..e72d4e2b61aa 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -1282,7 +1282,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
ret = dw_mipi_dsi_register(drm, dsi);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to register mipi_dsi: %d\n", ret);
-   goto err_pllref;
+   goto err_disable_pllref;
}
 
pm_runtime_enable(dev);
@@ -1292,23 +1292,24 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
ret = mipi_dsi_host_register(>dsi_host);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to register MIPI host: %d\n", ret);
-   goto err_cleanup;
+   goto err_disable_pm_runtime;
}
 
if (!dsi->panel) {
ret = -EPROBE_DEFER;
-   goto err_mipi_dsi_host;
+   goto err_unreg_mipi_dsi_host;
}
 
dev_set_drvdata(dev, dsi);
return 0;
 
-err_mipi_dsi_host:
+err_unreg_mipi_dsi_host:
mipi_dsi_host_unregister(>dsi_host);
-err_cleanup:
-   drm_encoder_cleanup(>encoder);
-   drm_connector_cleanup(>connector);
-err_pllref:
+err_disable_pm_runtime:
+   pm_runtime_disable(dev);
+   dsi->connector.funcs->destroy(>connector);
+   dsi->encoder.funcs->destroy(>encoder);
+err_disable_pllref:
clk_disable_unprepare(dsi->pllref_clk);
return ret;
 }
@@ -1320,6 +1321,10 @@ static void dw_mipi_dsi_unbind(struct device *dev, 
struct device *master,
 
mipi_dsi_host_unregister(>dsi_host);
pm_runtime_disable(dev);
+
+   dsi->connector.funcs->destroy(>connector);
+   dsi->encoder.funcs->destroy(>encoder);
+
clk_disable_unprepare(dsi->pllref_clk);
 }
 
-- 
2.11.0

Re: [PATCH 1/10] KVM: PPC: Book3S HV: Use ARRAY_SIZE macro

[Paul Mackerras]

On Sun, Sep 03, 2017 at 02:19:31PM +0200, Thomas Meyer wrote:
> Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> yourself.
> Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> /ARRAY_SIZE(\1)/g' and manual check/verification.
> 
> Signed-off-by: Thomas Meyer 

Thanks, applied to my kvm-ppc-next branch.

Paul.

[PATCH v6 10/10] drm/rockchip: dw_hdmi: Fix error handling path

[Jeffy Chen]

Add missing clk_disable_unprepare() in bind()'s error handling path and
unbind().

Also inline clk_prepare_enable() with bind().

Fixes: 12b9f204e804 ("drm: bridge/dw_hdmi: add rockchip rk3288 support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Add disable to unbind(), and inline clk_prepare_enable() with bind().

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 791ab938f998..e936dfe6c03d 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -193,13 +193,6 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi 
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
 
-   ret = clk_prepare_enable(hdmi->vpll_clk);
-   if (ret) {
-   DRM_DEV_ERROR(hdmi->dev,
- "Failed to enable HDMI vpll: %d\n", ret);
-   return ret;
-   }
-
return 0;
 }
 
@@ -374,6 +367,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
+   ret = clk_prepare_enable(hdmi->vpll_clk);
+   if (ret) {
+   DRM_DEV_ERROR(hdmi->dev,
+ "Failed to enable HDMI vpll: %d\n", ret);
+   return ret;
+   }
+
drm_encoder_helper_add(encoder, _hdmi_rockchip_encoder_helper_funcs);
drm_encoder_init(drm, encoder, _hdmi_rockchip_encoder_funcs,
 DRM_MODE_ENCODER_TMDS, NULL);
@@ -381,6 +381,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct 
device *master,
hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
if (IS_ERR(hdmi->hdmi)) {
encoder->funcs->destroy(encoder);
+   clk_disable_unprepare(hdmi->vpll_clk);
return PTR_ERR(hdmi->hdmi);
}
 
@@ -396,6 +397,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, 
struct device *master,
 
dw_hdmi_unbind(hdmi->hdmi);
hdmi->encoder.funcs->destroy(>encoder);
+   clk_disable_unprepare(hdmi->vpll_clk);
 }
 
 static const struct component_ops dw_hdmi_rockchip_ops = {
-- 
2.11.0

[PATCH v6 10/10] drm/rockchip: dw_hdmi: Fix error handling path

[Jeffy Chen]

Add missing clk_disable_unprepare() in bind()'s error handling path and
unbind().

Also inline clk_prepare_enable() with bind().

Fixes: 12b9f204e804 ("drm: bridge/dw_hdmi: add rockchip rk3288 support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Add disable to unbind(), and inline clk_prepare_enable() with bind().

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c 
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 791ab938f998..e936dfe6c03d 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -193,13 +193,6 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi 
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
 
-   ret = clk_prepare_enable(hdmi->vpll_clk);
-   if (ret) {
-   DRM_DEV_ERROR(hdmi->dev,
- "Failed to enable HDMI vpll: %d\n", ret);
-   return ret;
-   }
-
return 0;
 }
 
@@ -374,6 +367,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, 
struct device *master,
return ret;
}
 
+   ret = clk_prepare_enable(hdmi->vpll_clk);
+   if (ret) {
+   DRM_DEV_ERROR(hdmi->dev,
+ "Failed to enable HDMI vpll: %d\n", ret);
+   return ret;
+   }
+
drm_encoder_helper_add(encoder, _hdmi_rockchip_encoder_helper_funcs);
drm_encoder_init(drm, encoder, _hdmi_rockchip_encoder_funcs,
 DRM_MODE_ENCODER_TMDS, NULL);
@@ -381,6 +381,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct 
device *master,
hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
if (IS_ERR(hdmi->hdmi)) {
encoder->funcs->destroy(encoder);
+   clk_disable_unprepare(hdmi->vpll_clk);
return PTR_ERR(hdmi->hdmi);
}
 
@@ -396,6 +397,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, 
struct device *master,
 
dw_hdmi_unbind(hdmi->hdmi);
hdmi->encoder.funcs->destroy(>encoder);
+   clk_disable_unprepare(hdmi->vpll_clk);
 }
 
 static const struct component_ops dw_hdmi_rockchip_ops = {
-- 
2.11.0

[PATCH v6 09/10] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata

[Jeffy Chen]

Let plat drivers own the drvdata, so that they could cleanup resources
in their unbind().

Signed-off-by: Jeffy Chen 
Reviewed-by: Neil Armstrong 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 43 ++---
 drivers/gpu/drm/imx/dw_hdmi-imx.c   | 22 +--
 drivers/gpu/drm/meson/meson_dw_hdmi.c   | 20 ++
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c  | 14 --
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ---
 include/drm/bridge/dw_hdmi.h| 17 ++--
 6 files changed, 77 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ff1b3d2b5d06..6fbfafc5832b 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2072,7 +2072,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
return ret;
 }
 
-void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
 {
mutex_lock(>mutex);
 
@@ -2098,13 +2098,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool 
hpd, bool rx_sense)
}
mutex_unlock(>mutex);
 }
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
-{
-   struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
-   __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
-}
 EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
 
 static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
@@ -2140,9 +2133,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 */
if (intr_stat &
(HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
-   __dw_hdmi_setup_rx_sense(hdmi,
-phy_stat & HDMI_PHY_HPD,
-phy_stat & HDMI_PHY_RX_SENSE);
+   dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
+  phy_stat & HDMI_PHY_RX_SENSE);
 
if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
cec_notifier_set_phys_addr(hdmi->cec_notifier,
@@ -2512,8 +2504,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (hdmi->i2c)
dw_hdmi_i2c_init(hdmi);
 
-   platform_set_drvdata(pdev, hdmi);
-
return hdmi;
 
 err_iahb:
@@ -2559,25 +2549,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 /* 
-
  * Probe/remove API, used from platforms based on the DRM bridge API.
  */
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data)
 {
struct dw_hdmi *hdmi;
 
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
-   return PTR_ERR(hdmi);
+   return hdmi;
 
drm_bridge_add(>bridge);
 
-   return 0;
+   return hdmi;
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_probe);
 
-void dw_hdmi_remove(struct platform_device *pdev)
+void dw_hdmi_remove(struct dw_hdmi *hdmi)
 {
-   struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
-
drm_bridge_remove(>bridge);
 
__dw_hdmi_remove(hdmi);
@@ -2587,31 +2575,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
 /* 
-
  * Bind/unbind API, used from platforms based on the component framework.
  */
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
-const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
+   struct drm_encoder *encoder,
+   const struct dw_hdmi_plat_data *plat_data)
 {
struct dw_hdmi *hdmi;
int ret;
 
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
-   return PTR_ERR(hdmi);
+   return hdmi;
 
ret = drm_bridge_attach(encoder, >bridge, NULL);
if (ret) {
__dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
-   return ret;
+   return ERR_PTR(ret);
}
 
-   return 0;
+   return hdmi;
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_bind);
 
-void dw_hdmi_unbind(struct device *dev)
+void dw_hdmi_unbind(struct dw_hdmi *hdmi)
 {
-   struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
__dw_hdmi_remove(hdmi);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c 
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index b62763aa8706..b01d03e02ce0 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ 

[PATCH v6 08/10] drm/bridge/synopsys: dw-hdmi: Add missing bridge detach

[Jeffy Chen]

We inited connector in attach(), so need a detach() to cleanup.

Also fix wrong use of dw_hdmi_remove() in bind().

Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..ff1b3d2b5d06 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1966,6 +1966,13 @@ static int dw_hdmi_bridge_attach(struct drm_bridge 
*bridge)
return 0;
 }
 
+static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
+{
+   struct dw_hdmi *hdmi = bridge->driver_private;
+
+   drm_connector_cleanup(>connector);
+}
+
 static enum drm_mode_status
 dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
  const struct drm_display_mode *mode)
@@ -2022,6 +2029,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge 
*bridge)
 
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
.attach = dw_hdmi_bridge_attach,
+   .detach = dw_hdmi_bridge_detach,
.enable = dw_hdmi_bridge_enable,
.disable = dw_hdmi_bridge_disable,
.mode_set = dw_hdmi_bridge_mode_set,
@@ -2591,7 +2599,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct 
drm_encoder *encoder,
 
ret = drm_bridge_attach(encoder, >bridge, NULL);
if (ret) {
-   dw_hdmi_remove(pdev);
+   __dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
return ret;
}
-- 
2.11.0

[PATCH v6 09/10] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata

[Jeffy Chen]

Let plat drivers own the drvdata, so that they could cleanup resources
in their unbind().

Signed-off-by: Jeffy Chen 
Reviewed-by: Neil Armstrong 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 43 ++---
 drivers/gpu/drm/imx/dw_hdmi-imx.c   | 22 +--
 drivers/gpu/drm/meson/meson_dw_hdmi.c   | 20 ++
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c  | 14 --
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ---
 include/drm/bridge/dw_hdmi.h| 17 ++--
 6 files changed, 77 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ff1b3d2b5d06..6fbfafc5832b 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2072,7 +2072,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
return ret;
 }
 
-void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
 {
mutex_lock(>mutex);
 
@@ -2098,13 +2098,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool 
hpd, bool rx_sense)
}
mutex_unlock(>mutex);
 }
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
-{
-   struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
-   __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
-}
 EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
 
 static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
@@ -2140,9 +2133,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 */
if (intr_stat &
(HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
-   __dw_hdmi_setup_rx_sense(hdmi,
-phy_stat & HDMI_PHY_HPD,
-phy_stat & HDMI_PHY_RX_SENSE);
+   dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
+  phy_stat & HDMI_PHY_RX_SENSE);
 
if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
cec_notifier_set_phys_addr(hdmi->cec_notifier,
@@ -2512,8 +2504,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (hdmi->i2c)
dw_hdmi_i2c_init(hdmi);
 
-   platform_set_drvdata(pdev, hdmi);
-
return hdmi;
 
 err_iahb:
@@ -2559,25 +2549,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 /* 
-
  * Probe/remove API, used from platforms based on the DRM bridge API.
  */
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data)
 {
struct dw_hdmi *hdmi;
 
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
-   return PTR_ERR(hdmi);
+   return hdmi;
 
drm_bridge_add(>bridge);
 
-   return 0;
+   return hdmi;
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_probe);
 
-void dw_hdmi_remove(struct platform_device *pdev)
+void dw_hdmi_remove(struct dw_hdmi *hdmi)
 {
-   struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
-
drm_bridge_remove(>bridge);
 
__dw_hdmi_remove(hdmi);
@@ -2587,31 +2575,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
 /* 
-
  * Bind/unbind API, used from platforms based on the component framework.
  */
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
-const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
+   struct drm_encoder *encoder,
+   const struct dw_hdmi_plat_data *plat_data)
 {
struct dw_hdmi *hdmi;
int ret;
 
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
-   return PTR_ERR(hdmi);
+   return hdmi;
 
ret = drm_bridge_attach(encoder, >bridge, NULL);
if (ret) {
__dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
-   return ret;
+   return ERR_PTR(ret);
}
 
-   return 0;
+   return hdmi;
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_bind);
 
-void dw_hdmi_unbind(struct device *dev)
+void dw_hdmi_unbind(struct dw_hdmi *hdmi)
 {
-   struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
__dw_hdmi_remove(hdmi);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c 
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index b62763aa8706..b01d03e02ce0 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -26,6 +26,8 @@ struct 

[PATCH v6 08/10] drm/bridge/synopsys: dw-hdmi: Add missing bridge detach

[Jeffy Chen]

We inited connector in attach(), so need a detach() to cleanup.

Also fix wrong use of dw_hdmi_remove() in bind().

Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..ff1b3d2b5d06 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1966,6 +1966,13 @@ static int dw_hdmi_bridge_attach(struct drm_bridge 
*bridge)
return 0;
 }
 
+static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
+{
+   struct dw_hdmi *hdmi = bridge->driver_private;
+
+   drm_connector_cleanup(>connector);
+}
+
 static enum drm_mode_status
 dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
  const struct drm_display_mode *mode)
@@ -2022,6 +2029,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge 
*bridge)
 
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
.attach = dw_hdmi_bridge_attach,
+   .detach = dw_hdmi_bridge_detach,
.enable = dw_hdmi_bridge_enable,
.disable = dw_hdmi_bridge_disable,
.mode_set = dw_hdmi_bridge_mode_set,
@@ -2591,7 +2599,7 @@ int dw_hdmi_bind(struct platform_device *pdev, struct 
drm_encoder *encoder,
 
ret = drm_bridge_attach(encoder, >bridge, NULL);
if (ret) {
-   dw_hdmi_remove(pdev);
+   __dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
return ret;
}
-- 
2.11.0

[PATCH v6 07/10] drm/rockchip: inno_hdmi: Fix error handling path

[Jeffy Chen]

Add missing error handling in bind().

Fixes: 412d4ae6b7a5 ("drm/rockchip: hdmi: add Innosilicon HDMI support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Call the destroy hook in the error handling path like in unbind().
Update cleanup order in unbind().

 drivers/gpu/drm/rockchip/inno_hdmi.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index ee584d87111f..9a96ff6b022b 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -851,8 +851,10 @@ static int inno_hdmi_bind(struct device *dev, struct 
device *master,
}
 
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
-   return irq;
+   if (irq < 0) {
+   ret = irq;
+   goto err_disable_clk;
+   }
 
inno_hdmi_reset(hdmi);
 
@@ -860,7 +862,7 @@ static int inno_hdmi_bind(struct device *dev, struct device 
*master,
if (IS_ERR(hdmi->ddc)) {
ret = PTR_ERR(hdmi->ddc);
hdmi->ddc = NULL;
-   return ret;
+   goto err_disable_clk;
}
 
/*
@@ -874,7 +876,7 @@ static int inno_hdmi_bind(struct device *dev, struct device 
*master,
 
ret = inno_hdmi_register(drm, hdmi);
if (ret)
-   return ret;
+   goto err_put_adapter;
 
dev_set_drvdata(dev, hdmi);
 
@@ -884,7 +886,17 @@ static int inno_hdmi_bind(struct device *dev, struct 
device *master,
ret = devm_request_threaded_irq(dev, irq, inno_hdmi_hardirq,
inno_hdmi_irq, IRQF_SHARED,
dev_name(dev), hdmi);
+   if (ret < 0)
+   goto err_cleanup_hdmi;
 
+   return 0;
+err_cleanup_hdmi:
+   hdmi->connector.funcs->destroy(>connector);
+   hdmi->encoder.funcs->destroy(>encoder);
+err_put_adapter:
+   i2c_put_adapter(hdmi->ddc);
+err_disable_clk:
+   clk_disable_unprepare(hdmi->pclk);
return ret;
 }
 
@@ -896,8 +908,8 @@ static void inno_hdmi_unbind(struct device *dev, struct 
device *master,
hdmi->connector.funcs->destroy(>connector);
hdmi->encoder.funcs->destroy(>encoder);
 
-   clk_disable_unprepare(hdmi->pclk);
i2c_put_adapter(hdmi->ddc);
+   clk_disable_unprepare(hdmi->pclk);
 }
 
 static const struct component_ops inno_hdmi_ops = {
-- 
2.11.0

[PATCH v6 07/10] drm/rockchip: inno_hdmi: Fix error handling path

[Jeffy Chen]

Add missing error handling in bind().

Fixes: 412d4ae6b7a5 ("drm/rockchip: hdmi: add Innosilicon HDMI support")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5:
Call the destroy hook in the error handling path like in unbind().
Update cleanup order in unbind().

 drivers/gpu/drm/rockchip/inno_hdmi.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index ee584d87111f..9a96ff6b022b 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -851,8 +851,10 @@ static int inno_hdmi_bind(struct device *dev, struct 
device *master,
}
 
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
-   return irq;
+   if (irq < 0) {
+   ret = irq;
+   goto err_disable_clk;
+   }
 
inno_hdmi_reset(hdmi);
 
@@ -860,7 +862,7 @@ static int inno_hdmi_bind(struct device *dev, struct device 
*master,
if (IS_ERR(hdmi->ddc)) {
ret = PTR_ERR(hdmi->ddc);
hdmi->ddc = NULL;
-   return ret;
+   goto err_disable_clk;
}
 
/*
@@ -874,7 +876,7 @@ static int inno_hdmi_bind(struct device *dev, struct device 
*master,
 
ret = inno_hdmi_register(drm, hdmi);
if (ret)
-   return ret;
+   goto err_put_adapter;
 
dev_set_drvdata(dev, hdmi);
 
@@ -884,7 +886,17 @@ static int inno_hdmi_bind(struct device *dev, struct 
device *master,
ret = devm_request_threaded_irq(dev, irq, inno_hdmi_hardirq,
inno_hdmi_irq, IRQF_SHARED,
dev_name(dev), hdmi);
+   if (ret < 0)
+   goto err_cleanup_hdmi;
 
+   return 0;
+err_cleanup_hdmi:
+   hdmi->connector.funcs->destroy(>connector);
+   hdmi->encoder.funcs->destroy(>encoder);
+err_put_adapter:
+   i2c_put_adapter(hdmi->ddc);
+err_disable_clk:
+   clk_disable_unprepare(hdmi->pclk);
return ret;
 }
 
@@ -896,8 +908,8 @@ static void inno_hdmi_unbind(struct device *dev, struct 
device *master,
hdmi->connector.funcs->destroy(>connector);
hdmi->encoder.funcs->destroy(>encoder);
 
-   clk_disable_unprepare(hdmi->pclk);
i2c_put_adapter(hdmi->ddc);
+   clk_disable_unprepare(hdmi->pclk);
 }
 
 static const struct component_ops inno_hdmi_ops = {
-- 
2.11.0

Re: [PATCH 6/7] KVM: PPC: Cocci spatch "vma_pages"

[Paul Mackerras]

On Thu, Sep 21, 2017 at 12:29:36AM +0200, Thomas Meyer wrote:
> Use vma_pages function on vma object instead of explicit computation.
> Found by coccinelle spatch "api/vma_pages.cocci"
> 
> Signed-off-by: Thomas Meyer 

Thanks, applied to my kvm-ppc-next branch, with the headline "KVM:
PPC: BookE: Use vma_pages function".

Paul.

[PATCH v6 05/10] drm/rockchip: analogix_dp: Add a sanity check for rockchip_drm_psr_register()

[Jeffy Chen]

The rockchip_drm_psr_register() can fail, so add a sanity check for that.

Also reorder the calls in unbind() to match bind().

Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 117585df73e1..bd3567ad8b53 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -356,15 +356,22 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
INIT_WORK(>psr_work, analogix_dp_psr_work);
 
-   rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
+   ret = rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
+   if (ret < 0)
+   goto err_cleanup_encoder;
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
if (IS_ERR(dp->adp)) {
-   dp->encoder.funcs->destroy(>encoder);
-   return PTR_ERR(dp->adp);
+   ret = PTR_ERR(dp->adp);
+   goto err_unreg_psr;
}
 
return 0;
+err_unreg_psr:
+   rockchip_drm_psr_unregister(>encoder);
+err_cleanup_encoder:
+   dp->encoder.funcs->destroy(>encoder);
+   return ret;
 }
 
 static void rockchip_dp_unbind(struct device *dev, struct device *master,
@@ -372,8 +379,8 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
 {
struct rockchip_dp_device *dp = dev_get_drvdata(dev);
 
-   rockchip_drm_psr_unregister(>encoder);
analogix_dp_unbind(dp->adp);
+   rockchip_drm_psr_unregister(>encoder);
dp->encoder.funcs->destroy(>encoder);
 }
 
-- 
2.11.0

[PATCH v6 05/10] drm/rockchip: analogix_dp: Add a sanity check for rockchip_drm_psr_register()

[Jeffy Chen]

The rockchip_drm_psr_register() can fail, so add a sanity check for that.

Also reorder the calls in unbind() to match bind().

Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 117585df73e1..bd3567ad8b53 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -356,15 +356,22 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
INIT_WORK(>psr_work, analogix_dp_psr_work);
 
-   rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
+   ret = rockchip_drm_psr_register(>encoder, analogix_dp_psr_set);
+   if (ret < 0)
+   goto err_cleanup_encoder;
 
dp->adp = analogix_dp_bind(dev, dp->drm_dev, >plat_data);
if (IS_ERR(dp->adp)) {
-   dp->encoder.funcs->destroy(>encoder);
-   return PTR_ERR(dp->adp);
+   ret = PTR_ERR(dp->adp);
+   goto err_unreg_psr;
}
 
return 0;
+err_unreg_psr:
+   rockchip_drm_psr_unregister(>encoder);
+err_cleanup_encoder:
+   dp->encoder.funcs->destroy(>encoder);
+   return ret;
 }
 
 static void rockchip_dp_unbind(struct device *dev, struct device *master,
@@ -372,8 +379,8 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
 {
struct rockchip_dp_device *dp = dev_get_drvdata(dev);
 
-   rockchip_drm_psr_unregister(>encoder);
analogix_dp_unbind(dp->adp);
+   rockchip_drm_psr_unregister(>encoder);
dp->encoder.funcs->destroy(>encoder);
 }
 
-- 
2.11.0

Re: [PATCH 6/7] KVM: PPC: Cocci spatch "vma_pages"

[Paul Mackerras]

On Thu, Sep 21, 2017 at 12:29:36AM +0200, Thomas Meyer wrote:
> Use vma_pages function on vma object instead of explicit computation.
> Found by coccinelle spatch "api/vma_pages.cocci"
> 
> Signed-off-by: Thomas Meyer 

Thanks, applied to my kvm-ppc-next branch, with the headline "KVM:
PPC: BookE: Use vma_pages function".

Paul.

[PATCH v6 02/10] drm/rockchip: analogix_dp: Remove unnecessary init code

[Jeffy Chen]

Remove unnecessary init code, since we would do it in the power_on()
callback.

Also move of parse code to probe().

Fixes: 9e32e16e9e98 ("drm: rockchip: dp: add rockchip platform dp driver")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 27 ++---
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 4d3f6ad0abdd..8cae5ad926cd 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -269,7 +269,7 @@ static struct drm_encoder_funcs rockchip_dp_encoder_funcs = 
{
.destroy = rockchip_dp_drm_encoder_destroy,
 };
 
-static int rockchip_dp_init(struct rockchip_dp_device *dp)
+static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 {
struct device *dev = dp->dev;
struct device_node *np = dev->of_node;
@@ -303,19 +303,6 @@ static int rockchip_dp_init(struct rockchip_dp_device *dp)
return PTR_ERR(dp->rst);
}
 
-   ret = clk_prepare_enable(dp->pclk);
-   if (ret < 0) {
-   DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
-   return ret;
-   }
-
-   ret = rockchip_dp_pre_init(dp);
-   if (ret < 0) {
-   DRM_DEV_ERROR(dp->dev, "failed to pre init %d\n", ret);
-   clk_disable_unprepare(dp->pclk);
-   return ret;
-   }
-
return 0;
 }
 
@@ -361,10 +348,6 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
if (!dp_data)
return -ENODEV;
 
-   ret = rockchip_dp_init(dp);
-   if (ret < 0)
-   return ret;
-
dp->data = dp_data;
dp->drm_dev = drm_dev;
 
@@ -398,7 +381,6 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
rockchip_drm_psr_unregister(>encoder);
 
analogix_dp_unbind(dev, master, data);
-   clk_disable_unprepare(dp->pclk);
 }
 
 static const struct component_ops rockchip_dp_component_ops = {
@@ -414,7 +396,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
int ret;
 
ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL);
-   if (ret)
+   if (ret < 0)
return ret;
 
dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
@@ -422,9 +404,12 @@ static int rockchip_dp_probe(struct platform_device *pdev)
return -ENOMEM;
 
dp->dev = dev;
-
dp->plat_data.panel = panel;
 
+   ret = rockchip_dp_of_probe(dp);
+   if (ret < 0)
+   return ret;
+
/*
 * We just use the drvdata until driver run into component
 * add function, and then we would set drvdata to null, so
-- 
2.11.0

[PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata

[Jeffy Chen]

The driver that instantiates the bridge should own the drvdata, as all
driver model callbacks (probe, remove, shutdown, PM ops, etc.) are also
owned by its driver struct. Moreover, storing two different pointer
types in driver data depending on driver initialization status is barely
a good practice and in fact has led to many bugs in this driver.

Let's clean up this mess and change Analogix entry points to simply
accept some opaque struct pointer, adjusting their users at the same
time to avoid breaking the compilation.

Signed-off-by: Tomasz Figa 
Signed-off-by: Jeffy Chen 
Reviewed-by: Andrzej Hajda 
Reviewed-by: Sean Paul 
Acked-by: Jingoo Han 
Acked-by: Archit Taneja 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 50 +-
 drivers/gpu/drm/exynos/exynos_dp.c | 26 ++-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 49 -
 include/drm/bridge/analogix_dp.h   | 19 
 4 files changed, 74 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5dd3f1cd074a..74d274b6d31d 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -98,17 +98,15 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device 
*dp)
return 0;
 }
 
-int analogix_dp_psr_supported(struct device *dev)
+int analogix_dp_psr_supported(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
 
return dp->psr_support;
 }
 EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
 
-int analogix_dp_enable_psr(struct device *dev)
+int analogix_dp_enable_psr(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
 
if (!dp->psr_support)
@@ -129,9 +127,8 @@ int analogix_dp_enable_psr(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
-int analogix_dp_disable_psr(struct device *dev)
+int analogix_dp_disable_psr(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
int ret;
 
@@ -1281,8 +1278,9 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux 
*aux,
return analogix_dp_transfer(dp, msg);
 }
 
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
-struct analogix_dp_plat_data *plat_data)
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+struct analogix_dp_plat_data *plat_data)
 {
struct platform_device *pdev = to_platform_device(dev);
struct analogix_dp_device *dp;
@@ -1292,14 +1290,12 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
if (!plat_data) {
dev_err(dev, "Invalided input plat_data\n");
-   return -EINVAL;
+   return ERR_PTR(-EINVAL);
}
 
dp = devm_kzalloc(dev, sizeof(struct analogix_dp_device), GFP_KERNEL);
if (!dp)
-   return -ENOMEM;
-
-   dev_set_drvdata(dev, dp);
+   return ERR_PTR(-ENOMEM);
 
dp->dev = >dev;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1316,7 +1312,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
ret = analogix_dp_dt_parse_pdata(dp);
if (ret)
-   return ret;
+   return ERR_PTR(ret);
 
dp->phy = devm_phy_get(dp->dev, "dp");
if (IS_ERR(dp->phy)) {
@@ -1330,14 +1326,14 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
if (ret == -ENOSYS || ret == -ENODEV)
dp->phy = NULL;
else
-   return ret;
+   return ERR_PTR(ret);
}
}
 
dp->clock = devm_clk_get(>dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(>dev, "failed to get clock\n");
-   return PTR_ERR(dp->clock);
+   return ERR_CAST(dp->clock);
}
 
clk_prepare_enable(dp->clock);
@@ -1346,7 +1342,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
dp->reg_base = devm_ioremap_resource(>dev, res);
if (IS_ERR(dp->reg_base))
-   return PTR_ERR(dp->reg_base);
+   return ERR_CAST(dp->reg_base);
 
dp->force_hpd = of_property_read_bool(dev->of_node, "force-hpd");
 
@@ -1367,7 +1363,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
"hpd_gpio");
if (ret) {

[PATCH v6 02/10] drm/rockchip: analogix_dp: Remove unnecessary init code

[Jeffy Chen]

Remove unnecessary init code, since we would do it in the power_on()
callback.

Also move of parse code to probe().

Fixes: 9e32e16e9e98 ("drm: rockchip: dp: add rockchip platform dp driver")
Signed-off-by: Jeffy Chen 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 27 ++---
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 4d3f6ad0abdd..8cae5ad926cd 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -269,7 +269,7 @@ static struct drm_encoder_funcs rockchip_dp_encoder_funcs = 
{
.destroy = rockchip_dp_drm_encoder_destroy,
 };
 
-static int rockchip_dp_init(struct rockchip_dp_device *dp)
+static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 {
struct device *dev = dp->dev;
struct device_node *np = dev->of_node;
@@ -303,19 +303,6 @@ static int rockchip_dp_init(struct rockchip_dp_device *dp)
return PTR_ERR(dp->rst);
}
 
-   ret = clk_prepare_enable(dp->pclk);
-   if (ret < 0) {
-   DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
-   return ret;
-   }
-
-   ret = rockchip_dp_pre_init(dp);
-   if (ret < 0) {
-   DRM_DEV_ERROR(dp->dev, "failed to pre init %d\n", ret);
-   clk_disable_unprepare(dp->pclk);
-   return ret;
-   }
-
return 0;
 }
 
@@ -361,10 +348,6 @@ static int rockchip_dp_bind(struct device *dev, struct 
device *master,
if (!dp_data)
return -ENODEV;
 
-   ret = rockchip_dp_init(dp);
-   if (ret < 0)
-   return ret;
-
dp->data = dp_data;
dp->drm_dev = drm_dev;
 
@@ -398,7 +381,6 @@ static void rockchip_dp_unbind(struct device *dev, struct 
device *master,
rockchip_drm_psr_unregister(>encoder);
 
analogix_dp_unbind(dev, master, data);
-   clk_disable_unprepare(dp->pclk);
 }
 
 static const struct component_ops rockchip_dp_component_ops = {
@@ -414,7 +396,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
int ret;
 
ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL);
-   if (ret)
+   if (ret < 0)
return ret;
 
dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
@@ -422,9 +404,12 @@ static int rockchip_dp_probe(struct platform_device *pdev)
return -ENOMEM;
 
dp->dev = dev;
-
dp->plat_data.panel = panel;
 
+   ret = rockchip_dp_of_probe(dp);
+   if (ret < 0)
+   return ret;
+
/*
 * We just use the drvdata until driver run into component
 * add function, and then we would set drvdata to null, so
-- 
2.11.0

[PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata

[Jeffy Chen]

The driver that instantiates the bridge should own the drvdata, as all
driver model callbacks (probe, remove, shutdown, PM ops, etc.) are also
owned by its driver struct. Moreover, storing two different pointer
types in driver data depending on driver initialization status is barely
a good practice and in fact has led to many bugs in this driver.

Let's clean up this mess and change Analogix entry points to simply
accept some opaque struct pointer, adjusting their users at the same
time to avoid breaking the compilation.

Signed-off-by: Tomasz Figa 
Signed-off-by: Jeffy Chen 
Reviewed-by: Andrzej Hajda 
Reviewed-by: Sean Paul 
Acked-by: Jingoo Han 
Acked-by: Archit Taneja 
---

Changes in v6: None
Changes in v5: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 50 +-
 drivers/gpu/drm/exynos/exynos_dp.c | 26 ++-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 49 -
 include/drm/bridge/analogix_dp.h   | 19 
 4 files changed, 74 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5dd3f1cd074a..74d274b6d31d 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -98,17 +98,15 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device 
*dp)
return 0;
 }
 
-int analogix_dp_psr_supported(struct device *dev)
+int analogix_dp_psr_supported(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
 
return dp->psr_support;
 }
 EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
 
-int analogix_dp_enable_psr(struct device *dev)
+int analogix_dp_enable_psr(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
 
if (!dp->psr_support)
@@ -129,9 +127,8 @@ int analogix_dp_enable_psr(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
-int analogix_dp_disable_psr(struct device *dev)
+int analogix_dp_disable_psr(struct analogix_dp_device *dp)
 {
-   struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
int ret;
 
@@ -1281,8 +1278,9 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux 
*aux,
return analogix_dp_transfer(dp, msg);
 }
 
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
-struct analogix_dp_plat_data *plat_data)
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+struct analogix_dp_plat_data *plat_data)
 {
struct platform_device *pdev = to_platform_device(dev);
struct analogix_dp_device *dp;
@@ -1292,14 +1290,12 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
if (!plat_data) {
dev_err(dev, "Invalided input plat_data\n");
-   return -EINVAL;
+   return ERR_PTR(-EINVAL);
}
 
dp = devm_kzalloc(dev, sizeof(struct analogix_dp_device), GFP_KERNEL);
if (!dp)
-   return -ENOMEM;
-
-   dev_set_drvdata(dev, dp);
+   return ERR_PTR(-ENOMEM);
 
dp->dev = >dev;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1316,7 +1312,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
ret = analogix_dp_dt_parse_pdata(dp);
if (ret)
-   return ret;
+   return ERR_PTR(ret);
 
dp->phy = devm_phy_get(dp->dev, "dp");
if (IS_ERR(dp->phy)) {
@@ -1330,14 +1326,14 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
if (ret == -ENOSYS || ret == -ENODEV)
dp->phy = NULL;
else
-   return ret;
+   return ERR_PTR(ret);
}
}
 
dp->clock = devm_clk_get(>dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(>dev, "failed to get clock\n");
-   return PTR_ERR(dp->clock);
+   return ERR_CAST(dp->clock);
}
 
clk_prepare_enable(dp->clock);
@@ -1346,7 +1342,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
 
dp->reg_base = devm_ioremap_resource(>dev, res);
if (IS_ERR(dp->reg_base))
-   return PTR_ERR(dp->reg_base);
+   return ERR_CAST(dp->reg_base);
 
dp->force_hpd = of_property_read_bool(dev->of_node, "force-hpd");
 
@@ -1367,7 +1363,7 @@ int analogix_dp_bind(struct device *dev, struct 
drm_device *drm_dev,
"hpd_gpio");
if (ret) {
dev_err(>dev, "failed to get hpd gpio\n");
-   return ret;
+   return ERR_PTR(ret);