[PATCH 3/5] kconfig.h: use already defined macros for IS_REACHABLE() define

[Masahiro Yamada]

For the same reason as commit 02d699f1f464 ("include/linux/kconfig.h:
ese macros which are already defined"), it is better to use macros
IS_BUILTIN() and IS_MODULE() for defining IS_REACHABLE().

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index a94b5bf..722c7d2 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -42,8 +42,8 @@
  * This is similar to IS_ENABLED(), but returns false when invoked from
  * built-in code when CONFIG_FOO is set to 'm'.
  */
-#define IS_REACHABLE(option) (config_enabled(option) || \
-(config_enabled(option##_MODULE) && __is_defined(MODULE)))
+#define IS_REACHABLE(option) (IS_BUILTIN(option) || \
+(IS_MODULE(option) && __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
-- 
1.9.1

[PATCH 3/5] kconfig.h: use already defined macros for IS_REACHABLE() define

[Masahiro Yamada]

For the same reason as commit 02d699f1f464 ("include/linux/kconfig.h:
ese macros which are already defined"), it is better to use macros
IS_BUILTIN() and IS_MODULE() for defining IS_REACHABLE().

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index a94b5bf..722c7d2 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -42,8 +42,8 @@
  * This is similar to IS_ENABLED(), but returns false when invoked from
  * built-in code when CONFIG_FOO is set to 'm'.
  */
-#define IS_REACHABLE(option) (config_enabled(option) || \
-(config_enabled(option##_MODULE) && __is_defined(MODULE)))
+#define IS_REACHABLE(option) (IS_BUILTIN(option) || \
+(IS_MODULE(option) && __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
-- 
1.9.1

[PATCH 1/5] kconfig.h: use __is_defined() to check if MODULE is defined

[Masahiro Yamada]

The macro MODULE is not a config option, it is a per-file build
option.  So, config_enabled(MODULE) is not sensible.  (There is
another case in include/linux/export.h, where config_enabled() is
used against a non-config option.)

This commit renames some macros in include/linux/kconfig.h for the
use for non-config macros and replaces config_enabled(MODULE) with
__is_defined(MODULE).

I am keeping config_enabled() because it is still referenced from
some places, but I expect it would be deprecated in the future.

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index b33c779..a94b5bf 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -17,10 +17,11 @@
  * the last step cherry picks the 2nd arg, we get a zero.
  */
 #define __ARG_PLACEHOLDER_1 0,
-#define config_enabled(cfg) _config_enabled(cfg)
-#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
-#define ___config_enabled(__ignored, val, ...) val
+#define config_enabled(cfg)___is_defined(cfg)
+#define __is_defined(x)___is_defined(x)
+#define ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##val)
+#define is_defined(arg1_or_junk)   __take_second_arg(arg1_or_junk 1, 0)
+#define __take_second_arg(__ignored, val, ...) val
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
@@ -42,7 +43,7 @@
  * built-in code when CONFIG_FOO is set to 'm'.
  */
 #define IS_REACHABLE(option) (config_enabled(option) || \
-(config_enabled(option##_MODULE) && config_enabled(MODULE)))
+(config_enabled(option##_MODULE) && __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
-- 
1.9.1

[PATCH 4/5] kconfig.h: allow to use IS_{ENABLE,REACHABLE} in macro expansion

[Masahiro Yamada]

The typical usage of IS_ENABLED() is

if (IS_ENABLED(CONFIG_FOO)) {
...
}

or

#if IS_ENABLED(CONFIG_FOO)
...
#endif

The current implementation of IS_ENABLED() includes "||" operator,
which works well in those expressions like above.

However, there is a case where we want to evaluate a config option
beyond those use cases.

For example, the OF_TABLE() in include/asm-generic/vmlinux.lds.h
needs to evaluate a config option in macro expansion:

  #define ___OF_TABLE(cfg, name)  _OF_TABLE_##cfg(name)
  #define __OF_TABLE(cfg, name)   ___OF_TABLE(cfg, name)
  #define OF_TABLE(cfg, name) __OF_TABLE(config_enabled(cfg), name)
  #define _OF_TABLE_0(name)
  #define _OF_TABLE_1(name)  \
  ...

Here, we can not use IS_ENABLED() because of the "||" operator in
its define.  It is true config_enabled() works well, but it is a bit
ambiguous to be used against config options.

This commit makes IS_ENABLED() available in more generic context by
calculating "or" with macro expansion only.

Do likewise for IS_REACHABLE().

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 722c7d2..15ec117 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -3,6 +3,21 @@
 
 #include 
 
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+/*
+ * The use of "&&" / "||" is limited in certain expressions.
+ * The followings enable to calculate "and" / "or" with macro expansion only.
+ */
+#define __and(x, y)___and(x, y)
+#define ___and(x, y)   and(__ARG_PLACEHOLDER_##x, y)
+#define and(arg1_or_junk, y)   __take_second_arg(arg1_or_junk y, 0)
+
+#define __or(x, y) ___or(x, y)
+#define ___or(x, y)or(__ARG_PLACEHOLDER_##x, y)
+#define or(arg1_or_junk, y)__take_second_arg(arg1_or_junk 
1, y)
+
 /*
  * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
  * these only work with boolean and tristate options.
@@ -16,12 +31,10 @@
  * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
  * the last step cherry picks the 2nd arg, we get a zero.
  */
-#define __ARG_PLACEHOLDER_1 0,
 #define config_enabled(cfg)___is_defined(cfg)
 #define __is_defined(x)___is_defined(x)
 #define ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##val)
 #define is_defined(arg1_or_junk)   __take_second_arg(arg1_or_junk 1, 0)
-#define __take_second_arg(__ignored, val, ...) val
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
@@ -42,14 +55,13 @@
  * This is similar to IS_ENABLED(), but returns false when invoked from
  * built-in code when CONFIG_FOO is set to 'm'.
  */
-#define IS_REACHABLE(option) (IS_BUILTIN(option) || \
-(IS_MODULE(option) && __is_defined(MODULE)))
+#define IS_REACHABLE(option) __or(IS_BUILTIN(option), \
+   __and(IS_MODULE(option), __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
  * 0 otherwise.
  */
-#define IS_ENABLED(option) \
-   (IS_BUILTIN(option) || IS_MODULE(option))
+#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
 
 #endif /* __LINUX_KCONFIG_H */
-- 
1.9.1

[PATCH 2/5] export.h: use __is_defined() to check if __KSYM_* is defined

[Masahiro Yamada]

Here the need is for a macro that checks whether the given symbol is
defined or not, which has nothing to do with config.

The new macro __is_defined() is a better fit for this case.

Signed-off-by: Masahiro Yamada 
---

 include/linux/export.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/export.h b/include/linux/export.h
index 2f9ccbe..c565f87 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -82,7 +82,7 @@ extern struct module __this_module;
 #include 
 
 #define __EXPORT_SYMBOL(sym, sec)  \
-   __cond_export_sym(sym, sec, config_enabled(__KSYM_##sym))
+   __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
 #define __cond_export_sym(sym, sec, conf)  \
___cond_export_sym(sym, sec, conf)
 #define ___cond_export_sym(sym, sec, enabled)  \
-- 
1.9.1

[PATCH 1/5] kconfig.h: use __is_defined() to check if MODULE is defined

[Masahiro Yamada]

The macro MODULE is not a config option, it is a per-file build
option.  So, config_enabled(MODULE) is not sensible.  (There is
another case in include/linux/export.h, where config_enabled() is
used against a non-config option.)

This commit renames some macros in include/linux/kconfig.h for the
use for non-config macros and replaces config_enabled(MODULE) with
__is_defined(MODULE).

I am keeping config_enabled() because it is still referenced from
some places, but I expect it would be deprecated in the future.

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index b33c779..a94b5bf 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -17,10 +17,11 @@
  * the last step cherry picks the 2nd arg, we get a zero.
  */
 #define __ARG_PLACEHOLDER_1 0,
-#define config_enabled(cfg) _config_enabled(cfg)
-#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
-#define ___config_enabled(__ignored, val, ...) val
+#define config_enabled(cfg)___is_defined(cfg)
+#define __is_defined(x)___is_defined(x)
+#define ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##val)
+#define is_defined(arg1_or_junk)   __take_second_arg(arg1_or_junk 1, 0)
+#define __take_second_arg(__ignored, val, ...) val
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
@@ -42,7 +43,7 @@
  * built-in code when CONFIG_FOO is set to 'm'.
  */
 #define IS_REACHABLE(option) (config_enabled(option) || \
-(config_enabled(option##_MODULE) && config_enabled(MODULE)))
+(config_enabled(option##_MODULE) && __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
-- 
1.9.1

[PATCH 4/5] kconfig.h: allow to use IS_{ENABLE,REACHABLE} in macro expansion

[Masahiro Yamada]

The typical usage of IS_ENABLED() is

if (IS_ENABLED(CONFIG_FOO)) {
...
}

or

#if IS_ENABLED(CONFIG_FOO)
...
#endif

The current implementation of IS_ENABLED() includes "||" operator,
which works well in those expressions like above.

However, there is a case where we want to evaluate a config option
beyond those use cases.

For example, the OF_TABLE() in include/asm-generic/vmlinux.lds.h
needs to evaluate a config option in macro expansion:

  #define ___OF_TABLE(cfg, name)  _OF_TABLE_##cfg(name)
  #define __OF_TABLE(cfg, name)   ___OF_TABLE(cfg, name)
  #define OF_TABLE(cfg, name) __OF_TABLE(config_enabled(cfg), name)
  #define _OF_TABLE_0(name)
  #define _OF_TABLE_1(name)  \
  ...

Here, we can not use IS_ENABLED() because of the "||" operator in
its define.  It is true config_enabled() works well, but it is a bit
ambiguous to be used against config options.

This commit makes IS_ENABLED() available in more generic context by
calculating "or" with macro expansion only.

Do likewise for IS_REACHABLE().

Signed-off-by: Masahiro Yamada 
---

 include/linux/kconfig.h | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 722c7d2..15ec117 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -3,6 +3,21 @@
 
 #include 
 
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+/*
+ * The use of "&&" / "||" is limited in certain expressions.
+ * The followings enable to calculate "and" / "or" with macro expansion only.
+ */
+#define __and(x, y)___and(x, y)
+#define ___and(x, y)   and(__ARG_PLACEHOLDER_##x, y)
+#define and(arg1_or_junk, y)   __take_second_arg(arg1_or_junk y, 0)
+
+#define __or(x, y) ___or(x, y)
+#define ___or(x, y)or(__ARG_PLACEHOLDER_##x, y)
+#define or(arg1_or_junk, y)__take_second_arg(arg1_or_junk 
1, y)
+
 /*
  * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
  * these only work with boolean and tristate options.
@@ -16,12 +31,10 @@
  * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
  * the last step cherry picks the 2nd arg, we get a zero.
  */
-#define __ARG_PLACEHOLDER_1 0,
 #define config_enabled(cfg)___is_defined(cfg)
 #define __is_defined(x)___is_defined(x)
 #define ___is_defined(val) is_defined(__ARG_PLACEHOLDER_##val)
 #define is_defined(arg1_or_junk)   __take_second_arg(arg1_or_junk 1, 0)
-#define __take_second_arg(__ignored, val, ...) val
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
@@ -42,14 +55,13 @@
  * This is similar to IS_ENABLED(), but returns false when invoked from
  * built-in code when CONFIG_FOO is set to 'm'.
  */
-#define IS_REACHABLE(option) (IS_BUILTIN(option) || \
-(IS_MODULE(option) && __is_defined(MODULE)))
+#define IS_REACHABLE(option) __or(IS_BUILTIN(option), \
+   __and(IS_MODULE(option), __is_defined(MODULE)))
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
  * 0 otherwise.
  */
-#define IS_ENABLED(option) \
-   (IS_BUILTIN(option) || IS_MODULE(option))
+#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
 
 #endif /* __LINUX_KCONFIG_H */
-- 
1.9.1

[PATCH 2/5] export.h: use __is_defined() to check if __KSYM_* is defined

[Masahiro Yamada]

Here the need is for a macro that checks whether the given symbol is
defined or not, which has nothing to do with config.

The new macro __is_defined() is a better fit for this case.

Signed-off-by: Masahiro Yamada 
---

 include/linux/export.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/export.h b/include/linux/export.h
index 2f9ccbe..c565f87 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -82,7 +82,7 @@ extern struct module __this_module;
 #include 
 
 #define __EXPORT_SYMBOL(sym, sec)  \
-   __cond_export_sym(sym, sec, config_enabled(__KSYM_##sym))
+   __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
 #define __cond_export_sym(sym, sec, conf)  \
___cond_export_sym(sym, sec, conf)
 #define ___cond_export_sym(sym, sec, enabled)  \
-- 
1.9.1

Re: [Intel-gfx] [PATCH] drm/i915: Fix missing unlock on error in i915_ppgtt_info()

[Daniel Vetter]

On Mon, Jun 13, 2016 at 11:42:00PM +, weiyj...@163.com wrote:
> From: Wei Yongjun 
> 
> Add the missing unlock before return from function i915_ppgtt_info()
> in the error handling case.
> 
> Fixes: 1d2ac403ae3b(drm: Protect dev->filelist with its own mutex)
> Signed-off-by: Wei Yongjun 

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3269033..1035468 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2365,16 +2365,16 @@ static int i915_ppgtt_info(struct seq_file *m, void 
> *data)
>   task = get_pid_task(file->pid, PIDTYPE_PID);
>   if (!task) {
>   ret = -ESRCH;
> - goto out_put;
> + goto out_unlock;
>   }
>   seq_printf(m, "\nproc: %s\n", task->comm);
>   put_task_struct(task);
>   idr_for_each(_priv->context_idr, per_file_ctx,
>(void *)(unsigned long)m);
>   }
> +out_unlock:
>   mutex_unlock(>filelist_mutex);
>  
> -out_put:
>   intel_runtime_pm_put(dev_priv);
>   mutex_unlock(>struct_mutex);
> 
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH] drm/i915: Fix missing unlock on error in i915_ppgtt_info()

[Daniel Vetter]

On Mon, Jun 13, 2016 at 11:42:00PM +, weiyj...@163.com wrote:
> From: Wei Yongjun 
> 
> Add the missing unlock before return from function i915_ppgtt_info()
> in the error handling case.
> 
> Fixes: 1d2ac403ae3b(drm: Protect dev->filelist with its own mutex)
> Signed-off-by: Wei Yongjun 

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3269033..1035468 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2365,16 +2365,16 @@ static int i915_ppgtt_info(struct seq_file *m, void 
> *data)
>   task = get_pid_task(file->pid, PIDTYPE_PID);
>   if (!task) {
>   ret = -ESRCH;
> - goto out_put;
> + goto out_unlock;
>   }
>   seq_printf(m, "\nproc: %s\n", task->comm);
>   put_task_struct(task);
>   idr_for_each(_priv->context_idr, per_file_ctx,
>(void *)(unsigned long)m);
>   }
> +out_unlock:
>   mutex_unlock(>filelist_mutex);
>  
> -out_put:
>   intel_runtime_pm_put(dev_priv);
>   mutex_unlock(>struct_mutex);
> 
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH net-next v3 6/7] vmxnet3: introduce command to register memory region

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:

> +++ b/drivers/net/vmxnet3/vmxnet3_defs.h
> @@ -81,6 +81,7 @@ enum {
> VMXNET3_CMD_RESERVED2,
> VMXNET3_CMD_RESERVED3,
> VMXNET3_CMD_SET_COALESCE,
> +   VMXNET3_CMD_REGISTER_MEMREGS,
>
> VMXNET3_CMD_FIRST_GET = 0xF00D,
> VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> @@ -668,6 +669,22 @@ struct Vmxnet3_CoalesceScheme {
> } coalPara;
>  };
>
> +struct Vmxnet3_MemoryRegion {
> +   __le64  startPA;
> +   __le32  length;
> +   __le16  txQueueBits;
> +   __le16  rxQueueBits;
> +};


What is the use case for this command?

What's the role of the tx/rx queue bits fields?

Re: [PATCH net-next v3 6/7] vmxnet3: introduce command to register memory region

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:

> +++ b/drivers/net/vmxnet3/vmxnet3_defs.h
> @@ -81,6 +81,7 @@ enum {
> VMXNET3_CMD_RESERVED2,
> VMXNET3_CMD_RESERVED3,
> VMXNET3_CMD_SET_COALESCE,
> +   VMXNET3_CMD_REGISTER_MEMREGS,
>
> VMXNET3_CMD_FIRST_GET = 0xF00D,
> VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> @@ -668,6 +669,22 @@ struct Vmxnet3_CoalesceScheme {
> } coalPara;
>  };
>
> +struct Vmxnet3_MemoryRegion {
> +   __le64  startPA;
> +   __le32  length;
> +   __le16  txQueueBits;
> +   __le16  rxQueueBits;
> +};


What is the use case for this command?

What's the role of the tx/rx queue bits fields?

[PATCH v2 1/1] Staging: comedi: dmm32at: fix BIT macro issue.

[Ravishankar Karkala Mallikarjunayya]

This Replace all occurences of (1<
---
Changes V1 -> V2:
- BIT macros added(suggested by Ian Abbott)
-i.e.DMM32AT_AI_CFG_SCINT(x), DMM32AT_CTRL_PAGE(x)
---
 drivers/staging/comedi/drivers/dmm32at.c | 98 
 1 file changed, 50 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dmm32at.c 
b/drivers/staging/comedi/drivers/dmm32at.c
index 958c0d4..b8606de 100644
--- a/drivers/staging/comedi/drivers/dmm32at.c
+++ b/drivers/staging/comedi/drivers/dmm32at.c
@@ -46,73 +46,75 @@
 #define DMM32AT_AI_START_CONV_REG  0x00
 #define DMM32AT_AI_LSB_REG 0x00
 #define DMM32AT_AUX_DOUT_REG   0x01
-#define DMM32AT_AUX_DOUT2  (1 << 2)  /* J3.42 - OUT2 (OUT2EN) */
-#define DMM32AT_AUX_DOUT1  (1 << 1)  /* J3.43 */
-#define DMM32AT_AUX_DOUT0  (1 << 0)  /* J3.44 - OUT0 (OUT0EN) */
+#define DMM32AT_AUX_DOUT2  BIT(2)  /* J3.42 - OUT2 (OUT2EN) */
+#define DMM32AT_AUX_DOUT1  BIT(1)  /* J3.43 */
+#define DMM32AT_AUX_DOUT0  BIT(0)  /* J3.44 - OUT0 (OUT0EN) */
 #define DMM32AT_AI_MSB_REG 0x01
 #define DMM32AT_AI_LO_CHAN_REG 0x02
 #define DMM32AT_AI_HI_CHAN_REG 0x03
 #define DMM32AT_AUX_DI_REG 0x04
-#define DMM32AT_AUX_DI_DACBUSY (1 << 7)
-#define DMM32AT_AUX_DI_CALBUSY (1 << 6)
-#define DMM32AT_AUX_DI3(1 << 3)  /* J3.45 - ADCLK 
(CLKSEL) */
-#define DMM32AT_AUX_DI2(1 << 2)  /* J3.46 - GATE12 
(GT12EN) */
-#define DMM32AT_AUX_DI1(1 << 1)  /* J3.47 - GATE0 
(GT0EN) */
-#define DMM32AT_AUX_DI0(1 << 0)  /* J3.48 - CLK0 
(SRC0) */
+#define DMM32AT_AUX_DI_DACBUSY BIT(7)
+#define DMM32AT_AUX_DI_CALBUSY BIT(6)
+#define DMM32AT_AUX_DI3BIT(3)  /* J3.45 - ADCLK 
(CLKSEL) */
+#define DMM32AT_AUX_DI2BIT(2)  /* J3.46 - GATE12 
(GT12EN) */
+#define DMM32AT_AUX_DI1BIT(1)  /* J3.47 - GATE0 
(GT0EN) */
+#define DMM32AT_AUX_DI0BIT(0)  /* J3.48 - CLK0 (SRC0) 
*/
 #define DMM32AT_AO_LSB_REG 0x04
 #define DMM32AT_AO_MSB_REG 0x05
 #define DMM32AT_AO_MSB_DACH(x) ((x) << 6)
 #define DMM32AT_FIFO_DEPTH_REG 0x06
 #define DMM32AT_FIFO_CTRL_REG  0x07
-#define DMM32AT_FIFO_CTRL_FIFOEN   (1 << 3)
-#define DMM32AT_FIFO_CTRL_SCANEN   (1 << 2)
-#define DMM32AT_FIFO_CTRL_FIFORST  (1 << 1)
+#define DMM32AT_FIFO_CTRL_FIFOEN   BIT(3)
+#define DMM32AT_FIFO_CTRL_SCANEN   BIT(2)
+#define DMM32AT_FIFO_CTRL_FIFORST  BIT(1)
 #define DMM32AT_FIFO_STATUS_REG0x07
-#define DMM32AT_FIFO_STATUS_EF (1 << 7)
-#define DMM32AT_FIFO_STATUS_HF (1 << 6)
-#define DMM32AT_FIFO_STATUS_FF (1 << 5)
-#define DMM32AT_FIFO_STATUS_OVF(1 << 4)
-#define DMM32AT_FIFO_STATUS_FIFOEN (1 << 3)
-#define DMM32AT_FIFO_STATUS_SCANEN (1 << 2)
+#define DMM32AT_FIFO_STATUS_EF BIT(7)
+#define DMM32AT_FIFO_STATUS_HF BIT(6)
+#define DMM32AT_FIFO_STATUS_FF BIT(5)
+#define DMM32AT_FIFO_STATUS_OVFBIT(4)
+#define DMM32AT_FIFO_STATUS_FIFOEN BIT(3)
+#define DMM32AT_FIFO_STATUS_SCANEN BIT(2)
 #define DMM32AT_FIFO_STATUS_PAGE_MASK  (3 << 0)
 #define DMM32AT_CTRL_REG   0x08
-#define DMM32AT_CTRL_RESETA(1 << 5)
-#define DMM32AT_CTRL_RESETD(1 << 4)
-#define DMM32AT_CTRL_INTRST(1 << 3)
-#define DMM32AT_CTRL_PAGE_8254 (0 << 0)
-#define DMM32AT_CTRL_PAGE_8255 (1 << 0)
-#define DMM32AT_CTRL_PAGE_CALIB(3 << 0)
+#define DMM32AT_CTRL_RESETABIT(5)
+#define DMM32AT_CTRL_RESETDBIT(4)
+#define DMM32AT_CTRL_INTRSTBIT(3)
+#define DMM32AT_CTRL_PAGE(x)   ((x) << 0)
+#define DMM32AT_CTRL_PAGE_8254 DMM32AT_CTRL_PAGE(0)
+#define DMM32AT_CTRL_PAGE_8255 DMM32AT_CTRL_PAGE(1)
+#define DMM32AT_CTRL_PAGE_CALIBDMM32AT_CTRL_PAGE(3)
 #define DMM32AT_AI_STATUS_REG  0x08
-#define DMM32AT_AI_STATUS_STS  (1 << 7)
-#define DMM32AT_AI_STATUS_SD1  (1 << 6)
-#define DMM32AT_AI_STATUS_SD0  (1 << 5)
+#define DMM32AT_AI_STATUS_STS  BIT(7)
+#define DMM32AT_AI_STATUS_SD1  BIT(6)
+#define DMM32AT_AI_STATUS_SD0  BIT(5)
 #define DMM32AT_AI_STATUS_ADCH_MASK(0x1f << 0)
 #define DMM32AT_INTCLK_REG 0x09
-#define DMM32AT_INTCLK_ADINT   (1 << 7)
-#define DMM32AT_INTCLK_DINT(1 << 6)
-#define DMM32AT_INTCLK_TINT(1 << 5)
-#define DMM32AT_INTCLK_CLKEN   (1 << 1)  /* 1=see below  0=software */

[PATCH v2 1/1] Staging: comedi: dmm32at: fix BIT macro issue.

[Ravishankar Karkala Mallikarjunayya]

This Replace all occurences of (1<
---
Changes V1 -> V2:
- BIT macros added(suggested by Ian Abbott)
-i.e.DMM32AT_AI_CFG_SCINT(x), DMM32AT_CTRL_PAGE(x)
---
 drivers/staging/comedi/drivers/dmm32at.c | 98 
 1 file changed, 50 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dmm32at.c 
b/drivers/staging/comedi/drivers/dmm32at.c
index 958c0d4..b8606de 100644
--- a/drivers/staging/comedi/drivers/dmm32at.c
+++ b/drivers/staging/comedi/drivers/dmm32at.c
@@ -46,73 +46,75 @@
 #define DMM32AT_AI_START_CONV_REG  0x00
 #define DMM32AT_AI_LSB_REG 0x00
 #define DMM32AT_AUX_DOUT_REG   0x01
-#define DMM32AT_AUX_DOUT2  (1 << 2)  /* J3.42 - OUT2 (OUT2EN) */
-#define DMM32AT_AUX_DOUT1  (1 << 1)  /* J3.43 */
-#define DMM32AT_AUX_DOUT0  (1 << 0)  /* J3.44 - OUT0 (OUT0EN) */
+#define DMM32AT_AUX_DOUT2  BIT(2)  /* J3.42 - OUT2 (OUT2EN) */
+#define DMM32AT_AUX_DOUT1  BIT(1)  /* J3.43 */
+#define DMM32AT_AUX_DOUT0  BIT(0)  /* J3.44 - OUT0 (OUT0EN) */
 #define DMM32AT_AI_MSB_REG 0x01
 #define DMM32AT_AI_LO_CHAN_REG 0x02
 #define DMM32AT_AI_HI_CHAN_REG 0x03
 #define DMM32AT_AUX_DI_REG 0x04
-#define DMM32AT_AUX_DI_DACBUSY (1 << 7)
-#define DMM32AT_AUX_DI_CALBUSY (1 << 6)
-#define DMM32AT_AUX_DI3(1 << 3)  /* J3.45 - ADCLK 
(CLKSEL) */
-#define DMM32AT_AUX_DI2(1 << 2)  /* J3.46 - GATE12 
(GT12EN) */
-#define DMM32AT_AUX_DI1(1 << 1)  /* J3.47 - GATE0 
(GT0EN) */
-#define DMM32AT_AUX_DI0(1 << 0)  /* J3.48 - CLK0 
(SRC0) */
+#define DMM32AT_AUX_DI_DACBUSY BIT(7)
+#define DMM32AT_AUX_DI_CALBUSY BIT(6)
+#define DMM32AT_AUX_DI3BIT(3)  /* J3.45 - ADCLK 
(CLKSEL) */
+#define DMM32AT_AUX_DI2BIT(2)  /* J3.46 - GATE12 
(GT12EN) */
+#define DMM32AT_AUX_DI1BIT(1)  /* J3.47 - GATE0 
(GT0EN) */
+#define DMM32AT_AUX_DI0BIT(0)  /* J3.48 - CLK0 (SRC0) 
*/
 #define DMM32AT_AO_LSB_REG 0x04
 #define DMM32AT_AO_MSB_REG 0x05
 #define DMM32AT_AO_MSB_DACH(x) ((x) << 6)
 #define DMM32AT_FIFO_DEPTH_REG 0x06
 #define DMM32AT_FIFO_CTRL_REG  0x07
-#define DMM32AT_FIFO_CTRL_FIFOEN   (1 << 3)
-#define DMM32AT_FIFO_CTRL_SCANEN   (1 << 2)
-#define DMM32AT_FIFO_CTRL_FIFORST  (1 << 1)
+#define DMM32AT_FIFO_CTRL_FIFOEN   BIT(3)
+#define DMM32AT_FIFO_CTRL_SCANEN   BIT(2)
+#define DMM32AT_FIFO_CTRL_FIFORST  BIT(1)
 #define DMM32AT_FIFO_STATUS_REG0x07
-#define DMM32AT_FIFO_STATUS_EF (1 << 7)
-#define DMM32AT_FIFO_STATUS_HF (1 << 6)
-#define DMM32AT_FIFO_STATUS_FF (1 << 5)
-#define DMM32AT_FIFO_STATUS_OVF(1 << 4)
-#define DMM32AT_FIFO_STATUS_FIFOEN (1 << 3)
-#define DMM32AT_FIFO_STATUS_SCANEN (1 << 2)
+#define DMM32AT_FIFO_STATUS_EF BIT(7)
+#define DMM32AT_FIFO_STATUS_HF BIT(6)
+#define DMM32AT_FIFO_STATUS_FF BIT(5)
+#define DMM32AT_FIFO_STATUS_OVFBIT(4)
+#define DMM32AT_FIFO_STATUS_FIFOEN BIT(3)
+#define DMM32AT_FIFO_STATUS_SCANEN BIT(2)
 #define DMM32AT_FIFO_STATUS_PAGE_MASK  (3 << 0)
 #define DMM32AT_CTRL_REG   0x08
-#define DMM32AT_CTRL_RESETA(1 << 5)
-#define DMM32AT_CTRL_RESETD(1 << 4)
-#define DMM32AT_CTRL_INTRST(1 << 3)
-#define DMM32AT_CTRL_PAGE_8254 (0 << 0)
-#define DMM32AT_CTRL_PAGE_8255 (1 << 0)
-#define DMM32AT_CTRL_PAGE_CALIB(3 << 0)
+#define DMM32AT_CTRL_RESETABIT(5)
+#define DMM32AT_CTRL_RESETDBIT(4)
+#define DMM32AT_CTRL_INTRSTBIT(3)
+#define DMM32AT_CTRL_PAGE(x)   ((x) << 0)
+#define DMM32AT_CTRL_PAGE_8254 DMM32AT_CTRL_PAGE(0)
+#define DMM32AT_CTRL_PAGE_8255 DMM32AT_CTRL_PAGE(1)
+#define DMM32AT_CTRL_PAGE_CALIBDMM32AT_CTRL_PAGE(3)
 #define DMM32AT_AI_STATUS_REG  0x08
-#define DMM32AT_AI_STATUS_STS  (1 << 7)
-#define DMM32AT_AI_STATUS_SD1  (1 << 6)
-#define DMM32AT_AI_STATUS_SD0  (1 << 5)
+#define DMM32AT_AI_STATUS_STS  BIT(7)
+#define DMM32AT_AI_STATUS_SD1  BIT(6)
+#define DMM32AT_AI_STATUS_SD0  BIT(5)
 #define DMM32AT_AI_STATUS_ADCH_MASK(0x1f << 0)
 #define DMM32AT_INTCLK_REG 0x09
-#define DMM32AT_INTCLK_ADINT   (1 << 7)
-#define DMM32AT_INTCLK_DINT(1 << 6)
-#define DMM32AT_INTCLK_TINT(1 << 5)
-#define DMM32AT_INTCLK_CLKEN   (1 << 1)  /* 1=see below  0=software */
-#define DMM32AT_INTCLK_CLKSEL  (1 << 0)  /* 1=OUT2  0=EXTCLK */
+#define DMM32AT_INTCLK_ADINT   BIT(7)
+#define DMM32AT_INTCLK_DINTBIT(6)
+#define DMM32AT_INTCLK_TINTBIT(5)
+#define 

Re: [PATCH net-next v3 0/7] vmxnet3: upgrade to version 3

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:
> This patchset upgrades vmxnet3 to version 3.

Except for one patch, all the rest lack change-log, which makes it
somehow needlessly harder to review and maintain

> Shrikrishna Khare (7):
>   vmxnet3: prepare for version 3 changes
>   vmxnet3: introduce generic command interface to configure the device
>   vmxnet3: allow variable length transmit data ring buffer
>   vmxnet3: add receive data ring support
>   vmxnet3: add support for get_coalesce, set_coalesce ethtool operations
>   vmxnet3: introduce command to register memory region
>   vmxnet3: update to version 3

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
  #include 
  #include 
  #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

  /* Structure to hold entries from the MCFG table */
  struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
  /* List to save mcfg entries */
  static LIST_HEAD(pci_mcfg_list);

+extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[];
+extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[];
+
+struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
+{
+ int bus_num = root->secondary.start;
+ int domain = root->segment;
+ struct pci_cfg_fixup *f;
+
+ if (!mcfg_table)
+ return _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  then use OEM ID and
+  * OEM revision from MCFG table standard header.
+  */
+ for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; f++) {
+ if ((f->domain == domain || f->domain == PCI_MCFG_DOMAIN_ANY) &&
+ (f->bus_num == bus_num || f->bus_num == PCI_MCFG_BUS_ANY) &&
+ (!strncmp(f->oem_id, mcfg_table->header.oem_id,
+   ACPI_OEM_ID_SIZE)) &&
+ (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))


This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}



diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct 
acpi_device *dev)
  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);

  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
  {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
   int (*prepare_resources)(struct acpi_pci_root_info *info);
  };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
 static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
  __used __attribute__((__section__(".acpi_fixup_mcfg"), \
 aligned((sizeof(void *) =   \
 { ops, oem_id, rev, dom, bus };


V1 fixup exist the redefinition error when compiling mutiple 
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

In file included from drivers/pci/host/pcie-hisi-acpi.c:15:0:
include/linux/pci-acpi.h:98:43: error: redefinition of 

Re: [PATCH net-next v3 0/7] vmxnet3: upgrade to version 3

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:
> This patchset upgrades vmxnet3 to version 3.

Except for one patch, all the rest lack change-log, which makes it
somehow needlessly harder to review and maintain

> Shrikrishna Khare (7):
>   vmxnet3: prepare for version 3 changes
>   vmxnet3: introduce generic command interface to configure the device
>   vmxnet3: allow variable length transmit data ring buffer
>   vmxnet3: add receive data ring support
>   vmxnet3: add support for get_coalesce, set_coalesce ethtool operations
>   vmxnet3: introduce command to register memory region
>   vmxnet3: update to version 3

Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Dongdong Liu]

Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:

On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
 wrote:

Hi Dongdong,

On 06/13/2016 09:02 AM, Dongdong Liu wrote:

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index d3c3e85..49612b3 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,10 @@
  #include 
  #include 
  #include 
+#include 
+
+/* Root pointer to the mapped MCFG table */
+static struct acpi_table_mcfg *mcfg_table;

  /* Structure to hold entries from the MCFG table */
  struct mcfg_entry {
@@ -35,6 +39,38 @@ struct mcfg_entry {
  /* List to save mcfg entries */
  static LIST_HEAD(pci_mcfg_list);

+extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[];
+extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[];
+
+struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
+{
+ int bus_num = root->secondary.start;
+ int domain = root->segment;
+ struct pci_cfg_fixup *f;
+
+ if (!mcfg_table)
+ return _generic_ecam_ops;
+
+ /*
+  * Match against platform specific quirks and return corresponding
+  * CAM ops.
+  *
+  * First match against PCI topology  then use OEM ID and
+  * OEM revision from MCFG table standard header.
+  */
+ for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; f++) {
+ if ((f->domain == domain || f->domain == PCI_MCFG_DOMAIN_ANY) &&
+ (f->bus_num == bus_num || f->bus_num == PCI_MCFG_BUS_ANY) &&
+ (!strncmp(f->oem_id, mcfg_table->header.oem_id,
+   ACPI_OEM_ID_SIZE)) &&
+ (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id,
+   ACPI_OEM_TABLE_ID_SIZE)))


This would just be a small convenience, but if the character count used here 
were

min(strlen(f->oem_id), ACPI_OEM_ID_SIZE)

then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and
wouldn't need to be padded out to the full length.


+ return f->ops;
+ }
+ /* No quirks, use ECAM */
+ return _generic_ecam_ops;
+}



diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..088a1da 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,7 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct 
acpi_device *dev)
  extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);

  extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
+extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root);

  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
  {
@@ -72,6 +73,25 @@ struct acpi_pci_root_ops {
   int (*prepare_resources)(struct acpi_pci_root_info *info);
  };

+struct pci_cfg_fixup {
+ struct pci_ecam_ops *ops;
+ char *oem_id;
+ char *oem_table_id;
+ int domain;
+ int bus_num;
+};
+
+#define PCI_MCFG_DOMAIN_ANY  -1
+#define PCI_MCFG_BUS_ANY -1
+
+/* Designate a routine to fix up buggy MCFG */
+#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \
+ static const struct pci_cfg_fixup   \
+ __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I'm not entirely sure that this is the right fix--I'm pretty blindly
following a GCC documentation suggestion [1]--but removing the first two
preprocessor concatenation operators "##" solved the following build error
for me.

include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does 
not give a valid preprocessing token
   __mcfg_fixup_##oem_id##oem_table_id##dom##bus   \


I think the problem is gcc is not happy with quoted string when
processing these tokens
(""QCOM"", the extra "" are added by gcc). So should we not concat
string tokens and
use the fixup definition in v1 of this RFC:
/* Designate a routine to fix up buggy MCFG */
#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \
 static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\
  __used __attribute__((__section__(".acpi_fixup_mcfg"), \
 aligned((sizeof(void *) =   \
 { ops, oem_id, rev, dom, bus };


V1 fixup exist the redefinition error when compiling mutiple 
DECLARE_ACPI_MCFG_FIXUP
with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY.

#define EFI_ACPI_HISI_OEM_ID "HISI"
#define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02"
#define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03"

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

DECLARE_ACPI_MCFG_FIXUP(_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID,
   EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);

In file included from drivers/pci/host/pcie-hisi-acpi.c:15:0:
include/linux/pci-acpi.h:98:43: error: redefinition of 

Re: [GIT PULL] SCSI fixes for 4.7-rc2

[Linus Torvalds]

On Mon, Jun 13, 2016 at 12:04 AM, Hannes Reinecke  wrote:
>
> And we have been running the very patch in SLES for over a year now,
> without a single issue being reported.

Oh, ok. So it's not "all qemu kvm instances are broken", it was a very
unusual issue, and the patch has actually gotten wider testing.

That makes me much happier about it.

Linus

Re: [GIT PULL] SCSI fixes for 4.7-rc2

[Linus Torvalds]

On Mon, Jun 13, 2016 at 12:04 AM, Hannes Reinecke  wrote:
>
> And we have been running the very patch in SLES for over a year now,
> without a single issue being reported.

Oh, ok. So it's not "all qemu kvm instances are broken", it was a very
unusual issue, and the patch has actually gotten wider testing.

That makes me much happier about it.

Linus

Re: [PATCH v2 1/7] mm/compaction: split freepages without holding the zone lock

[Joonsoo Kim]

On Mon, Jun 13, 2016 at 04:31:15PM -0400, Sasha Levin wrote:
> On 05/25/2016 10:37 PM, js1...@gmail.com wrote:
> > From: Joonsoo Kim 
> > 
> > We don't need to split freepages with holding the zone lock. It will cause
> > more contention on zone lock so not desirable.
> > 
> > Signed-off-by: Joonsoo Kim 
> 
> Hey Joonsoo,

Hello, Sasha.
> 
> I'm seeing the following corruption/crash which seems to be related to
> this patch:

Could you tell me why you think that following corruption is related
to this patch? list_del() in __isolate_free_page() is unchanged part.

Before this patch, we did it by split_free_page() ->
__isolate_free_page() -> list_del(). With this patch, we do it by
calling __isolate_free_page() directly.

Thanks.


> [ 3777.807224] [ cut here ]
> 
> [ 3777.807834] WARNING: CPU: 5 PID: 3270 at lib/list_debug.c:62 
> __list_del_entry+0x14e/0x280
> 
> [ 3777.808562] list_del corruption. next->prev should be ea0004a76120, 
> but was ea0004a72120
> 
> [ 3777.809498] Modules linked in:
> 
> [ 3777.809923] CPU: 5 PID: 3270 Comm: khugepaged Tainted: GW   
> 4.7.0-rc2-next-20160609-sasha-00024-g30ecaf6 #3101
> 
> [ 3777.811014]  1100f9315d7b 0bb7299a 8807c98aec60 
> a0035b2b
> 
> [ 3777.811816]  0005 fbfff5630bf4 41b58ab3 
> aaaf18e0
> 
> [ 3777.812662]  a00359bc 9e54d4a0 a8b2ade0 
> 8807c98aece0
> 
> [ 3777.813493] Call Trace:
> 
> [ 3777.813796] dump_stack (lib/dump_stack.c:53)
> [ 3777.814310] ? arch_local_irq_restore 
> (./arch/x86/include/asm/paravirt.h:134)
> [ 3777.814947] ? is_module_text_address (kernel/module.c:4185)
> [ 3777.815571] ? __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.816174] ? vprintk_default (kernel/printk/printk.c:1886)
> [ 3777.816761] ? __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.817381] __warn (kernel/panic.c:518)
> [ 3777.817867] warn_slowpath_fmt (kernel/panic.c:526)
> [ 3777.818428] ? __warn (kernel/panic.c:526)
> [ 3777.819001] ? __schedule (kernel/sched/core.c:2858 
> kernel/sched/core.c:3345)
> [ 3777.819541] __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.820116] ? __list_add (lib/list_debug.c:45)
> [ 3777.820721] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
> [ 3777.821347] list_del (lib/list_debug.c:78)
> [ 3777.821829] __isolate_free_page (mm/page_alloc.c:2514)
> [ 3777.822400] ? __zone_watermark_ok (mm/page_alloc.c:2493)
> [ 3777.823007] isolate_freepages_block (mm/compaction.c:498)
> [ 3777.823629] ? compact_unlock_should_abort (mm/compaction.c:417)
> [ 3777.824312] compaction_alloc (mm/compaction.c:1112 mm/compaction.c:1156)
> [ 3777.824871] ? isolate_freepages_block (mm/compaction.c:1146)
> [ 3777.825512] ? __page_cache_release (mm/swap.c:73)
> [ 3777.826127] migrate_pages (mm/migrate.c:1079 mm/migrate.c:1325)
> [ 3777.826712] ? __reset_isolation_suitable (mm/compaction.c:1175)
> [ 3777.827398] ? isolate_freepages_block (mm/compaction.c:1146)
> [ 3777.828109] ? buffer_migrate_page (mm/migrate.c:1301)
> [ 3777.828727] compact_zone (mm/compaction.c:1555)
> [ 3777.829290] ? compaction_restarting (mm/compaction.c:1476)
> [ 3777.829969] ? _raw_spin_unlock_irq (./arch/x86/include/asm/preempt.h:92 
> include/linux/spinlock_api_smp.h:171 kernel/locking/spinlock.c:199)
> [ 3777.830607] compact_zone_order (mm/compaction.c:1653)
> [ 3777.831204] ? kick_process (kernel/sched/core.c:2692)
> [ 3777.831774] ? compact_zone (mm/compaction.c:1637)
> [ 3777.832336] ? io_schedule_timeout (kernel/sched/core.c:3266)
> [ 3777.832934] try_to_compact_pages (mm/compaction.c:1717)
> [ 3777.833550] ? compaction_zonelist_suitable (mm/compaction.c:1679)
> [ 3777.834265] __alloc_pages_direct_compact (mm/page_alloc.c:3180)
> [ 3777.834922] ? get_page_from_freelist (mm/page_alloc.c:3172)
> [ 3777.835549] __alloc_pages_slowpath (mm/page_alloc.c:3741)
> [ 3777.836210] ? kvm_clock_read (./arch/x86/include/asm/preempt.h:84 
> arch/x86/kernel/kvmclock.c:92)
> [ 3777.836744] ? __alloc_pages_direct_compact (mm/page_alloc.c:3546)
> [ 3777.837429] ? get_page_from_freelist (mm/page_alloc.c:2950)
> [ 3777.838072] ? release_pages (mm/swap.c:731)
> [ 3777.838610] ? __isolate_free_page (mm/page_alloc.c:2883)
> [ 3777.839209] ? ___might_sleep (kernel/sched/core.c:7540 (discriminator 1))
> [ 3777.839826] ? __might_sleep (kernel/sched/core.c:7532 (discriminator 14))
> [ 3777.840427] __alloc_pages_nodemask (mm/page_alloc.c:3841)
> [ 3777.841071] ? rwsem_wake (kernel/locking/rwsem-xadd.c:580)
> [ 3777.841608] ? __alloc_pages_slowpath (mm/page_alloc.c:3757)
> [ 3777.842253] ? call_rwsem_wake (arch/x86/lib/rwsem.S:129)
> [ 3777.842839] ? up_write (kernel/locking/rwsem.c:112)
> [ 3777.843350] ? pmdp_huge_clear_flush (mm/pgtable-generic.c:131)
> [ 3777.844125] khugepaged_alloc_page (mm/khugepaged.c:752)
> [ 3777.844719] collapse_huge_page (mm/khugepaged.c:948)
> [ 

Re: [PATCH v2 1/7] mm/compaction: split freepages without holding the zone lock

[Joonsoo Kim]

On Mon, Jun 13, 2016 at 04:31:15PM -0400, Sasha Levin wrote:
> On 05/25/2016 10:37 PM, js1...@gmail.com wrote:
> > From: Joonsoo Kim 
> > 
> > We don't need to split freepages with holding the zone lock. It will cause
> > more contention on zone lock so not desirable.
> > 
> > Signed-off-by: Joonsoo Kim 
> 
> Hey Joonsoo,

Hello, Sasha.
> 
> I'm seeing the following corruption/crash which seems to be related to
> this patch:

Could you tell me why you think that following corruption is related
to this patch? list_del() in __isolate_free_page() is unchanged part.

Before this patch, we did it by split_free_page() ->
__isolate_free_page() -> list_del(). With this patch, we do it by
calling __isolate_free_page() directly.

Thanks.


> [ 3777.807224] [ cut here ]
> 
> [ 3777.807834] WARNING: CPU: 5 PID: 3270 at lib/list_debug.c:62 
> __list_del_entry+0x14e/0x280
> 
> [ 3777.808562] list_del corruption. next->prev should be ea0004a76120, 
> but was ea0004a72120
> 
> [ 3777.809498] Modules linked in:
> 
> [ 3777.809923] CPU: 5 PID: 3270 Comm: khugepaged Tainted: GW   
> 4.7.0-rc2-next-20160609-sasha-00024-g30ecaf6 #3101
> 
> [ 3777.811014]  1100f9315d7b 0bb7299a 8807c98aec60 
> a0035b2b
> 
> [ 3777.811816]  0005 fbfff5630bf4 41b58ab3 
> aaaf18e0
> 
> [ 3777.812662]  a00359bc 9e54d4a0 a8b2ade0 
> 8807c98aece0
> 
> [ 3777.813493] Call Trace:
> 
> [ 3777.813796] dump_stack (lib/dump_stack.c:53)
> [ 3777.814310] ? arch_local_irq_restore 
> (./arch/x86/include/asm/paravirt.h:134)
> [ 3777.814947] ? is_module_text_address (kernel/module.c:4185)
> [ 3777.815571] ? __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.816174] ? vprintk_default (kernel/printk/printk.c:1886)
> [ 3777.816761] ? __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.817381] __warn (kernel/panic.c:518)
> [ 3777.817867] warn_slowpath_fmt (kernel/panic.c:526)
> [ 3777.818428] ? __warn (kernel/panic.c:526)
> [ 3777.819001] ? __schedule (kernel/sched/core.c:2858 
> kernel/sched/core.c:3345)
> [ 3777.819541] __list_del_entry (lib/list_debug.c:60 (discriminator 1))
> [ 3777.820116] ? __list_add (lib/list_debug.c:45)
> [ 3777.820721] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
> [ 3777.821347] list_del (lib/list_debug.c:78)
> [ 3777.821829] __isolate_free_page (mm/page_alloc.c:2514)
> [ 3777.822400] ? __zone_watermark_ok (mm/page_alloc.c:2493)
> [ 3777.823007] isolate_freepages_block (mm/compaction.c:498)
> [ 3777.823629] ? compact_unlock_should_abort (mm/compaction.c:417)
> [ 3777.824312] compaction_alloc (mm/compaction.c:1112 mm/compaction.c:1156)
> [ 3777.824871] ? isolate_freepages_block (mm/compaction.c:1146)
> [ 3777.825512] ? __page_cache_release (mm/swap.c:73)
> [ 3777.826127] migrate_pages (mm/migrate.c:1079 mm/migrate.c:1325)
> [ 3777.826712] ? __reset_isolation_suitable (mm/compaction.c:1175)
> [ 3777.827398] ? isolate_freepages_block (mm/compaction.c:1146)
> [ 3777.828109] ? buffer_migrate_page (mm/migrate.c:1301)
> [ 3777.828727] compact_zone (mm/compaction.c:1555)
> [ 3777.829290] ? compaction_restarting (mm/compaction.c:1476)
> [ 3777.829969] ? _raw_spin_unlock_irq (./arch/x86/include/asm/preempt.h:92 
> include/linux/spinlock_api_smp.h:171 kernel/locking/spinlock.c:199)
> [ 3777.830607] compact_zone_order (mm/compaction.c:1653)
> [ 3777.831204] ? kick_process (kernel/sched/core.c:2692)
> [ 3777.831774] ? compact_zone (mm/compaction.c:1637)
> [ 3777.832336] ? io_schedule_timeout (kernel/sched/core.c:3266)
> [ 3777.832934] try_to_compact_pages (mm/compaction.c:1717)
> [ 3777.833550] ? compaction_zonelist_suitable (mm/compaction.c:1679)
> [ 3777.834265] __alloc_pages_direct_compact (mm/page_alloc.c:3180)
> [ 3777.834922] ? get_page_from_freelist (mm/page_alloc.c:3172)
> [ 3777.835549] __alloc_pages_slowpath (mm/page_alloc.c:3741)
> [ 3777.836210] ? kvm_clock_read (./arch/x86/include/asm/preempt.h:84 
> arch/x86/kernel/kvmclock.c:92)
> [ 3777.836744] ? __alloc_pages_direct_compact (mm/page_alloc.c:3546)
> [ 3777.837429] ? get_page_from_freelist (mm/page_alloc.c:2950)
> [ 3777.838072] ? release_pages (mm/swap.c:731)
> [ 3777.838610] ? __isolate_free_page (mm/page_alloc.c:2883)
> [ 3777.839209] ? ___might_sleep (kernel/sched/core.c:7540 (discriminator 1))
> [ 3777.839826] ? __might_sleep (kernel/sched/core.c:7532 (discriminator 14))
> [ 3777.840427] __alloc_pages_nodemask (mm/page_alloc.c:3841)
> [ 3777.841071] ? rwsem_wake (kernel/locking/rwsem-xadd.c:580)
> [ 3777.841608] ? __alloc_pages_slowpath (mm/page_alloc.c:3757)
> [ 3777.842253] ? call_rwsem_wake (arch/x86/lib/rwsem.S:129)
> [ 3777.842839] ? up_write (kernel/locking/rwsem.c:112)
> [ 3777.843350] ? pmdp_huge_clear_flush (mm/pgtable-generic.c:131)
> [ 3777.844125] khugepaged_alloc_page (mm/khugepaged.c:752)
> [ 3777.844719] collapse_huge_page (mm/khugepaged.c:948)
> [ 3777.845332] ? khugepaged_scan_shmem 

Re: [PATCH] locking/qspinlock: Use atomic_sub_return_release in queued_spin_unlock

[Boqun Feng]

On Mon, Jun 13, 2016 at 12:45:23PM -0700, Davidlohr Bueso wrote:
> On Fri, 03 Jun 2016, Pan Xinhui wrote:
> 
> > The existing version uses a heavy barrier while only release semantics
> > is required. So use atomic_sub_return_release instead.
> > 
> > Suggested-by: Peter Zijlstra (Intel) 
> > Signed-off-by: Pan Xinhui 
> 
> I just noticed this change in -tip and, while I know that saving a barrier
> in core spinlock paths is perhaps a worthy exception, I cannot help but
> wonder if this is the begging of the end for smp__{before,after}_atomic().

This is surely a good direction I think, that is using _acquire and
_release primitives to replace those barriers. However, I think we
should do this carefully, because the _acquire and _release primitives
are RCpc because they are on PPC, IOW, a ACQUIRE and RELEASE pair is not
a full barrier nor provides global transivity. I'm worried about there
are some users depending on the full-barrier semantics, which means we
must audit each use carefully before we make the change.

Besides, if we want to do the conversion, we'd better have _acquire and
_release variants for non-value-returning atomic operations.

I remember you were working on those variants. How is that going?

Regards,
Boqun


signature.asc
Description: PGP signature

Re: [PATCH] locking/qspinlock: Use atomic_sub_return_release in queued_spin_unlock

[Boqun Feng]

On Mon, Jun 13, 2016 at 12:45:23PM -0700, Davidlohr Bueso wrote:
> On Fri, 03 Jun 2016, Pan Xinhui wrote:
> 
> > The existing version uses a heavy barrier while only release semantics
> > is required. So use atomic_sub_return_release instead.
> > 
> > Suggested-by: Peter Zijlstra (Intel) 
> > Signed-off-by: Pan Xinhui 
> 
> I just noticed this change in -tip and, while I know that saving a barrier
> in core spinlock paths is perhaps a worthy exception, I cannot help but
> wonder if this is the begging of the end for smp__{before,after}_atomic().

This is surely a good direction I think, that is using _acquire and
_release primitives to replace those barriers. However, I think we
should do this carefully, because the _acquire and _release primitives
are RCpc because they are on PPC, IOW, a ACQUIRE and RELEASE pair is not
a full barrier nor provides global transivity. I'm worried about there
are some users depending on the full-barrier semantics, which means we
must audit each use carefully before we make the change.

Besides, if we want to do the conversion, we'd better have _acquire and
_release variants for non-value-returning atomic operations.

I remember you were working on those variants. How is that going?

Regards,
Boqun


signature.asc
Description: PGP signature

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:28 PM, Michal Suchanek  wrote:
> On 14 June 2016 at 06:47, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
>>> Hello,
>>>
>>> On 14 June 2016 at 01:43, Julian Calaby  wrote:
 Hi Michal,

 On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  
 wrote:
> The drivers are very similar and share multiple flaws which needed
> separate fixes for both drivers.
>
> Signed-off-by: Michal Suchanek 
> ---
>  drivers/spi/Kconfig |   8 +-
>  drivers/spi/Makefile|   1 -
>  drivers/spi/spi-sun4i.c | 156 +++--
>  drivers/spi/spi-sun6i.c | 598 
> 
>  4 files changed, 143 insertions(+), 620 deletions(-)
>  delete mode 100644 drivers/spi/spi-sun6i.c
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 0b8e6c6..c76f8e4 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
> *master,
> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>
> /* Reset FIFOs */
> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +   if (sspi->type == SPI_SUN4I)
> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +   else
> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));

 If we're already doing different stuff for each generation of the IP,
 why not just use the register offsets and bit definitions directly?
>>>
>>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>>> makes my eyes bleed and you cannot use the check that you are
>>> accessing a register that actually exists.
>>
>> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
>> indirection you added and using them directly, i.e.
>>
>> #define SUN4I_TFR_CTL_RF_RST BIT(x)
>> #define SUN4I_TFR_CTL_TF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>>
>> then
>>
>> if (sspi->type == SPI_SUN4I)
>> sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
>> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
>> else
>> sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
>> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>>
>> I.e. the bits that need setting are in different registers, so you
>> have to have an if statement and separate calls. Therefore there's no
>> real benefit from the indirection you've introduced here, unless
>> you're expecting the SUN8I variant to use different bits in one of
>> those two registers.
>>
>
> That looks nice for this particular case.

There was another case I pointed out in this driver that could
potentially benefit from this.

> Still you will have to remember that these bits are specified directly
> while other bits on the register are mapped which is not so nice.

True. I'm not sure which path is best in this case. To my eye, your
indirection scheme seems like the "heavy" solution, however I have no
idea what form a "lighter" solution would take.

Other drivers I've seen which have tackled similar problems have used
a large struct to hold all the variant-specific stuff, whether that's
function pointers, register offsets, constants, etc. (so this code
could theoretically be re-written as sunxi_spi_write(sspi,
sspi->fifo_reg, reg | sspi->fifo_reset_arg) or sspi->reset_fifo())
however the driver I saw doing this (rtl8xxxu) was handling much more
significant differences between device variants than you are.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:28 PM, Michal Suchanek  wrote:
> On 14 June 2016 at 06:47, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
>>> Hello,
>>>
>>> On 14 June 2016 at 01:43, Julian Calaby  wrote:
 Hi Michal,

 On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  
 wrote:
> The drivers are very similar and share multiple flaws which needed
> separate fixes for both drivers.
>
> Signed-off-by: Michal Suchanek 
> ---
>  drivers/spi/Kconfig |   8 +-
>  drivers/spi/Makefile|   1 -
>  drivers/spi/spi-sun4i.c | 156 +++--
>  drivers/spi/spi-sun6i.c | 598 
> 
>  4 files changed, 143 insertions(+), 620 deletions(-)
>  delete mode 100644 drivers/spi/spi-sun6i.c
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 0b8e6c6..c76f8e4 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
> *master,
> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>
> /* Reset FIFOs */
> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +   if (sspi->type == SPI_SUN4I)
> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +   else
> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));

 If we're already doing different stuff for each generation of the IP,
 why not just use the register offsets and bit definitions directly?
>>>
>>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>>> makes my eyes bleed and you cannot use the check that you are
>>> accessing a register that actually exists.
>>
>> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
>> indirection you added and using them directly, i.e.
>>
>> #define SUN4I_TFR_CTL_RF_RST BIT(x)
>> #define SUN4I_TFR_CTL_TF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>>
>> then
>>
>> if (sspi->type == SPI_SUN4I)
>> sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
>> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
>> else
>> sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
>> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>>
>> I.e. the bits that need setting are in different registers, so you
>> have to have an if statement and separate calls. Therefore there's no
>> real benefit from the indirection you've introduced here, unless
>> you're expecting the SUN8I variant to use different bits in one of
>> those two registers.
>>
>
> That looks nice for this particular case.

There was another case I pointed out in this driver that could
potentially benefit from this.

> Still you will have to remember that these bits are specified directly
> while other bits on the register are mapped which is not so nice.

True. I'm not sure which path is best in this case. To my eye, your
indirection scheme seems like the "heavy" solution, however I have no
idea what form a "lighter" solution would take.

Other drivers I've seen which have tackled similar problems have used
a large struct to hold all the variant-specific stuff, whether that's
function pointers, register offsets, constants, etc. (so this code
could theoretically be re-written as sunxi_spi_write(sspi,
sspi->fifo_reg, reg | sspi->fifo_reset_arg) or sspi->reset_fifo())
however the driver I saw doing this (rtl8xxxu) was handling much more
significant differences between device variants than you are.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 1/2] drm/mediatek: Add gamma correction

[Daniel Vetter]

On Tue, Jun 14, 2016 at 10:55:52AM +0800, Bibby Hsieh wrote:
> Apply gamma function to correct brightness values.
> It applies arbitrary mapping curve to compensate the
> incorrect transfer function of the panel.
> 
> Signed-off-by: Bibby Hsieh 

I think it would be much better to use the new atomic color management
support, which includes gamma. See drm_crtc_enable_color_mgmt.
-Daniel

> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |   12 ++
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h |1 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   56 
> ++-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |9 +
>  4 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 24aa3ba..1b38406 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -127,6 +127,16 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>   state->base.crtc = crtc;
>  }
>  
> +static void mtk_crtc_gamma_set(struct drm_crtc *crtc, u16 *r,
> +u16 *g, u16 *b, uint32_t start, uint32_t size)
> +{
> + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> + unsigned int i;
> +
> + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> + mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], r, g, b, start, size);
> +}
> +
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc 
> *crtc)
>  {
>   struct mtk_crtc_state *state;
> @@ -418,6 +428,7 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = {
>   .reset  = mtk_drm_crtc_reset,
>   .atomic_duplicate_state = mtk_drm_crtc_duplicate_state,
>   .atomic_destroy_state   = mtk_drm_crtc_destroy_state,
> + .gamma_set  = mtk_crtc_gamma_set,
>  };
>  
>  static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = {
> @@ -569,6 +580,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
>   if (ret < 0)
>   goto unprepare;
>  
> + drm_mode_crtc_set_gamma_size(_crtc->base, MTK_LUT_SIZE);
>   priv->crtc[pipe] = _crtc->base;
>   priv->num_pipes++;
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 81e5566..d332564 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -19,6 +19,7 @@
>  #include "mtk_drm_plane.h"
>  
>  #define OVL_LAYER_NR 4
> +#define MTK_LUT_SIZE 512
>  
>  int mtk_drm_crtc_enable_vblank(struct drm_device *drm, unsigned int pipe);
>  void mtk_drm_crtc_disable_vblank(struct drm_device *drm, unsigned int pipe);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 3970fcf..3fd5141 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -24,6 +24,7 @@
>  #include "mtk_drm_drv.h"
>  #include "mtk_drm_plane.h"
>  #include "mtk_drm_ddp_comp.h"
> +#include "mtk_drm_crtc.h"
>  
>  #define DISP_OD_EN   0x
>  #define DISP_OD_INTEN0x0008
> @@ -38,6 +39,18 @@
>  #define DISP_COLOR_WIDTH 0x0c50
>  #define DISP_COLOR_HEIGHT0x0c54
>  
> +#define DISP_AAL_EN  0x000
> +#define DISP_AAL_SIZE0x030
> +
> +#define DISP_GAMMA_CFG   0x020
> +#define DISP_GAMMA_LUT   0x700
> +
> +#define LUT_10BIT_MASK   0x3ff
> +
> +#define AAL_EN   BIT(0)
> +
> +#define GAMMA_LUT_EN BIT(1)
> +
>  #define  OD_RELAY_MODE   BIT(0)
>  
>  #define  UFO_BYPASS  BIT(2)
> @@ -76,6 +89,40 @@ static void mtk_ufoe_start(struct mtk_ddp_comp *comp)
>   writel(UFO_BYPASS, comp->regs + DISP_REG_UFO_START);
>  }
>  
> +static void mtk_aal_config(struct mtk_ddp_comp *comp, unsigned int w,
> +unsigned int h, unsigned int vrefresh)
> +{
> + writel(h << 16 | w, comp->regs + DISP_AAL_SIZE);
> +}
> +
> +static void mtk_aal_start(struct mtk_ddp_comp *comp)
> +{
> + writel(AAL_EN, comp->regs  + DISP_AAL_EN);
> +}
> +
> +static void mtk_aal_stop(struct mtk_ddp_comp *comp)
> +{
> + writel_relaxed(0x0, comp->regs  + DISP_AAL_EN);
> +}
> +
> +static void mtk_gamma_set(struct mtk_ddp_comp *comp, u16 *r, u16 *g,
> +   u16 *b, uint32_t start, uint32_t size)
> +{
> + int i;
> + unsigned int lut;
> + int end = (start + size > MTK_LUT_SIZE) ? MTK_LUT_SIZE : start + size;
> + void __iomem *lut_base;
> +
> + writel(GAMMA_LUT_EN, comp->regs + DISP_GAMMA_CFG);
> + lut_base = comp->regs + DISP_GAMMA_LUT;
> + for (i = start; i < end; i++) {
> + lut = (((r[i] >> 6) & LUT_10BIT_MASK) << 

Re: [PATCH 1/2] drm/mediatek: Add gamma correction

[Daniel Vetter]

On Tue, Jun 14, 2016 at 10:55:52AM +0800, Bibby Hsieh wrote:
> Apply gamma function to correct brightness values.
> It applies arbitrary mapping curve to compensate the
> incorrect transfer function of the panel.
> 
> Signed-off-by: Bibby Hsieh 

I think it would be much better to use the new atomic color management
support, which includes gamma. See drm_crtc_enable_color_mgmt.
-Daniel

> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |   12 ++
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h |1 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   56 
> ++-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |9 +
>  4 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 24aa3ba..1b38406 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -127,6 +127,16 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>   state->base.crtc = crtc;
>  }
>  
> +static void mtk_crtc_gamma_set(struct drm_crtc *crtc, u16 *r,
> +u16 *g, u16 *b, uint32_t start, uint32_t size)
> +{
> + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> + unsigned int i;
> +
> + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> + mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], r, g, b, start, size);
> +}
> +
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc 
> *crtc)
>  {
>   struct mtk_crtc_state *state;
> @@ -418,6 +428,7 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = {
>   .reset  = mtk_drm_crtc_reset,
>   .atomic_duplicate_state = mtk_drm_crtc_duplicate_state,
>   .atomic_destroy_state   = mtk_drm_crtc_destroy_state,
> + .gamma_set  = mtk_crtc_gamma_set,
>  };
>  
>  static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = {
> @@ -569,6 +580,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
>   if (ret < 0)
>   goto unprepare;
>  
> + drm_mode_crtc_set_gamma_size(_crtc->base, MTK_LUT_SIZE);
>   priv->crtc[pipe] = _crtc->base;
>   priv->num_pipes++;
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 81e5566..d332564 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -19,6 +19,7 @@
>  #include "mtk_drm_plane.h"
>  
>  #define OVL_LAYER_NR 4
> +#define MTK_LUT_SIZE 512
>  
>  int mtk_drm_crtc_enable_vblank(struct drm_device *drm, unsigned int pipe);
>  void mtk_drm_crtc_disable_vblank(struct drm_device *drm, unsigned int pipe);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 3970fcf..3fd5141 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -24,6 +24,7 @@
>  #include "mtk_drm_drv.h"
>  #include "mtk_drm_plane.h"
>  #include "mtk_drm_ddp_comp.h"
> +#include "mtk_drm_crtc.h"
>  
>  #define DISP_OD_EN   0x
>  #define DISP_OD_INTEN0x0008
> @@ -38,6 +39,18 @@
>  #define DISP_COLOR_WIDTH 0x0c50
>  #define DISP_COLOR_HEIGHT0x0c54
>  
> +#define DISP_AAL_EN  0x000
> +#define DISP_AAL_SIZE0x030
> +
> +#define DISP_GAMMA_CFG   0x020
> +#define DISP_GAMMA_LUT   0x700
> +
> +#define LUT_10BIT_MASK   0x3ff
> +
> +#define AAL_EN   BIT(0)
> +
> +#define GAMMA_LUT_EN BIT(1)
> +
>  #define  OD_RELAY_MODE   BIT(0)
>  
>  #define  UFO_BYPASS  BIT(2)
> @@ -76,6 +89,40 @@ static void mtk_ufoe_start(struct mtk_ddp_comp *comp)
>   writel(UFO_BYPASS, comp->regs + DISP_REG_UFO_START);
>  }
>  
> +static void mtk_aal_config(struct mtk_ddp_comp *comp, unsigned int w,
> +unsigned int h, unsigned int vrefresh)
> +{
> + writel(h << 16 | w, comp->regs + DISP_AAL_SIZE);
> +}
> +
> +static void mtk_aal_start(struct mtk_ddp_comp *comp)
> +{
> + writel(AAL_EN, comp->regs  + DISP_AAL_EN);
> +}
> +
> +static void mtk_aal_stop(struct mtk_ddp_comp *comp)
> +{
> + writel_relaxed(0x0, comp->regs  + DISP_AAL_EN);
> +}
> +
> +static void mtk_gamma_set(struct mtk_ddp_comp *comp, u16 *r, u16 *g,
> +   u16 *b, uint32_t start, uint32_t size)
> +{
> + int i;
> + unsigned int lut;
> + int end = (start + size > MTK_LUT_SIZE) ? MTK_LUT_SIZE : start + size;
> + void __iomem *lut_base;
> +
> + writel(GAMMA_LUT_EN, comp->regs + DISP_GAMMA_CFG);
> + lut_base = comp->regs + DISP_GAMMA_LUT;
> + for (i = start; i < end; i++) {
> + lut = (((r[i] >> 6) & LUT_10BIT_MASK) << 20) +
> + 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
> > However I'm wondering if we shouldn't simply remove at least the s390
> > specific hwswampler code from the oprofile module. This would still leave
> > the common code timer based sampling mode for oprofile working on s390.
> > 
> > It looks like the oprofile user space utility nowadays (since 2012) uses
> > the kernel perf interface instead of the oprofile interface anyway, if
> > present. So the oprofile module itself doesn't seem to have too many users
> > left.
> > 
> > Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.

Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
it ok for you if I add the patch to the s390 kernel tree?
The patch would only remove s390 specific architecture code.

I have this pending:

s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 

 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 -
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
> > However I'm wondering if we shouldn't simply remove at least the s390
> > specific hwswampler code from the oprofile module. This would still leave
> > the common code timer based sampling mode for oprofile working on s390.
> > 
> > It looks like the oprofile user space utility nowadays (since 2012) uses
> > the kernel perf interface instead of the oprofile interface anyway, if
> > present. So the oprofile module itself doesn't seem to have too many users
> > left.
> > 
> > Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.

Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
it ok for you if I add the patch to the s390 kernel tree?
The patch would only remove s390 specific architecture code.

I have this pending:

s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 

 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 -
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

On 14 June 2016 at 06:47, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
>> Hello,
>>
>> On 14 June 2016 at 01:43, Julian Calaby  wrote:
>>> Hi Michal,
>>>
>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
 The drivers are very similar and share multiple flaws which needed
 separate fixes for both drivers.

 Signed-off-by: Michal Suchanek 
 ---
  drivers/spi/Kconfig |   8 +-
  drivers/spi/Makefile|   1 -
  drivers/spi/spi-sun4i.c | 156 +++--
  drivers/spi/spi-sun6i.c | 598 
 
  4 files changed, 143 insertions(+), 620 deletions(-)
  delete mode 100644 drivers/spi/spi-sun6i.c

 diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
 index 0b8e6c6..c76f8e4 100644
 --- a/drivers/spi/spi-sun4i.c
 +++ b/drivers/spi/spi-sun4i.c
 @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
 *master,
 reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);

 /* Reset FIFOs */
 -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
 +   if (sspi->type == SPI_SUN4I)
 +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
 +   else
 +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
 +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>
>>> If we're already doing different stuff for each generation of the IP,
>>> why not just use the register offsets and bit definitions directly?
>>
>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>> makes my eyes bleed and you cannot use the check that you are
>> accessing a register that actually exists.
>
> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
> indirection you added and using them directly, i.e.
>
> #define SUN4I_TFR_CTL_RF_RST BIT(x)
> #define SUN4I_TFR_CTL_TF_RST BIT(x)
> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>
> then
>
> if (sspi->type == SPI_SUN4I)
> sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
> else
> sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>
> I.e. the bits that need setting are in different registers, so you
> have to have an if statement and separate calls. Therefore there's no
> real benefit from the indirection you've introduced here, unless
> you're expecting the SUN8I variant to use different bits in one of
> those two registers.
>

That looks nice for this particular case.

Still you will have to remember that these bits are specified directly
while other bits on the register are mapped which is not so nice.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

On 14 June 2016 at 06:47, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
>> Hello,
>>
>> On 14 June 2016 at 01:43, Julian Calaby  wrote:
>>> Hi Michal,
>>>
>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
 The drivers are very similar and share multiple flaws which needed
 separate fixes for both drivers.

 Signed-off-by: Michal Suchanek 
 ---
  drivers/spi/Kconfig |   8 +-
  drivers/spi/Makefile|   1 -
  drivers/spi/spi-sun4i.c | 156 +++--
  drivers/spi/spi-sun6i.c | 598 
 
  4 files changed, 143 insertions(+), 620 deletions(-)
  delete mode 100644 drivers/spi/spi-sun6i.c

 diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
 index 0b8e6c6..c76f8e4 100644
 --- a/drivers/spi/spi-sun4i.c
 +++ b/drivers/spi/spi-sun4i.c
 @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
 *master,
 reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);

 /* Reset FIFOs */
 -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
 +   if (sspi->type == SPI_SUN4I)
 +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
 +   else
 +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
 +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
 +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>
>>> If we're already doing different stuff for each generation of the IP,
>>> why not just use the register offsets and bit definitions directly?
>>
>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>> makes my eyes bleed and you cannot use the check that you are
>> accessing a register that actually exists.
>
> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
> indirection you added and using them directly, i.e.
>
> #define SUN4I_TFR_CTL_RF_RST BIT(x)
> #define SUN4I_TFR_CTL_TF_RST BIT(x)
> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>
> then
>
> if (sspi->type == SPI_SUN4I)
> sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
> else
> sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>
> I.e. the bits that need setting are in different registers, so you
> have to have an if statement and separate calls. Therefore there's no
> real benefit from the indirection you've introduced here, unless
> you're expecting the SUN8I variant to use different bits in one of
> those two registers.
>

That looks nice for this particular case.

Still you will have to remember that these bits are specified directly
while other bits on the register are mapped which is not so nice.

Thanks

Michal

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

[Julia Lawall]

On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:

> On Mon, Jun 13, 2016 at 09:50:15PM +0200, Julia Lawall wrote:
> > 
> > 
> > On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:
> > 
> > > On Fri, Jun 10, 2016 at 11:21:28PM +0200, Julia Lawall wrote:
> > > > 
> > > > 
> > > > On Fri, 10 Jun 2016, Luis R. Rodriguez wrote:
> > > > 
> > > > > On Fri, Jun 10, 2016 at 11:02:38PM +0200, Julia Lawall wrote:
> > > > > > 
> > > > > > 
> > > > > > On Fri, 10 Jun 2016, Luis R. Rodriguez wrote:
> > > > > > 
> > > > > > > Enable indexing optimizations heuristics. Coccinelle has
> > > > > > > support to make use of its own enhanced "grep" mechanisms
> > > > > > > instead of using regular grep for searching code 'coccigrep',
> > > > > > > in practice though this seems to not perform better than
> > > > > > > regular grep however its expected to help with some use cases
> > > > > > > so we use that if you have no other indexing options in place
> > > > > > > available.
> > > > > > > 
> > > > > > > Since git has its own index, support for using 'git grep' has been
> > > > > > > added to Coccinelle, that should on average perform better than
> > > > > > > using the internal cocci grep, and regular grep. Lastly, 
> > > > > > > Coccinelle
> > > > > > > has had support for glimpseindex for a long while, however the
> > > > > > > tool was previously closed source, its now open sourced, and
> > > > > > > provides the best performance, so support that if we can detect
> > > > > > > you have a glimpse index.
> > > > > > > 
> > > > > > > These tests have been run on an 8 core system:
> > > > > > > 
> > > > > > > Before:
> > > > > > > 
> > > > > > > $ export COCCI=scripts/coccinelle/free/kfree.cocci
> > > > > > > $ time make coccicheck MODE=report
> > > > > > > 
> > > > > > > Before this patch with no indexing or anything:
> > > > > > > 
> > > > > > > real16m22.435s
> > > > > > > user128m30.060s
> > > > > > > sys 0m2.712s
> > > > > > > 
> > > > > > > Using coccigrep (after this patch if you have no .git):
> > > > > > > 
> > > > > > > real16m27.650s
> > > > > > > user128m47.904s
> > > > > > > sys 0m2.176s
> > > > > > > 
> > > > > > > If you have .git and therefore use gitgrep:
> > > > > > > 
> > > > > > > real16m21.220s
> > > > > > > user129m30.940s
> > > > > > > sys 0m2.060s
> > > > > > > 
> > > > > > > And if you have a .glimpse_index:
> > > > > > > 
> > > > > > > real16m14.794s
> > > > > > > user128m42.356s
> > > > > > > sys 0m1.880s
> > > > > > 
> > > > > > I don't see any convincing differences in these times.
> > > > > > 
> > > > > > I believe that Coccinelle's internal grep is always used, even with 
> > > > > > no 
> > > > > > option.
> > > > > 
> > > > > Ah that would explain it. This uses coccinelle 1.0.5, is the default
> > > > > there to use --use-coccigrep if no other index is specified ?
> > > > 
> > > > It has been the default for a long time.
> > > > 
> > > > > > I'm puzzled why glimpse gives no benefit.
> > > > > 
> > > > > Well, slightly better.
> > > > 
> > > > No, it should be much better.  You would have to look at the standard 
> > > > error to see if you are getting any benefit.  There should be very few 
> > > > occurrences of Skipping if you are really using glimpse.  In any case, 
> > > > if 
> > > > you asked for glimpse and it was not able to provide it, there should 
> > > > be 
> > > > warning messages at the top of stderr.
> > > 
> > > I'll redirect stderr to stdout by default when parmap support is used 
> > > then.
> > 
> > Usually I put them in different files.
> 
> We can do that as well but I would only want to deal with parmap support 
> case. Any preference? How about .coccicheck.stderr.$PID where PID would 
> be the PID of the shell script?

I don't understand the connection with parmap.

Originally our use of parmap made output files based on pids.  Maybe this 
is the default for parmap.  I found this completely unusable.  I guess one 
could look at the dates to see which file is the most recent one, but it 
seems tedious.  If you are putting the standard output in x.out, then put 
the standard error in x.err.

julia

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

[Julia Lawall]

On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:

> On Mon, Jun 13, 2016 at 09:50:15PM +0200, Julia Lawall wrote:
> > 
> > 
> > On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:
> > 
> > > On Fri, Jun 10, 2016 at 11:21:28PM +0200, Julia Lawall wrote:
> > > > 
> > > > 
> > > > On Fri, 10 Jun 2016, Luis R. Rodriguez wrote:
> > > > 
> > > > > On Fri, Jun 10, 2016 at 11:02:38PM +0200, Julia Lawall wrote:
> > > > > > 
> > > > > > 
> > > > > > On Fri, 10 Jun 2016, Luis R. Rodriguez wrote:
> > > > > > 
> > > > > > > Enable indexing optimizations heuristics. Coccinelle has
> > > > > > > support to make use of its own enhanced "grep" mechanisms
> > > > > > > instead of using regular grep for searching code 'coccigrep',
> > > > > > > in practice though this seems to not perform better than
> > > > > > > regular grep however its expected to help with some use cases
> > > > > > > so we use that if you have no other indexing options in place
> > > > > > > available.
> > > > > > > 
> > > > > > > Since git has its own index, support for using 'git grep' has been
> > > > > > > added to Coccinelle, that should on average perform better than
> > > > > > > using the internal cocci grep, and regular grep. Lastly, 
> > > > > > > Coccinelle
> > > > > > > has had support for glimpseindex for a long while, however the
> > > > > > > tool was previously closed source, its now open sourced, and
> > > > > > > provides the best performance, so support that if we can detect
> > > > > > > you have a glimpse index.
> > > > > > > 
> > > > > > > These tests have been run on an 8 core system:
> > > > > > > 
> > > > > > > Before:
> > > > > > > 
> > > > > > > $ export COCCI=scripts/coccinelle/free/kfree.cocci
> > > > > > > $ time make coccicheck MODE=report
> > > > > > > 
> > > > > > > Before this patch with no indexing or anything:
> > > > > > > 
> > > > > > > real16m22.435s
> > > > > > > user128m30.060s
> > > > > > > sys 0m2.712s
> > > > > > > 
> > > > > > > Using coccigrep (after this patch if you have no .git):
> > > > > > > 
> > > > > > > real16m27.650s
> > > > > > > user128m47.904s
> > > > > > > sys 0m2.176s
> > > > > > > 
> > > > > > > If you have .git and therefore use gitgrep:
> > > > > > > 
> > > > > > > real16m21.220s
> > > > > > > user129m30.940s
> > > > > > > sys 0m2.060s
> > > > > > > 
> > > > > > > And if you have a .glimpse_index:
> > > > > > > 
> > > > > > > real16m14.794s
> > > > > > > user128m42.356s
> > > > > > > sys 0m1.880s
> > > > > > 
> > > > > > I don't see any convincing differences in these times.
> > > > > > 
> > > > > > I believe that Coccinelle's internal grep is always used, even with 
> > > > > > no 
> > > > > > option.
> > > > > 
> > > > > Ah that would explain it. This uses coccinelle 1.0.5, is the default
> > > > > there to use --use-coccigrep if no other index is specified ?
> > > > 
> > > > It has been the default for a long time.
> > > > 
> > > > > > I'm puzzled why glimpse gives no benefit.
> > > > > 
> > > > > Well, slightly better.
> > > > 
> > > > No, it should be much better.  You would have to look at the standard 
> > > > error to see if you are getting any benefit.  There should be very few 
> > > > occurrences of Skipping if you are really using glimpse.  In any case, 
> > > > if 
> > > > you asked for glimpse and it was not able to provide it, there should 
> > > > be 
> > > > warning messages at the top of stderr.
> > > 
> > > I'll redirect stderr to stdout by default when parmap support is used 
> > > then.
> > 
> > Usually I put them in different files.
> 
> We can do that as well but I would only want to deal with parmap support 
> case. Any preference? How about .coccicheck.stderr.$PID where PID would 
> be the PID of the shell script?

I don't understand the connection with parmap.

Originally our use of parmap made output files based on pids.  Maybe this 
is the default for parmap.  I found this completely unusable.  I guess one 
could look at the dates to see which file is the most recent one, but it 
seems tedious.  If you are putting the standard output in x.out, then put 
the standard error in x.err.

julia

Re: [PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.

[Pranay Srivastava]

Hello,

On Thu, Jun 9, 2016 at 3:33 PM, Pranay Srivastava  wrote:
>
> Hello
>
>
> On Thu, Jun 2, 2016 at 3:54 PM, Pranay Kr. Srivastava  
> wrote:
> > spinlocked ranges should be small and not contain calls into huge
> > subfunctions. Fix my mistake and just get the pointer to the socket
> > instead of doing everything with spinlock held.
> >
> > Reported-by: Mikulas Patocka 
> > Signed-off-by: Markus Pargmann 
> >
> > Changelog:
> > Pranay Kr. Srivastava:
> >
> > 1) Use spin_lock instead of irq version for sock_shutdown.
> >
> > 2) Use system work queue to actually trigger the shutdown of
> >socket. This solves the issue when kernel_sendmsg is currently
> >blocked while a timeout occurs.
> >
> > Signed-off-by: Pranay Kr. Srivastava 
> > ---
> >  drivers/block/nbd.c | 65 
> > ++---
> >  1 file changed, 42 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index 31e73a7..0339d40 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -39,6 +39,7 @@
> >  #include 
> >
> >  #include 
> > +#include 
> >
> >  struct nbd_device {
> > u32 flags;
> > @@ -69,6 +70,10 @@ struct nbd_device {
> >  #if IS_ENABLED(CONFIG_DEBUG_FS)
> > struct dentry *dbg_dir;
> >  #endif
> > +   /*
> > +*This is specifically for calling sock_shutdown, for now.
> > +*/
> > +   struct work_struct ws_shutdown;
> >  };
> >
> >  #if IS_ENABLED(CONFIG_DEBUG_FS)
> > @@ -95,6 +100,11 @@ static int max_part;
> >   */
> >  static DEFINE_SPINLOCK(nbd_lock);
> >
> > +/*
> > + * Shutdown function for nbd_dev work struct.
> > + */
> > +static void nbd_ws_func_shutdown(struct work_struct *);
> > +
> >  static inline struct device *nbd_to_dev(struct nbd_device *nbd)
> >  {
> > return disk_to_dev(nbd->disk);
> > @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, 
> > struct request *req)
> >   */
> >  static void sock_shutdown(struct nbd_device *nbd)
> >  {
> > -   spin_lock_irq(>sock_lock);
> > -
> > -   if (!nbd->sock) {
> > -   spin_unlock_irq(>sock_lock);
> > -   return;
> > -   }
> > +   struct socket *sock;
> >
> > -   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > -   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > -   sockfd_put(nbd->sock);
> > +   spin_lock(>sock_lock);
> > +   sock = nbd->sock;
> > nbd->sock = NULL;
> > -   spin_unlock_irq(>sock_lock);
> > +   spin_unlock(>sock_lock);
> > +
> > +   if (!sock)
> > +   return;
> >
> > del_timer(>timeout_timer);
> > +   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > +   kernel_sock_shutdown(sock, SHUT_RDWR);
> > +   sockfd_put(sock);
> >  }
> >
> >  static void nbd_xmit_timeout(unsigned long arg)
> >  {
> > struct nbd_device *nbd = (struct nbd_device *)arg;
> > -   unsigned long flags;
> >
> > if (list_empty(>queue_head))
> > return;
> > -
> > -   spin_lock_irqsave(>sock_lock, flags);
> > -
> > nbd->timedout = true;
> > -
> > -   if (nbd->sock)
> > -   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > -
> > -   spin_unlock_irqrestore(>sock_lock, flags);
> > -
> > +   schedule_work(>ws_shutdown);
> > +   /*
> > +* Make sure sender thread sees nbd->timedout.
> > +*/
> > +   smp_wmb();
> > +   wake_up(>waiting_wq);
> > dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down 
> > connection\n");
> >  }
> >
> > @@ -592,7 +598,11 @@ static int nbd_thread_send(void *data)
> > spin_unlock_irq(>queue_lock);
> >
> > /* handle request */
> > -   nbd_handle_req(nbd, req);
> > +   if (nbd->timedout) {
> > +   req->errors++;
> > +   nbd_end_request(nbd, req);
> > +   } else
> > +   nbd_handle_req(nbd, req);
> > }
> >
> > nbd->task_send = NULL;
> > @@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd)
> > set_capacity(nbd->disk, 0);
> > nbd->flags = 0;
> > nbd->xmit_timeout = 0;
> > +   INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
> > queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
> > del_timer_sync(>timeout_timer);
> >  }
> > @@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, 
> > struct nbd_device *nbd,
> > nbd_dev_dbg_close(nbd);
> > kthread_stop(thread);
> >
> > -   mutex_lock(>tx_lock);
> > -
> > sock_shutdown(nbd);
> > +   mutex_lock(>tx_lock);
> > nbd_clear_que(nbd);
> > kill_bdev(bdev);
> > 

Re: [PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.

[Pranay Srivastava]

Hello,

On Thu, Jun 9, 2016 at 3:33 PM, Pranay Srivastava  wrote:
>
> Hello
>
>
> On Thu, Jun 2, 2016 at 3:54 PM, Pranay Kr. Srivastava  
> wrote:
> > spinlocked ranges should be small and not contain calls into huge
> > subfunctions. Fix my mistake and just get the pointer to the socket
> > instead of doing everything with spinlock held.
> >
> > Reported-by: Mikulas Patocka 
> > Signed-off-by: Markus Pargmann 
> >
> > Changelog:
> > Pranay Kr. Srivastava:
> >
> > 1) Use spin_lock instead of irq version for sock_shutdown.
> >
> > 2) Use system work queue to actually trigger the shutdown of
> >socket. This solves the issue when kernel_sendmsg is currently
> >blocked while a timeout occurs.
> >
> > Signed-off-by: Pranay Kr. Srivastava 
> > ---
> >  drivers/block/nbd.c | 65 
> > ++---
> >  1 file changed, 42 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index 31e73a7..0339d40 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -39,6 +39,7 @@
> >  #include 
> >
> >  #include 
> > +#include 
> >
> >  struct nbd_device {
> > u32 flags;
> > @@ -69,6 +70,10 @@ struct nbd_device {
> >  #if IS_ENABLED(CONFIG_DEBUG_FS)
> > struct dentry *dbg_dir;
> >  #endif
> > +   /*
> > +*This is specifically for calling sock_shutdown, for now.
> > +*/
> > +   struct work_struct ws_shutdown;
> >  };
> >
> >  #if IS_ENABLED(CONFIG_DEBUG_FS)
> > @@ -95,6 +100,11 @@ static int max_part;
> >   */
> >  static DEFINE_SPINLOCK(nbd_lock);
> >
> > +/*
> > + * Shutdown function for nbd_dev work struct.
> > + */
> > +static void nbd_ws_func_shutdown(struct work_struct *);
> > +
> >  static inline struct device *nbd_to_dev(struct nbd_device *nbd)
> >  {
> > return disk_to_dev(nbd->disk);
> > @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, 
> > struct request *req)
> >   */
> >  static void sock_shutdown(struct nbd_device *nbd)
> >  {
> > -   spin_lock_irq(>sock_lock);
> > -
> > -   if (!nbd->sock) {
> > -   spin_unlock_irq(>sock_lock);
> > -   return;
> > -   }
> > +   struct socket *sock;
> >
> > -   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > -   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > -   sockfd_put(nbd->sock);
> > +   spin_lock(>sock_lock);
> > +   sock = nbd->sock;
> > nbd->sock = NULL;
> > -   spin_unlock_irq(>sock_lock);
> > +   spin_unlock(>sock_lock);
> > +
> > +   if (!sock)
> > +   return;
> >
> > del_timer(>timeout_timer);
> > +   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > +   kernel_sock_shutdown(sock, SHUT_RDWR);
> > +   sockfd_put(sock);
> >  }
> >
> >  static void nbd_xmit_timeout(unsigned long arg)
> >  {
> > struct nbd_device *nbd = (struct nbd_device *)arg;
> > -   unsigned long flags;
> >
> > if (list_empty(>queue_head))
> > return;
> > -
> > -   spin_lock_irqsave(>sock_lock, flags);
> > -
> > nbd->timedout = true;
> > -
> > -   if (nbd->sock)
> > -   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > -
> > -   spin_unlock_irqrestore(>sock_lock, flags);
> > -
> > +   schedule_work(>ws_shutdown);
> > +   /*
> > +* Make sure sender thread sees nbd->timedout.
> > +*/
> > +   smp_wmb();
> > +   wake_up(>waiting_wq);
> > dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down 
> > connection\n");
> >  }
> >
> > @@ -592,7 +598,11 @@ static int nbd_thread_send(void *data)
> > spin_unlock_irq(>queue_lock);
> >
> > /* handle request */
> > -   nbd_handle_req(nbd, req);
> > +   if (nbd->timedout) {
> > +   req->errors++;
> > +   nbd_end_request(nbd, req);
> > +   } else
> > +   nbd_handle_req(nbd, req);
> > }
> >
> > nbd->task_send = NULL;
> > @@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd)
> > set_capacity(nbd->disk, 0);
> > nbd->flags = 0;
> > nbd->xmit_timeout = 0;
> > +   INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
> > queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
> > del_timer_sync(>timeout_timer);
> >  }
> > @@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, 
> > struct nbd_device *nbd,
> > nbd_dev_dbg_close(nbd);
> > kthread_stop(thread);
> >
> > -   mutex_lock(>tx_lock);
> > -
> > sock_shutdown(nbd);
> > +   mutex_lock(>tx_lock);
> > nbd_clear_que(nbd);
> > kill_bdev(bdev);
> > nbd_bdev_reset(bdev);
> >
> > if (nbd->disconnect) /* user requested, ignore socket 
> > errors */
> >  

806ebc1465: BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS

[kernel test robot]

FYI, we noticed the following commit:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git x86/uaccess
commit 806ebc146567cb0030460ebf34ebecfb7c67eb76 ("[DEBUG] force 
CONFIG_DEBUG_UACCESS")


on test machine: vm-lkp-wsx03-openwrt-i386: 1 threads qemu-system-i386 
-enable-kvm with 192M memory

caused below changes:


++++
|| 899f263944 | 806ebc1465 |
++++
| boot_successes | 6  | 0  |
| boot_failures  | 4  | 12 |
| IP-Config:Auto-configuration_of_network_failed | 4  ||
| BUG:tried_to_access_memory_at#while_not_in_USER_DS | 0  | 12 |
| kernel_BUG_at_arch/x86/mm/extable.c| 0  | 12 |
| invalid_opcode:#[##]SMP| 0  | 12 |
| EIP_is_at_bad_uaccess_kernel_ds| 0  | 12 |
| Kernel_panic-not_syncing:Fatal_exception   | 0  | 12 |
| backtrace:do_vfs_ioctl | 0  | 6  |
| backtrace:SyS_ioctl| 0  | 6  |
| backtrace:do_execve| 0  | 2  |
| backtrace:SyS_execve   | 0  | 2  |
++++



[   10.893589] Write protecting the kernel read-only data: 3932k
procd: Console is alive
procd: - watchdog -
[   10.922394] BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS
[   10.922394] BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS
[   10.923957] [ cut here ]
[   10.923957] [ cut here ]
[   10.924977] kernel BUG at arch/x86/mm/extable.c:189!
[   10.924977] kernel BUG at arch/x86/mm/extable.c:189!
[   10.926382] invalid opcode:  [#1] SMP
[   10.926382] invalid opcode:  [#1] SMP
[   10.927270] CPU: 0 PID: 1 Comm: init Not tainted 4.7.0-rc2-00014-g806ebc1 
#403
[   10.927270] CPU: 0 PID: 1 Comm: init Not tainted 4.7.0-rc2-00014-g806ebc1 
#403
[   10.928893] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Debian-1.8.2-1 04/01/2014
[   10.928893] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Debian-1.8.2-1 04/01/2014
[   10.930915] task: 8b44 ti: 8b426000 task.ti: 8b426000
[   10.930915] task: 8b44 ti: 8b426000 task.ti: 8b426000
[   10.932179] EIP: 0060:[<8102e631>] EFLAGS: 00010246 CPU: 0
[   10.932179] EIP: 0060:[<8102e631>] EFLAGS: 00010246 CPU: 0
[   10.933391] EIP is at bad_uaccess_kernel_ds+0xe/0x10
[   10.933391] EIP is at bad_uaccess_kernel_ds+0xe/0x10
[   10.934483] EAX: 003e EBX: 813de9b8 ECX: 81077607 EDX: 8b44
[   10.934483] EAX: 003e EBX: 813de9b8 ECX: 81077607 EDX: 8b44
[   10.935909] ESI: 7ff6afd0 EDI: fff7 EBP: 8b427ef8 ESP: 8b427ef0
[   10.935909] ESI: 7ff6afd0 EDI: fff7 EBP: 8b427ef8 ESP: 8b427ef0
[   10.937353]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   10.937353]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   10.938552] CR0: 80050033 CR2: bc60 CR3: 0087a000 CR4: 0690
[   10.938552] CR0: 80050033 CR2: bc60 CR3: 0087a000 CR4: 0690
[   10.939936] Stack:
[   10.939936] Stack:
[   10.940386]  81a330a5
[   10.940386]  81a330a5 7ff6afd3 7ff6afd3 8b427f08 8b427f08 813deab5 813deab5 
813de9b8 813de9b8 899cb780 899cb780 8b427f14 8b427f14 810edd0a 810edd0a

[   10.942231]  899cb780
[   10.942231]  899cb780 8b427f68 8b427f68 810ee465 810ee465 0002 0002 
2180 2180 899cb3c8 899cb3c8 88d146a8 88d146a8 810e100b 810e100b

[   10.944062]  0001
[   10.944062]  0001     899cb3c0 899cb3c0 
0014 0014 0804a2a5 0804a2a5 8b427f60 8b427f60 810e1d7c 810e1d7c

[   10.945951] Call Trace:
[   10.945951] Call Trace:
[   10.946493]  [<813deab5>] wafwdt_ioctl+0xfd/0x14a
[   10.946493]  [<813deab5>] wafwdt_ioctl+0xfd/0x14a
[   10.947528]  [<813de9b8>] ? wafwdt_write+0x63/0x63
[   10.947528]  [<813de9b8>] ? wafwdt_write+0x63/0x63
[   10.948611]  [<810edd0a>] vfs_ioctl+0x17/0x21
[   10.948611]  [<810edd0a>] vfs_ioctl+0x17/0x21
[   10.949568]  [<810ee465>] do_vfs_ioctl+0x5bd/0x5ef
[   10.949568]  [<810ee465>] do_vfs_ioctl+0x5bd/0x5ef
[   10.950620]  [<810e100b>] ? fsnotify_modify+0x48/0x53
[   10.950620]  [<810e100b>] ? fsnotify_modify+0x48/0x53
[   10.951781]  [<810e1d7c>] ? vfs_write+0x9a/0xa6
[   10.951781]  [<810e1d7c>] ? vfs_write+0x9a/0xa6
[   10.952781]  [<810f61f6>] ? __fget_light+0x38/0x5a
[   10.952781]  [<810f61f6>] ? __fget_light+0x38/0x5a
[   10.953909]  [<810ee4c3>] SyS_ioctl+0x2c/0x45
[   10.953909]  [<810ee4c3>] SyS_ioctl+0x2c/0x45
[   10.954866]  [<81000d1c>] 

Re: [PATCH v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:

Hi Andrew,

> Hi,
> 
> On 8 June 2016 at 21:14, Mat Martineau
> 
>  wrote:
> > On Wed, 8 Jun 2016, Stephan Mueller wrote:
> >> What is your concern?
> > 
> > Userspace must allocate larger buffers than it knows are necessary for
> > expected results.
> > 
> > It looks like the software rsa implementation handles shorter output
> > buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
> > too small), however I see at least one hardware rsa driver that requires
> > the output buffer to be the maximum size. But this inconsistency might be
> > best addressed within the software cipher or drivers rather than in
> > recvmsg.
> Should the hardware drivers fix this instead?  I've looked at the qat
> and caam drivers, they both require the destination buffer size to be
> the key size and in both cases there would be no penalty for dropping
> this requirement as far as I see.  Both do a memmove if the result
> ends up being shorter than key size.  In case the caller knows it is
> expecting a specific output size, the driver will have to use a self
> allocated buffer + a memcpy in those same cases where it would later
> use memmove instead.  Alternatively the sg passed to dma_map_sg can be
> prepended with a dummy segment the right size to save the memcpy.
> 
> akcipher.h only says:
> @dst_len: Size of the output buffer. It needs to be at least as big as
> the expected result depending on the operation
> 
> Note that for random input data the memmove will be done about 1 in
> 256 times but with PKCS#1 padding the signature always has a leading
> zero.
> 
> Requiring buffers bigger than needed makes the added work of dropping
> the zero bytes from the sglist and potentially re-adding them in the
> client difficult to justify.  RSA doing this sets a precedent for a
> future pkcs1pad (or other algorithm) implementation to do the same
> thing and a portable client having to always know the key size and use
> key-sized buffers.

I think we have agreed on dropping the length enforcement at the interface 
level.

Ciao
Stephan

806ebc1465: BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS

[kernel test robot]

FYI, we noticed the following commit:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git x86/uaccess
commit 806ebc146567cb0030460ebf34ebecfb7c67eb76 ("[DEBUG] force 
CONFIG_DEBUG_UACCESS")


on test machine: vm-lkp-wsx03-openwrt-i386: 1 threads qemu-system-i386 
-enable-kvm with 192M memory

caused below changes:


++++
|| 899f263944 | 806ebc1465 |
++++
| boot_successes | 6  | 0  |
| boot_failures  | 4  | 12 |
| IP-Config:Auto-configuration_of_network_failed | 4  ||
| BUG:tried_to_access_memory_at#while_not_in_USER_DS | 0  | 12 |
| kernel_BUG_at_arch/x86/mm/extable.c| 0  | 12 |
| invalid_opcode:#[##]SMP| 0  | 12 |
| EIP_is_at_bad_uaccess_kernel_ds| 0  | 12 |
| Kernel_panic-not_syncing:Fatal_exception   | 0  | 12 |
| backtrace:do_vfs_ioctl | 0  | 6  |
| backtrace:SyS_ioctl| 0  | 6  |
| backtrace:do_execve| 0  | 2  |
| backtrace:SyS_execve   | 0  | 2  |
++++



[   10.893589] Write protecting the kernel read-only data: 3932k
procd: Console is alive
procd: - watchdog -
[   10.922394] BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS
[   10.922394] BUG: tried to access memory at 0x7ff6afd3 while not in USER_DS
[   10.923957] [ cut here ]
[   10.923957] [ cut here ]
[   10.924977] kernel BUG at arch/x86/mm/extable.c:189!
[   10.924977] kernel BUG at arch/x86/mm/extable.c:189!
[   10.926382] invalid opcode:  [#1] SMP
[   10.926382] invalid opcode:  [#1] SMP
[   10.927270] CPU: 0 PID: 1 Comm: init Not tainted 4.7.0-rc2-00014-g806ebc1 
#403
[   10.927270] CPU: 0 PID: 1 Comm: init Not tainted 4.7.0-rc2-00014-g806ebc1 
#403
[   10.928893] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Debian-1.8.2-1 04/01/2014
[   10.928893] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Debian-1.8.2-1 04/01/2014
[   10.930915] task: 8b44 ti: 8b426000 task.ti: 8b426000
[   10.930915] task: 8b44 ti: 8b426000 task.ti: 8b426000
[   10.932179] EIP: 0060:[<8102e631>] EFLAGS: 00010246 CPU: 0
[   10.932179] EIP: 0060:[<8102e631>] EFLAGS: 00010246 CPU: 0
[   10.933391] EIP is at bad_uaccess_kernel_ds+0xe/0x10
[   10.933391] EIP is at bad_uaccess_kernel_ds+0xe/0x10
[   10.934483] EAX: 003e EBX: 813de9b8 ECX: 81077607 EDX: 8b44
[   10.934483] EAX: 003e EBX: 813de9b8 ECX: 81077607 EDX: 8b44
[   10.935909] ESI: 7ff6afd0 EDI: fff7 EBP: 8b427ef8 ESP: 8b427ef0
[   10.935909] ESI: 7ff6afd0 EDI: fff7 EBP: 8b427ef8 ESP: 8b427ef0
[   10.937353]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   10.937353]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   10.938552] CR0: 80050033 CR2: bc60 CR3: 0087a000 CR4: 0690
[   10.938552] CR0: 80050033 CR2: bc60 CR3: 0087a000 CR4: 0690
[   10.939936] Stack:
[   10.939936] Stack:
[   10.940386]  81a330a5
[   10.940386]  81a330a5 7ff6afd3 7ff6afd3 8b427f08 8b427f08 813deab5 813deab5 
813de9b8 813de9b8 899cb780 899cb780 8b427f14 8b427f14 810edd0a 810edd0a

[   10.942231]  899cb780
[   10.942231]  899cb780 8b427f68 8b427f68 810ee465 810ee465 0002 0002 
2180 2180 899cb3c8 899cb3c8 88d146a8 88d146a8 810e100b 810e100b

[   10.944062]  0001
[   10.944062]  0001     899cb3c0 899cb3c0 
0014 0014 0804a2a5 0804a2a5 8b427f60 8b427f60 810e1d7c 810e1d7c

[   10.945951] Call Trace:
[   10.945951] Call Trace:
[   10.946493]  [<813deab5>] wafwdt_ioctl+0xfd/0x14a
[   10.946493]  [<813deab5>] wafwdt_ioctl+0xfd/0x14a
[   10.947528]  [<813de9b8>] ? wafwdt_write+0x63/0x63
[   10.947528]  [<813de9b8>] ? wafwdt_write+0x63/0x63
[   10.948611]  [<810edd0a>] vfs_ioctl+0x17/0x21
[   10.948611]  [<810edd0a>] vfs_ioctl+0x17/0x21
[   10.949568]  [<810ee465>] do_vfs_ioctl+0x5bd/0x5ef
[   10.949568]  [<810ee465>] do_vfs_ioctl+0x5bd/0x5ef
[   10.950620]  [<810e100b>] ? fsnotify_modify+0x48/0x53
[   10.950620]  [<810e100b>] ? fsnotify_modify+0x48/0x53
[   10.951781]  [<810e1d7c>] ? vfs_write+0x9a/0xa6
[   10.951781]  [<810e1d7c>] ? vfs_write+0x9a/0xa6
[   10.952781]  [<810f61f6>] ? __fget_light+0x38/0x5a
[   10.952781]  [<810f61f6>] ? __fget_light+0x38/0x5a
[   10.953909]  [<810ee4c3>] SyS_ioctl+0x2c/0x45
[   10.953909]  [<810ee4c3>] SyS_ioctl+0x2c/0x45
[   10.954866]  [<81000d1c>] 

Re: [PATCH v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:

Hi Andrew,

> Hi,
> 
> On 8 June 2016 at 21:14, Mat Martineau
> 
>  wrote:
> > On Wed, 8 Jun 2016, Stephan Mueller wrote:
> >> What is your concern?
> > 
> > Userspace must allocate larger buffers than it knows are necessary for
> > expected results.
> > 
> > It looks like the software rsa implementation handles shorter output
> > buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
> > too small), however I see at least one hardware rsa driver that requires
> > the output buffer to be the maximum size. But this inconsistency might be
> > best addressed within the software cipher or drivers rather than in
> > recvmsg.
> Should the hardware drivers fix this instead?  I've looked at the qat
> and caam drivers, they both require the destination buffer size to be
> the key size and in both cases there would be no penalty for dropping
> this requirement as far as I see.  Both do a memmove if the result
> ends up being shorter than key size.  In case the caller knows it is
> expecting a specific output size, the driver will have to use a self
> allocated buffer + a memcpy in those same cases where it would later
> use memmove instead.  Alternatively the sg passed to dma_map_sg can be
> prepended with a dummy segment the right size to save the memcpy.
> 
> akcipher.h only says:
> @dst_len: Size of the output buffer. It needs to be at least as big as
> the expected result depending on the operation
> 
> Note that for random input data the memmove will be done about 1 in
> 256 times but with PKCS#1 padding the signature always has a leading
> zero.
> 
> Requiring buffers bigger than needed makes the added work of dropping
> the zero bytes from the sglist and potentially re-adding them in the
> client difficult to justify.  RSA doing this sets a precedent for a
> future pkcs1pad (or other algorithm) implementation to do the same
> thing and a portable client having to always know the key size and use
> key-sized buffers.

I think we have agreed on dropping the length enforcement at the interface 
level.

Ciao
Stephan

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Julia Lawall]

On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:

> On Mon, Jun 13, 2016 at 09:48:30PM +0200, Julia Lawall wrote:
> > 
> > 
> > On Mon, 13 Jun 2016, Wolfram Sang wrote:
> > 
> > > 
> > > > Is there another scripts/coccinelle/ file I can use to test against to 
> > > > demo
> > > > against glimpse/idutils/gitgrep best?
> > > 
> > > I'd think this one may be a candidate:
> > > 
> > > scripts/coccinelle/misc/irqf_oneshot.cocci
> > > 
> > > Not too many, but quite some matches over the tree.
> > 
> > Seems like a reasonable choice.
> 
> With this one on a 32-core system, I get:
> 
> glimpse:
> real0m6.549s
> user0m49.136s
> sys 0m3.076s
> 
> idutils:
> real0m6.749s
> user0m51.936s
> sys 0m3.876s
> 
> gitgrep:
> real0m6.805s
> user0m51.572s
> sys 0m4.432s
> 
> coccigrep:
> real0m16.369s
> user0m58.712s
> sys 0m5.064s
> 
> I redirected stderr to stdout, and verified glimpse output has:
> 
> glimpse request = request_threaded_irq
> 
> Does this match expectations?

Yes.  I'm not sure that gitgrep would work as well when there are multiple 
keywords.  It may descend to coccigrep.

julia

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Julia Lawall]

On Mon, 13 Jun 2016, Luis R. Rodriguez wrote:

> On Mon, Jun 13, 2016 at 09:48:30PM +0200, Julia Lawall wrote:
> > 
> > 
> > On Mon, 13 Jun 2016, Wolfram Sang wrote:
> > 
> > > 
> > > > Is there another scripts/coccinelle/ file I can use to test against to 
> > > > demo
> > > > against glimpse/idutils/gitgrep best?
> > > 
> > > I'd think this one may be a candidate:
> > > 
> > > scripts/coccinelle/misc/irqf_oneshot.cocci
> > > 
> > > Not too many, but quite some matches over the tree.
> > 
> > Seems like a reasonable choice.
> 
> With this one on a 32-core system, I get:
> 
> glimpse:
> real0m6.549s
> user0m49.136s
> sys 0m3.076s
> 
> idutils:
> real0m6.749s
> user0m51.936s
> sys 0m3.876s
> 
> gitgrep:
> real0m6.805s
> user0m51.572s
> sys 0m4.432s
> 
> coccigrep:
> real0m16.369s
> user0m58.712s
> sys 0m5.064s
> 
> I redirected stderr to stdout, and verified glimpse output has:
> 
> glimpse request = request_threaded_irq
> 
> Does this match expectations?

Yes.  I'm not sure that gitgrep would work as well when there are multiple 
keywords.  It may descend to coccigrep.

julia

linux-next: manual merge of the staging tree with the block tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the staging tree got a conflict in:

  drivers/staging/lustre/lustre/llite/lloop.c

between commit:

  95fe6c1a209e ("block, fs, mm, drivers: use bio set/get op accessors")

from the block tree and commit:

  67b1a24e883c ("staging: lustre: llite: remove lloop device")

from the staging tree.

I fixed it up (I removed the file) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the staging tree with the block tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the staging tree got a conflict in:

  drivers/staging/lustre/lustre/llite/lloop.c

between commit:

  95fe6c1a209e ("block, fs, mm, drivers: use bio set/get op accessors")

from the block tree and commit:

  67b1a24e883c ("staging: lustre: llite: remove lloop device")

from the staging tree.

I fixed it up (I removed the file) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: [PATCH 2/2] dmaengine: bcm2835: Avoid splitting periods into very small chunks

[Eric Anholt]

Matthias Reichl  writes:

> The current cyclic DMA period splitting implementation can generate
> very small chunks at the end of each period. For example a 65536 byte
> period will be split into a 65532 byte chunk and a 4 byte chunk on
> the "lite" DMA channels.
>
> This increases pressure on the RAM controller as the DMA controller
> needs to fetch two control blocks from RAM in quick succession and
> could potentially cause latency issues if the RAM is tied up by other
> devices.
>
> We can easily avoid these situations by distributing the remaining
> length evenly between the last-but-one and the last chunk, making
> sure that split chunks will be at least half the maximum length the
> DMA controller can handle.
>
> This patch checks if the last chunk would be less than half of
> the maximum DMA length and if yes distributes the max len+4...max_len*1.5
> bytes evenly between the last 2 chunks. This results in chunk sizes
> between max_len/2 and max_len*0.75 bytes.
>
> Signed-off-by: Matthias Reichl 
> Signed-off-by: Martin Sperl 
> Tested-by: Clive Messer 
> ---
>  drivers/dma/bcm2835-dma.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index 344bcf92..36b998d 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -252,6 +252,20 @@ static void bcm2835_dma_create_cb_set_length(
>  
>   /* have we filled in period_length yet? */
>   if (*total_len + control_block->length < period_len) {
> + /*
> +  * If the next control block is the last in the period
> +  * and it's length would be less than half of max_len
> +  * change it so that both control blocks are (almost)
> +  * equally long. This avoids generating very short
> +  * control blocks (worst case would be 4 bytes) which
> +  * might be problematic. We also have to make sure the
> +  * new length is a multiple of 4 bytes.
> +  */
> + if (*total_len + control_block->length + max_len / 2 >
> + period_len) {
> + control_block->length =
> + DIV_ROUND_UP(period_len - *total_len, 8) * 4;
> + }
>   /* update number of bytes in this period so far */
>   *total_len += control_block->length;
>   return;

It seems to me like this would all be a lot simpler if we always split
the last 2 control blocks evenly (other than 4-byte rounding):

u32 period_remaining = period_len - *total_len;

/* Early exit if we aren't finishing this period */
if (period_remaining >= max_len) {
/*
 * Split the length between the last 2 CBs, to help hide the
 * latency of fetching the CBs.
 */
if (period_remaining < max_len * 2) {
control_block->length =
DIV_ROUND_UP(period_remaining, 8) * 4;
}
/* update number of bytes in this period so far */
*total_len += control_block->length;
}

I'm about to go semi-AFK for a couple weeks.  If there's a good reason
to only do this when the last block is very short, I'm fine with:

Acked-by: Eric Anholt 


signature.asc
Description: PGP signature

Re: [PATCH 2/2] dmaengine: bcm2835: Avoid splitting periods into very small chunks

[Eric Anholt]

Matthias Reichl  writes:

> The current cyclic DMA period splitting implementation can generate
> very small chunks at the end of each period. For example a 65536 byte
> period will be split into a 65532 byte chunk and a 4 byte chunk on
> the "lite" DMA channels.
>
> This increases pressure on the RAM controller as the DMA controller
> needs to fetch two control blocks from RAM in quick succession and
> could potentially cause latency issues if the RAM is tied up by other
> devices.
>
> We can easily avoid these situations by distributing the remaining
> length evenly between the last-but-one and the last chunk, making
> sure that split chunks will be at least half the maximum length the
> DMA controller can handle.
>
> This patch checks if the last chunk would be less than half of
> the maximum DMA length and if yes distributes the max len+4...max_len*1.5
> bytes evenly between the last 2 chunks. This results in chunk sizes
> between max_len/2 and max_len*0.75 bytes.
>
> Signed-off-by: Matthias Reichl 
> Signed-off-by: Martin Sperl 
> Tested-by: Clive Messer 
> ---
>  drivers/dma/bcm2835-dma.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index 344bcf92..36b998d 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -252,6 +252,20 @@ static void bcm2835_dma_create_cb_set_length(
>  
>   /* have we filled in period_length yet? */
>   if (*total_len + control_block->length < period_len) {
> + /*
> +  * If the next control block is the last in the period
> +  * and it's length would be less than half of max_len
> +  * change it so that both control blocks are (almost)
> +  * equally long. This avoids generating very short
> +  * control blocks (worst case would be 4 bytes) which
> +  * might be problematic. We also have to make sure the
> +  * new length is a multiple of 4 bytes.
> +  */
> + if (*total_len + control_block->length + max_len / 2 >
> + period_len) {
> + control_block->length =
> + DIV_ROUND_UP(period_len - *total_len, 8) * 4;
> + }
>   /* update number of bytes in this period so far */
>   *total_len += control_block->length;
>   return;

It seems to me like this would all be a lot simpler if we always split
the last 2 control blocks evenly (other than 4-byte rounding):

u32 period_remaining = period_len - *total_len;

/* Early exit if we aren't finishing this period */
if (period_remaining >= max_len) {
/*
 * Split the length between the last 2 CBs, to help hide the
 * latency of fetching the CBs.
 */
if (period_remaining < max_len * 2) {
control_block->length =
DIV_ROUND_UP(period_remaining, 8) * 4;
}
/* update number of bytes in this period so far */
*total_len += control_block->length;
}

I'm about to go semi-AFK for a couple weeks.  If there's a good reason
to only do this when the last block is very short, I'm fine with:

Acked-by: Eric Anholt 


signature.asc
Description: PGP signature

linux-next: manual merge of the staging tree with the staging.current tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the staging tree got a conflict in:

  drivers/iio/industrialio-trigger.c

between commit:

  995438233579 ("iio: Fix error handling in iio_trigger_attach_poll_func")

from the staging.current tree and commit:

  ef2d71d6b7fb ("iio: triggers: Make trigger ops structure explicitly non 
optional.")

from the staging tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/iio/industrialio-trigger.c
index 0c52dfe64977,672911293987..
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@@ -220,14 -217,15 +223,14 @@@ static int iio_trigger_attach_poll_func
ret = request_threaded_irq(pf->irq, pf->h, pf->thread,
   pf->type, pf->name,
   pf);
 -  if (ret < 0) {
 -  module_put(pf->indio_dev->info->driver_module);
 -  return ret;
 -  }
 +  if (ret < 0)
 +  goto out_put_irq;
  
 +  /* Enable trigger in driver */
-   if (trig->ops && trig->ops->set_trigger_state && notinuse) {
+   if (trig->ops->set_trigger_state && notinuse) {
ret = trig->ops->set_trigger_state(trig, true);
if (ret < 0)
 -  module_put(pf->indio_dev->info->driver_module);
 +  goto out_free_irq;
}
  
return ret;

linux-next: manual merge of the staging tree with the staging.current tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the staging tree got a conflict in:

  drivers/iio/industrialio-trigger.c

between commit:

  995438233579 ("iio: Fix error handling in iio_trigger_attach_poll_func")

from the staging.current tree and commit:

  ef2d71d6b7fb ("iio: triggers: Make trigger ops structure explicitly non 
optional.")

from the staging tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/iio/industrialio-trigger.c
index 0c52dfe64977,672911293987..
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@@ -220,14 -217,15 +223,14 @@@ static int iio_trigger_attach_poll_func
ret = request_threaded_irq(pf->irq, pf->h, pf->thread,
   pf->type, pf->name,
   pf);
 -  if (ret < 0) {
 -  module_put(pf->indio_dev->info->driver_module);
 -  return ret;
 -  }
 +  if (ret < 0)
 +  goto out_put_irq;
  
 +  /* Enable trigger in driver */
-   if (trig->ops && trig->ops->set_trigger_state && notinuse) {
+   if (trig->ops->set_trigger_state && notinuse) {
ret = trig->ops->set_trigger_state(trig, true);
if (ret < 0)
 -  module_put(pf->indio_dev->info->driver_module);
 +  goto out_free_irq;
}
  
return ret;

Re: [PATCH v2 3/3] ARM64: dts: meson-gxbb: Add Hardware Random Generator node

[Neil Armstrong]

On 06/14/2016 12:09 AM, Kevin Hilman wrote:
> Hi Neil,
> 
> Neil Armstrong  writes:
> 
>> Signed-off-by: Neil Armstrong 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 832815d..8353621 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -167,6 +167,11 @@
>>  };
>>  };
>>  
>> +rng {
>> +compatible = "amlogic,meson-rng";
>> +reg = <0x0 0xc8834000 0x0 0x4>;
>> +};
> 
> This should be under the periphs bus, with an offset of 0x0 instead of
> at the top level.
> 
> See the for-next branch in the linux-amlogic tree[1] which has separate
> busses for periphs and hiu.

Yes, It was introduced lately.

Herbert,

I'll repost this patch separately to arm-soc directly rebased on [1].

Thanks,
Neil

> Kevin
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git
> 

Re: [PATCH v2 3/3] ARM64: dts: meson-gxbb: Add Hardware Random Generator node

[Neil Armstrong]

On 06/14/2016 12:09 AM, Kevin Hilman wrote:
> Hi Neil,
> 
> Neil Armstrong  writes:
> 
>> Signed-off-by: Neil Armstrong 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 832815d..8353621 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -167,6 +167,11 @@
>>  };
>>  };
>>  
>> +rng {
>> +compatible = "amlogic,meson-rng";
>> +reg = <0x0 0xc8834000 0x0 0x4>;
>> +};
> 
> This should be under the periphs bus, with an offset of 0x0 instead of
> at the top level.
> 
> See the for-next branch in the linux-amlogic tree[1] which has separate
> busses for periphs and hiu.

Yes, It was introduced lately.

Herbert,

I'll repost this patch separately to arm-soc directly rebased on [1].

Thanks,
Neil

> Kevin
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git
> 

Re: [RESEND PATCH v11] mtd: spi-nor: add hisilicon spi-nor flash controller driver

[kbuild test robot]

Hi,

[auto build test ERROR on mtd/master]
[also build test ERROR on v4.7-rc3 next-20160609]
[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/Jiancheng-Xue/mtd-spi-nor-add-hisilicon-spi-nor-flash-controller-driver/20160613-163520
base:   git://git.infradead.org/linux-mtd.git master
config: m32r-allmodconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r 

All errors (new ones prefixed by >>):

   ERROR: "bad_dma_ops" [sound/soc/fsl/snd-soc-fsl-asrc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/soc/atmel/snd-soc-atmel-pcm-pdc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/core/snd-pcm.ko] undefined!
   ERROR: "dma_common_mmap" [sound/core/snd-pcm.ko] undefined!
   ERROR: "__ucmpdi2" [lib/842/842_decompress.ko] undefined!
   ERROR: "__ucmpdi2" [fs/btrfs/btrfs.ko] undefined!
>> ERROR: "dmam_alloc_coherent" [drivers/mtd/spi-nor/hisi-sfc.ko] undefined!
>> ERROR: "bad_dma_ops" [drivers/mtd/spi-nor/hisi-sfc.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/media/i2c/adv7842.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/md/bcache/bcache.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/iio/imu/inv_mpu6050/inv-mpu6050.ko] undefined!
   ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined!

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


.config.gz
Description: Binary data

Re: [RESEND PATCH v11] mtd: spi-nor: add hisilicon spi-nor flash controller driver

[kbuild test robot]

Hi,

[auto build test ERROR on mtd/master]
[also build test ERROR on v4.7-rc3 next-20160609]
[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/Jiancheng-Xue/mtd-spi-nor-add-hisilicon-spi-nor-flash-controller-driver/20160613-163520
base:   git://git.infradead.org/linux-mtd.git master
config: m32r-allmodconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r 

All errors (new ones prefixed by >>):

   ERROR: "bad_dma_ops" [sound/soc/fsl/snd-soc-fsl-asrc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/soc/atmel/snd-soc-atmel-pcm-pdc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/core/snd-pcm.ko] undefined!
   ERROR: "dma_common_mmap" [sound/core/snd-pcm.ko] undefined!
   ERROR: "__ucmpdi2" [lib/842/842_decompress.ko] undefined!
   ERROR: "__ucmpdi2" [fs/btrfs/btrfs.ko] undefined!
>> ERROR: "dmam_alloc_coherent" [drivers/mtd/spi-nor/hisi-sfc.ko] undefined!
>> ERROR: "bad_dma_ops" [drivers/mtd/spi-nor/hisi-sfc.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/media/i2c/adv7842.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/md/bcache/bcache.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/iio/imu/inv_mpu6050/inv-mpu6050.ko] undefined!
   ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined!

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


.config.gz
Description: Binary data

Re: linux-next: manual merge of the kvms390 tree with the s390 tree

[Heiko Carstens]

On Tue, Jun 14, 2016 at 02:51:17PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the kvms390 tree got a conflict in:
> 
>   arch/s390/hypfs/hypfs_diag.c
> 
> between commit:
> 
>   6c22c9863760 ("s390: avoid extable collisions")
> 
> from the s390 tree and commit:
> 
>   e65f30e0cb29 ("s390: hypfs: Move diag implementation and data definitions")
> 
> from the kvms390 tree.
> 
> I fixed it up (using the kvms390 version and then adding the following
> patch) and can carry the fix as necessary. This is now fixed as far as
> linux-next is concerned, but any non trivial conflicts should be
> mentioned to your upstream maintainer when your tree is submitted for
> merging.  You may also want to consider cooperating with the maintainer
> of the conflicting tree to minimise any particularly complex conflicts.
> 
> From: Stephen Rothwell 
> Date: Tue, 14 Jun 2016 14:47:33 +1000
> Subject: [PATCH] s390: merge fix up for __diag204 move
> 
> Signed-off-by: Stephen Rothwell 
> ---
>  arch/s390/kernel/diag.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kernel/diag.c b/arch/s390/kernel/diag.c
> index a44faf4a0454..2289d6f8bec0 100644
> --- a/arch/s390/kernel/diag.c
> +++ b/arch/s390/kernel/diag.c
> @@ -169,7 +169,7 @@ static inline int __diag204(unsigned long subcode, 
> unsigned long size, void *add
> 
>   asm volatile(
>   "   diag%2,%0,0x204\n"
> - "0:\n"
> + "0: nopr%%r7\n"
>   EX_TABLE(0b,0b)
>   : "+d" (_subcode), "+d" (_size) : "d" (addr) : "memory");
>   if (_subcode)

The patch looks good. Thank you!

Re: linux-next: manual merge of the kvms390 tree with the s390 tree

[Heiko Carstens]

On Tue, Jun 14, 2016 at 02:51:17PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the kvms390 tree got a conflict in:
> 
>   arch/s390/hypfs/hypfs_diag.c
> 
> between commit:
> 
>   6c22c9863760 ("s390: avoid extable collisions")
> 
> from the s390 tree and commit:
> 
>   e65f30e0cb29 ("s390: hypfs: Move diag implementation and data definitions")
> 
> from the kvms390 tree.
> 
> I fixed it up (using the kvms390 version and then adding the following
> patch) and can carry the fix as necessary. This is now fixed as far as
> linux-next is concerned, but any non trivial conflicts should be
> mentioned to your upstream maintainer when your tree is submitted for
> merging.  You may also want to consider cooperating with the maintainer
> of the conflicting tree to minimise any particularly complex conflicts.
> 
> From: Stephen Rothwell 
> Date: Tue, 14 Jun 2016 14:47:33 +1000
> Subject: [PATCH] s390: merge fix up for __diag204 move
> 
> Signed-off-by: Stephen Rothwell 
> ---
>  arch/s390/kernel/diag.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kernel/diag.c b/arch/s390/kernel/diag.c
> index a44faf4a0454..2289d6f8bec0 100644
> --- a/arch/s390/kernel/diag.c
> +++ b/arch/s390/kernel/diag.c
> @@ -169,7 +169,7 @@ static inline int __diag204(unsigned long subcode, 
> unsigned long size, void *add
> 
>   asm volatile(
>   "   diag%2,%0,0x204\n"
> - "0:\n"
> + "0: nopr%%r7\n"
>   EX_TABLE(0b,0b)
>   : "+d" (_subcode), "+d" (_size) : "d" (addr) : "memory");
>   if (_subcode)

The patch looks good. Thank you!

linux-next: manual merge of the kvms390 tree with the s390 tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the kvms390 tree got a conflict in:

  arch/s390/hypfs/hypfs_diag.c

between commit:

  6c22c9863760 ("s390: avoid extable collisions")

from the s390 tree and commit:

  e65f30e0cb29 ("s390: hypfs: Move diag implementation and data definitions")

from the kvms390 tree.

I fixed it up (using the kvms390 version and then adding the following
patch) and can carry the fix as necessary. This is now fixed as far as
linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

From: Stephen Rothwell 
Date: Tue, 14 Jun 2016 14:47:33 +1000
Subject: [PATCH] s390: merge fix up for __diag204 move

Signed-off-by: Stephen Rothwell 
---
 arch/s390/kernel/diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kernel/diag.c b/arch/s390/kernel/diag.c
index a44faf4a0454..2289d6f8bec0 100644
--- a/arch/s390/kernel/diag.c
+++ b/arch/s390/kernel/diag.c
@@ -169,7 +169,7 @@ static inline int __diag204(unsigned long subcode, unsigned 
long size, void *add
 
asm volatile(
"   diag%2,%0,0x204\n"
-   "0:\n"
+   "0: nopr%%r7\n"
EX_TABLE(0b,0b)
: "+d" (_subcode), "+d" (_size) : "d" (addr) : "memory");
if (_subcode)
-- 
2.8.1

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the kvms390 tree with the s390 tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the kvms390 tree got a conflict in:

  arch/s390/hypfs/hypfs_diag.c

between commit:

  6c22c9863760 ("s390: avoid extable collisions")

from the s390 tree and commit:

  e65f30e0cb29 ("s390: hypfs: Move diag implementation and data definitions")

from the kvms390 tree.

I fixed it up (using the kvms390 version and then adding the following
patch) and can carry the fix as necessary. This is now fixed as far as
linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

From: Stephen Rothwell 
Date: Tue, 14 Jun 2016 14:47:33 +1000
Subject: [PATCH] s390: merge fix up for __diag204 move

Signed-off-by: Stephen Rothwell 
---
 arch/s390/kernel/diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kernel/diag.c b/arch/s390/kernel/diag.c
index a44faf4a0454..2289d6f8bec0 100644
--- a/arch/s390/kernel/diag.c
+++ b/arch/s390/kernel/diag.c
@@ -169,7 +169,7 @@ static inline int __diag204(unsigned long subcode, unsigned 
long size, void *add
 
asm volatile(
"   diag%2,%0,0x204\n"
-   "0:\n"
+   "0: nopr%%r7\n"
EX_TABLE(0b,0b)
: "+d" (_subcode), "+d" (_size) : "d" (addr) : "memory");
if (_subcode)
-- 
2.8.1

-- 
Cheers,
Stephen Rothwell

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

On 13 June 2016 at 21:57, Maxime Ripard
 wrote:
> On Mon, Jun 13, 2016 at 05:46:48PM -, Michal Suchanek wrote:
>> Hello,
>>
>> This is update of the sunxi spi patches that should give full-featured SPI
>> driver.
>>
>> First three patches fix issues with the current driver and can be of use for
>> stable kernels so adding cc for those.
>>
>> I merged the sun4i and sun6i driver because there several issues that need to
>> be fixed in both separately and they are even out of sync wrt some fixes.
>> I guess some of the merge patches can be squashed.
>>
>> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> tell if that side was broken by this patchset - especially the last patch 
>> that
>> adds DMA was afaik never tested on sun6i.
>
> So, you didn't run that code through checkpatch and you rewrite the
> whole thing entirely without even testing it... Awesome.

Aside from the DMA part this is not a rewrite.

And I did run the code through checkpatch. It still gives some
warnings about BUG_ON and overly long lines, sure. II don't think
those are that serious or that fixing them would improve the code.

Thanks

Michal

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

On 13 June 2016 at 21:57, Maxime Ripard
 wrote:
> On Mon, Jun 13, 2016 at 05:46:48PM -, Michal Suchanek wrote:
>> Hello,
>>
>> This is update of the sunxi spi patches that should give full-featured SPI
>> driver.
>>
>> First three patches fix issues with the current driver and can be of use for
>> stable kernels so adding cc for those.
>>
>> I merged the sun4i and sun6i driver because there several issues that need to
>> be fixed in both separately and they are even out of sync wrt some fixes.
>> I guess some of the merge patches can be squashed.
>>
>> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> tell if that side was broken by this patchset - especially the last patch 
>> that
>> adds DMA was afaik never tested on sun6i.
>
> So, you didn't run that code through checkpatch and you rewrite the
> whole thing entirely without even testing it... Awesome.

Aside from the DMA part this is not a rewrite.

And I did run the code through checkpatch. It still gives some
warnings about BUG_ON and overly long lines, sure. II don't think
those are that serious or that fixing them would improve the code.

Thanks

Michal

Re: [PATCH 1/2] dmaengine: bcm2835: Fix cyclic DMA period splitting

[Eric Anholt]

Matthias Reichl  writes:

> The code responsible for splitting periods into chunks that
> can be handled by the DMA controller missed to update total_len,
> the number of bytes processed in the current period, when there
> are more chunks to follow.
>
> Therefore total_len was stuck at 0 and the code didn't work at all.
> This resulted in a wrong control block layout and audio issues because
> the cyclic DMA callback wasn't executing on period boundaries.
>
> Fix this by adding the missing total_len update.

It looks like this issue has been around for a long time, and this fix
is pretty dependent on the recent refactors.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature

Re: [PATCH 1/2] dmaengine: bcm2835: Fix cyclic DMA period splitting

[Eric Anholt]

Matthias Reichl  writes:

> The code responsible for splitting periods into chunks that
> can be handled by the DMA controller missed to update total_len,
> the number of bytes processed in the current period, when there
> are more chunks to follow.
>
> Therefore total_len was stuck at 0 and the code didn't work at all.
> This resulted in a wrong control block layout and audio issues because
> the cyclic DMA callback wasn't executing on period boundaries.
>
> Fix this by adding the missing total_len update.

It looks like this issue has been around for a long time, and this fix
is pretty dependent on the recent refactors.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature

Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:40 PM, Michal Suchanek  wrote:
> On 14 June 2016 at 01:45, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>>> Signed-off-by: Michal Suchanek 
>>> ---
>>>  .../devicetree/bindings/spi/spi-sun4i.txt  | 21 ++-
>>>  .../devicetree/bindings/spi/spi-sun6i.txt  | 24 
>>> --
>>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt 
>>> b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> index de827f5..329e543 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> @@ -1,7 +1,8 @@
>>> -Allwinner A10 SPI controller
>>> +Allwinner A10/A31 SPI controller
>>>
>>>  Required properties:
>>> -- compatible: Should be "allwinner,sun4-a10-spi".
>>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>>> +   "allwinner,sun6i-a31-spi"
>>>  - reg: Should contain register location and length.
>>>  - interrupts: Should contain interrupt.
>>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>>> @@ -9,16 +10,16 @@ Required properties:
>>>- "ahb": the gated AHB parent clock
>>>- "mod": the parent module clock
>>>  - clock-names: Must contain the clock names described just above
>>> +- resets: (sun6i only) phandle to the reset controller asserting
>>> + this device in reset
>>>
>>>  Example:
>>>
>>> -spi1: spi@01c06000 {
>>> -   compatible = "allwinner,sun4i-a10-spi";
>>> -   reg = <0x01c06000 0x1000>;
>>> -   interrupts = <11>;
>>> -   clocks = <_gates 21>, <_clk>;
>>> +spi1: spi@01c69000 {
>>> +   compatible = "allwinner,sun6i-a31-spi";
>>> +   reg = <0x01c69000 0x1000>;
>>> +   interrupts = <0 66 4>;
>>> +   clocks = <_gates 21>, <_clk>;
>>> clock-names = "ahb", "mod";
>>> -   status = "disabled";
>>> -   #address-cells = <1>;
>>> -   #size-cells = <0>;
>>> +   resets = <_rst 21>;
>>
>> Why not have an example of each type?
>
> How many binding docs have examples of all types?

I'm pretty sure that there's a few. This was only a suggestion, so if
it's not to your taste, ignore it.

> There are actual DTs using these so you can look at those as well.
>
> This driver covers 3 types of bindings which look different in the DT:
>
> sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
> and sun6i IP with GIC.

Fair point.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:40 PM, Michal Suchanek  wrote:
> On 14 June 2016 at 01:45, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>>> Signed-off-by: Michal Suchanek 
>>> ---
>>>  .../devicetree/bindings/spi/spi-sun4i.txt  | 21 ++-
>>>  .../devicetree/bindings/spi/spi-sun6i.txt  | 24 
>>> --
>>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt 
>>> b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> index de827f5..329e543 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> @@ -1,7 +1,8 @@
>>> -Allwinner A10 SPI controller
>>> +Allwinner A10/A31 SPI controller
>>>
>>>  Required properties:
>>> -- compatible: Should be "allwinner,sun4-a10-spi".
>>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>>> +   "allwinner,sun6i-a31-spi"
>>>  - reg: Should contain register location and length.
>>>  - interrupts: Should contain interrupt.
>>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>>> @@ -9,16 +10,16 @@ Required properties:
>>>- "ahb": the gated AHB parent clock
>>>- "mod": the parent module clock
>>>  - clock-names: Must contain the clock names described just above
>>> +- resets: (sun6i only) phandle to the reset controller asserting
>>> + this device in reset
>>>
>>>  Example:
>>>
>>> -spi1: spi@01c06000 {
>>> -   compatible = "allwinner,sun4i-a10-spi";
>>> -   reg = <0x01c06000 0x1000>;
>>> -   interrupts = <11>;
>>> -   clocks = <_gates 21>, <_clk>;
>>> +spi1: spi@01c69000 {
>>> +   compatible = "allwinner,sun6i-a31-spi";
>>> +   reg = <0x01c69000 0x1000>;
>>> +   interrupts = <0 66 4>;
>>> +   clocks = <_gates 21>, <_clk>;
>>> clock-names = "ahb", "mod";
>>> -   status = "disabled";
>>> -   #address-cells = <1>;
>>> -   #size-cells = <0>;
>>> +   resets = <_rst 21>;
>>
>> Why not have an example of each type?
>
> How many binding docs have examples of all types?

I'm pretty sure that there's a few. This was only a suggestion, so if
it's not to your taste, ignore it.

> There are actual DTs using these so you can look at those as well.
>
> This driver covers 3 types of bindings which look different in the DT:
>
> sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
> and sun6i IP with GIC.

Fair point.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
> Hello,
>
> On 14 June 2016 at 01:43, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>>> The drivers are very similar and share multiple flaws which needed
>>> separate fixes for both drivers.
>>>
>>> Signed-off-by: Michal Suchanek 
>>> ---
>>>  drivers/spi/Kconfig |   8 +-
>>>  drivers/spi/Makefile|   1 -
>>>  drivers/spi/spi-sun4i.c | 156 +++--
>>>  drivers/spi/spi-sun6i.c | 598 
>>> 
>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>
>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>> index 0b8e6c6..c76f8e4 100644
>>> --- a/drivers/spi/spi-sun4i.c
>>> +++ b/drivers/spi/spi-sun4i.c
>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
>>> *master,
>>> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>
>>> /* Reset FIFOs */
>>> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +   if (sspi->type == SPI_SUN4I)
>>> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +   else
>>> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>
>> If we're already doing different stuff for each generation of the IP,
>> why not just use the register offsets and bit definitions directly?
>
> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
> makes my eyes bleed and you cannot use the check that you are
> accessing a register that actually exists.

I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
indirection you added and using them directly, i.e.

#define SUN4I_TFR_CTL_RF_RST BIT(x)
#define SUN4I_TFR_CTL_TF_RST BIT(x)
#define SUN6I_FIFO_CTL_RF_RST BIT(x)
#define SUN6I_FIFO_CTL_TF_RST BIT(x)

then

if (sspi->type == SPI_SUN4I)
sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
else
sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);

I.e. the bits that need setting are in different registers, so you
have to have an if statement and separate calls. Therefore there's no
real benefit from the indirection you've introduced here, unless
you're expecting the SUN8I variant to use different bits in one of
those two registers.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek  wrote:
> Hello,
>
> On 14 June 2016 at 01:43, Julian Calaby  wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>>> The drivers are very similar and share multiple flaws which needed
>>> separate fixes for both drivers.
>>>
>>> Signed-off-by: Michal Suchanek 
>>> ---
>>>  drivers/spi/Kconfig |   8 +-
>>>  drivers/spi/Makefile|   1 -
>>>  drivers/spi/spi-sun4i.c | 156 +++--
>>>  drivers/spi/spi-sun6i.c | 598 
>>> 
>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>
>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>> index 0b8e6c6..c76f8e4 100644
>>> --- a/drivers/spi/spi-sun4i.c
>>> +++ b/drivers/spi/spi-sun4i.c
>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
>>> *master,
>>> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>
>>> /* Reset FIFOs */
>>> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +   if (sspi->type == SPI_SUN4I)
>>> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +   else
>>> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>
>> If we're already doing different stuff for each generation of the IP,
>> why not just use the register offsets and bit definitions directly?
>
> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
> makes my eyes bleed and you cannot use the check that you are
> accessing a register that actually exists.

I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
indirection you added and using them directly, i.e.

#define SUN4I_TFR_CTL_RF_RST BIT(x)
#define SUN4I_TFR_CTL_TF_RST BIT(x)
#define SUN6I_FIFO_CTL_RF_RST BIT(x)
#define SUN6I_FIFO_CTL_TF_RST BIT(x)

then

if (sspi->type == SPI_SUN4I)
sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
else
sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);

I.e. the bits that need setting are in different registers, so you
have to have an if statement and separate calls. Therefore there's no
real benefit from the indirection you've introduced here, unless
you're expecting the SUN8I variant to use different bits in one of
those two registers.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:31, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> SUNXI_CTL_ -> SUNXI_TFR_CTL_
>> SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS
>
> I don't know these abbreviations, are they both referring to the same thing?
>
>> SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL
>
> It looks like you're making the constant name less descriptive here.
> Is the old version (CS_ACTIVE_LOW) incorrect?
>
>> and some SUNXI_???_CTL_ -> SUNXI_CTL_
>> for constants migrated to different registers between sun4i and sun6i
>>
>> No functional change.
>>
>>  #define SUNXI_INT_CTL_REG  0x0c
>> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
>> index a27bf8f..f26b52a 100644
>> --- a/drivers/spi/spi-sun6i.c
>> +++ b/drivers/spi/spi-sun6i.c
>> @@ -26,9 +26,9 @@
>>  #define SUNXI_FIFO_DEPTH   128
>>
>>  #define SUNXI_GBL_CTL_REG  0x04
>> -#define SUNXI_GBL_CTL_BUS_ENABLE   BIT(0)
>> -#define SUNXI_GBL_CTL_MASTER   BIT(1)
>> -#define SUNXI_GBL_CTL_TP   BIT(7)
>> +#define SUNXI_CTL_ENABLE   BIT(0)
>> +#define SUNXI_CTL_MASTER   BIT(1)
>> +#define SUNXI_CTL_TP   BIT(7)
>
> If these are bit definitions for the GBL register, why throw that
> information away?

Those bits are on the TFR register in the earlier IP so it makes
perfect sense to me this way.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:31, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> SUNXI_CTL_ -> SUNXI_TFR_CTL_
>> SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS
>
> I don't know these abbreviations, are they both referring to the same thing?
>
>> SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL
>
> It looks like you're making the constant name less descriptive here.
> Is the old version (CS_ACTIVE_LOW) incorrect?
>
>> and some SUNXI_???_CTL_ -> SUNXI_CTL_
>> for constants migrated to different registers between sun4i and sun6i
>>
>> No functional change.
>>
>>  #define SUNXI_INT_CTL_REG  0x0c
>> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
>> index a27bf8f..f26b52a 100644
>> --- a/drivers/spi/spi-sun6i.c
>> +++ b/drivers/spi/spi-sun6i.c
>> @@ -26,9 +26,9 @@
>>  #define SUNXI_FIFO_DEPTH   128
>>
>>  #define SUNXI_GBL_CTL_REG  0x04
>> -#define SUNXI_GBL_CTL_BUS_ENABLE   BIT(0)
>> -#define SUNXI_GBL_CTL_MASTER   BIT(1)
>> -#define SUNXI_GBL_CTL_TP   BIT(7)
>> +#define SUNXI_CTL_ENABLE   BIT(0)
>> +#define SUNXI_CTL_MASTER   BIT(1)
>> +#define SUNXI_CTL_TP   BIT(7)
>
> If these are bit definitions for the GBL register, why throw that
> information away?

Those bits are on the TFR register in the earlier IP so it makes
perfect sense to me this way.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Michal Suchanek]

On 14 June 2016 at 01:45, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> Signed-off-by: Michal Suchanek 
>> ---
>>  .../devicetree/bindings/spi/spi-sun4i.txt  | 21 ++-
>>  .../devicetree/bindings/spi/spi-sun6i.txt  | 24 
>> --
>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt 
>> b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> index de827f5..329e543 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> @@ -1,7 +1,8 @@
>> -Allwinner A10 SPI controller
>> +Allwinner A10/A31 SPI controller
>>
>>  Required properties:
>> -- compatible: Should be "allwinner,sun4-a10-spi".
>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>> +   "allwinner,sun6i-a31-spi"
>>  - reg: Should contain register location and length.
>>  - interrupts: Should contain interrupt.
>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>> @@ -9,16 +10,16 @@ Required properties:
>>- "ahb": the gated AHB parent clock
>>- "mod": the parent module clock
>>  - clock-names: Must contain the clock names described just above
>> +- resets: (sun6i only) phandle to the reset controller asserting
>> + this device in reset
>>
>>  Example:
>>
>> -spi1: spi@01c06000 {
>> -   compatible = "allwinner,sun4i-a10-spi";
>> -   reg = <0x01c06000 0x1000>;
>> -   interrupts = <11>;
>> -   clocks = <_gates 21>, <_clk>;
>> +spi1: spi@01c69000 {
>> +   compatible = "allwinner,sun6i-a31-spi";
>> +   reg = <0x01c69000 0x1000>;
>> +   interrupts = <0 66 4>;
>> +   clocks = <_gates 21>, <_clk>;
>> clock-names = "ahb", "mod";
>> -   status = "disabled";
>> -   #address-cells = <1>;
>> -   #size-cells = <0>;
>> +   resets = <_rst 21>;
>
> Why not have an example of each type?

How many binding docs have examples of all types?

There are actual DTs using these so you can look at those as well.

This driver covers 3 types of bindings which look different in the DT:

sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
and sun6i IP with GIC.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Michal Suchanek]

On 14 June 2016 at 01:45, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> Signed-off-by: Michal Suchanek 
>> ---
>>  .../devicetree/bindings/spi/spi-sun4i.txt  | 21 ++-
>>  .../devicetree/bindings/spi/spi-sun6i.txt  | 24 
>> --
>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt 
>> b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> index de827f5..329e543 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> @@ -1,7 +1,8 @@
>> -Allwinner A10 SPI controller
>> +Allwinner A10/A31 SPI controller
>>
>>  Required properties:
>> -- compatible: Should be "allwinner,sun4-a10-spi".
>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>> +   "allwinner,sun6i-a31-spi"
>>  - reg: Should contain register location and length.
>>  - interrupts: Should contain interrupt.
>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>> @@ -9,16 +10,16 @@ Required properties:
>>- "ahb": the gated AHB parent clock
>>- "mod": the parent module clock
>>  - clock-names: Must contain the clock names described just above
>> +- resets: (sun6i only) phandle to the reset controller asserting
>> + this device in reset
>>
>>  Example:
>>
>> -spi1: spi@01c06000 {
>> -   compatible = "allwinner,sun4i-a10-spi";
>> -   reg = <0x01c06000 0x1000>;
>> -   interrupts = <11>;
>> -   clocks = <_gates 21>, <_clk>;
>> +spi1: spi@01c69000 {
>> +   compatible = "allwinner,sun6i-a31-spi";
>> +   reg = <0x01c69000 0x1000>;
>> +   interrupts = <0 66 4>;
>> +   clocks = <_gates 21>, <_clk>;
>> clock-names = "ahb", "mod";
>> -   status = "disabled";
>> -   #address-cells = <1>;
>> -   #size-cells = <0>;
>> +   resets = <_rst 21>;
>
> Why not have an example of each type?

How many binding docs have examples of all types?

There are actual DTs using these so you can look at those as well.

This driver covers 3 types of bindings which look different in the DT:

sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
and sun6i IP with GIC.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:43, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> The drivers are very similar and share multiple flaws which needed
>> separate fixes for both drivers.
>>
>> Signed-off-by: Michal Suchanek 
>> ---
>>  drivers/spi/Kconfig |   8 +-
>>  drivers/spi/Makefile|   1 -
>>  drivers/spi/spi-sun4i.c | 156 +++--
>>  drivers/spi/spi-sun6i.c | 598 
>> 
>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>
>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>> index 0b8e6c6..c76f8e4 100644
>> --- a/drivers/spi/spi-sun4i.c
>> +++ b/drivers/spi/spi-sun4i.c
>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
>> *master,
>> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>
>> /* Reset FIFOs */
>> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +   if (sspi->type == SPI_SUN4I)
>> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +   else
>> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>
> If we're already doing different stuff for each generation of the IP,
> why not just use the register offsets and bit definitions directly?

Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
makes my eyes bleed and you cannot use the check that you are
accessing a register that actually exists.

>> @@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device 
>> *pdev)
>> }
>>
>> sspi->master = master;
>> -   sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> -   sspi->type = SPI_SUN4I;
>> -   sspi->regmap = _regmap;
>> -   sspi->bitmap = _bitmap;
>> +   if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
>> +   sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> +   sspi->type = SPI_SUN4I;
>> +   sspi->regmap = _regmap;
>> +   sspi->bitmap = _bitmap;
>> +   } else if (of_device_is_compatible(pdev->dev.of_node,
>> +  SUN6I_COMPATIBLE)) {
>> +   sspi->fifo_depth = SUN6I_FIFO_DEPTH;
>> +   sspi->type = SPI_SUN6I;
>> +   sspi->regmap = _regmap;
>> +   sspi->bitmap = _bitmap;
>
> Can you store data in the match table instead of doing this?

That might be nicer. Will look into this.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:43, Julian Calaby  wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek  wrote:
>> The drivers are very similar and share multiple flaws which needed
>> separate fixes for both drivers.
>>
>> Signed-off-by: Michal Suchanek 
>> ---
>>  drivers/spi/Kconfig |   8 +-
>>  drivers/spi/Makefile|   1 -
>>  drivers/spi/spi-sun4i.c | 156 +++--
>>  drivers/spi/spi-sun6i.c | 598 
>> 
>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>
>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>> index 0b8e6c6..c76f8e4 100644
>> --- a/drivers/spi/spi-sun4i.c
>> +++ b/drivers/spi/spi-sun4i.c
>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master 
>> *master,
>> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>
>> /* Reset FIFOs */
>> -   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> -   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> -   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +   if (sspi->type == SPI_SUN4I)
>> +   sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> +   reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +   else
>> +   sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>> +   sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +   sspi_bits(sspi, SUNXI_CTL_TF_RST));
>
> If we're already doing different stuff for each generation of the IP,
> why not just use the register offsets and bit definitions directly?

Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
makes my eyes bleed and you cannot use the check that you are
accessing a register that actually exists.

>> @@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device 
>> *pdev)
>> }
>>
>> sspi->master = master;
>> -   sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> -   sspi->type = SPI_SUN4I;
>> -   sspi->regmap = _regmap;
>> -   sspi->bitmap = _bitmap;
>> +   if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
>> +   sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> +   sspi->type = SPI_SUN4I;
>> +   sspi->regmap = _regmap;
>> +   sspi->bitmap = _bitmap;
>> +   } else if (of_device_is_compatible(pdev->dev.of_node,
>> +  SUN6I_COMPATIBLE)) {
>> +   sspi->fifo_depth = SUN6I_FIFO_DEPTH;
>> +   sspi->type = SPI_SUN6I;
>> +   sspi->regmap = _regmap;
>> +   sspi->bitmap = _bitmap;
>
> Can you store data in the match table instead of doing this?

That might be nicer. Will look into this.

Thanks

Michal

Re: [PATCH] crypto : async implementation for sha1-mb

[Herbert Xu]

On Mon, Jun 13, 2016 at 07:10:26PM +, Dey, Megha wrote:
>
> > In the current implementation, the inner algorithm is called directly, and 
> > we use the outer algorithm's callback. We do not use the callback in inner 
> > algorithm. We are actually calling the child functions directly and the 
> > child is using the parent's call back function. Probably I can add a 
> > comment before the set callback function.. will this be ok?

No this is a hack and you should not do that.

You can of course set the inner request's callback to that of the outer request.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto : async implementation for sha1-mb

[Herbert Xu]

On Mon, Jun 13, 2016 at 07:10:26PM +, Dey, Megha wrote:
>
> > In the current implementation, the inner algorithm is called directly, and 
> > we use the outer algorithm's callback. We do not use the callback in inner 
> > algorithm. We are actually calling the child functions directly and the 
> > child is using the parent's call back function. Probably I can add a 
> > comment before the set callback function.. will this be ok?

No this is a hack and you should not do that.

You can of course set the inner request's callback to that of the outer request.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: duplicate patches in the kspp and kbuild trees

[Stephen Rothwell]

Hi Kees,

On Mon, 13 Jun 2016 16:57:15 -0700 Kees Cook  wrote:
>
> On Mon, Jun 13, 2016 at 4:53 PM, Kees Cook  wrote:
> >
> > Strange, I pulled these directly from linux-next. Michal had an
> > auto-responder saying he was going to be out-of-office, so I wanted to
> > make sure the !COMPILE_TEST fix got in.
> >
> > Sounds like I should merge the kbuild tree, rather than cherry-picking
> > from linux-next? I will adjust.  

Cherry-picking produces new commits (with new SHA1s etc), while merging
(or rebasing on top of the other versions) will have the same commits
(not just patches).

Having the same commits means that they never produce conflicts after
further changes to the same files (unless both sides of the merge make
further changes to the same files).

> I've done this merge correctly now and pushed a forced update on the kspp 
> tree.

Thanks for that.  Now you just have to hope that Michal never rebases
that part of his tree from under you.  (Michal: hint! :-))

-- 
Cheers,
Stephen Rothwell

Re: linux-next: duplicate patches in the kspp and kbuild trees

[Stephen Rothwell]

Hi Kees,

On Mon, 13 Jun 2016 16:57:15 -0700 Kees Cook  wrote:
>
> On Mon, Jun 13, 2016 at 4:53 PM, Kees Cook  wrote:
> >
> > Strange, I pulled these directly from linux-next. Michal had an
> > auto-responder saying he was going to be out-of-office, so I wanted to
> > make sure the !COMPILE_TEST fix got in.
> >
> > Sounds like I should merge the kbuild tree, rather than cherry-picking
> > from linux-next? I will adjust.  

Cherry-picking produces new commits (with new SHA1s etc), while merging
(or rebasing on top of the other versions) will have the same commits
(not just patches).

Having the same commits means that they never produce conflicts after
further changes to the same files (unless both sides of the merge make
further changes to the same files).

> I've done this merge correctly now and pushed a forced update on the kspp 
> tree.

Thanks for that.  Now you just have to hope that Michal never rebases
that part of his tree from under you.  (Michal: hint! :-))

-- 
Cheers,
Stephen Rothwell

Re: Internal error xfs_trans_cancel

[Josh Poimboeuf]

On Wed, Jun 01, 2016 at 07:52:31AM +0200, Daniel Wagner wrote:
> Hi,
> 
> I got the error message below while compiling a kernel 
> on that system. I can't really say if I did something
> which made the file system unhappy before the crash.
> 
> 
> [Jun 1 07:41] XFS (sde1): Internal error xfs_trans_cancel at line 984 of file 
> fs/xfs/xfs_trans.c.  Caller xfs_rename+0x453/0x960 [xfs]
> [  +0.95] CPU: 22 PID: 8640 Comm: gcc Not tainted 4.7.0-rc1 #16
> [  +0.35] Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 
> 01/16/2014
> [  +0.48]  0286 c8be6bc3 885fa9473cb0 
> 813d146e
> [  +0.56]  885fa9ac5ed0 0001 885fa9473cc8 
> a0213cdc
> [  +0.53]  a02257b3 885fa9473cf0 a022eb36 
> 883faa502d00
> [  +0.53] Call Trace:
> [  +0.28]  [] dump_stack+0x63/0x85
> [  +0.69]  [] xfs_error_report+0x3c/0x40 [xfs]
> [  +0.65]  [] ? xfs_rename+0x453/0x960 [xfs]
> [  +0.64]  [] xfs_trans_cancel+0xb6/0xe0 [xfs]
> [  +0.65]  [] xfs_rename+0x453/0x960 [xfs]
> [  +0.62]  [] xfs_vn_rename+0xb3/0xf0 [xfs]
> [  +0.40]  [] vfs_rename+0x58c/0x8d0
> [  +0.32]  [] SyS_rename+0x371/0x390
> [  +0.36]  [] entry_SYSCALL_64_fastpath+0x1a/0xa4
> [  +0.40] XFS (sde1): xfs_do_force_shutdown(0x8) called from line 985 of 
> file fs/xfs/xfs_trans.c.  Return address = 0xa022eb4f
> [  +0.027680] XFS (sde1): Corruption of in-memory data detected.  Shutting 
> down filesystem
> [  +0.57] XFS (sde1): Please umount the filesystem and rectify the 
> problem(s)
> [Jun 1 07:42] XFS (sde1): xfs_log_force: error -5 returned.
> [ +30.081016] XFS (sde1): xfs_log_force: error -5 returned.

I saw this today.  I was just building/installing kernels, rebooting,
running kexec, running perf.


[ 1359.005573] [ cut here ]
[ 1359.010191] WARNING: CPU: 4 PID: 6031 at fs/inode.c:280 drop_nlink+0x3e/0x50
[ 1359.017231] Modules linked in: rpcrdma ib_isert iscsi_target_mod ib_iser 
libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp 
ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_powerclamp 
coretemp kvm_intel kvm nfsd ipmi_ssif ipmi_devintf ipmi_si iTCO_wdt irqbypass 
iTCO_vendor_support ipmi_msghandler i7core_edac shpchp sg edac_core pcspkr wmi 
lpc_ich dcdbas mfd_core acpi_power_meter auth_rpcgss acpi_cpufreq nfs_acl lockd 
grace sunrpc ip_tables xfs libcrc32c sd_mod sr_mod cdrom iw_cxgb3 ib_core 
mgag200 ata_generic pata_acpi i2c_algo_bit drm_kms_helper syscopyarea 
sysfillrect sysimgblt fb_sys_fops ttm drm mptsas scsi_transport_sas ata_piix 
mptscsih libata cxgb3 crc32c_intel i2c_core serio_raw mptbase bnx2 fjes mdio 
dm_mirror dm_region_hash dm_log dm_mod
[ 1359.088447] CPU: 4 PID: 6031 Comm: depmod Tainted: G  I 
4.7.0-rc3+ #4
[ 1359.095911] Hardware name: Dell Inc. PowerEdge R410/0N051F, BIOS 1.11.0 
07/20/2012
[ 1359.103461]  0286 a0bc39d9 8802143dfd18 
8134bb7f
[ 1359.110871]    8802143dfd58 
8108b671
[ 1359.118280]  0118575f7d13 880222c9a6e8 8803ec3874d8 
880428827000
[ 1359.125693] Call Trace:
[ 1359.128133]  [] dump_stack+0x63/0x84
[ 1359.133259]  [] __warn+0xd1/0xf0
[ 1359.138037]  [] warn_slowpath_null+0x1d/0x20
[ 1359.143855]  [] drop_nlink+0x3e/0x50
[ 1359.149017]  [] xfs_droplink+0x28/0x60 [xfs]
[ 1359.154864]  [] xfs_remove+0x231/0x350 [xfs]
[ 1359.160682]  [] ? security_inode_permission+0x3a/0x60
[ 1359.167309]  [] xfs_vn_unlink+0x58/0xa0 [xfs]
[ 1359.173213]  [] ? selinux_inode_unlink+0x13/0x20
[ 1359.179379]  [] vfs_unlink+0xda/0x190
[ 1359.184590]  [] do_unlinkat+0x263/0x2a0
[ 1359.189974]  [] SyS_unlinkat+0x1b/0x30
[ 1359.195272]  [] do_syscall_64+0x62/0x110
[ 1359.200743]  [] entry_SYSCALL64_slow_path+0x25/0x25
[ 1359.207178] ---[ end trace 0d397afdaff9f340 ]---
[ 1359.211830] XFS (dm-0): Internal error xfs_trans_cancel at line 984 of file 
fs/xfs/xfs_trans.c.  Caller xfs_remove+0x1d1/0x350 [xfs]
[ 1359.223723] CPU: 4 PID: 6031 Comm: depmod Tainted: GW I 
4.7.0-rc3+ #4
[ 1359.231185] Hardware name: Dell Inc. PowerEdge R410/0N051F, BIOS 1.11.0 
07/20/2012
[ 1359.238736]  0286 a0bc39d9 8802143dfd60 
8134bb7f
[ 1359.246147]  8803ec3874d8 0001 8802143dfd78 
a03176bb
[ 1359.253559]  a0328c21 8802143dfda0 a03327a6 
880222e7e180
[ 1359.260969] Call Trace:
[ 1359.263407]  [] dump_stack+0x63/0x84
[ 1359.268560]  [] xfs_error_report+0x3b/0x40 [xfs]
[ 1359.274755]  [] ? xfs_remove+0x1d1/0x350 [xfs]
[ 1359.280778]  [] xfs_trans_cancel+0xb6/0xe0 [xfs]
[ 1359.286973]  [] xfs_remove+0x1d1/0x350 [xfs]
[ 1359.292820]  [] xfs_vn_unlink+0x58/0xa0 [xfs]
[ 1359.298724]  [] ? selinux_inode_unlink+0x13/0x20
[ 1359.304890]  [] vfs_unlink+0xda/0x190
[ 1359.310100]  [] do_unlinkat+0x263/0x2a0
[ 1359.315486]  [] SyS_unlinkat+0x1b/0x30
[ 

Re: Internal error xfs_trans_cancel

[Josh Poimboeuf]

On Wed, Jun 01, 2016 at 07:52:31AM +0200, Daniel Wagner wrote:
> Hi,
> 
> I got the error message below while compiling a kernel 
> on that system. I can't really say if I did something
> which made the file system unhappy before the crash.
> 
> 
> [Jun 1 07:41] XFS (sde1): Internal error xfs_trans_cancel at line 984 of file 
> fs/xfs/xfs_trans.c.  Caller xfs_rename+0x453/0x960 [xfs]
> [  +0.95] CPU: 22 PID: 8640 Comm: gcc Not tainted 4.7.0-rc1 #16
> [  +0.35] Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 
> 01/16/2014
> [  +0.48]  0286 c8be6bc3 885fa9473cb0 
> 813d146e
> [  +0.56]  885fa9ac5ed0 0001 885fa9473cc8 
> a0213cdc
> [  +0.53]  a02257b3 885fa9473cf0 a022eb36 
> 883faa502d00
> [  +0.53] Call Trace:
> [  +0.28]  [] dump_stack+0x63/0x85
> [  +0.69]  [] xfs_error_report+0x3c/0x40 [xfs]
> [  +0.65]  [] ? xfs_rename+0x453/0x960 [xfs]
> [  +0.64]  [] xfs_trans_cancel+0xb6/0xe0 [xfs]
> [  +0.65]  [] xfs_rename+0x453/0x960 [xfs]
> [  +0.62]  [] xfs_vn_rename+0xb3/0xf0 [xfs]
> [  +0.40]  [] vfs_rename+0x58c/0x8d0
> [  +0.32]  [] SyS_rename+0x371/0x390
> [  +0.36]  [] entry_SYSCALL_64_fastpath+0x1a/0xa4
> [  +0.40] XFS (sde1): xfs_do_force_shutdown(0x8) called from line 985 of 
> file fs/xfs/xfs_trans.c.  Return address = 0xa022eb4f
> [  +0.027680] XFS (sde1): Corruption of in-memory data detected.  Shutting 
> down filesystem
> [  +0.57] XFS (sde1): Please umount the filesystem and rectify the 
> problem(s)
> [Jun 1 07:42] XFS (sde1): xfs_log_force: error -5 returned.
> [ +30.081016] XFS (sde1): xfs_log_force: error -5 returned.

I saw this today.  I was just building/installing kernels, rebooting,
running kexec, running perf.


[ 1359.005573] [ cut here ]
[ 1359.010191] WARNING: CPU: 4 PID: 6031 at fs/inode.c:280 drop_nlink+0x3e/0x50
[ 1359.017231] Modules linked in: rpcrdma ib_isert iscsi_target_mod ib_iser 
libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp 
ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_powerclamp 
coretemp kvm_intel kvm nfsd ipmi_ssif ipmi_devintf ipmi_si iTCO_wdt irqbypass 
iTCO_vendor_support ipmi_msghandler i7core_edac shpchp sg edac_core pcspkr wmi 
lpc_ich dcdbas mfd_core acpi_power_meter auth_rpcgss acpi_cpufreq nfs_acl lockd 
grace sunrpc ip_tables xfs libcrc32c sd_mod sr_mod cdrom iw_cxgb3 ib_core 
mgag200 ata_generic pata_acpi i2c_algo_bit drm_kms_helper syscopyarea 
sysfillrect sysimgblt fb_sys_fops ttm drm mptsas scsi_transport_sas ata_piix 
mptscsih libata cxgb3 crc32c_intel i2c_core serio_raw mptbase bnx2 fjes mdio 
dm_mirror dm_region_hash dm_log dm_mod
[ 1359.088447] CPU: 4 PID: 6031 Comm: depmod Tainted: G  I 
4.7.0-rc3+ #4
[ 1359.095911] Hardware name: Dell Inc. PowerEdge R410/0N051F, BIOS 1.11.0 
07/20/2012
[ 1359.103461]  0286 a0bc39d9 8802143dfd18 
8134bb7f
[ 1359.110871]    8802143dfd58 
8108b671
[ 1359.118280]  0118575f7d13 880222c9a6e8 8803ec3874d8 
880428827000
[ 1359.125693] Call Trace:
[ 1359.128133]  [] dump_stack+0x63/0x84
[ 1359.133259]  [] __warn+0xd1/0xf0
[ 1359.138037]  [] warn_slowpath_null+0x1d/0x20
[ 1359.143855]  [] drop_nlink+0x3e/0x50
[ 1359.149017]  [] xfs_droplink+0x28/0x60 [xfs]
[ 1359.154864]  [] xfs_remove+0x231/0x350 [xfs]
[ 1359.160682]  [] ? security_inode_permission+0x3a/0x60
[ 1359.167309]  [] xfs_vn_unlink+0x58/0xa0 [xfs]
[ 1359.173213]  [] ? selinux_inode_unlink+0x13/0x20
[ 1359.179379]  [] vfs_unlink+0xda/0x190
[ 1359.184590]  [] do_unlinkat+0x263/0x2a0
[ 1359.189974]  [] SyS_unlinkat+0x1b/0x30
[ 1359.195272]  [] do_syscall_64+0x62/0x110
[ 1359.200743]  [] entry_SYSCALL64_slow_path+0x25/0x25
[ 1359.207178] ---[ end trace 0d397afdaff9f340 ]---
[ 1359.211830] XFS (dm-0): Internal error xfs_trans_cancel at line 984 of file 
fs/xfs/xfs_trans.c.  Caller xfs_remove+0x1d1/0x350 [xfs]
[ 1359.223723] CPU: 4 PID: 6031 Comm: depmod Tainted: GW I 
4.7.0-rc3+ #4
[ 1359.231185] Hardware name: Dell Inc. PowerEdge R410/0N051F, BIOS 1.11.0 
07/20/2012
[ 1359.238736]  0286 a0bc39d9 8802143dfd60 
8134bb7f
[ 1359.246147]  8803ec3874d8 0001 8802143dfd78 
a03176bb
[ 1359.253559]  a0328c21 8802143dfda0 a03327a6 
880222e7e180
[ 1359.260969] Call Trace:
[ 1359.263407]  [] dump_stack+0x63/0x84
[ 1359.268560]  [] xfs_error_report+0x3b/0x40 [xfs]
[ 1359.274755]  [] ? xfs_remove+0x1d1/0x350 [xfs]
[ 1359.280778]  [] xfs_trans_cancel+0xb6/0xe0 [xfs]
[ 1359.286973]  [] xfs_remove+0x1d1/0x350 [xfs]
[ 1359.292820]  [] xfs_vn_unlink+0x58/0xa0 [xfs]
[ 1359.298724]  [] ? selinux_inode_unlink+0x13/0x20
[ 1359.304890]  [] vfs_unlink+0xda/0x190
[ 1359.310100]  [] do_unlinkat+0x263/0x2a0
[ 1359.315486]  [] SyS_unlinkat+0x1b/0x30
[ 

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Huang, Tao]

Hi Daniel:
On 2016年06月13日 21:06, Daniel Lezcano wrote:
> On Tue, Jun 07, 2016 at 12:54:29PM +0800, Caesar Wang wrote:
>> This series patches had been tested on rockchip inside kernel.
>> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
>> save power in idle.
> 
> For my personnal information, are the arch_timer in the same power domain 
> than the CPU ? IOW, what is the 'always-on' property in the DT ?

Yes. In our SoC design, all arch (generic) timer in the same power
domain of CPU core. So if one CPU core power down, the arch (generic)
timer will lose it's state and stop working.
While rk timer maybe in peri power domain or pmu power domain, so the
timer will still work when CPU power down.

But before RK3399, all SoCs with CPU power domain, do not support auto
power down while cpu idle. So the arch timer can be seem as always on,
i.e. we don't need a broadcast timer at all.

> 
>> Okay, it still works bootup on rk3288/other SoCs, even though many socs 
>> hasn't used
>> the broadcast timer.
> 
> Yes, unfortunately the SoC design on rk3288 and the previous ones do not 
> allow to use a cpuidle driver with cpu/cluster power down, so obviously the 
> broadcast timer is pointless on these boards :)
> 

You are right.

>> History version:
>> v1:
>> https://lkml.org/lkml/2016/5/25/186
>>
>> Easy to test for my borad.
>> localhost / # cat /proc/interrupts
>> CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
>> 1:  0  0  0  0  0  0 
>> GICv3  29 Edge  arch_timer
>> ...
>> 5:  0  0  0  0  0  0 
>> GICv3 113 Level rk_timer
>> ..
>>
>> localhost / # cat /proc/timer_list | grep event_handler
>> get "event_handler:  hrtimer_interrupt"
>> event_handler:  tick_handle_oneshot_broadcast
>> event_handler:  hrtimer_interrupt
> 
> What are you trying to demonstrate here ? There are no interrupts for both 
> arch_timer and rk_timer.

I don't know. Maybe Caesar do something wrong :(
This is my output:
   CPU0   CPU1   CPU2   CPU3   CPU4   CPU5

...
  2:   2911   1967   1588   1608   1295   1606
   GICv3  30 Edge  arch_timer
  5:578637684626161165
   GICv3 113 Level rk_timer

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Huang, Tao]

Hi Daniel:
On 2016年06月13日 21:06, Daniel Lezcano wrote:
> On Tue, Jun 07, 2016 at 12:54:29PM +0800, Caesar Wang wrote:
>> This series patches had been tested on rockchip inside kernel.
>> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
>> save power in idle.
> 
> For my personnal information, are the arch_timer in the same power domain 
> than the CPU ? IOW, what is the 'always-on' property in the DT ?

Yes. In our SoC design, all arch (generic) timer in the same power
domain of CPU core. So if one CPU core power down, the arch (generic)
timer will lose it's state and stop working.
While rk timer maybe in peri power domain or pmu power domain, so the
timer will still work when CPU power down.

But before RK3399, all SoCs with CPU power domain, do not support auto
power down while cpu idle. So the arch timer can be seem as always on,
i.e. we don't need a broadcast timer at all.

> 
>> Okay, it still works bootup on rk3288/other SoCs, even though many socs 
>> hasn't used
>> the broadcast timer.
> 
> Yes, unfortunately the SoC design on rk3288 and the previous ones do not 
> allow to use a cpuidle driver with cpu/cluster power down, so obviously the 
> broadcast timer is pointless on these boards :)
> 

You are right.

>> History version:
>> v1:
>> https://lkml.org/lkml/2016/5/25/186
>>
>> Easy to test for my borad.
>> localhost / # cat /proc/interrupts
>> CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
>> 1:  0  0  0  0  0  0 
>> GICv3  29 Edge  arch_timer
>> ...
>> 5:  0  0  0  0  0  0 
>> GICv3 113 Level rk_timer
>> ..
>>
>> localhost / # cat /proc/timer_list | grep event_handler
>> get "event_handler:  hrtimer_interrupt"
>> event_handler:  tick_handle_oneshot_broadcast
>> event_handler:  hrtimer_interrupt
> 
> What are you trying to demonstrate here ? There are no interrupts for both 
> arch_timer and rk_timer.

I don't know. Maybe Caesar do something wrong :(
This is my output:
   CPU0   CPU1   CPU2   CPU3   CPU4   CPU5

...
  2:   2911   1967   1588   1608   1295   1606
   GICv3  30 Edge  arch_timer
  5:578637684626161165
   GICv3 113 Level rk_timer

linux-next: manual merge of the md tree with the block tree

[Stephen Rothwell]

Hi Shaohua,

Today's linux-next merge of the md tree got a conflict in:

  drivers/md/raid1.c

between commit:

  796a5cf083c2 ("md: use bio op accessors")

from the block tree and commit:

  707a6a420ccf ("md/raid1: add rcu protection to rdev in fix_read_error")

from the md tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/md/raid1.c
index 41d9c31da3b3,5027ef4752ac..
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@@ -2086,15 -2069,20 +2067,21 @@@ static void fix_read_error(struct r1con
 (!test_bit(Faulty, >flags) &&
  rdev->recovery_offset >= sect + s)) &&
is_badblock(rdev, sect, s,
-   _bad, _sectors) == 0 &&
-   sync_page_io(rdev, sect, s<<9,
-conf->tmppage, REQ_OP_READ, 0, false))
-   success = 1;
-   else {
-   d++;
-   if (d == conf->raid_disks * 2)
-   d = 0;
-   }
+   _bad, _sectors) == 0) {
+   atomic_inc(>nr_pending);
+   rcu_read_unlock();
+   if (sync_page_io(rdev, sect, s<<9,
 -   conf->tmppage, READ, false))
++   conf->tmppage, REQ_OP_READ, 0,
++   false))
+   success = 1;
+   rdev_dec_pending(rdev, mddev);
+   if (success)
+   break;
+   } else
+   rcu_read_unlock();
+   d++;
+   if (d == conf->raid_disks * 2)
+   d = 0;
} while (!success && d != read_disk);
  
if (!success) {

linux-next: manual merge of the md tree with the block tree

[Stephen Rothwell]

Hi Shaohua,

Today's linux-next merge of the md tree got a conflict in:

  drivers/md/raid1.c

between commit:

  796a5cf083c2 ("md: use bio op accessors")

from the block tree and commit:

  707a6a420ccf ("md/raid1: add rcu protection to rdev in fix_read_error")

from the md tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/md/raid1.c
index 41d9c31da3b3,5027ef4752ac..
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@@ -2086,15 -2069,20 +2067,21 @@@ static void fix_read_error(struct r1con
 (!test_bit(Faulty, >flags) &&
  rdev->recovery_offset >= sect + s)) &&
is_badblock(rdev, sect, s,
-   _bad, _sectors) == 0 &&
-   sync_page_io(rdev, sect, s<<9,
-conf->tmppage, REQ_OP_READ, 0, false))
-   success = 1;
-   else {
-   d++;
-   if (d == conf->raid_disks * 2)
-   d = 0;
-   }
+   _bad, _sectors) == 0) {
+   atomic_inc(>nr_pending);
+   rcu_read_unlock();
+   if (sync_page_io(rdev, sect, s<<9,
 -   conf->tmppage, READ, false))
++   conf->tmppage, REQ_OP_READ, 0,
++   false))
+   success = 1;
+   rdev_dec_pending(rdev, mddev);
+   if (success)
+   break;
+   } else
+   rcu_read_unlock();
+   d++;
+   if (d == conf->raid_disks * 2)
+   d = 0;
} while (!success && d != read_disk);
  
if (!success) {

linux-next: manual merge of the md tree with the block tree

[Stephen Rothwell]

Hi Shaohua,

Today's linux-next merge of the md tree got a conflict in:

  drivers/md/raid10.c

between commit:

  796a5cf083c2 ("md: use bio op accessors")

from the block tree and commits:

  f90145f317ef ("md/raid10: add rcu protection to rdev access in 
raid10_sync_request."
  d094d6860b66 ("md/raid10: add rcu protection to rdev access during reshape.")

from the md tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/md/raid10.c
index 26ae74fd0d01,8ee5d96e6a2d..
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@@ -3063,10 -3092,10 +3091,10 @@@ static sector_t raid10_sync_request(str
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr
-   + rdev->data_offset;
-   bio->bi_bdev = rdev->bdev;
+   + mrdev->data_offset;
+   bio->bi_bdev = mrdev->bdev;
atomic_inc(_bio->remaining);
} else
r10_bio->devs[1].bio->bi_end_io = NULL;
@@@ -3092,10 -3120,10 +3119,10 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr +
-   rdev->data_offset;
-   bio->bi_bdev = rdev->bdev;
+   mreplace->data_offset;
+   bio->bi_bdev = mreplace->bdev;
atomic_inc(_bio->remaining);
break;
}
@@@ -3212,16 -3251,18 +3250,18 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_read;
 -  bio->bi_rw = READ;
 +  bio_set_op_attrs(bio, REQ_OP_READ, 0);
-   bio->bi_iter.bi_sector = sector +
-   conf->mirrors[d].rdev->data_offset;
-   bio->bi_bdev = conf->mirrors[d].rdev->bdev;
+   bio->bi_iter.bi_sector = sector + rdev->data_offset;
+   bio->bi_bdev = rdev->bdev;
count++;
  
-   if (conf->mirrors[d].replacement == NULL ||
-   test_bit(Faulty,
->mirrors[d].replacement->flags))
+   rdev = rcu_dereference(conf->mirrors[d].replacement);
+   if (rdev == NULL || test_bit(Faulty, >flags)) {
+   rcu_read_unlock();
continue;
+   }
+   atomic_inc(>nr_pending);
+   rcu_read_unlock();
  
/* Need to set up for writing to the replacement */
bio = r10_bio->devs[i].repl_bio;
@@@ -3234,10 -3274,9 +3273,9 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-   bio->bi_iter.bi_sector = sector +
-   conf->mirrors[d].replacement->data_offset;
-   bio->bi_bdev = conf->mirrors[d].replacement->bdev;
+   bio->bi_iter.bi_sector = sector + rdev->data_offset;
+   bio->bi_bdev = rdev->bdev;
count++;
}
  
@@@ -4521,7 -4569,9 +4568,9 @@@ static int handle_reshape_read_error(st
   addr,
   s << 9,
   bvec[idx].bv_page,
 - READ, false);
 + REQ_OP_READ, 0, 

linux-next: manual merge of the md tree with the block tree

[Stephen Rothwell]

Hi Shaohua,

Today's linux-next merge of the md tree got a conflict in:

  drivers/md/raid10.c

between commit:

  796a5cf083c2 ("md: use bio op accessors")

from the block tree and commits:

  f90145f317ef ("md/raid10: add rcu protection to rdev access in 
raid10_sync_request."
  d094d6860b66 ("md/raid10: add rcu protection to rdev access during reshape.")

from the md tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/md/raid10.c
index 26ae74fd0d01,8ee5d96e6a2d..
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@@ -3063,10 -3092,10 +3091,10 @@@ static sector_t raid10_sync_request(str
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr
-   + rdev->data_offset;
-   bio->bi_bdev = rdev->bdev;
+   + mrdev->data_offset;
+   bio->bi_bdev = mrdev->bdev;
atomic_inc(_bio->remaining);
} else
r10_bio->devs[1].bio->bi_end_io = NULL;
@@@ -3092,10 -3120,10 +3119,10 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr +
-   rdev->data_offset;
-   bio->bi_bdev = rdev->bdev;
+   mreplace->data_offset;
+   bio->bi_bdev = mreplace->bdev;
atomic_inc(_bio->remaining);
break;
}
@@@ -3212,16 -3251,18 +3250,18 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_read;
 -  bio->bi_rw = READ;
 +  bio_set_op_attrs(bio, REQ_OP_READ, 0);
-   bio->bi_iter.bi_sector = sector +
-   conf->mirrors[d].rdev->data_offset;
-   bio->bi_bdev = conf->mirrors[d].rdev->bdev;
+   bio->bi_iter.bi_sector = sector + rdev->data_offset;
+   bio->bi_bdev = rdev->bdev;
count++;
  
-   if (conf->mirrors[d].replacement == NULL ||
-   test_bit(Faulty,
->mirrors[d].replacement->flags))
+   rdev = rcu_dereference(conf->mirrors[d].replacement);
+   if (rdev == NULL || test_bit(Faulty, >flags)) {
+   rcu_read_unlock();
continue;
+   }
+   atomic_inc(>nr_pending);
+   rcu_read_unlock();
  
/* Need to set up for writing to the replacement */
bio = r10_bio->devs[i].repl_bio;
@@@ -3234,10 -3274,9 +3273,9 @@@
biolist = bio;
bio->bi_private = r10_bio;
bio->bi_end_io = end_sync_write;
 -  bio->bi_rw = WRITE;
 +  bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-   bio->bi_iter.bi_sector = sector +
-   conf->mirrors[d].replacement->data_offset;
-   bio->bi_bdev = conf->mirrors[d].replacement->bdev;
+   bio->bi_iter.bi_sector = sector + rdev->data_offset;
+   bio->bi_bdev = rdev->bdev;
count++;
}
  
@@@ -4521,7 -4569,9 +4568,9 @@@ static int handle_reshape_read_error(st
   addr,
   s << 9,
   bvec[idx].bv_page,
 - READ, false);
 + REQ_OP_READ, 0, 

Re: [PATCH] clk: rockchip: add pclk_vio_grf to critical clock on the RK3399

[Doug Anderson]

Hi,

On Mon, Jun 13, 2016 at 8:02 PM, Xing Zheng  wrote:
> Hi Doug,
>
> On 2016年06月14日 07:46, Doug Anderson wrote:
>>
>>
>> Even if it's not much power, it seems like we should still turn it off
>> and on in the right place.  Unless I'm mistaken it should be such a
>> simple patch provide the clock to the right driver and then get the
>> clock when appropriate.
>
> Yes, I talked with Yakir and we intent to enable/disable the pclk_vio_grf in
> video drivers,
> so this patch will be dropped.
>>>
>>> I will refer the latest TRM to update a new patch for always enable these
>>> GRFs.
>>
>> Does that mean you're going to make these all critical clocks?  That
>> doesn't sound so great...
>>
>> -Doug
>
> Maybe, I heard that they are removed in the updated TRM, but I have not got
> the TRM yet.
> I will double check it, and it seems that you do not agree to remove these
> clock...

Well, if it were to be removed from the TRM then that would be a
strong sign that the SoC designers think that this clock should never
ever be turned off.  If that were the case I don't think I could
object to leaving this clock on all the time.  Presumably then we'd
totally remove the clock from the clock tree and rely on firmware to
leave it on?  Technically removing this clock is not really
device-tree backward compatible, but I guess if there are no current
users...

...note: if the clock IS listed in the TRM and there's ever a chance
that we'd want to turn it off, it's much easier to set that up all
now.  Trying to later go in and decide that these clocks are no longer
"always on" gets into all sorts of weird device tree backward
compatibility corner cases.



-Doug

Re: [PATCH] clk: rockchip: add pclk_vio_grf to critical clock on the RK3399

[Doug Anderson]

Hi,

On Mon, Jun 13, 2016 at 8:02 PM, Xing Zheng  wrote:
> Hi Doug,
>
> On 2016年06月14日 07:46, Doug Anderson wrote:
>>
>>
>> Even if it's not much power, it seems like we should still turn it off
>> and on in the right place.  Unless I'm mistaken it should be such a
>> simple patch provide the clock to the right driver and then get the
>> clock when appropriate.
>
> Yes, I talked with Yakir and we intent to enable/disable the pclk_vio_grf in
> video drivers,
> so this patch will be dropped.
>>>
>>> I will refer the latest TRM to update a new patch for always enable these
>>> GRFs.
>>
>> Does that mean you're going to make these all critical clocks?  That
>> doesn't sound so great...
>>
>> -Doug
>
> Maybe, I heard that they are removed in the updated TRM, but I have not got
> the TRM yet.
> I will double check it, and it seems that you do not agree to remove these
> clock...

Well, if it were to be removed from the TRM then that would be a
strong sign that the SoC designers think that this clock should never
ever be turned off.  If that were the case I don't think I could
object to leaving this clock on all the time.  Presumably then we'd
totally remove the clock from the clock tree and rely on firmware to
leave it on?  Technically removing this clock is not really
device-tree backward compatible, but I guess if there are no current
users...

...note: if the clock IS listed in the TRM and there's ever a chance
that we'd want to turn it off, it's much easier to set that up all
now.  Trying to later go in and decide that these clocks are no longer
"always on" gets into all sorts of weird device tree backward
compatibility corner cases.



-Doug

Re: [PATCH 2/2] perf annotate: add powerpc support

[Michael Ellerman]

On Fri, 2016-06-10 at 20:08 +0530, Naveen N. Rao wrote:
> On 2016/06/10 10:36AM, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Jun 10, 2016 at 06:32:51PM +0530, Naveen N. Rao escreveu:
> > > Convert ins__find() to a __weak function for generic functionality,
> > > while adding a powerpc-specific variant. We look at the function name
> > > for branch instructions and classify the instructions to one among a
> > > branch, a function call (branch with LR update) or a function return
> > > (branch to LR).
> > 
> > How would this allow one to get a perf.data collected on a powerpc
> > system, transfer it to a x86-64 (or aarch64, to mention another
> > workstation wannabe chip) system and then try annotating it?
> > 
> > There was a previous discussion about this, and it involved having all
> > yout ppc tables available as well as other arches tables, and then
> > choosing which one to use based on:
> > 
> > normalize_arch(thread->mg->machine->env->arch)
> > 
> > just like was done for support cross unwinding, see recent patch kit by
> > He Kuang, CCed.
> 
> Nice. This would be good to have. I will look at adding powerpc support 
> for cross-architecture unwind.
> 
> However, for cross-architecture annotation, I think there will be a lot 
> more dependencies since perf currently uses objdump to obtain the 
> disassembly. In addition, the actual binaries will also be needed.

It's possible to build a multi-arch objdump, I don't know if it's packaged on
all distros, or if perf wants to depend on it.

cheers

Re: [PATCH 2/2] perf annotate: add powerpc support

[Michael Ellerman]

On Fri, 2016-06-10 at 20:08 +0530, Naveen N. Rao wrote:
> On 2016/06/10 10:36AM, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Jun 10, 2016 at 06:32:51PM +0530, Naveen N. Rao escreveu:
> > > Convert ins__find() to a __weak function for generic functionality,
> > > while adding a powerpc-specific variant. We look at the function name
> > > for branch instructions and classify the instructions to one among a
> > > branch, a function call (branch with LR update) or a function return
> > > (branch to LR).
> > 
> > How would this allow one to get a perf.data collected on a powerpc
> > system, transfer it to a x86-64 (or aarch64, to mention another
> > workstation wannabe chip) system and then try annotating it?
> > 
> > There was a previous discussion about this, and it involved having all
> > yout ppc tables available as well as other arches tables, and then
> > choosing which one to use based on:
> > 
> > normalize_arch(thread->mg->machine->env->arch)
> > 
> > just like was done for support cross unwinding, see recent patch kit by
> > He Kuang, CCed.
> 
> Nice. This would be good to have. I will look at adding powerpc support 
> for cross-architecture unwind.
> 
> However, for cross-architecture annotation, I think there will be a lot 
> more dependencies since perf currently uses objdump to obtain the 
> disassembly. In addition, the actual binaries will also be needed.

It's possible to build a multi-arch objdump, I don't know if it's packaged on
all distros, or if perf wants to depend on it.

cheers

Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

[Eric Dumazet]

On Mon, 2016-06-13 at 20:12 -0700, Eric Dumazet wrote:

> 
> Have you tested this patch ?

Hmm, please ignore this question ;)

Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

[Eric Dumazet]

On Mon, 2016-06-13 at 20:12 -0700, Eric Dumazet wrote:

> 
> Have you tested this patch ?

Hmm, please ignore this question ;)

Re: [PATCH v2] iommu/amd: Set AMD iommu callbacks for platform bus driver

[Wan ZongShun]

2016-05-10 21:21 GMT+08:00 Wan Zongshun :
> From: Wan Zongshun 
>
> AMD has more drivers will use ACPI to platform bus driver later,
> all those devices need iommu support, for example: eMMC driver.
>
> For latest AMD eMMC controller, it will utilize sdhci-acpi.c driver,
> which will rely on platform bus to match device and driver, where we
> will set 'dev' of struct platform_device as map_sg parameter passing
> to iommu driver for DMA request, so the iommu-ops are needed on the
> platform bus.

Joerg,

How about this patch? it will impact our new eMMC controller driver bringup.


Thanks.

>
> Signed-off-by: Wan Zongshun 
>
> ---
> changes from v1: Add comment why the iommu-ops are needed on platform bus.
> ---
>  drivers/iommu/amd_iommu.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index c430c10..547cdd4 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2980,6 +2981,9 @@ int __init amd_iommu_init_api(void)
> if (err)
> return err;
>  #endif
> +   err = bus_set_iommu(_bus_type, _iommu_ops);
> +   if (err)
> +   return err;
> return 0;
>  }
>
> --
> 1.9.1
>



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

Re: [PATCH v2] iommu/amd: Set AMD iommu callbacks for platform bus driver

[Wan ZongShun]

2016-05-10 21:21 GMT+08:00 Wan Zongshun :
> From: Wan Zongshun 
>
> AMD has more drivers will use ACPI to platform bus driver later,
> all those devices need iommu support, for example: eMMC driver.
>
> For latest AMD eMMC controller, it will utilize sdhci-acpi.c driver,
> which will rely on platform bus to match device and driver, where we
> will set 'dev' of struct platform_device as map_sg parameter passing
> to iommu driver for DMA request, so the iommu-ops are needed on the
> platform bus.

Joerg,

How about this patch? it will impact our new eMMC controller driver bringup.


Thanks.

>
> Signed-off-by: Wan Zongshun 
>
> ---
> changes from v1: Add comment why the iommu-ops are needed on platform bus.
> ---
>  drivers/iommu/amd_iommu.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index c430c10..547cdd4 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2980,6 +2981,9 @@ int __init amd_iommu_init_api(void)
> if (err)
> return err;
>  #endif
> +   err = bus_set_iommu(_bus_type, _iommu_ops);
> +   if (err)
> +   return err;
> return 0;
>  }
>
> --
> 1.9.1
>



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

[Eric Dumazet]

On Tue, 2016-06-14 at 02:43 +0100, Quentin Armitage wrote:
> When other settings are changed in the socket it is locked, so
> lock the socket before setting SK_CAN_REUSE.
> 
> Signed-off-by: Quentin Armitage 
> ---
>  net/netfilter/ipvs/ip_vs_sync.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 29d73d8..dfac9ef 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1540,7 +1540,9 @@ static struct socket *make_receive_sock(struct 
> netns_ipvs *ipvs, int id, int ifi
>   }
>  
>   /* it is equivalent to the REUSEADDR option in user-space */
> + lock_sock(sock->sk);
>   sock->sk->sk_reuse = SK_CAN_REUSE;
> + release_sock(sock->sk);
>   result = sysctl_sync_sock_size(ipvs);
>   if (result > 0)
>   set_sock_size(sock->sk, 0, result);


Have you tested this patch ?

Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

[Eric Dumazet]

On Tue, 2016-06-14 at 02:43 +0100, Quentin Armitage wrote:
> When other settings are changed in the socket it is locked, so
> lock the socket before setting SK_CAN_REUSE.
> 
> Signed-off-by: Quentin Armitage 
> ---
>  net/netfilter/ipvs/ip_vs_sync.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 29d73d8..dfac9ef 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1540,7 +1540,9 @@ static struct socket *make_receive_sock(struct 
> netns_ipvs *ipvs, int id, int ifi
>   }
>  
>   /* it is equivalent to the REUSEADDR option in user-space */
> + lock_sock(sock->sk);
>   sock->sk->sk_reuse = SK_CAN_REUSE;
> + release_sock(sock->sk);
>   result = sysctl_sync_sock_size(ipvs);
>   if (result > 0)
>   set_sock_size(sock->sk, 0, result);


Have you tested this patch ?

Re: [PATCH] iommu/arm-smmu: request pcie devices to enable ACS

[Wei Chen]

On 13 June 2016 at 20:45, Will Deacon  wrote:
> On Mon, Jun 13, 2016 at 05:20:17PM +0800, Wei Chen wrote:
>> The PCIe ACS capability will affect the layout of iommu groups.
>> Generally speaking, if the path from root port to the PCIe device
>> is ACS enabled, the iommu will create a single iommu group for this
>> PCIe device. If all PCIe devices on the path are ACS enabled then
>> Linux can determine this path is ACS enabled.
>>
>> Linux use two PCIe configuration registers to determine the ACS
>> status of PCIe devices:
>> ACS Capability Register and ACS Control Register.
>>
>> The first register is used to check the implementation of ACS function
>> of a PCIe device, the second register is used to check the enable status
>> of ACS function. If one PCIe device has implemented and enabled the ACS
>> function then Linux will determine this PCIe device enabled ACS.
>>
>> From the Chapter:6.12 of PCI Express Base Specification Revision 3.1a,
>> we can find that when a PCIe device implements ACS function, the enable
>> status is set to disabled by default and can be enabled by ACS-aware
>> software.
>>
>> ACS will affect the iommu groups topology, so, the iommu driver is
>> ACS-aware software. This patch adds a call to pci_request_acs() to the
>> arm-smmu driver to enable the ACS function in PCIe devices that support
>> it.
>>
>> Signed-off-by: Wei Chen 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 ++
>>  drivers/iommu/arm-smmu.c| 4 +++-
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> Thanks, queued for 4.8 w/ Robin and Eric's reviewed-by tags and the minor
> commit wording change.
>

Thanks, I will post a v2 patch to include above changes.

> Will
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] iommu/arm-smmu: request pcie devices to enable ACS

[Wei Chen]

On 13 June 2016 at 20:45, Will Deacon  wrote:
> On Mon, Jun 13, 2016 at 05:20:17PM +0800, Wei Chen wrote:
>> The PCIe ACS capability will affect the layout of iommu groups.
>> Generally speaking, if the path from root port to the PCIe device
>> is ACS enabled, the iommu will create a single iommu group for this
>> PCIe device. If all PCIe devices on the path are ACS enabled then
>> Linux can determine this path is ACS enabled.
>>
>> Linux use two PCIe configuration registers to determine the ACS
>> status of PCIe devices:
>> ACS Capability Register and ACS Control Register.
>>
>> The first register is used to check the implementation of ACS function
>> of a PCIe device, the second register is used to check the enable status
>> of ACS function. If one PCIe device has implemented and enabled the ACS
>> function then Linux will determine this PCIe device enabled ACS.
>>
>> From the Chapter:6.12 of PCI Express Base Specification Revision 3.1a,
>> we can find that when a PCIe device implements ACS function, the enable
>> status is set to disabled by default and can be enabled by ACS-aware
>> software.
>>
>> ACS will affect the iommu groups topology, so, the iommu driver is
>> ACS-aware software. This patch adds a call to pci_request_acs() to the
>> arm-smmu driver to enable the ACS function in PCIe devices that support
>> it.
>>
>> Signed-off-by: Wei Chen 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 ++
>>  drivers/iommu/arm-smmu.c| 4 +++-
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> Thanks, queued for 4.8 w/ Robin and Eric's reviewed-by tags and the minor
> commit wording change.
>

Thanks, I will post a v2 patch to include above changes.

> Will
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH] sys_read: add a compat_sys_read for 64bit system

[Weidong Wang]

On 2016/6/10 1:08, Andy Lutomirski wrote:
> On Tue, Jun 7, 2016 at 7:14 PM, Zhangjian (Bamvor)
>  wrote:
>> Hi,
>>
>> On 2016/6/8 9:33, Weidong Wang wrote:
>>>
>>> Test 32 progress and 64 progress on the 64bit system with
>>> this progress:
>>>
>>> int main(int argc, char **argv)
>>> {
>>>  int fd = 0;
>>>  int i, ret = 0;
>>>  char buf[512];
>>>  unsigned long count = -1;
>>>
>>>  fd = open("/tmp", O_RDONLY);
>>>  if (fd < -1) {
>>>  printf("Pls check the directory is exist?\n");
>>>  return -1;
>>>  }
>>>  errno = 0;
>>>  ret = read(fd, NULL, count);
>>>  printf("Ret is %d errno %d\n", ret, errno);
>>>  close(fd);
>>>
>>>  return 0;
>>> }
>>>
>>> we get the different errno. The 64 progress we get errno is -14 while
>>> the 32 progress is -21.
> 
> On 64-bit, you get -14 == -EFAULT.  Seems reasonable: you passed a bad 
> pointer.
> 
> On 32-bit, you get -21 == -EISDIR.  Also seems reasonable: fd is a directory.
> 
>>>
>>> The reason is that, the user progress would use a 32bit count, while
>>> the sys_read size_t in kernel is 64bit.  When the uesrspace count is
>>> -1(0x), it goes to the sys_read, it would be change to a positive
>>> number.
> 
> That parameter is size_t, which is unsigned.  It's a positive number
> in both cases.
> 
> I don't think there's a bug here.
> 

Yep.
In the progress open the '/tmp' is a directory. If we do open a file 
'/tmp/files' (exist file),
the result would be different on x86-64bit machine.

On 64-bit, we get -14 == -EFAULT.
On 32-bit, we get the length of the file, the errno is 0.

Regards,
Weidong

> .
> 

Re: [RFC PATCH] sys_read: add a compat_sys_read for 64bit system

[Weidong Wang]

On 2016/6/10 1:08, Andy Lutomirski wrote:
> On Tue, Jun 7, 2016 at 7:14 PM, Zhangjian (Bamvor)
>  wrote:
>> Hi,
>>
>> On 2016/6/8 9:33, Weidong Wang wrote:
>>>
>>> Test 32 progress and 64 progress on the 64bit system with
>>> this progress:
>>>
>>> int main(int argc, char **argv)
>>> {
>>>  int fd = 0;
>>>  int i, ret = 0;
>>>  char buf[512];
>>>  unsigned long count = -1;
>>>
>>>  fd = open("/tmp", O_RDONLY);
>>>  if (fd < -1) {
>>>  printf("Pls check the directory is exist?\n");
>>>  return -1;
>>>  }
>>>  errno = 0;
>>>  ret = read(fd, NULL, count);
>>>  printf("Ret is %d errno %d\n", ret, errno);
>>>  close(fd);
>>>
>>>  return 0;
>>> }
>>>
>>> we get the different errno. The 64 progress we get errno is -14 while
>>> the 32 progress is -21.
> 
> On 64-bit, you get -14 == -EFAULT.  Seems reasonable: you passed a bad 
> pointer.
> 
> On 32-bit, you get -21 == -EISDIR.  Also seems reasonable: fd is a directory.
> 
>>>
>>> The reason is that, the user progress would use a 32bit count, while
>>> the sys_read size_t in kernel is 64bit.  When the uesrspace count is
>>> -1(0x), it goes to the sys_read, it would be change to a positive
>>> number.
> 
> That parameter is size_t, which is unsigned.  It's a positive number
> in both cases.
> 
> I don't think there's a bug here.
> 

Yep.
In the progress open the '/tmp' is a directory. If we do open a file 
'/tmp/files' (exist file),
the result would be different on x86-64bit machine.

On 64-bit, we get -14 == -EFAULT.
On 32-bit, we get the length of the file, the errno is 0.

Regards,
Weidong

> .
> 

[PATCH 1/3] arm64: dts: uniphier: add SoC-Glue node to UniPhier 64bit SoCs

[Masahiro Yamada]

This node consists of various system-level configuration registers.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
index 9532880..31dc51b 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
@@ -264,9 +264,13 @@
reg = <0x59801000 0x400>;
};
 
-   pinctrl: pinctrl@5f801000 {
-   compatible = "socionext,ph1-ld20-pinctrl", "syscon";
-   reg = <0x5f801000 0xe00>;
+   soc-glue@5f80 {
+   compatible = "simple-mfd", "syscon";
+   reg = <0x5f80 0x2000>;
+
+   pinctrl: pinctrl {
+compatible = "socionext,uniphier-ld20-pinctrl";
+   };
};
 
gic: interrupt-controller@5fe0 {
-- 
1.9.1

[PATCH 1/3] arm64: dts: uniphier: add SoC-Glue node to UniPhier 64bit SoCs

[Masahiro Yamada]

This node consists of various system-level configuration registers.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
index 9532880..31dc51b 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
@@ -264,9 +264,13 @@
reg = <0x59801000 0x400>;
};
 
-   pinctrl: pinctrl@5f801000 {
-   compatible = "socionext,ph1-ld20-pinctrl", "syscon";
-   reg = <0x5f801000 0xe00>;
+   soc-glue@5f80 {
+   compatible = "simple-mfd", "syscon";
+   reg = <0x5f80 0x2000>;
+
+   pinctrl: pinctrl {
+compatible = "socionext,uniphier-ld20-pinctrl";
+   };
};
 
gic: interrupt-controller@5fe0 {
-- 
1.9.1

Re: [PATCH] clk: rockchip: add pclk_vio_grf to critical clock on the RK3399

[Xing Zheng]

Hi Doug,

On 2016年06月14日 07:46, Doug Anderson wrote:


Even if it's not much power, it seems like we should still turn it off
and on in the right place.  Unless I'm mistaken it should be such a
simple patch provide the clock to the right driver and then get the
clock when appropriate.
Yes, I talked with Yakir and we intent to enable/disable the 
pclk_vio_grf in video drivers,

so this patch will be dropped.

I will refer the latest TRM to update a new patch for always enable these
GRFs.

Does that mean you're going to make these all critical clocks?  That
doesn't sound so great...

-Doug
Maybe, I heard that they are removed in the updated TRM, but I have not 
got the TRM yet.
I will double check it, and it seems that you do not agree to remove 
these clock...


Thanks.

--
- Xing Zheng

Re: [PATCH] clk: rockchip: add pclk_vio_grf to critical clock on the RK3399

[Xing Zheng]

Hi Doug,

On 2016年06月14日 07:46, Doug Anderson wrote:


Even if it's not much power, it seems like we should still turn it off
and on in the right place.  Unless I'm mistaken it should be such a
simple patch provide the clock to the right driver and then get the
clock when appropriate.
Yes, I talked with Yakir and we intent to enable/disable the 
pclk_vio_grf in video drivers,

so this patch will be dropped.

I will refer the latest TRM to update a new patch for always enable these
GRFs.

Does that mean you're going to make these all critical clocks?  That
doesn't sound so great...

-Doug
Maybe, I heard that they are removed in the updated TRM, but I have not 
got the TRM yet.
I will double check it, and it seems that you do not agree to remove 
these clock...


Thanks.

--
- Xing Zheng