Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test

[Dmitry Safonov]

2016-05-21 23:27 GMT+03:00 Ingo Molnar :
>
> * Andy Lutomirski  wrote:
>
>> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar  wrote:
>> >
>> > * Dmitry Safonov  wrote:
>> >
>> >> Should print on success:
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>   AT_SYSINFO_EHDR is 0xf773f000
>> >> [NOTE]Moving vDSO: [f773f000, f774] -> [a00, a001000]
>> >> [OK]
>> >> Or segfault if landing was bad (before patches):
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>   AT_SYSINFO_EHDR is 0xf774f000
>> >> [NOTE]Moving vDSO: [f774f000, f775] -> [a00, a001000]
>> >> Segmentation fault (core dumped)
>> >
>> > So I still think that generating potential segfaults is not a proper way 
>> > to test a
>> > new feature. How are we supposed to tell the feature still works? I 
>> > realize that
>> > glibc is a problem here - but that doesn't really change the QA equation: 
>> > we are
>> > adding new kernel code to help essentially a single application out of 
>> > tens of
>> > thousands of applications.
>> >
>> > At minimum we should have a robust testcase ...
>>
>> I think it's robust enough.  It will print "[OK]" and exit with 0 on
>> success and it will crash on failure.  The latter should cause make
>> run_tests to fail reliably.
>
> Indeed, you are right - I somehow mis-read it as potentially segfaulting on 
> fixed
> kernels as well...
>
> Will look at applying this after the merge window.

Great! Thanks, Ingo - maybe I should have wrote test's patch description better.
Thanks again, Andy.

Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test

[Dmitry Safonov]

2016-05-21 23:27 GMT+03:00 Ingo Molnar :
>
> * Andy Lutomirski  wrote:
>
>> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar  wrote:
>> >
>> > * Dmitry Safonov  wrote:
>> >
>> >> Should print on success:
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>   AT_SYSINFO_EHDR is 0xf773f000
>> >> [NOTE]Moving vDSO: [f773f000, f774] -> [a00, a001000]
>> >> [OK]
>> >> Or segfault if landing was bad (before patches):
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>   AT_SYSINFO_EHDR is 0xf774f000
>> >> [NOTE]Moving vDSO: [f774f000, f775] -> [a00, a001000]
>> >> Segmentation fault (core dumped)
>> >
>> > So I still think that generating potential segfaults is not a proper way 
>> > to test a
>> > new feature. How are we supposed to tell the feature still works? I 
>> > realize that
>> > glibc is a problem here - but that doesn't really change the QA equation: 
>> > we are
>> > adding new kernel code to help essentially a single application out of 
>> > tens of
>> > thousands of applications.
>> >
>> > At minimum we should have a robust testcase ...
>>
>> I think it's robust enough.  It will print "[OK]" and exit with 0 on
>> success and it will crash on failure.  The latter should cause make
>> run_tests to fail reliably.
>
> Indeed, you are right - I somehow mis-read it as potentially segfaulting on 
> fixed
> kernels as well...
>
> Will look at applying this after the merge window.

Great! Thanks, Ingo - maybe I should have wrote test's patch description better.
Thanks again, Andy.

[PATCH] clk: fixed-rate: add clk_hw_unregister_fixed_rate()

[Masahiro Yamada]

This will be used to migrate to the clk_hw APIs.

Signed-off-by: Masahiro Yamada 
---

 drivers/clk/clk-fixed-rate.c | 11 +++
 include/linux/clk-provider.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 8e4453e..2edb393 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -145,6 +145,17 @@ void clk_unregister_fixed_rate(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_unregister_fixed_rate);
 
+void clk_hw_unregister_fixed_rate(struct clk_hw *hw)
+{
+   struct clk_fixed_rate *fixed;
+
+   fixed = to_clk_fixed_rate(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fixed);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_fixed_rate);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_clk_setup() - Setup function for simple fixed rate clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 0c72204..94e00fe 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -293,6 +293,7 @@ void clk_unregister_fixed_rate(struct clk *clk);
 struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy);
+void clk_hw_unregister_fixed_rate(struct clk_hw *hw);
 
 void of_fixed_clk_setup(struct device_node *np);
 
-- 
1.9.1

[PATCH] clk: fixed-rate: add clk_hw_unregister_fixed_rate()

[Masahiro Yamada]

This will be used to migrate to the clk_hw APIs.

Signed-off-by: Masahiro Yamada 
---

 drivers/clk/clk-fixed-rate.c | 11 +++
 include/linux/clk-provider.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 8e4453e..2edb393 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -145,6 +145,17 @@ void clk_unregister_fixed_rate(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_unregister_fixed_rate);
 
+void clk_hw_unregister_fixed_rate(struct clk_hw *hw)
+{
+   struct clk_fixed_rate *fixed;
+
+   fixed = to_clk_fixed_rate(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fixed);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_fixed_rate);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_clk_setup() - Setup function for simple fixed rate clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 0c72204..94e00fe 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -293,6 +293,7 @@ void clk_unregister_fixed_rate(struct clk *clk);
 struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy);
+void clk_hw_unregister_fixed_rate(struct clk_hw *hw);
 
 void of_fixed_clk_setup(struct device_node *np);
 
-- 
1.9.1

Re: [rcutorture] 8704baab9b: WARNING: CPU: 0 PID: 30 at kernel/rcu/rcuperf.c:363 rcu_perf_writer

[Paul E. McKenney]

On Sun, May 22, 2016 at 10:36:00AM +0800, kernel test robot wrote:
> Greetings,
> 
> 0day kernel testing robot got the below dmesg and the first bad commit is
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> 
> commit 8704baab9bc848b58c129fed6b591bb84ec02f41
> Author: Paul E. McKenney 
> AuthorDate: Thu Dec 31 18:33:22 2015 -0800
> Commit: Paul E. McKenney 
> CommitDate: Thu Mar 31 13:37:38 2016 -0700
> 
> rcutorture: Add RCU grace-period performance tests
> 
> This commit adds a new rcuperf module that carries out simple performance
> tests of RCU grace periods.
> 
> Signed-off-by: Paul E. McKenney 

???

This commit adds a default-n performance-test module.  I don't believe
that this would result in boot failures.  False bisection?

Thanx, Paul

> +---++++
> |   | 291783b8ad | 
> 8704baab9b | ce82e4a05f |
> +---++++
> | boot_successes| 57 | 0  
> | 0  |
> | boot_failures | 6  | 22 
> | 13 |
> | BUG:unable_to_handle_kernel   | 6  | 22 
> ||
> | Oops  | 6  | 22 
> ||
> | EIP_is_at_get_perf_callchain  | 6  |
> ||
> | Kernel_panic-not_syncing:Fatal_exception  | 5  | 22 
> ||
> | backtrace:acpi_get_cpuid  | 6  | 22 
> | 13 |
> | backtrace:early_init_pdc  | 6  | 22 
> | 13 |
> | backtrace:acpi_early_processor_set_pdc| 6  | 22 
> | 13 |
> | backtrace:acpi_init   | 6  | 22 
> | 13 |
> | backtrace:kernel_init_freeable| 6  | 22 
> | 13 |
> | Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 1  |
> ||
> | backtrace:vfs_fstatat | 2  |
> ||
> | backtrace:SyS_fstatat64   | 2  |
> ||
> | backtrace:SYSC_socketcall | 2  |
> ||
> | backtrace:SyS_socketcall  | 2  | 0  
> | 6  |
> | WARNING:at_kernel/rcu/rcuperf.c:#rcu_perf_writer  | 0  | 22 
> | 13 |
> | BUG:spinlock_bad_magic_on_CPU | 0  | 22 
> ||
> | BUG:spinlock_lockup_suspected_on_CPU  | 0  | 22 
> ||
> | EIP_is_at__wake_up_common | 0  | 22 
> ||
> | backtrace:rcu_perf_writer | 0  | 22 
> | 13 |
> | backtrace:sock_setsockopt | 0  | 0  
> | 6  |
> | backtrace:rht_deferred_worker | 0  | 0  
> | 3  |
> +---++++
> 
> [1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.6.0-rc1-5-g8704baa #3
> [1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.6.0-rc1-5-g8704baa #3
> [1.062485]  
> [1.062485]   cf03bc70 cf03bc70 cf03bc50 cf03bc50 c15192fc 
> c15192fc cf03bc5c cf03bc5c c157a45b c157a45b c1ed4940 c1ed4940 cf03bcac 
> cf03bcac
> 
> [1.064620]  c157aa41
> [1.064620]  c157aa41 c1bba04c c1bba04c cf03bc74 cf03bc74 c1ed4958 
> c1ed4958 0202 0202 c100312d c100312d cf03bc80 cf03bc80 c10ab3fb 
> c10ab3fb
> 
> [1.066740]  cf03bc90
> [1.066740]  cf03bc90 0203 0203 cf0a8634 cf0a8634 0382 
> 0382 cf03bcc4 cf03bcc4 c11a701a c11a701a 0010 0010 eb0e29d5 
> eb0e29d5
> 
> [1.068833] Call Trace:
> [1.068833] Call Trace:
> [1.072825]  [] dump_stack+0x16/0x1a
> [1.072825]  [] dump_stack+0x16/0x1a
> [1.073958]  [] ubsan_epilogue+0xb/0x40
> [1.073958]  [] ubsan_epilogue+0xb/0x40
> [1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
> [1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
> [1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
> [1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
> [1.081379]  [] ? trace_hardirqs_on+0xb/0x10
> [1.081379]  [] ? 

Re: [rcutorture] 8704baab9b: WARNING: CPU: 0 PID: 30 at kernel/rcu/rcuperf.c:363 rcu_perf_writer

[Paul E. McKenney]

On Sun, May 22, 2016 at 10:36:00AM +0800, kernel test robot wrote:
> Greetings,
> 
> 0day kernel testing robot got the below dmesg and the first bad commit is
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> 
> commit 8704baab9bc848b58c129fed6b591bb84ec02f41
> Author: Paul E. McKenney 
> AuthorDate: Thu Dec 31 18:33:22 2015 -0800
> Commit: Paul E. McKenney 
> CommitDate: Thu Mar 31 13:37:38 2016 -0700
> 
> rcutorture: Add RCU grace-period performance tests
> 
> This commit adds a new rcuperf module that carries out simple performance
> tests of RCU grace periods.
> 
> Signed-off-by: Paul E. McKenney 

???

This commit adds a default-n performance-test module.  I don't believe
that this would result in boot failures.  False bisection?

Thanx, Paul

> +---++++
> |   | 291783b8ad | 
> 8704baab9b | ce82e4a05f |
> +---++++
> | boot_successes| 57 | 0  
> | 0  |
> | boot_failures | 6  | 22 
> | 13 |
> | BUG:unable_to_handle_kernel   | 6  | 22 
> ||
> | Oops  | 6  | 22 
> ||
> | EIP_is_at_get_perf_callchain  | 6  |
> ||
> | Kernel_panic-not_syncing:Fatal_exception  | 5  | 22 
> ||
> | backtrace:acpi_get_cpuid  | 6  | 22 
> | 13 |
> | backtrace:early_init_pdc  | 6  | 22 
> | 13 |
> | backtrace:acpi_early_processor_set_pdc| 6  | 22 
> | 13 |
> | backtrace:acpi_init   | 6  | 22 
> | 13 |
> | backtrace:kernel_init_freeable| 6  | 22 
> | 13 |
> | Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 1  |
> ||
> | backtrace:vfs_fstatat | 2  |
> ||
> | backtrace:SyS_fstatat64   | 2  |
> ||
> | backtrace:SYSC_socketcall | 2  |
> ||
> | backtrace:SyS_socketcall  | 2  | 0  
> | 6  |
> | WARNING:at_kernel/rcu/rcuperf.c:#rcu_perf_writer  | 0  | 22 
> | 13 |
> | BUG:spinlock_bad_magic_on_CPU | 0  | 22 
> ||
> | BUG:spinlock_lockup_suspected_on_CPU  | 0  | 22 
> ||
> | EIP_is_at__wake_up_common | 0  | 22 
> ||
> | backtrace:rcu_perf_writer | 0  | 22 
> | 13 |
> | backtrace:sock_setsockopt | 0  | 0  
> | 6  |
> | backtrace:rht_deferred_worker | 0  | 0  
> | 3  |
> +---++++
> 
> [1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.6.0-rc1-5-g8704baa #3
> [1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.6.0-rc1-5-g8704baa #3
> [1.062485]  
> [1.062485]   cf03bc70 cf03bc70 cf03bc50 cf03bc50 c15192fc 
> c15192fc cf03bc5c cf03bc5c c157a45b c157a45b c1ed4940 c1ed4940 cf03bcac 
> cf03bcac
> 
> [1.064620]  c157aa41
> [1.064620]  c157aa41 c1bba04c c1bba04c cf03bc74 cf03bc74 c1ed4958 
> c1ed4958 0202 0202 c100312d c100312d cf03bc80 cf03bc80 c10ab3fb 
> c10ab3fb
> 
> [1.066740]  cf03bc90
> [1.066740]  cf03bc90 0203 0203 cf0a8634 cf0a8634 0382 
> 0382 cf03bcc4 cf03bcc4 c11a701a c11a701a 0010 0010 eb0e29d5 
> eb0e29d5
> 
> [1.068833] Call Trace:
> [1.068833] Call Trace:
> [1.072825]  [] dump_stack+0x16/0x1a
> [1.072825]  [] dump_stack+0x16/0x1a
> [1.073958]  [] ubsan_epilogue+0xb/0x40
> [1.073958]  [] ubsan_epilogue+0xb/0x40
> [1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
> [1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
> [1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
> [1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
> [1.081379]  [] ? trace_hardirqs_on+0xb/0x10
> [1.081379]  [] ? trace_hardirqs_on+0xb/0x10
> [1.086052]  [] ? slob_free+0x15a/0xa00
> [1.086052]  [] ? 

[PATCH v3] Drivers: hv: vmbus: fix the race when querying & updating the percpu list

[Dexuan Cui]

There is a rare race when we remove an entry from the global list
hv_context.percpu_list[cpu] in hv_process_channel_removal() ->
percpu_channel_deq() -> list_del(): at this time, if vmbus_on_event() ->
process_chn_event() -> pcpu_relid2channel() is trying to query the list,
we can get the kernel fault.

Similarly, we also have the issue in the code path: vmbus_process_offer() ->
percpu_channel_enq().

We can resolve the issue by disabling the tasklet when updating the list.

The patch also moves vmbus_release_relid() to a later place where
the channel has been removed from the per-cpu and the global lists.

Reported-by: Rolf Neugebauer 
Cc: Vitaly Kuznetsov 
Signed-off-by: Dexuan Cui 
---

v2: added tasklet_schedule() after tasklet_enable(). Thanks, Vitaly!

v3: I shouldn't have moved percpu_channel_deq()
from  hv_process_channel_removal() to vmbus_close_internal(): the
channel couldn't be removed from the per-cpu list, if the channel state
was not CHANNEL_OPENED_STATE. v3 fixed this.

v3 also added 2 wrapper functions for the disable/enable stuff.
v3 also moved vmbus_release_relid() to a later safe place.

You can also get v3 by
https://github.com/dcui/linux/commit/2f314fb9352020f70b094bf31539afd3a18a5545

 drivers/hv/channel.c  |  6 ++
 drivers/hv/channel_mgmt.c | 32 
 include/linux/hyperv.h|  3 +++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 56dd261..ef8e9b5 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -512,7 +512,6 @@ static void reset_channel_cb(void *arg)
 static int vmbus_close_internal(struct vmbus_channel *channel)
 {
struct vmbus_channel_close_channel *msg;
-   struct tasklet_struct *tasklet;
int ret;
 
/*
@@ -524,8 +523,7 @@ static int vmbus_close_internal(struct vmbus_channel 
*channel)
 * To resolve the race, we can serialize them by disabling the
 * tasklet when the latter is running here.
 */
-   tasklet = hv_context.event_dpc[channel->target_cpu];
-   tasklet_disable(tasklet);
+   hv_event_tasklet_disable(channel);
 
/*
 * In case a device driver's probe() fails (e.g.,
@@ -591,7 +589,7 @@ static int vmbus_close_internal(struct vmbus_channel 
*channel)
get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
 
 out:
-   tasklet_enable(tasklet);
+   hv_event_tasklet_enable(channel);
 
return ret;
 }
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 38b682ba..3bcf141 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -303,16 +304,32 @@ static void vmbus_release_relid(u32 relid)
vmbus_post_msg(, sizeof(struct vmbus_channel_relid_released));
 }
 
+void hv_event_tasklet_disable(struct vmbus_channel *channel)
+{
+   struct tasklet_struct *tasklet;
+   tasklet = hv_context.event_dpc[channel->target_cpu];
+   tasklet_disable(tasklet);
+}
+
+void hv_event_tasklet_enable(struct vmbus_channel *channel)
+{
+   struct tasklet_struct *tasklet;
+   tasklet = hv_context.event_dpc[channel->target_cpu];
+   tasklet_enable(tasklet);
+
+   /* In case there is any pending event */
+   tasklet_schedule(tasklet);
+}
+
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 {
unsigned long flags;
struct vmbus_channel *primary_channel;
 
-   vmbus_release_relid(relid);
-
BUG_ON(!channel->rescind);
BUG_ON(!mutex_is_locked(_connection.channel_mutex));
 
+   hv_event_tasklet_disable(channel);
if (channel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(channel->target_cpu,
@@ -321,6 +338,7 @@ void hv_process_channel_removal(struct vmbus_channel 
*channel, u32 relid)
percpu_channel_deq(channel);
put_cpu();
}
+   hv_event_tasklet_enable(channel);
 
if (channel->primary_channel == NULL) {
list_del(>listentry);
@@ -341,6 +359,8 @@ void hv_process_channel_removal(struct vmbus_channel 
*channel, u32 relid)
cpumask_clear_cpu(channel->target_cpu,
  _channel->alloced_cpus_in_node);
 
+   vmbus_release_relid(relid);
+
free_channel(channel);
 }
 
@@ -409,6 +429,7 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
 
init_vp_index(newchannel, dev_type);
 
+   hv_event_tasklet_disable(channel);
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -418,6 +439,7 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
percpu_channel_enq(newchannel);
  

[PATCH v3] Drivers: hv: vmbus: fix the race when querying & updating the percpu list

[Dexuan Cui]

There is a rare race when we remove an entry from the global list
hv_context.percpu_list[cpu] in hv_process_channel_removal() ->
percpu_channel_deq() -> list_del(): at this time, if vmbus_on_event() ->
process_chn_event() -> pcpu_relid2channel() is trying to query the list,
we can get the kernel fault.

Similarly, we also have the issue in the code path: vmbus_process_offer() ->
percpu_channel_enq().

We can resolve the issue by disabling the tasklet when updating the list.

The patch also moves vmbus_release_relid() to a later place where
the channel has been removed from the per-cpu and the global lists.

Reported-by: Rolf Neugebauer 
Cc: Vitaly Kuznetsov 
Signed-off-by: Dexuan Cui 
---

v2: added tasklet_schedule() after tasklet_enable(). Thanks, Vitaly!

v3: I shouldn't have moved percpu_channel_deq()
from  hv_process_channel_removal() to vmbus_close_internal(): the
channel couldn't be removed from the per-cpu list, if the channel state
was not CHANNEL_OPENED_STATE. v3 fixed this.

v3 also added 2 wrapper functions for the disable/enable stuff.
v3 also moved vmbus_release_relid() to a later safe place.

You can also get v3 by
https://github.com/dcui/linux/commit/2f314fb9352020f70b094bf31539afd3a18a5545

 drivers/hv/channel.c  |  6 ++
 drivers/hv/channel_mgmt.c | 32 
 include/linux/hyperv.h|  3 +++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 56dd261..ef8e9b5 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -512,7 +512,6 @@ static void reset_channel_cb(void *arg)
 static int vmbus_close_internal(struct vmbus_channel *channel)
 {
struct vmbus_channel_close_channel *msg;
-   struct tasklet_struct *tasklet;
int ret;
 
/*
@@ -524,8 +523,7 @@ static int vmbus_close_internal(struct vmbus_channel 
*channel)
 * To resolve the race, we can serialize them by disabling the
 * tasklet when the latter is running here.
 */
-   tasklet = hv_context.event_dpc[channel->target_cpu];
-   tasklet_disable(tasklet);
+   hv_event_tasklet_disable(channel);
 
/*
 * In case a device driver's probe() fails (e.g.,
@@ -591,7 +589,7 @@ static int vmbus_close_internal(struct vmbus_channel 
*channel)
get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
 
 out:
-   tasklet_enable(tasklet);
+   hv_event_tasklet_enable(channel);
 
return ret;
 }
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 38b682ba..3bcf141 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -303,16 +304,32 @@ static void vmbus_release_relid(u32 relid)
vmbus_post_msg(, sizeof(struct vmbus_channel_relid_released));
 }
 
+void hv_event_tasklet_disable(struct vmbus_channel *channel)
+{
+   struct tasklet_struct *tasklet;
+   tasklet = hv_context.event_dpc[channel->target_cpu];
+   tasklet_disable(tasklet);
+}
+
+void hv_event_tasklet_enable(struct vmbus_channel *channel)
+{
+   struct tasklet_struct *tasklet;
+   tasklet = hv_context.event_dpc[channel->target_cpu];
+   tasklet_enable(tasklet);
+
+   /* In case there is any pending event */
+   tasklet_schedule(tasklet);
+}
+
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 {
unsigned long flags;
struct vmbus_channel *primary_channel;
 
-   vmbus_release_relid(relid);
-
BUG_ON(!channel->rescind);
BUG_ON(!mutex_is_locked(_connection.channel_mutex));
 
+   hv_event_tasklet_disable(channel);
if (channel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(channel->target_cpu,
@@ -321,6 +338,7 @@ void hv_process_channel_removal(struct vmbus_channel 
*channel, u32 relid)
percpu_channel_deq(channel);
put_cpu();
}
+   hv_event_tasklet_enable(channel);
 
if (channel->primary_channel == NULL) {
list_del(>listentry);
@@ -341,6 +359,8 @@ void hv_process_channel_removal(struct vmbus_channel 
*channel, u32 relid)
cpumask_clear_cpu(channel->target_cpu,
  _channel->alloced_cpus_in_node);
 
+   vmbus_release_relid(relid);
+
free_channel(channel);
 }
 
@@ -409,6 +429,7 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
 
init_vp_index(newchannel, dev_type);
 
+   hv_event_tasklet_disable(channel);
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -418,6 +439,7 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
percpu_channel_enq(newchannel);
put_cpu();
}
+   

[rcutorture] 8704baab9b: WARNING: CPU: 0 PID: 30 at kernel/rcu/rcuperf.c:363 rcu_perf_writer

[kernel test robot]

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

commit 8704baab9bc848b58c129fed6b591bb84ec02f41
Author: Paul E. McKenney 
AuthorDate: Thu Dec 31 18:33:22 2015 -0800
Commit: Paul E. McKenney 
CommitDate: Thu Mar 31 13:37:38 2016 -0700

rcutorture: Add RCU grace-period performance tests

This commit adds a new rcuperf module that carries out simple performance
tests of RCU grace periods.

Signed-off-by: Paul E. McKenney 

+---++++
|   | 291783b8ad | 
8704baab9b | ce82e4a05f |
+---++++
| boot_successes| 57 | 0
  | 0  |
| boot_failures | 6  | 22   
  | 13 |
| BUG:unable_to_handle_kernel   | 6  | 22   
  ||
| Oops  | 6  | 22   
  ||
| EIP_is_at_get_perf_callchain  | 6  |  
  ||
| Kernel_panic-not_syncing:Fatal_exception  | 5  | 22   
  ||
| backtrace:acpi_get_cpuid  | 6  | 22   
  | 13 |
| backtrace:early_init_pdc  | 6  | 22   
  | 13 |
| backtrace:acpi_early_processor_set_pdc| 6  | 22   
  | 13 |
| backtrace:acpi_init   | 6  | 22   
  | 13 |
| backtrace:kernel_init_freeable| 6  | 22   
  | 13 |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 1  |  
  ||
| backtrace:vfs_fstatat | 2  |  
  ||
| backtrace:SyS_fstatat64   | 2  |  
  ||
| backtrace:SYSC_socketcall | 2  |  
  ||
| backtrace:SyS_socketcall  | 2  | 0
  | 6  |
| WARNING:at_kernel/rcu/rcuperf.c:#rcu_perf_writer  | 0  | 22   
  | 13 |
| BUG:spinlock_bad_magic_on_CPU | 0  | 22   
  ||
| BUG:spinlock_lockup_suspected_on_CPU  | 0  | 22   
  ||
| EIP_is_at__wake_up_common | 0  | 22   
  ||
| backtrace:rcu_perf_writer | 0  | 22   
  | 13 |
| backtrace:sock_setsockopt | 0  | 0
  | 6  |
| backtrace:rht_deferred_worker | 0  | 0
  | 3  |
+---++++

[1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 4.6.0-rc1-5-g8704baa 
#3
[1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 4.6.0-rc1-5-g8704baa 
#3
[1.062485]  
[1.062485]   cf03bc70 cf03bc70 cf03bc50 cf03bc50 c15192fc c15192fc 
cf03bc5c cf03bc5c c157a45b c157a45b c1ed4940 c1ed4940 cf03bcac cf03bcac

[1.064620]  c157aa41
[1.064620]  c157aa41 c1bba04c c1bba04c cf03bc74 cf03bc74 c1ed4958 c1ed4958 
0202 0202 c100312d c100312d cf03bc80 cf03bc80 c10ab3fb c10ab3fb

[1.066740]  cf03bc90
[1.066740]  cf03bc90 0203 0203 cf0a8634 cf0a8634 0382 0382 
cf03bcc4 cf03bcc4 c11a701a c11a701a 0010 0010 eb0e29d5 eb0e29d5

[1.068833] Call Trace:
[1.068833] Call Trace:
[1.072825]  [] dump_stack+0x16/0x1a
[1.072825]  [] dump_stack+0x16/0x1a
[1.073958]  [] ubsan_epilogue+0xb/0x40
[1.073958]  [] ubsan_epilogue+0xb/0x40
[1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
[1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
[1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
[1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
[1.081379]  [] ? trace_hardirqs_on+0xb/0x10
[1.081379]  [] ? trace_hardirqs_on+0xb/0x10
[1.086052]  [] ? slob_free+0x15a/0xa00
[1.086052]  [] ? slob_free+0x15a/0xa00
[1.087383]  [] acpi_ds_create_operand+0x20b/0x294
[1.087383]  [] acpi_ds_create_operand+0x20b/0x294
[1.088807]  [] ? __kmem_cache_free+0x3d/0x60
[1.088807]  [] ? __kmem_cache_free+0x3d/0x60
[1.095377]  [] acpi_ds_create_operands+0xf7/0x139
[1.095377]  [] acpi_ds_create_operands+0xf7/0x139
[1.096857]  [] ? acpi_os_release_object+0x8/0xc
[1.096857] 

[rcutorture] 8704baab9b: WARNING: CPU: 0 PID: 30 at kernel/rcu/rcuperf.c:363 rcu_perf_writer

[kernel test robot]

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

commit 8704baab9bc848b58c129fed6b591bb84ec02f41
Author: Paul E. McKenney 
AuthorDate: Thu Dec 31 18:33:22 2015 -0800
Commit: Paul E. McKenney 
CommitDate: Thu Mar 31 13:37:38 2016 -0700

rcutorture: Add RCU grace-period performance tests

This commit adds a new rcuperf module that carries out simple performance
tests of RCU grace periods.

Signed-off-by: Paul E. McKenney 

+---++++
|   | 291783b8ad | 
8704baab9b | ce82e4a05f |
+---++++
| boot_successes| 57 | 0
  | 0  |
| boot_failures | 6  | 22   
  | 13 |
| BUG:unable_to_handle_kernel   | 6  | 22   
  ||
| Oops  | 6  | 22   
  ||
| EIP_is_at_get_perf_callchain  | 6  |  
  ||
| Kernel_panic-not_syncing:Fatal_exception  | 5  | 22   
  ||
| backtrace:acpi_get_cpuid  | 6  | 22   
  | 13 |
| backtrace:early_init_pdc  | 6  | 22   
  | 13 |
| backtrace:acpi_early_processor_set_pdc| 6  | 22   
  | 13 |
| backtrace:acpi_init   | 6  | 22   
  | 13 |
| backtrace:kernel_init_freeable| 6  | 22   
  | 13 |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 1  |  
  ||
| backtrace:vfs_fstatat | 2  |  
  ||
| backtrace:SyS_fstatat64   | 2  |  
  ||
| backtrace:SYSC_socketcall | 2  |  
  ||
| backtrace:SyS_socketcall  | 2  | 0
  | 6  |
| WARNING:at_kernel/rcu/rcuperf.c:#rcu_perf_writer  | 0  | 22   
  | 13 |
| BUG:spinlock_bad_magic_on_CPU | 0  | 22   
  ||
| BUG:spinlock_lockup_suspected_on_CPU  | 0  | 22   
  ||
| EIP_is_at__wake_up_common | 0  | 22   
  ||
| backtrace:rcu_perf_writer | 0  | 22   
  | 13 |
| backtrace:sock_setsockopt | 0  | 0
  | 6  |
| backtrace:rht_deferred_worker | 0  | 0
  | 3  |
+---++++

[1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 4.6.0-rc1-5-g8704baa 
#3
[1.054065] CPU: 0 PID: 1 Comm: swapper Not tainted 4.6.0-rc1-5-g8704baa 
#3
[1.062485]  
[1.062485]   cf03bc70 cf03bc70 cf03bc50 cf03bc50 c15192fc c15192fc 
cf03bc5c cf03bc5c c157a45b c157a45b c1ed4940 c1ed4940 cf03bcac cf03bcac

[1.064620]  c157aa41
[1.064620]  c157aa41 c1bba04c c1bba04c cf03bc74 cf03bc74 c1ed4958 c1ed4958 
0202 0202 c100312d c100312d cf03bc80 cf03bc80 c10ab3fb c10ab3fb

[1.066740]  cf03bc90
[1.066740]  cf03bc90 0203 0203 cf0a8634 cf0a8634 0382 0382 
cf03bcc4 cf03bcc4 c11a701a c11a701a 0010 0010 eb0e29d5 eb0e29d5

[1.068833] Call Trace:
[1.068833] Call Trace:
[1.072825]  [] dump_stack+0x16/0x1a
[1.072825]  [] dump_stack+0x16/0x1a
[1.073958]  [] ubsan_epilogue+0xb/0x40
[1.073958]  [] ubsan_epilogue+0xb/0x40
[1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
[1.075151]  [] __ubsan_handle_out_of_bounds+0x61/0x80
[1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
[1.080046]  [] ? allocate_fake_cpuc+0x7d/0x90
[1.081379]  [] ? trace_hardirqs_on+0xb/0x10
[1.081379]  [] ? trace_hardirqs_on+0xb/0x10
[1.086052]  [] ? slob_free+0x15a/0xa00
[1.086052]  [] ? slob_free+0x15a/0xa00
[1.087383]  [] acpi_ds_create_operand+0x20b/0x294
[1.087383]  [] acpi_ds_create_operand+0x20b/0x294
[1.088807]  [] ? __kmem_cache_free+0x3d/0x60
[1.088807]  [] ? __kmem_cache_free+0x3d/0x60
[1.095377]  [] acpi_ds_create_operands+0xf7/0x139
[1.095377]  [] acpi_ds_create_operands+0xf7/0x139
[1.096857]  [] ? acpi_os_release_object+0x8/0xc
[1.096857]  [] ? acpi_os_release_object+0x8/0xc
[1.098240]  [] ? 

Re: [PATCH 1/4] x86: Save return value from kernel_thread

[Brian Gerst]

On Sat, May 21, 2016 at 9:44 PM, Andy Lutomirski  wrote:
> On Sat, May 21, 2016 at 9:04 AM, Brian Gerst  wrote:
>> Kernel threads should always return zero on success after calling 
>> do_execve().  The
>> two existing cases in the kernel (kernel_init() and 
>> call_usermodehelper_exec_async())
>> correctly do this.  Save a few bytes by storing EAX/RAX instead of an 
>> immediate zero.
>> Also fix the 64-bit case which should save the full 64-bits.
>
> Does this have any additional motivation beyond cleanup and fixing an
> inconsequential bug?  I.e. does the rest of your series need this?
>
> --Andy

It's just a minor cleanup, not necessary.

--
Brian Gerst

Re: [PATCH 1/4] x86: Save return value from kernel_thread

[Brian Gerst]

On Sat, May 21, 2016 at 9:44 PM, Andy Lutomirski  wrote:
> On Sat, May 21, 2016 at 9:04 AM, Brian Gerst  wrote:
>> Kernel threads should always return zero on success after calling 
>> do_execve().  The
>> two existing cases in the kernel (kernel_init() and 
>> call_usermodehelper_exec_async())
>> correctly do this.  Save a few bytes by storing EAX/RAX instead of an 
>> immediate zero.
>> Also fix the 64-bit case which should save the full 64-bits.
>
> Does this have any additional motivation beyond cleanup and fixing an
> inconsequential bug?  I.e. does the rest of your series need this?
>
> --Andy

It's just a minor cleanup, not necessary.

--
Brian Gerst

Re: [PATCH 1/4] x86: Save return value from kernel_thread

[Andy Lutomirski]

On Sat, May 21, 2016 at 9:04 AM, Brian Gerst  wrote:
> Kernel threads should always return zero on success after calling 
> do_execve().  The
> two existing cases in the kernel (kernel_init() and 
> call_usermodehelper_exec_async())
> correctly do this.  Save a few bytes by storing EAX/RAX instead of an 
> immediate zero.
> Also fix the 64-bit case which should save the full 64-bits.

Does this have any additional motivation beyond cleanup and fixing an
inconsequential bug?  I.e. does the rest of your series need this?

--Andy

Re: [PATCH 1/4] x86: Save return value from kernel_thread

[Andy Lutomirski]

On Sat, May 21, 2016 at 9:04 AM, Brian Gerst  wrote:
> Kernel threads should always return zero on success after calling 
> do_execve().  The
> two existing cases in the kernel (kernel_init() and 
> call_usermodehelper_exec_async())
> correctly do this.  Save a few bytes by storing EAX/RAX instead of an 
> immediate zero.
> Also fix the 64-bit case which should save the full 64-bits.

Does this have any additional motivation beyond cleanup and fixing an
inconsequential bug?  I.e. does the rest of your series need this?

--Andy

[GIT PULL] f2fs for 4.7-rc1

[Jaegeuk Kim]

Hi Linus,

In this round, as Ted pointed out, fscrypto allows one more key prefix given by
filesystem to resolve backward compatibility issue. Other than that, we could
fix several error handling flow by introducing fault injection facility. We've
also achieved performance improvement in some workloads as well as a bunch of
bug fixes.

Could you consider pulling the below patches?

Thanks,

The following changes since commit 806fdcce017dc98c4dbf8ed001750a0d7d2bb0af:

  Merge branch 'x86-urgent-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2016-04-14 19:53:46 
-0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
tags/for-f2fs-4.7

for you to fetch changes up to 0f3311a8c266b9f4fae4e5cdfcd9a86970e2b9bd:

  f2fs: fix to update dirty page count correctly (2016-05-20 14:55:41 -0700)


Enhancement
- fs-specific prefix for fscrypto
- fault injection facility
- expose validity bitmaps for user to be aware of fragmentation
- fallocate/rm/preallocation speed up
- use percpu counters

Bug fixes
- some inline_dentry/inline_data bugs
- error handling for atomic/volatile/orphan inodes
- recover broken superblock


Chao Yu (22):
  f2fs: fix to convert inline directory correctly
  MAINTAINERS: update my email address
  f2fs: be aware of invalid filename length
  f2fs: move node pages only in victim section during GC
  f2fs: fix to clear private data in page
  f2fs: fix to clear page private flag
  f2fs: factor out fsync inode entry operations
  f2fs: remove unneeded readahead in find_fsync_dnodes
  f2fs: remove unneeded memset when updating xattr
  f2fs: reuse get_extent_info
  f2fs: shrink size of struct seg_entry
  f2fs: fix incorrect mapping in ->bmap
  f2fs: avoid panic when truncating to max filesize
  f2fs: fix inode cache leak
  f2fs: use mnt_{want,drop}_write_file in ioctl
  f2fs: make atomic/volatile operation exclusive
  f2fs: support in batch multi blocks preallocation
  f2fs: support in batch fzero in dnode page
  f2fs: fix deadlock when flush inline data
  f2fs: fix i_current_depth during inline dentry conversion
  f2fs: fix incorrect error path handling in f2fs_move_rehashed_dirents
  f2fs: fix to update dirty page count correctly

Jaegeuk Kim (46):
  f2fs: give RO message when recovering superblock
  f2fs: recover superblock at RW remounts
  f2fs: give -EINVAL for norecovery and rw mount
  f2fs: treat as a normal umount when remounting ro
  f2fs: show current mount status
  f2fs: use PGP_LOCK to check its truncation
  f2fs: add BUG_ON to avoid unnecessary flow
  f2fs: fix dropping inmemory pages in a wrong time
  f2fs: unset atomic/volatile flag in f2fs_release_file
  f2fs: remove redundant condition check
  f2fs: give -E2BIG for no space in xattr
  f2fs: don't invalidate atomic page if successful
  f2fs: flush dirty pages before starting atomic writes
  f2fs: avoid needless lock for node pages when fsyncing a file
  f2fs: avoid writing 0'th page in volatile writes
  f2fs: split sync_node_pages with fsync_node_pages
  f2fs: report unwritten status in fsync_node_pages
  f2fs: set fsync mark only for the last dnode
  f2fs: issue cache flush on direct IO
  f2fs: introduce macros for proc entries
  f2fs: add proc entry to show valid block bitmap
  f2fs: introduce f2fs_kmalloc to wrap kmalloc
  f2fs: use f2fs_grab_cache_page instead of grab_cache_page
  f2fs: add mount option to select fault injection ratio
  f2fs: inject kmalloc failure
  f2fs: inject page allocation failures
  f2fs: inject ENOSPC failures
  f2fs: revisit error handling flows
  f2fs: fix leak of orphan inode objects
  f2fs: retry to truncate blocks in -ENOMEM case
  f2fs: don't worry about inode leak in evict_inode
  f2fs: remove an obsolete variable
  fscrypto/f2fs: allow fs-specific key prefix for fs encryption
  f2fs: fallocate data blocks in single locked node page
  f2fs: read node blocks ahead when truncating blocks
  f2fs: do not preallocate block unaligned to 4KB
  f2fs: show # of orphan inodes
  f2fs: avoid f2fs_bug_on during recovery
  f2fs: manipulate dirty file inodes when DATA_FLUSH is set
  f2fs: use bio count instead of F2FS_WRITEBACK page count
  f2fs: use percpu_counter for page counters
  f2fs: use percpu_counter for # of dirty pages in inode
  f2fs: use percpu_counter for alloc_valid_block_count
  f2fs: use percpu_counter for total_valid_inode_count
  f2fs: avoid ENOSPC fault in the recovery process
  f2fs: flush pending bios right away when error occurs

Sheng Yong (2):
  f2fs: correct return value type of f2fs_fill_super
  f2fs: add fault 

[GIT PULL] f2fs for 4.7-rc1

[Jaegeuk Kim]

Hi Linus,

In this round, as Ted pointed out, fscrypto allows one more key prefix given by
filesystem to resolve backward compatibility issue. Other than that, we could
fix several error handling flow by introducing fault injection facility. We've
also achieved performance improvement in some workloads as well as a bunch of
bug fixes.

Could you consider pulling the below patches?

Thanks,

The following changes since commit 806fdcce017dc98c4dbf8ed001750a0d7d2bb0af:

  Merge branch 'x86-urgent-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2016-04-14 19:53:46 
-0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
tags/for-f2fs-4.7

for you to fetch changes up to 0f3311a8c266b9f4fae4e5cdfcd9a86970e2b9bd:

  f2fs: fix to update dirty page count correctly (2016-05-20 14:55:41 -0700)


Enhancement
- fs-specific prefix for fscrypto
- fault injection facility
- expose validity bitmaps for user to be aware of fragmentation
- fallocate/rm/preallocation speed up
- use percpu counters

Bug fixes
- some inline_dentry/inline_data bugs
- error handling for atomic/volatile/orphan inodes
- recover broken superblock


Chao Yu (22):
  f2fs: fix to convert inline directory correctly
  MAINTAINERS: update my email address
  f2fs: be aware of invalid filename length
  f2fs: move node pages only in victim section during GC
  f2fs: fix to clear private data in page
  f2fs: fix to clear page private flag
  f2fs: factor out fsync inode entry operations
  f2fs: remove unneeded readahead in find_fsync_dnodes
  f2fs: remove unneeded memset when updating xattr
  f2fs: reuse get_extent_info
  f2fs: shrink size of struct seg_entry
  f2fs: fix incorrect mapping in ->bmap
  f2fs: avoid panic when truncating to max filesize
  f2fs: fix inode cache leak
  f2fs: use mnt_{want,drop}_write_file in ioctl
  f2fs: make atomic/volatile operation exclusive
  f2fs: support in batch multi blocks preallocation
  f2fs: support in batch fzero in dnode page
  f2fs: fix deadlock when flush inline data
  f2fs: fix i_current_depth during inline dentry conversion
  f2fs: fix incorrect error path handling in f2fs_move_rehashed_dirents
  f2fs: fix to update dirty page count correctly

Jaegeuk Kim (46):
  f2fs: give RO message when recovering superblock
  f2fs: recover superblock at RW remounts
  f2fs: give -EINVAL for norecovery and rw mount
  f2fs: treat as a normal umount when remounting ro
  f2fs: show current mount status
  f2fs: use PGP_LOCK to check its truncation
  f2fs: add BUG_ON to avoid unnecessary flow
  f2fs: fix dropping inmemory pages in a wrong time
  f2fs: unset atomic/volatile flag in f2fs_release_file
  f2fs: remove redundant condition check
  f2fs: give -E2BIG for no space in xattr
  f2fs: don't invalidate atomic page if successful
  f2fs: flush dirty pages before starting atomic writes
  f2fs: avoid needless lock for node pages when fsyncing a file
  f2fs: avoid writing 0'th page in volatile writes
  f2fs: split sync_node_pages with fsync_node_pages
  f2fs: report unwritten status in fsync_node_pages
  f2fs: set fsync mark only for the last dnode
  f2fs: issue cache flush on direct IO
  f2fs: introduce macros for proc entries
  f2fs: add proc entry to show valid block bitmap
  f2fs: introduce f2fs_kmalloc to wrap kmalloc
  f2fs: use f2fs_grab_cache_page instead of grab_cache_page
  f2fs: add mount option to select fault injection ratio
  f2fs: inject kmalloc failure
  f2fs: inject page allocation failures
  f2fs: inject ENOSPC failures
  f2fs: revisit error handling flows
  f2fs: fix leak of orphan inode objects
  f2fs: retry to truncate blocks in -ENOMEM case
  f2fs: don't worry about inode leak in evict_inode
  f2fs: remove an obsolete variable
  fscrypto/f2fs: allow fs-specific key prefix for fs encryption
  f2fs: fallocate data blocks in single locked node page
  f2fs: read node blocks ahead when truncating blocks
  f2fs: do not preallocate block unaligned to 4KB
  f2fs: show # of orphan inodes
  f2fs: avoid f2fs_bug_on during recovery
  f2fs: manipulate dirty file inodes when DATA_FLUSH is set
  f2fs: use bio count instead of F2FS_WRITEBACK page count
  f2fs: use percpu_counter for page counters
  f2fs: use percpu_counter for # of dirty pages in inode
  f2fs: use percpu_counter for alloc_valid_block_count
  f2fs: use percpu_counter for total_valid_inode_count
  f2fs: avoid ENOSPC fault in the recovery process
  f2fs: flush pending bios right away when error occurs

Sheng Yong (2):
  f2fs: correct return value type of f2fs_fill_super
  f2fs: add fault 

Re: [PATCH] dell-smm-hwmon: Detect fan with index=2

[Pali Rohár]

On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote:
> On 05/21/2016 07:52 AM, Pali Rohár wrote:
> > Some Dell machines (e.g. Dell Precision M3800) have two fans, first
> > with index=0 and second with index=2. So export also attributes
> > for third fan device with index=2.
> > 
> > Reported-by: Tolga Cakir 
> > Signed-off-by: Pali Rohár 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   20 +++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > ---
> > 
> > Hi Tolga! Can you test this patch if sensors see fan device
> > correctly?
> 
> I assume this means I should wait for test results before applying
> the patch ?

Yes, testing should be done.

Do you have some pending/testing/next tree/branch for such patches?

> Thanks,
> Guenter
> 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
> > 
> >   #define I8K_HWMON_HAVE_TEMP4  (1 << 3)
> >   #define I8K_HWMON_HAVE_FAN1   (1 << 4)
> >   #define I8K_HWMON_HAVE_FAN2   (1 << 5)
> > 
> > +#define I8K_HWMON_HAVE_FAN3(1 << 6)
> > 
> >   MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
> >   MODULE_AUTHOR("Pali Rohár ");
> > 
> > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan)
> > 
> >   static int i8k_get_fan_type(int fan)
> >   {
> >   
> > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values
> > */
> > 
> > -   static int types[2] = { INT_MIN, INT_MIN };
> > +   static int types[3] = { INT_MIN, INT_MIN, INT_MIN };
> > 
> > if (types[fan] == INT_MIN)
> > 
> > types[fan] = _i8k_get_fan_type(fan);
> > 
> > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL,
> > 
> >   1);
> >   
> >   static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR,
> >   i8k_hwmon_show_pwm,
> >   
> >   i8k_hwmon_set_pwm, 1);
> > 
> > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan,
> > NULL, +   2);
> > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL, + 2);
> > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR,
> > i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2);
> > 
> >   static struct attribute *i8k_attrs[] = {
> >   
> > _dev_attr_temp1_input.dev_attr.attr, /* 0 */
> > 
> > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
> > 
> > _dev_attr_fan2_input.dev_attr.attr,  /* 11 */
> > _dev_attr_fan2_label.dev_attr.attr,  /* 12 */
> > _dev_attr_pwm2.dev_attr.attr,/* 13 */
> > 
> > +   _dev_attr_fan3_input.dev_attr.attr,  /* 14 */
> > +   _dev_attr_fan3_label.dev_attr.attr,  /* 15 */
> > +   _dev_attr_pwm3.dev_attr.attr,/* 16 */
> > 
> > NULL
> >   
> >   };
> > 
> > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject
> > *kobj, struct attribute *attr,
> > 
> > if (index >= 11 && index <= 13 &&
> > 
> > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
> > 
> > return 0;
> > 
> > +   if (index >= 14 && index <= 16 &&
> > +   !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
> > +   return 0;
> > 
> > return attr->mode;
> >   
> >   }
> > 
> > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void)
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > 
> > +   /* Third fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(2);
> > +   if (err >= 0)
> > +   i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
> > +
> > 
> > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > "dell_smm",
> > 
> >   NULL, i8k_groups);
> > 
> > if (IS_ERR(i8k_hwmon_dev)) {

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] dell-smm-hwmon: Detect fan with index=2

[Pali Rohár]

On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote:
> On 05/21/2016 07:52 AM, Pali Rohár wrote:
> > Some Dell machines (e.g. Dell Precision M3800) have two fans, first
> > with index=0 and second with index=2. So export also attributes
> > for third fan device with index=2.
> > 
> > Reported-by: Tolga Cakir 
> > Signed-off-by: Pali Rohár 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   20 +++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > ---
> > 
> > Hi Tolga! Can you test this patch if sensors see fan device
> > correctly?
> 
> I assume this means I should wait for test results before applying
> the patch ?

Yes, testing should be done.

Do you have some pending/testing/next tree/branch for such patches?

> Thanks,
> Guenter
> 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
> > 
> >   #define I8K_HWMON_HAVE_TEMP4  (1 << 3)
> >   #define I8K_HWMON_HAVE_FAN1   (1 << 4)
> >   #define I8K_HWMON_HAVE_FAN2   (1 << 5)
> > 
> > +#define I8K_HWMON_HAVE_FAN3(1 << 6)
> > 
> >   MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
> >   MODULE_AUTHOR("Pali Rohár ");
> > 
> > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan)
> > 
> >   static int i8k_get_fan_type(int fan)
> >   {
> >   
> > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values
> > */
> > 
> > -   static int types[2] = { INT_MIN, INT_MIN };
> > +   static int types[3] = { INT_MIN, INT_MIN, INT_MIN };
> > 
> > if (types[fan] == INT_MIN)
> > 
> > types[fan] = _i8k_get_fan_type(fan);
> > 
> > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL,
> > 
> >   1);
> >   
> >   static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR,
> >   i8k_hwmon_show_pwm,
> >   
> >   i8k_hwmon_set_pwm, 1);
> > 
> > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan,
> > NULL, +   2);
> > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL, + 2);
> > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR,
> > i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2);
> > 
> >   static struct attribute *i8k_attrs[] = {
> >   
> > _dev_attr_temp1_input.dev_attr.attr, /* 0 */
> > 
> > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
> > 
> > _dev_attr_fan2_input.dev_attr.attr,  /* 11 */
> > _dev_attr_fan2_label.dev_attr.attr,  /* 12 */
> > _dev_attr_pwm2.dev_attr.attr,/* 13 */
> > 
> > +   _dev_attr_fan3_input.dev_attr.attr,  /* 14 */
> > +   _dev_attr_fan3_label.dev_attr.attr,  /* 15 */
> > +   _dev_attr_pwm3.dev_attr.attr,/* 16 */
> > 
> > NULL
> >   
> >   };
> > 
> > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject
> > *kobj, struct attribute *attr,
> > 
> > if (index >= 11 && index <= 13 &&
> > 
> > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
> > 
> > return 0;
> > 
> > +   if (index >= 14 && index <= 16 &&
> > +   !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
> > +   return 0;
> > 
> > return attr->mode;
> >   
> >   }
> > 
> > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void)
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > 
> > +   /* Third fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(2);
> > +   if (err >= 0)
> > +   i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
> > +
> > 
> > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > "dell_smm",
> > 
> >   NULL, i8k_groups);
> > 
> > if (IS_ERR(i8k_hwmon_dev)) {

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

[Pali Rohár]

On Sunday 22 May 2016 02:19:48 Guenter Roeck wrote:
> On 05/21/2016 07:46 AM, Pali Rohár wrote:
> > On more Dell machines (e.g. Dell Precision M3800)
> > I8K_SMM_GET_FAN_TYPE call is too expensive (CPU is too long in SMM
> > mode) and cause kernel to hang. This patch cache type for each fan
> > (as it should not change) and change the way how fan presense is
> > detected. It revert and use function fan_status() as was before
> > commit f989e55452c7 ("i8k: Add support for fan labels").
> > 
> > Moreover, kernel hangs for 2 - 3 seconds only sometimes and only on
> > some Dell machines. When kernel hangs fan speed is at max. So it
> > was hard to debug and bisect where is root of this problem. It
> > looks like this is bug in Dell BIOS which implement fan type SMM
> > code... and there is no way how to fix it in kernel.
> > 
> > Signed-off-by: Pali Rohár 
> > Reviewed-by: Jean Delvare 
> > Reported-and-tested-by: Tolga Cakir 
> > Fixes: f989e55452c7 ("i8k: Add support for fan labels")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
> > Cc: sta...@vger.kernel.org # v4.0+, will need backport
> 
> Should this patch be applied, or do you wait for more testing ?

I would like to hear some confirmation from people with affected machine.

> Guenter
> 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   21 -
> >   1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > ---
> > 
> > Hi!
> > 
> > Jan C Peters, Thorsten Leemhuis, David Santamaría Rogado, Peter
> > Saunderson and others, can you test this patch if it fixes your
> > freeze problem at boottime and when using "sensors" program?
> > 
> > I need to know if this patch fixes problem on Dell Studio XPS 8000
> > and Dell Studio XPS 8100 machines, so we can revert git commits:
> > 
> > 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000")
> > a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")
> > 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index c43318d..577445f 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -235,7 +235,7 @@ static int i8k_get_fan_speed(int fan)
> > 
> >   /*
> >   
> >* Read the fan type.
> >*/
> > 
> > -static int i8k_get_fan_type(int fan)
> > +static int _i8k_get_fan_type(int fan)
> > 
> >   {
> >   
> > struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
> > 
> > @@ -243,6 +243,17 @@ static int i8k_get_fan_type(int fan)
> > 
> > return i8k_smm() ? : regs.eax & 0xff;
> >   
> >   }
> > 
> > +static int i8k_get_fan_type(int fan)
> > +{
> > +   /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
> > +   static int types[2] = { INT_MIN, INT_MIN };
> > +
> > +   if (types[fan] == INT_MIN)
> > +   types[fan] = _i8k_get_fan_type(fan);
> > +
> > +   return types[fan];
> > +}
> > +
> > 
> >   /*
> >   
> >* Read the fan nominal rpm for specific fan speed.
> >*/
> > 
> > @@ -767,13 +778,13 @@ static int __init i8k_init_hwmon(void)
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
> > 
> > -   /* First fan attributes, if fan type is OK */
> > -   err = i8k_get_fan_type(0);
> > +   /* First fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(0);
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
> > 
> > -   /* Second fan attributes, if fan type is OK */
> > -   err = i8k_get_fan_type(1);
> > +   /* Second fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(1);
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;


-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

[Pali Rohár]

On Sunday 22 May 2016 02:19:48 Guenter Roeck wrote:
> On 05/21/2016 07:46 AM, Pali Rohár wrote:
> > On more Dell machines (e.g. Dell Precision M3800)
> > I8K_SMM_GET_FAN_TYPE call is too expensive (CPU is too long in SMM
> > mode) and cause kernel to hang. This patch cache type for each fan
> > (as it should not change) and change the way how fan presense is
> > detected. It revert and use function fan_status() as was before
> > commit f989e55452c7 ("i8k: Add support for fan labels").
> > 
> > Moreover, kernel hangs for 2 - 3 seconds only sometimes and only on
> > some Dell machines. When kernel hangs fan speed is at max. So it
> > was hard to debug and bisect where is root of this problem. It
> > looks like this is bug in Dell BIOS which implement fan type SMM
> > code... and there is no way how to fix it in kernel.
> > 
> > Signed-off-by: Pali Rohár 
> > Reviewed-by: Jean Delvare 
> > Reported-and-tested-by: Tolga Cakir 
> > Fixes: f989e55452c7 ("i8k: Add support for fan labels")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
> > Cc: sta...@vger.kernel.org # v4.0+, will need backport
> 
> Should this patch be applied, or do you wait for more testing ?

I would like to hear some confirmation from people with affected machine.

> Guenter
> 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   21 -
> >   1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > ---
> > 
> > Hi!
> > 
> > Jan C Peters, Thorsten Leemhuis, David Santamaría Rogado, Peter
> > Saunderson and others, can you test this patch if it fixes your
> > freeze problem at boottime and when using "sensors" program?
> > 
> > I need to know if this patch fixes problem on Dell Studio XPS 8000
> > and Dell Studio XPS 8100 machines, so we can revert git commits:
> > 
> > 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000")
> > a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")
> > 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index c43318d..577445f 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -235,7 +235,7 @@ static int i8k_get_fan_speed(int fan)
> > 
> >   /*
> >   
> >* Read the fan type.
> >*/
> > 
> > -static int i8k_get_fan_type(int fan)
> > +static int _i8k_get_fan_type(int fan)
> > 
> >   {
> >   
> > struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
> > 
> > @@ -243,6 +243,17 @@ static int i8k_get_fan_type(int fan)
> > 
> > return i8k_smm() ? : regs.eax & 0xff;
> >   
> >   }
> > 
> > +static int i8k_get_fan_type(int fan)
> > +{
> > +   /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
> > +   static int types[2] = { INT_MIN, INT_MIN };
> > +
> > +   if (types[fan] == INT_MIN)
> > +   types[fan] = _i8k_get_fan_type(fan);
> > +
> > +   return types[fan];
> > +}
> > +
> > 
> >   /*
> >   
> >* Read the fan nominal rpm for specific fan speed.
> >*/
> > 
> > @@ -767,13 +778,13 @@ static int __init i8k_init_hwmon(void)
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
> > 
> > -   /* First fan attributes, if fan type is OK */
> > -   err = i8k_get_fan_type(0);
> > +   /* First fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(0);
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
> > 
> > -   /* Second fan attributes, if fan type is OK */
> > -   err = i8k_get_fan_type(1);
> > +   /* Second fan attributes, if fan status is OK */
> > +   err = i8k_get_fan_status(1);
> > 
> > if (err >= 0)
> > 
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;


-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] dell-smm-hwmon: Detect fan with index=2

[Guenter Roeck]

On 05/21/2016 07:52 AM, Pali Rohár wrote:

Some Dell machines (e.g. Dell Precision M3800) have two fans, first with
index=0 and second with index=2. So export also attributes for third fan
device with index=2.

Reported-by: Tolga Cakir 
Signed-off-by: Pali Rohár 
---
  drivers/hwmon/dell-smm-hwmon.c |   20 +++-
  1 file changed, 19 insertions(+), 1 deletion(-)
---

Hi Tolga! Can you test this patch if sensors see fan device correctly?



I assume this means I should wait for test results before applying the patch ?

Thanks,
Guenter


diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 577445f..7a2764a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
  #define I8K_HWMON_HAVE_TEMP4  (1 << 3)
  #define I8K_HWMON_HAVE_FAN1   (1 << 4)
  #define I8K_HWMON_HAVE_FAN2   (1 << 5)
+#define I8K_HWMON_HAVE_FAN3(1 << 6)

  MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
  MODULE_AUTHOR("Pali Rohár ");
@@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan)
  static int i8k_get_fan_type(int fan)
  {
/* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
-   static int types[2] = { INT_MIN, INT_MIN };
+   static int types[3] = { INT_MIN, INT_MIN, INT_MIN };

if (types[fan] == INT_MIN)
types[fan] = _i8k_get_fan_type(fan);
@@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, 
i8k_hwmon_show_fan_label, NULL,
  1);
  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 1);
+static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
+ i8k_hwmon_set_pwm, 2);

  static struct attribute *i8k_attrs[] = {
_dev_attr_temp1_input.dev_attr.attr, /* 0 */
@@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
_dev_attr_fan2_input.dev_attr.attr,  /* 11 */
_dev_attr_fan2_label.dev_attr.attr,  /* 12 */
_dev_attr_pwm2.dev_attr.attr,/* 13 */
+   _dev_attr_fan3_input.dev_attr.attr,  /* 14 */
+   _dev_attr_fan3_label.dev_attr.attr,  /* 15 */
+   _dev_attr_pwm3.dev_attr.attr,/* 16 */
NULL
  };

@@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct 
attribute *attr,
if (index >= 11 && index <= 13 &&
!(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
return 0;
+   if (index >= 14 && index <= 16 &&
+   !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
+   return 0;

return attr->mode;
  }
@@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;

+   /* Third fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(2);
+   if (err >= 0)
+   i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
+
i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm",
  NULL, i8k_groups);
if (IS_ERR(i8k_hwmon_dev)) {

Re: [PATCH] dell-smm-hwmon: Detect fan with index=2

[Guenter Roeck]

On 05/21/2016 07:52 AM, Pali Rohár wrote:

Some Dell machines (e.g. Dell Precision M3800) have two fans, first with
index=0 and second with index=2. So export also attributes for third fan
device with index=2.

Reported-by: Tolga Cakir 
Signed-off-by: Pali Rohár 
---
  drivers/hwmon/dell-smm-hwmon.c |   20 +++-
  1 file changed, 19 insertions(+), 1 deletion(-)
---

Hi Tolga! Can you test this patch if sensors see fan device correctly?



I assume this means I should wait for test results before applying the patch ?

Thanks,
Guenter


diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 577445f..7a2764a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
  #define I8K_HWMON_HAVE_TEMP4  (1 << 3)
  #define I8K_HWMON_HAVE_FAN1   (1 << 4)
  #define I8K_HWMON_HAVE_FAN2   (1 << 5)
+#define I8K_HWMON_HAVE_FAN3(1 << 6)

  MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
  MODULE_AUTHOR("Pali Rohár ");
@@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan)
  static int i8k_get_fan_type(int fan)
  {
/* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
-   static int types[2] = { INT_MIN, INT_MIN };
+   static int types[3] = { INT_MIN, INT_MIN, INT_MIN };

if (types[fan] == INT_MIN)
types[fan] = _i8k_get_fan_type(fan);
@@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, 
i8k_hwmon_show_fan_label, NULL,
  1);
  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 1);
+static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
+ i8k_hwmon_set_pwm, 2);

  static struct attribute *i8k_attrs[] = {
_dev_attr_temp1_input.dev_attr.attr, /* 0 */
@@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
_dev_attr_fan2_input.dev_attr.attr,  /* 11 */
_dev_attr_fan2_label.dev_attr.attr,  /* 12 */
_dev_attr_pwm2.dev_attr.attr,/* 13 */
+   _dev_attr_fan3_input.dev_attr.attr,  /* 14 */
+   _dev_attr_fan3_label.dev_attr.attr,  /* 15 */
+   _dev_attr_pwm3.dev_attr.attr,/* 16 */
NULL
  };

@@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct 
attribute *attr,
if (index >= 11 && index <= 13 &&
!(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
return 0;
+   if (index >= 14 && index <= 16 &&
+   !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
+   return 0;

return attr->mode;
  }
@@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;

+   /* Third fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(2);
+   if (err >= 0)
+   i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
+
i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm",
  NULL, i8k_groups);
if (IS_ERR(i8k_hwmon_dev)) {

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

[Guenter Roeck]

On 05/21/2016 07:46 AM, Pali Rohár wrote:

On more Dell machines (e.g. Dell Precision M3800) I8K_SMM_GET_FAN_TYPE call
is too expensive (CPU is too long in SMM mode) and cause kernel to hang.
This patch cache type for each fan (as it should not change) and change the
way how fan presense is detected. It revert and use function fan_status()
as was before commit f989e55452c7 ("i8k: Add support for fan labels").

Moreover, kernel hangs for 2 - 3 seconds only sometimes and only on some
Dell machines. When kernel hangs fan speed is at max. So it was hard to
debug and bisect where is root of this problem. It looks like this is bug
in Dell BIOS which implement fan type SMM code... and there is no way how
to fix it in kernel.

Signed-off-by: Pali Rohár 
Reviewed-by: Jean Delvare 
Reported-and-tested-by: Tolga Cakir 
Fixes: f989e55452c7 ("i8k: Add support for fan labels")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
Cc: sta...@vger.kernel.org # v4.0+, will need backport


Should this patch be applied, or do you wait for more testing ?

Guenter


---
  drivers/hwmon/dell-smm-hwmon.c |   21 -
  1 file changed, 16 insertions(+), 5 deletions(-)
---

Hi!

Jan C Peters, Thorsten Leemhuis, David Santamaría Rogado, Peter Saunderson
and others, can you test this patch if it fixes your freeze problem at
boottime and when using "sensors" program?

I need to know if this patch fixes problem on Dell Studio XPS 8000 and
Dell Studio XPS 8100 machines, so we can revert git commits:

6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000")
a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c43318d..577445f 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -235,7 +235,7 @@ static int i8k_get_fan_speed(int fan)
  /*
   * Read the fan type.
   */
-static int i8k_get_fan_type(int fan)
+static int _i8k_get_fan_type(int fan)
  {
struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };

@@ -243,6 +243,17 @@ static int i8k_get_fan_type(int fan)
return i8k_smm() ? : regs.eax & 0xff;
  }

+static int i8k_get_fan_type(int fan)
+{
+   /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
+   static int types[2] = { INT_MIN, INT_MIN };
+
+   if (types[fan] == INT_MIN)
+   types[fan] = _i8k_get_fan_type(fan);
+
+   return types[fan];
+}
+
  /*
   * Read the fan nominal rpm for specific fan speed.
   */
@@ -767,13 +778,13 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;

-   /* First fan attributes, if fan type is OK */
-   err = i8k_get_fan_type(0);
+   /* First fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(0);
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;

-   /* Second fan attributes, if fan type is OK */
-   err = i8k_get_fan_type(1);
+   /* Second fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(1);
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

[Guenter Roeck]

On 05/21/2016 07:46 AM, Pali Rohár wrote:

On more Dell machines (e.g. Dell Precision M3800) I8K_SMM_GET_FAN_TYPE call
is too expensive (CPU is too long in SMM mode) and cause kernel to hang.
This patch cache type for each fan (as it should not change) and change the
way how fan presense is detected. It revert and use function fan_status()
as was before commit f989e55452c7 ("i8k: Add support for fan labels").

Moreover, kernel hangs for 2 - 3 seconds only sometimes and only on some
Dell machines. When kernel hangs fan speed is at max. So it was hard to
debug and bisect where is root of this problem. It looks like this is bug
in Dell BIOS which implement fan type SMM code... and there is no way how
to fix it in kernel.

Signed-off-by: Pali Rohár 
Reviewed-by: Jean Delvare 
Reported-and-tested-by: Tolga Cakir 
Fixes: f989e55452c7 ("i8k: Add support for fan labels")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
Cc: sta...@vger.kernel.org # v4.0+, will need backport


Should this patch be applied, or do you wait for more testing ?

Guenter


---
  drivers/hwmon/dell-smm-hwmon.c |   21 -
  1 file changed, 16 insertions(+), 5 deletions(-)
---

Hi!

Jan C Peters, Thorsten Leemhuis, David Santamaría Rogado, Peter Saunderson
and others, can you test this patch if it fixes your freeze problem at
boottime and when using "sensors" program?

I need to know if this patch fixes problem on Dell Studio XPS 8000 and
Dell Studio XPS 8100 machines, so we can revert git commits:

6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000")
a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c43318d..577445f 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -235,7 +235,7 @@ static int i8k_get_fan_speed(int fan)
  /*
   * Read the fan type.
   */
-static int i8k_get_fan_type(int fan)
+static int _i8k_get_fan_type(int fan)
  {
struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };

@@ -243,6 +243,17 @@ static int i8k_get_fan_type(int fan)
return i8k_smm() ? : regs.eax & 0xff;
  }

+static int i8k_get_fan_type(int fan)
+{
+   /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
+   static int types[2] = { INT_MIN, INT_MIN };
+
+   if (types[fan] == INT_MIN)
+   types[fan] = _i8k_get_fan_type(fan);
+
+   return types[fan];
+}
+
  /*
   * Read the fan nominal rpm for specific fan speed.
   */
@@ -767,13 +778,13 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;

-   /* First fan attributes, if fan type is OK */
-   err = i8k_get_fan_type(0);
+   /* First fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(0);
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;

-   /* Second fan attributes, if fan type is OK */
-   err = i8k_get_fan_type(1);
+   /* Second fan attributes, if fan status is OK */
+   err = i8k_get_fan_status(1);
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;

Re: [PATCH] drm/gma500: use vma_pages().

[Patrik Jakobsson]

On Sat, May 21, 2016 at 3:21 PM, Muhammad Falak R Wani
 wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages()
>
> Signed-off-by: Muhammad Falak R Wani 

Thanks, queued for gma500-next

> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 7440bf9..5486e7e 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -125,7 +125,7 @@ static int psbfb_vm_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
> unsigned long phys_addr = (unsigned long)dev_priv->stolen_base +
>   psbfb->gtt->offset;
>
> -   page_num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +   page_num = vma_pages(vma);
> address = (unsigned long)vmf->virtual_address - (vmf->pgoff << 
> PAGE_SHIFT);
>
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> --
> 1.9.1
>

Re: [PATCH] drm/gma500: use vma_pages().

[Patrik Jakobsson]

On Sat, May 21, 2016 at 3:21 PM, Muhammad Falak R Wani
 wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages()
>
> Signed-off-by: Muhammad Falak R Wani 

Thanks, queued for gma500-next

> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 7440bf9..5486e7e 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -125,7 +125,7 @@ static int psbfb_vm_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
> unsigned long phys_addr = (unsigned long)dev_priv->stolen_base +
>   psbfb->gtt->offset;
>
> -   page_num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +   page_num = vma_pages(vma);
> address = (unsigned long)vmf->virtual_address - (vmf->pgoff << 
> PAGE_SHIFT);
>
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> --
> 1.9.1
>

[rcu:dev.2016.05.18a 28/28] kernel/rcu/tree_plugin.h:1168:2: error: implicit declaration of function 'for_each_leaf_node_cpu_bit'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
dev.2016.05.18a
head:   a396e98dae720a48c49795892d23ca17c87319b0
commit: a396e98dae720a48c49795892d23ca17c87319b0 [28/28] rcu: Correctly handle 
sparse possible CPUs
config: x86_64-randconfig-n0-05220622 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
git checkout a396e98dae720a48c49795892d23ca17c87319b0
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the rcu/dev.2016.05.18a HEAD a396e98dae720a48c49795892d23ca17c87319b0 
builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   In file included from kernel/rcu/tree.c:4209:0:
   kernel/rcu/tree_plugin.h: In function 'rcu_boost_kthread_setaffinity':
>> kernel/rcu/tree_plugin.h:1168:2: error: implicit declaration of function 
>> 'for_each_leaf_node_cpu_bit' [-Werror=implicit-function-declaration]
 for_each_leaf_node_cpu_bit(rnp, cpu, bit)
 ^~
>> kernel/rcu/tree_plugin.h:1169:3: error: expected ';' before 'if'
  if ((mask & bit) && cpu != outgoingcpu)
  ^~
   kernel/rcu/tree_plugin.h:1159:16: warning: unused variable 'mask' 
[-Wunused-variable]
 unsigned long mask = rcu_rnp_online_cpus(rnp);
   ^~~~
   cc1: some warnings being treated as errors

vim +/for_each_leaf_node_cpu_bit +1168 kernel/rcu/tree_plugin.h

  1162  int cpu;
  1163  
  1164  if (!t)
  1165  return;
  1166  if (!zalloc_cpumask_var(, GFP_KERNEL))
  1167  return;
> 1168  for_each_leaf_node_cpu_bit(rnp, cpu, bit)
> 1169  if ((mask & bit) && cpu != outgoingcpu)
  1170  cpumask_set_cpu(cpu, cm);
  1171  if (cpumask_weight(cm) == 0)
  1172  cpumask_setall(cm);

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


.config.gz
Description: Binary data

[rcu:dev.2016.05.18a 28/28] kernel/rcu/tree_plugin.h:1168:2: error: implicit declaration of function 'for_each_leaf_node_cpu_bit'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
dev.2016.05.18a
head:   a396e98dae720a48c49795892d23ca17c87319b0
commit: a396e98dae720a48c49795892d23ca17c87319b0 [28/28] rcu: Correctly handle 
sparse possible CPUs
config: x86_64-randconfig-n0-05220622 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
git checkout a396e98dae720a48c49795892d23ca17c87319b0
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the rcu/dev.2016.05.18a HEAD a396e98dae720a48c49795892d23ca17c87319b0 
builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   In file included from kernel/rcu/tree.c:4209:0:
   kernel/rcu/tree_plugin.h: In function 'rcu_boost_kthread_setaffinity':
>> kernel/rcu/tree_plugin.h:1168:2: error: implicit declaration of function 
>> 'for_each_leaf_node_cpu_bit' [-Werror=implicit-function-declaration]
 for_each_leaf_node_cpu_bit(rnp, cpu, bit)
 ^~
>> kernel/rcu/tree_plugin.h:1169:3: error: expected ';' before 'if'
  if ((mask & bit) && cpu != outgoingcpu)
  ^~
   kernel/rcu/tree_plugin.h:1159:16: warning: unused variable 'mask' 
[-Wunused-variable]
 unsigned long mask = rcu_rnp_online_cpus(rnp);
   ^~~~
   cc1: some warnings being treated as errors

vim +/for_each_leaf_node_cpu_bit +1168 kernel/rcu/tree_plugin.h

  1162  int cpu;
  1163  
  1164  if (!t)
  1165  return;
  1166  if (!zalloc_cpumask_var(, GFP_KERNEL))
  1167  return;
> 1168  for_each_leaf_node_cpu_bit(rnp, cpu, bit)
> 1169  if ((mask & bit) && cpu != outgoingcpu)
  1170  cpumask_set_cpu(cpu, cm);
  1171  if (cpumask_weight(cm) == 0)
  1172  cpumask_setall(cm);

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


.config.gz
Description: Binary data

[PATCH v2 1/1] rtc: ds1685: correct check of day of month

[Heinrich Schuchardt]

The day of month is checked in ds1685_rtc_read_alarm
and ds1685_rtc_set_alarm.

Multiple errors exist in the day of month check.

Operator ! has a higher priority than &&.
(!(mday >= 1) && (mday <= 31)) is false for mday == 32.

When verifying the day of month the binary and the BCD mode
have to be considered.

v2:
consider ds1685_rtc_read_alarm
consider rtc->bcd_mode as indicated by Alexandre Belloni

Signed-off-by: Heinrich Schuchardt 
---
 drivers/rtc/rtc-ds1685.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
index b3ce3c6..92e2f27 100644
--- a/drivers/rtc/rtc-ds1685.c
+++ b/drivers/rtc/rtc-ds1685.c
@@ -103,6 +103,26 @@ ds1685_rtc_bin2bcd(struct ds1685_priv *rtc, u8 val, u8 
bin_mask, u8 bcd_mask)
 }
 
 /**
+ * s1685_rtc_check_mday - check validity of the day of month.
+ * @rtc: pointer to the ds1685 rtc structure.
+ * @mday: day of month.
+ *
+ * Returns -EDOM if the day of month is not within 1..31 range.
+ */
+static inline int
+ds1685_rtc_check_mday(struct ds1685_priv *rtc, u8 mday)
+{
+   if (rtc->bcd_mode) {
+   if (mday < 0x01 || mday > 0x31 || (mday & 0x0f) > 0x09)
+   return -EDOM;
+   } else {
+   if (mday < 1 || mday > 31)
+   return -EDOM;
+   }
+   return 0;
+}
+
+/**
  * ds1685_rtc_switch_to_bank0 - switch the rtc to bank 0.
  * @rtc: pointer to the ds1685 rtc structure.
  */
@@ -377,6 +397,7 @@ ds1685_rtc_read_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
struct platform_device *pdev = to_platform_device(dev);
struct ds1685_priv *rtc = platform_get_drvdata(pdev);
u8 seconds, minutes, hours, mday, ctrlb, ctrlc;
+   int ret;
 
/* Fetch the alarm info from the RTC alarm registers. */
ds1685_rtc_begin_data_access(rtc);
@@ -388,9 +409,10 @@ ds1685_rtc_read_alarm(struct device *dev, struct 
rtc_wkalrm *alrm)
ctrlc   = rtc->read(rtc, RTC_CTRL_C);
ds1685_rtc_end_data_access(rtc);
 
-   /* Check month date. */
-   if (!(mday >= 1) && (mday <= 31))
-   return -EDOM;
+   /* Check the month date for validity. */
+   ret = ds1685_rtc_check_mday(rtc, mday);
+   if (ret)
+   return ret;
 
/*
 * Check the three alarm bytes.
@@ -445,6 +467,7 @@ ds1685_rtc_set_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
struct platform_device *pdev = to_platform_device(dev);
struct ds1685_priv *rtc = platform_get_drvdata(pdev);
u8 ctrlb, seconds, minutes, hours, mday;
+   int ret;
 
/* Fetch the alarm info and convert to BCD. */
seconds = ds1685_rtc_bin2bcd(rtc, alrm->time.tm_sec,
@@ -461,8 +484,9 @@ ds1685_rtc_set_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
 RTC_MDAY_BCD_MASK);
 
/* Check the month date for validity. */
-   if (!(mday >= 1) && (mday <= 31))
-   return -EDOM;
+   ret = ds1685_rtc_check_mday(rtc, mday);
+   if (ret)
+   return ret;
 
/*
 * Check the three alarm bytes.
-- 
2.1.4

[PATCH v2 1/1] rtc: ds1685: correct check of day of month

[Heinrich Schuchardt]

The day of month is checked in ds1685_rtc_read_alarm
and ds1685_rtc_set_alarm.

Multiple errors exist in the day of month check.

Operator ! has a higher priority than &&.
(!(mday >= 1) && (mday <= 31)) is false for mday == 32.

When verifying the day of month the binary and the BCD mode
have to be considered.

v2:
consider ds1685_rtc_read_alarm
consider rtc->bcd_mode as indicated by Alexandre Belloni

Signed-off-by: Heinrich Schuchardt 
---
 drivers/rtc/rtc-ds1685.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
index b3ce3c6..92e2f27 100644
--- a/drivers/rtc/rtc-ds1685.c
+++ b/drivers/rtc/rtc-ds1685.c
@@ -103,6 +103,26 @@ ds1685_rtc_bin2bcd(struct ds1685_priv *rtc, u8 val, u8 
bin_mask, u8 bcd_mask)
 }
 
 /**
+ * s1685_rtc_check_mday - check validity of the day of month.
+ * @rtc: pointer to the ds1685 rtc structure.
+ * @mday: day of month.
+ *
+ * Returns -EDOM if the day of month is not within 1..31 range.
+ */
+static inline int
+ds1685_rtc_check_mday(struct ds1685_priv *rtc, u8 mday)
+{
+   if (rtc->bcd_mode) {
+   if (mday < 0x01 || mday > 0x31 || (mday & 0x0f) > 0x09)
+   return -EDOM;
+   } else {
+   if (mday < 1 || mday > 31)
+   return -EDOM;
+   }
+   return 0;
+}
+
+/**
  * ds1685_rtc_switch_to_bank0 - switch the rtc to bank 0.
  * @rtc: pointer to the ds1685 rtc structure.
  */
@@ -377,6 +397,7 @@ ds1685_rtc_read_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
struct platform_device *pdev = to_platform_device(dev);
struct ds1685_priv *rtc = platform_get_drvdata(pdev);
u8 seconds, minutes, hours, mday, ctrlb, ctrlc;
+   int ret;
 
/* Fetch the alarm info from the RTC alarm registers. */
ds1685_rtc_begin_data_access(rtc);
@@ -388,9 +409,10 @@ ds1685_rtc_read_alarm(struct device *dev, struct 
rtc_wkalrm *alrm)
ctrlc   = rtc->read(rtc, RTC_CTRL_C);
ds1685_rtc_end_data_access(rtc);
 
-   /* Check month date. */
-   if (!(mday >= 1) && (mday <= 31))
-   return -EDOM;
+   /* Check the month date for validity. */
+   ret = ds1685_rtc_check_mday(rtc, mday);
+   if (ret)
+   return ret;
 
/*
 * Check the three alarm bytes.
@@ -445,6 +467,7 @@ ds1685_rtc_set_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
struct platform_device *pdev = to_platform_device(dev);
struct ds1685_priv *rtc = platform_get_drvdata(pdev);
u8 ctrlb, seconds, minutes, hours, mday;
+   int ret;
 
/* Fetch the alarm info and convert to BCD. */
seconds = ds1685_rtc_bin2bcd(rtc, alrm->time.tm_sec,
@@ -461,8 +484,9 @@ ds1685_rtc_set_alarm(struct device *dev, struct rtc_wkalrm 
*alrm)
 RTC_MDAY_BCD_MASK);
 
/* Check the month date for validity. */
-   if (!(mday >= 1) && (mday <= 31))
-   return -EDOM;
+   ret = ds1685_rtc_check_mday(rtc, mday);
+   if (ret)
+   return ret;
 
/*
 * Check the three alarm bytes.
-- 
2.1.4

[PATCH] libnvdimm, dax: fix deletion

[Dan Williams]

The ndctl unit tests discovered that the dax enabling omitted updates to
nd_detach_and_reset().  This routine clears device the configuration
when the namespace is detached.  Without this clearing userspace may
assume that the device is in the process of being configured by another
agent in the system.

Signed-off-by: Dan Williams 
---
 drivers/nvdimm/claim.c|   23 +--
 drivers/nvdimm/nd-core.h  |1 +
 drivers/nvdimm/pfn_devs.c |   19 ---
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 5f53db59a058..8b2e3c4fb0ad 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -93,6 +93,25 @@ static bool is_idle(struct device *dev, struct 
nd_namespace_common *ndns)
return true;
 }
 
+struct nd_pfn *to_nd_pfn_safe(struct device *dev)
+{
+   /*
+* pfn device attributes are re-used by dax device instances, so we
+* need to be careful to correct device-to-nd_pfn conversion.
+*/
+   if (is_nd_pfn(dev))
+   return to_nd_pfn(dev);
+
+   if (is_nd_dax(dev)) {
+   struct nd_dax *nd_dax = to_nd_dax(dev);
+
+   return _dax->nd_pfn;
+   }
+
+   WARN_ON(1);
+   return NULL;
+}
+
 static void nd_detach_and_reset(struct device *dev,
struct nd_namespace_common **_ndns)
 {
@@ -106,8 +125,8 @@ static void nd_detach_and_reset(struct device *dev,
nd_btt->lbasize = 0;
kfree(nd_btt->uuid);
nd_btt->uuid = NULL;
-   } else if (is_nd_pfn(dev)) {
-   struct nd_pfn *nd_pfn = to_nd_pfn(dev);
+   } else if (is_nd_pfn(dev) || is_nd_dax(dev)) {
+   struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev);
 
kfree(nd_pfn->uuid);
nd_pfn->uuid = NULL;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 4136c1a82539..6c42eda025f9 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -94,4 +94,5 @@ bool __nd_attach_ndns(struct device *dev, struct 
nd_namespace_common *attach,
 ssize_t nd_namespace_store(struct device *dev,
struct nd_namespace_common **_ndns, const char *buf,
size_t len);
+struct nd_pfn *to_nd_pfn_safe(struct device *dev);
 #endif /* __ND_CORE_H__ */
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 04f71d6d304d..436191c47077 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -54,25 +54,6 @@ struct nd_pfn *to_nd_pfn(struct device *dev)
 }
 EXPORT_SYMBOL(to_nd_pfn);
 
-static struct nd_pfn *to_nd_pfn_safe(struct device *dev)
-{
-   /*
-* pfn device attributes are re-used by dax device instances, so we
-* need to be careful to correct device-to-nd_pfn conversion.
-*/
-   if (is_nd_pfn(dev))
-   return to_nd_pfn(dev);
-
-   if (is_nd_dax(dev)) {
-   struct nd_dax *nd_dax = to_nd_dax(dev);
-
-   return _dax->nd_pfn;
-   }
-
-   WARN_ON(1);
-   return NULL;
-}
-
 static ssize_t mode_show(struct device *dev,
struct device_attribute *attr, char *buf)
 {

[PATCH] libnvdimm, dax: fix deletion

[Dan Williams]

The ndctl unit tests discovered that the dax enabling omitted updates to
nd_detach_and_reset().  This routine clears device the configuration
when the namespace is detached.  Without this clearing userspace may
assume that the device is in the process of being configured by another
agent in the system.

Signed-off-by: Dan Williams 
---
 drivers/nvdimm/claim.c|   23 +--
 drivers/nvdimm/nd-core.h  |1 +
 drivers/nvdimm/pfn_devs.c |   19 ---
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 5f53db59a058..8b2e3c4fb0ad 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -93,6 +93,25 @@ static bool is_idle(struct device *dev, struct 
nd_namespace_common *ndns)
return true;
 }
 
+struct nd_pfn *to_nd_pfn_safe(struct device *dev)
+{
+   /*
+* pfn device attributes are re-used by dax device instances, so we
+* need to be careful to correct device-to-nd_pfn conversion.
+*/
+   if (is_nd_pfn(dev))
+   return to_nd_pfn(dev);
+
+   if (is_nd_dax(dev)) {
+   struct nd_dax *nd_dax = to_nd_dax(dev);
+
+   return _dax->nd_pfn;
+   }
+
+   WARN_ON(1);
+   return NULL;
+}
+
 static void nd_detach_and_reset(struct device *dev,
struct nd_namespace_common **_ndns)
 {
@@ -106,8 +125,8 @@ static void nd_detach_and_reset(struct device *dev,
nd_btt->lbasize = 0;
kfree(nd_btt->uuid);
nd_btt->uuid = NULL;
-   } else if (is_nd_pfn(dev)) {
-   struct nd_pfn *nd_pfn = to_nd_pfn(dev);
+   } else if (is_nd_pfn(dev) || is_nd_dax(dev)) {
+   struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev);
 
kfree(nd_pfn->uuid);
nd_pfn->uuid = NULL;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 4136c1a82539..6c42eda025f9 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -94,4 +94,5 @@ bool __nd_attach_ndns(struct device *dev, struct 
nd_namespace_common *attach,
 ssize_t nd_namespace_store(struct device *dev,
struct nd_namespace_common **_ndns, const char *buf,
size_t len);
+struct nd_pfn *to_nd_pfn_safe(struct device *dev);
 #endif /* __ND_CORE_H__ */
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 04f71d6d304d..436191c47077 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -54,25 +54,6 @@ struct nd_pfn *to_nd_pfn(struct device *dev)
 }
 EXPORT_SYMBOL(to_nd_pfn);
 
-static struct nd_pfn *to_nd_pfn_safe(struct device *dev)
-{
-   /*
-* pfn device attributes are re-used by dax device instances, so we
-* need to be careful to correct device-to-nd_pfn conversion.
-*/
-   if (is_nd_pfn(dev))
-   return to_nd_pfn(dev);
-
-   if (is_nd_dax(dev)) {
-   struct nd_dax *nd_dax = to_nd_dax(dev);
-
-   return _dax->nd_pfn;
-   }
-
-   WARN_ON(1);
-   return NULL;
-}
-
 static ssize_t mode_show(struct device *dev,
struct device_attribute *attr, char *buf)
 {

Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test

[Ingo Molnar]

* Andy Lutomirski  wrote:

> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar  wrote:
> >
> > * Dmitry Safonov  wrote:
> >
> >> Should print on success:
> >> [root@localhost ~]# ./test_mremap_vdso_32
> >>   AT_SYSINFO_EHDR is 0xf773f000
> >> [NOTE]Moving vDSO: [f773f000, f774] -> [a00, a001000]
> >> [OK]
> >> Or segfault if landing was bad (before patches):
> >> [root@localhost ~]# ./test_mremap_vdso_32
> >>   AT_SYSINFO_EHDR is 0xf774f000
> >> [NOTE]Moving vDSO: [f774f000, f775] -> [a00, a001000]
> >> Segmentation fault (core dumped)
> >
> > So I still think that generating potential segfaults is not a proper way to 
> > test a
> > new feature. How are we supposed to tell the feature still works? I realize 
> > that
> > glibc is a problem here - but that doesn't really change the QA equation: 
> > we are
> > adding new kernel code to help essentially a single application out of tens 
> > of
> > thousands of applications.
> >
> > At minimum we should have a robust testcase ...
> 
> I think it's robust enough.  It will print "[OK]" and exit with 0 on
> success and it will crash on failure.  The latter should cause make
> run_tests to fail reliably.

Indeed, you are right - I somehow mis-read it as potentially segfaulting on 
fixed 
kernels as well...

Will look at applying this after the merge window.

Thanks,

Ingo

Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test

[Ingo Molnar]

* Andy Lutomirski  wrote:

> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar  wrote:
> >
> > * Dmitry Safonov  wrote:
> >
> >> Should print on success:
> >> [root@localhost ~]# ./test_mremap_vdso_32
> >>   AT_SYSINFO_EHDR is 0xf773f000
> >> [NOTE]Moving vDSO: [f773f000, f774] -> [a00, a001000]
> >> [OK]
> >> Or segfault if landing was bad (before patches):
> >> [root@localhost ~]# ./test_mremap_vdso_32
> >>   AT_SYSINFO_EHDR is 0xf774f000
> >> [NOTE]Moving vDSO: [f774f000, f775] -> [a00, a001000]
> >> Segmentation fault (core dumped)
> >
> > So I still think that generating potential segfaults is not a proper way to 
> > test a
> > new feature. How are we supposed to tell the feature still works? I realize 
> > that
> > glibc is a problem here - but that doesn't really change the QA equation: 
> > we are
> > adding new kernel code to help essentially a single application out of tens 
> > of
> > thousands of applications.
> >
> > At minimum we should have a robust testcase ...
> 
> I think it's robust enough.  It will print "[OK]" and exit with 0 on
> success and it will crash on failure.  The latter should cause make
> run_tests to fail reliably.

Indeed, you are right - I somehow mis-read it as potentially segfaulting on 
fixed 
kernels as well...

Will look at applying this after the merge window.

Thanks,

Ingo

[PATCH] seqlock: fix raw_read_seqcount_latch()

[Alexey Dobriyan]

lockless_dereference() is supposed to take pointer not integer.

Signed-off-by: Alexey Dobriyan 
---

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

--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -277,7 +277,7 @@ static inline void raw_write_seqcount_barrier(seqcount_t *s)
 
 static inline int raw_read_seqcount_latch(seqcount_t *s)
 {
-   return lockless_dereference(s->sequence);
+   return lockless_dereference(s)->sequence;
 }
 
 /**
@@ -331,7 +331,7 @@ static inline int raw_read_seqcount_latch(seqcount_t *s)
  * unsigned seq, idx;
  *
  * do {
- * seq = lockless_dereference(latch->seq);
+ * seq = lockless_dereference(latch)->seq;
  *
  * idx = seq & 0x01;
  * entry = data_query(latch->data[idx], ...);

[PATCH] seqlock: fix raw_read_seqcount_latch()

[Alexey Dobriyan]

lockless_dereference() is supposed to take pointer not integer.

Signed-off-by: Alexey Dobriyan 
---

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

--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -277,7 +277,7 @@ static inline void raw_write_seqcount_barrier(seqcount_t *s)
 
 static inline int raw_read_seqcount_latch(seqcount_t *s)
 {
-   return lockless_dereference(s->sequence);
+   return lockless_dereference(s)->sequence;
 }
 
 /**
@@ -331,7 +331,7 @@ static inline int raw_read_seqcount_latch(seqcount_t *s)
  * unsigned seq, idx;
  *
  * do {
- * seq = lockless_dereference(latch->seq);
+ * seq = lockless_dereference(latch)->seq;
  *
  * idx = seq & 0x01;
  * entry = data_query(latch->data[idx], ...);

Re: [PATCH 3/5] sched: cpufreq: call cpufreq hook from remote CPUs

[Steve Muckle]

Hi Peter, Ingo,

On Thu, May 19, 2016 at 04:04:19PM -0700, Steve Muckle wrote:
> On Thu, May 19, 2016 at 11:06:14PM +0200, Rafael J. Wysocki wrote:
> > > In the case of a remote update the hook has to run (or not) after it is
> > > known whether preemption will occur so we don't do needless work or
> > > IPIs. If the policy CPUs aren't known in the scheduler then the early
> > > hook will always need to be called along with an indication that it is
> > > the early hook being called. If it turns out to be a remote update it
> > > could then be deferred to the later hook, which would only be called
> > > when a remote update has been deferred and preemption has not occurred.
> > >
> > > This means two hook inovcations for a remote non-preempting wakeup
> > > though instead of one.  Perhaps this is a good middle ground on code
> > > churn vs. optimization though.
> > 
> > I would think so.
> 
> Ok, I will pursue this approach.

I'd like to get your opinion here before proceeding further...

To catch you up and summarize, I'm trying to implement support for
calling the scheduler cpufreq callback during remote wakeups.  Currently
the scheduler cpufreq callback is only called when the target CPU is the
current CPU. If a remote wakeup does not result in preemption, the CPU
frequency may currently not be adjusted appropriately for up to a tick,
when we are guaranteed to call the hook again.

Invoking schedutil promptly for the target CPU in this situation means
sending an IPI if the current CPU is not in the target CPU's frequency
domain. This is because often a cpufreq driver must run on a CPU within
the frequency domain it is bound to.  But the catch is that we should
not do this and incur the overhead of an IPI if preemption will occur,
as in that case the scheduler (and schedutil) will run soon on the
target CPU anyway, potentially as a result of the scheduler sending its
own IPI.

I figured this unnecessary overhead would be unacceptable and so have
been working on an approach to avoid it. Unfortunately the current hooks
happen before the preemption decision is made. My current implementation
sets a flag if schedutil sees a remote wakeup and then bails. There's a
test to call the hook again at the end of check_preempt_curr() if the flag
is set.  The flag is cleared by resched_curr() as that means preemption
will happen on the target CPU. The flag currently lives at the end of
the rq struct. I could move it into the update_util_data hook structure
or elsewhere, but that would mean accessing another per-cpu item in
hot scheduler paths.

Thoughts? Note the current implementation described above differs a bit
from the last posting in this thread, per discussion with Rafael.

thanks,
Steve

Re: [PATCH 3/5] sched: cpufreq: call cpufreq hook from remote CPUs

[Steve Muckle]

Hi Peter, Ingo,

On Thu, May 19, 2016 at 04:04:19PM -0700, Steve Muckle wrote:
> On Thu, May 19, 2016 at 11:06:14PM +0200, Rafael J. Wysocki wrote:
> > > In the case of a remote update the hook has to run (or not) after it is
> > > known whether preemption will occur so we don't do needless work or
> > > IPIs. If the policy CPUs aren't known in the scheduler then the early
> > > hook will always need to be called along with an indication that it is
> > > the early hook being called. If it turns out to be a remote update it
> > > could then be deferred to the later hook, which would only be called
> > > when a remote update has been deferred and preemption has not occurred.
> > >
> > > This means two hook inovcations for a remote non-preempting wakeup
> > > though instead of one.  Perhaps this is a good middle ground on code
> > > churn vs. optimization though.
> > 
> > I would think so.
> 
> Ok, I will pursue this approach.

I'd like to get your opinion here before proceeding further...

To catch you up and summarize, I'm trying to implement support for
calling the scheduler cpufreq callback during remote wakeups.  Currently
the scheduler cpufreq callback is only called when the target CPU is the
current CPU. If a remote wakeup does not result in preemption, the CPU
frequency may currently not be adjusted appropriately for up to a tick,
when we are guaranteed to call the hook again.

Invoking schedutil promptly for the target CPU in this situation means
sending an IPI if the current CPU is not in the target CPU's frequency
domain. This is because often a cpufreq driver must run on a CPU within
the frequency domain it is bound to.  But the catch is that we should
not do this and incur the overhead of an IPI if preemption will occur,
as in that case the scheduler (and schedutil) will run soon on the
target CPU anyway, potentially as a result of the scheduler sending its
own IPI.

I figured this unnecessary overhead would be unacceptable and so have
been working on an approach to avoid it. Unfortunately the current hooks
happen before the preemption decision is made. My current implementation
sets a flag if schedutil sees a remote wakeup and then bails. There's a
test to call the hook again at the end of check_preempt_curr() if the flag
is set.  The flag is cleared by resched_curr() as that means preemption
will happen on the target CPU. The flag currently lives at the end of
the rq struct. I could move it into the update_util_data hook structure
or elsewhere, but that would mean accessing another per-cpu item in
hot scheduler paths.

Thoughts? Note the current implementation described above differs a bit
from the last posting in this thread, per discussion with Rafael.

thanks,
Steve

Re: [GIT PULL] RTC for 4.7

[Alexandre Belloni]

On 21/05/2016 at 11:18:15 -0700, Linus Torvalds wrote :
> On Sat, May 21, 2016 at 8:15 AM, Alexandre Belloni
>  wrote:
> >
> > Here is the pull-request for the RTC subsystem for 4.7.
> 
> Grr. I noticed this too late, but this has all been rebased very recently.
> 
> Don't f*cking do this! Why do I have to tell people every single merge window?
> 

Yeah, sorry about that. I realized I forgot to rework a few commit
messages (Fixes: tags and author names due to patchwork messing with
them) before applying so I did it before sending the pull request. I'll
abstain next time.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [GIT PULL] RTC for 4.7

[Alexandre Belloni]

On 21/05/2016 at 11:18:15 -0700, Linus Torvalds wrote :
> On Sat, May 21, 2016 at 8:15 AM, Alexandre Belloni
>  wrote:
> >
> > Here is the pull-request for the RTC subsystem for 4.7.
> 
> Grr. I noticed this too late, but this has all been rebased very recently.
> 
> Don't f*cking do this! Why do I have to tell people every single merge window?
> 

Yeah, sorry about that. I realized I forgot to rework a few commit
messages (Fixes: tags and author names due to patchwork messing with
them) before applying so I did it before sending the pull request. I'll
abstain next time.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] max44000: Remove scale from proximity

[Jonathan Cameron]

On 20/05/16 15:44, Crestez Dan Leonard wrote:
> This is not implemented and doesn't really make sense because IIO
> proximity is unit-less.
> 
> Remove IIO_CHAN_INFO_SCALE from info_mask because so that the _scale
> sysfs entry won't appear. This fixes userspace tools like generic_buffer
> which abort when reads returns an error.
> 
Applied to the fixes-togreg-post-rc1 branch of iio.git.

Thanks,

Jonathan
> Signed-off-by: Crestez Dan Leonard 
> ---
>  drivers/iio/light/max44000.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index e01e58a..f17cb2e 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -147,7 +147,6 @@ static const struct iio_chan_spec max44000_channels[] = {
>   {
>   .type = IIO_PROXIMITY,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>   .scan_index = MAX44000_SCAN_INDEX_PRX,
>   .scan_type = {
>   .sign   = 'u',
> 

Re: [PATCH] max44000: Remove scale from proximity

[Jonathan Cameron]

On 20/05/16 15:44, Crestez Dan Leonard wrote:
> This is not implemented and doesn't really make sense because IIO
> proximity is unit-less.
> 
> Remove IIO_CHAN_INFO_SCALE from info_mask because so that the _scale
> sysfs entry won't appear. This fixes userspace tools like generic_buffer
> which abort when reads returns an error.
> 
Applied to the fixes-togreg-post-rc1 branch of iio.git.

Thanks,

Jonathan
> Signed-off-by: Crestez Dan Leonard 
> ---
>  drivers/iio/light/max44000.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index e01e58a..f17cb2e 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -147,7 +147,6 @@ static const struct iio_chan_spec max44000_channels[] = {
>   {
>   .type = IIO_PROXIMITY,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>   .scan_index = MAX44000_SCAN_INDEX_PRX,
>   .scan_type = {
>   .sign   = 'u',
> 

Re: [PATCH] iio: light: jsa1212: remove unneeded i2c check functionality test

[Jonathan Cameron]

On 19/05/16 21:19, Matt Ranostay wrote:
> Reviewed-by: Matt Ranostay 
Applied to the togreg branch of iio.git.

Thanks,

Jonathan
> 
> On Thu, May 19, 2016 at 11:37 AM, sathyanarayanan kuppuswamy
>  wrote:
>> Looks Good.
>>
>> Reviewed-by:Kuppuswamy Sathyanarayanan
>> 
>>
>>
>> On 05/15/2016 11:37 AM, Alison Schofield wrote:
>>>
>>> This driver does not call i2c_smbus_read|write_byte_data(),
>>> so remove the corresponding functionality test. It uses regmap
>>> to handle byte transfers transparently.
>>>
>>> Signed-off-by: Alison Schofield 
>>> ---
>>> Found & fixed by inspection based on same defect recently fixed
>>> in light/tpl0102 driver.
>>>
>>>
>>>   drivers/iio/light/jsa1212.c | 3 ---
>>>   1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
>>> index 99a6281..e8a8931 100644
>>> --- a/drivers/iio/light/jsa1212.c
>>> +++ b/drivers/iio/light/jsa1212.c
>>> @@ -325,9 +325,6 @@ static int jsa1212_probe(struct i2c_client *client,
>>> struct regmap *regmap;
>>> int ret;
>>>   - if (!i2c_check_functionality(client->adapter,
>>> I2C_FUNC_SMBUS_BYTE_DATA))
>>> -   return -EOPNOTSUPP;
>>> -
>>> indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
>>> if (!indio_dev)
>>> return -ENOMEM;
>>
>>
>> --
>> Sathyanarayanan Kuppuswamy
>> Android kernel developer
>>

Re: [PATCH] iio: light: jsa1212: remove unneeded i2c check functionality test

[Jonathan Cameron]

On 19/05/16 21:19, Matt Ranostay wrote:
> Reviewed-by: Matt Ranostay 
Applied to the togreg branch of iio.git.

Thanks,

Jonathan
> 
> On Thu, May 19, 2016 at 11:37 AM, sathyanarayanan kuppuswamy
>  wrote:
>> Looks Good.
>>
>> Reviewed-by:Kuppuswamy Sathyanarayanan
>> 
>>
>>
>> On 05/15/2016 11:37 AM, Alison Schofield wrote:
>>>
>>> This driver does not call i2c_smbus_read|write_byte_data(),
>>> so remove the corresponding functionality test. It uses regmap
>>> to handle byte transfers transparently.
>>>
>>> Signed-off-by: Alison Schofield 
>>> ---
>>> Found & fixed by inspection based on same defect recently fixed
>>> in light/tpl0102 driver.
>>>
>>>
>>>   drivers/iio/light/jsa1212.c | 3 ---
>>>   1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
>>> index 99a6281..e8a8931 100644
>>> --- a/drivers/iio/light/jsa1212.c
>>> +++ b/drivers/iio/light/jsa1212.c
>>> @@ -325,9 +325,6 @@ static int jsa1212_probe(struct i2c_client *client,
>>> struct regmap *regmap;
>>> int ret;
>>>   - if (!i2c_check_functionality(client->adapter,
>>> I2C_FUNC_SMBUS_BYTE_DATA))
>>> -   return -EOPNOTSUPP;
>>> -
>>> indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
>>> if (!indio_dev)
>>> return -ENOMEM;
>>
>>
>> --
>> Sathyanarayanan Kuppuswamy
>> Android kernel developer
>>

Re: [PATCH v2 03/12] of: add J-Core interrupt controller bindings

[Rich Felker]

On Sat, May 21, 2016 at 08:07:54PM +0200, Geert Uytterhoeven wrote:
> On Sat, May 21, 2016 at 12:34 AM, Rich Felker  wrote:
> > On Fri, May 20, 2016 at 10:04:26AM +0200, Geert Uytterhoeven wrote:
> >> On Fri, May 20, 2016 at 4:53 AM, Rich Felker  wrote:
> >> > +Additional properties required for aic1:
> >> > +
> >> > +- reg : Memory region for configuration.
> >> > +
> >> > +- cpu-offset : For SMP, the offset to the per-cpu memory region for
> >> > +  configuration, to be scaled by the cpu number.
> >>
> >> Does cpu-offset apply to aic1 only?
> >
> > The current kernel driver doesn't have any reason to _need_ cpu-offset
> > for aic2, but since there are registers there that a driver (even a
> > non-Linux one) may want to use, I think it makes sense that it should
> > be present in the bindings.
> 
> Hence the "for aic1" should be dropped?

Yes, I've fixed that locally. Moved reg to required and cpu-offset
to optional but needed for smp.

Rich

Re: [PATCH v2 03/12] of: add J-Core interrupt controller bindings

[Rich Felker]

On Sat, May 21, 2016 at 08:07:54PM +0200, Geert Uytterhoeven wrote:
> On Sat, May 21, 2016 at 12:34 AM, Rich Felker  wrote:
> > On Fri, May 20, 2016 at 10:04:26AM +0200, Geert Uytterhoeven wrote:
> >> On Fri, May 20, 2016 at 4:53 AM, Rich Felker  wrote:
> >> > +Additional properties required for aic1:
> >> > +
> >> > +- reg : Memory region for configuration.
> >> > +
> >> > +- cpu-offset : For SMP, the offset to the per-cpu memory region for
> >> > +  configuration, to be scaled by the cpu number.
> >>
> >> Does cpu-offset apply to aic1 only?
> >
> > The current kernel driver doesn't have any reason to _need_ cpu-offset
> > for aic2, but since there are registers there that a driver (even a
> > non-Linux one) may want to use, I think it makes sense that it should
> > be present in the bindings.
> 
> Hence the "for aic1" should be dropped?

Yes, I've fixed that locally. Moved reg to required and cpu-offset
to optional but needed for smp.

Rich

Re: [PATCH v5] iio: max5487: Add support for Maxim digital potentiometers

[Jonathan Cameron]

On 19/05/16 06:55, Cristina Moraru wrote:
> Add implementation for Maxim MAX5487, MAX5488, MAX5489
> digital potentiometers.
> 
> Datasheet:
> http://datasheets.maximintegrated.com/en/ds/MAX5487-MAX5489.pdf
> 
> Signed-off-by: Cristina Moraru 
> CC: Daniel Baluta 
Applied to the togreg branch of iio.git - initially pushed out as
testing.  As ever with stuff in testing I can still modify the
commit if anyone has anything to add (acks etc welcome of course!)

Jonathan
> ---
> Changes since v4:
>   Add year for copyright
> Changes since v3:
> Fix size issue in max5487_write_cmd
> Fix typo
> Changes since v2:
> Change switch statement in max5487_write_raw
> to if statement for consistency
> Add blank line before return statement
> Eliminate regmap support and use spi_write
> Use iio_device_register instead of devm_ version
> Changes since v1:
> Fix spacing
> Eliminate max5487_cfg struct
> Add kohms as driver data
> 
>  drivers/iio/potentiometer/Kconfig   |  11 +++
>  drivers/iio/potentiometer/Makefile  |   1 +
>  drivers/iio/potentiometer/max5487.c | 161 
> 
>  3 files changed, 173 insertions(+)
>  create mode 100644 drivers/iio/potentiometer/max5487.c
> 
> diff --git a/drivers/iio/potentiometer/Kconfig 
> b/drivers/iio/potentiometer/Kconfig
> index 6acb238..0941c8d4 100644
> --- a/drivers/iio/potentiometer/Kconfig
> +++ b/drivers/iio/potentiometer/Kconfig
> @@ -15,6 +15,17 @@ config DS1803
> To compile this driver as a module, choose M here: the
> module will be called ds1803.
>  
> +config MAX5487
> +tristate "Maxim MAX5487/MAX5488/MAX5489 Digital Potentiometer driver"
> +depends on SPI
> +help
> +  Say yes here to build support for the Maxim
> +  MAX5487, MAX5488, MAX5489 digital potentiometer
> +  chips.
> +
> +  To compile this driver as a module, choose M here: the
> +  module will be called max5487.
> +
>  config MCP4131
>   tristate "Microchip MCP413X/414X/415X/416X/423X/424X/425X/426X Digital 
> Potentiometer driver"
>   depends on SPI
> diff --git a/drivers/iio/potentiometer/Makefile 
> b/drivers/iio/potentiometer/Makefile
> index 6007faa..8adb58f 100644
> --- a/drivers/iio/potentiometer/Makefile
> +++ b/drivers/iio/potentiometer/Makefile
> @@ -4,6 +4,7 @@
>  
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_DS1803) += ds1803.o
> +obj-$(CONFIG_MAX5487) += max5487.o
>  obj-$(CONFIG_MCP4131) += mcp4131.o
>  obj-$(CONFIG_MCP4531) += mcp4531.o
>  obj-$(CONFIG_TPL0102) += tpl0102.o
> diff --git a/drivers/iio/potentiometer/max5487.c 
> b/drivers/iio/potentiometer/max5487.c
> new file mode 100644
> index 000..6c50939
> --- /dev/null
> +++ b/drivers/iio/potentiometer/max5487.c
> @@ -0,0 +1,161 @@
> +/*
> + * max5487.c - Support for MAX5487, MAX5488, MAX5489 digital potentiometers
> + *
> + * Copyright (C) 2016 Cristina-Gabriela Moraru 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define MAX5487_WRITE_WIPER_A(0x01 << 8)
> +#define MAX5487_WRITE_WIPER_B(0x02 << 8)
> +
> +/* copy both wiper regs to NV regs */
> +#define MAX5487_COPY_AB_TO_NV(0x23 << 8)
> +/* copy both NV regs to wiper regs */
> +#define MAX5487_COPY_NV_TO_AB(0x33 << 8)
> +
> +#define MAX5487_MAX_POS  255
> +
> +struct max5487_data {
> + struct spi_device *spi;
> + int kohms;
> +};
> +
> +#define MAX5487_CHANNEL(ch, addr) {  \
> + .type = IIO_RESISTANCE, \
> + .indexed = 1,   \
> + .output = 1,\
> + .channel = ch,  \
> + .address = addr,\
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> +}
> +
> +static const struct iio_chan_spec max5487_channels[] = {
> + MAX5487_CHANNEL(0, MAX5487_WRITE_WIPER_A),
> + MAX5487_CHANNEL(1, MAX5487_WRITE_WIPER_B),
> +};
> +
> +static int max5487_write_cmd(struct spi_device *spi, u16 cmd)
> +{
> + return spi_write(spi, (const void *) , sizeof(u16));
> +}
> +
> +static int max5487_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max5487_data *data = iio_priv(indio_dev);
> +
> + if (mask != IIO_CHAN_INFO_SCALE)
> + return -EINVAL;

Re: [PATCH v5] iio: max5487: Add support for Maxim digital potentiometers

[Jonathan Cameron]

On 19/05/16 06:55, Cristina Moraru wrote:
> Add implementation for Maxim MAX5487, MAX5488, MAX5489
> digital potentiometers.
> 
> Datasheet:
> http://datasheets.maximintegrated.com/en/ds/MAX5487-MAX5489.pdf
> 
> Signed-off-by: Cristina Moraru 
> CC: Daniel Baluta 
Applied to the togreg branch of iio.git - initially pushed out as
testing.  As ever with stuff in testing I can still modify the
commit if anyone has anything to add (acks etc welcome of course!)

Jonathan
> ---
> Changes since v4:
>   Add year for copyright
> Changes since v3:
> Fix size issue in max5487_write_cmd
> Fix typo
> Changes since v2:
> Change switch statement in max5487_write_raw
> to if statement for consistency
> Add blank line before return statement
> Eliminate regmap support and use spi_write
> Use iio_device_register instead of devm_ version
> Changes since v1:
> Fix spacing
> Eliminate max5487_cfg struct
> Add kohms as driver data
> 
>  drivers/iio/potentiometer/Kconfig   |  11 +++
>  drivers/iio/potentiometer/Makefile  |   1 +
>  drivers/iio/potentiometer/max5487.c | 161 
> 
>  3 files changed, 173 insertions(+)
>  create mode 100644 drivers/iio/potentiometer/max5487.c
> 
> diff --git a/drivers/iio/potentiometer/Kconfig 
> b/drivers/iio/potentiometer/Kconfig
> index 6acb238..0941c8d4 100644
> --- a/drivers/iio/potentiometer/Kconfig
> +++ b/drivers/iio/potentiometer/Kconfig
> @@ -15,6 +15,17 @@ config DS1803
> To compile this driver as a module, choose M here: the
> module will be called ds1803.
>  
> +config MAX5487
> +tristate "Maxim MAX5487/MAX5488/MAX5489 Digital Potentiometer driver"
> +depends on SPI
> +help
> +  Say yes here to build support for the Maxim
> +  MAX5487, MAX5488, MAX5489 digital potentiometer
> +  chips.
> +
> +  To compile this driver as a module, choose M here: the
> +  module will be called max5487.
> +
>  config MCP4131
>   tristate "Microchip MCP413X/414X/415X/416X/423X/424X/425X/426X Digital 
> Potentiometer driver"
>   depends on SPI
> diff --git a/drivers/iio/potentiometer/Makefile 
> b/drivers/iio/potentiometer/Makefile
> index 6007faa..8adb58f 100644
> --- a/drivers/iio/potentiometer/Makefile
> +++ b/drivers/iio/potentiometer/Makefile
> @@ -4,6 +4,7 @@
>  
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_DS1803) += ds1803.o
> +obj-$(CONFIG_MAX5487) += max5487.o
>  obj-$(CONFIG_MCP4131) += mcp4131.o
>  obj-$(CONFIG_MCP4531) += mcp4531.o
>  obj-$(CONFIG_TPL0102) += tpl0102.o
> diff --git a/drivers/iio/potentiometer/max5487.c 
> b/drivers/iio/potentiometer/max5487.c
> new file mode 100644
> index 000..6c50939
> --- /dev/null
> +++ b/drivers/iio/potentiometer/max5487.c
> @@ -0,0 +1,161 @@
> +/*
> + * max5487.c - Support for MAX5487, MAX5488, MAX5489 digital potentiometers
> + *
> + * Copyright (C) 2016 Cristina-Gabriela Moraru 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define MAX5487_WRITE_WIPER_A(0x01 << 8)
> +#define MAX5487_WRITE_WIPER_B(0x02 << 8)
> +
> +/* copy both wiper regs to NV regs */
> +#define MAX5487_COPY_AB_TO_NV(0x23 << 8)
> +/* copy both NV regs to wiper regs */
> +#define MAX5487_COPY_NV_TO_AB(0x33 << 8)
> +
> +#define MAX5487_MAX_POS  255
> +
> +struct max5487_data {
> + struct spi_device *spi;
> + int kohms;
> +};
> +
> +#define MAX5487_CHANNEL(ch, addr) {  \
> + .type = IIO_RESISTANCE, \
> + .indexed = 1,   \
> + .output = 1,\
> + .channel = ch,  \
> + .address = addr,\
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> +}
> +
> +static const struct iio_chan_spec max5487_channels[] = {
> + MAX5487_CHANNEL(0, MAX5487_WRITE_WIPER_A),
> + MAX5487_CHANNEL(1, MAX5487_WRITE_WIPER_B),
> +};
> +
> +static int max5487_write_cmd(struct spi_device *spi, u16 cmd)
> +{
> + return spi_write(spi, (const void *) , sizeof(u16));
> +}
> +
> +static int max5487_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max5487_data *data = iio_priv(indio_dev);
> +
> + if (mask != IIO_CHAN_INFO_SCALE)
> + return -EINVAL;
> +
> + *val = 1000 * data->kohms;
> + *val2 = MAX5487_MAX_POS;
> +
> +

Re: [PATCH] iio: Export I2C module alias information

[Jonathan Cameron]

On 17/05/16 17:25, Javier Martinez Canillas wrote:
> The I2C drivers have an i2c_device_id array but that information isn't
> exported to the modules using the MODULE_DEVICE_TABLE() macro. So the
> modules autoloading won't work if the I2C device is registered using
> OF or legacy board files due missing alias information in the modules.
> 
> The issue was found using Kieran Bingham's coccinelle semantic patch:
> https://lkml.org/lkml/2016/5/10/520
> 
> Signed-off-by: Javier Martinez Canillas 
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

thanks

Jonathan
> 
> ---
> 
>  drivers/iio/humidity/am2315.c | 1 +
>  drivers/iio/humidity/htu21.c  | 1 +
>  drivers/iio/pressure/hp206c.c | 1 +
>  drivers/iio/pressure/ms5637.c | 1 +
>  drivers/iio/temperature/tsys02d.c | 1 +
>  5 files changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/humidity/am2315.c b/drivers/iio/humidity/am2315.c
> index 3be6d209a159..8de39bd349f9 100644
> --- a/drivers/iio/humidity/am2315.c
> +++ b/drivers/iio/humidity/am2315.c
> @@ -278,6 +278,7 @@ static const struct i2c_device_id am2315_i2c_id[] = {
>   {"am2315", 0},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, am2315_i2c_id);
>  
>  static const struct acpi_device_id am2315_acpi_id[] = {
>   {"AOS2315", 0},
> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
> index 11cbc38b450f..0fbbd8c40894 100644
> --- a/drivers/iio/humidity/htu21.c
> +++ b/drivers/iio/humidity/htu21.c
> @@ -236,6 +236,7 @@ static const struct i2c_device_id htu21_id[] = {
>   {"ms8607-humidity", MS8607},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
>  
>  static struct i2c_driver htu21_driver = {
>   .probe = htu21_probe,
> diff --git a/drivers/iio/pressure/hp206c.c b/drivers/iio/pressure/hp206c.c
> index 90f2b6e4a920..12f769e86355 100644
> --- a/drivers/iio/pressure/hp206c.c
> +++ b/drivers/iio/pressure/hp206c.c
> @@ -401,6 +401,7 @@ static const struct i2c_device_id hp206c_id[] = {
>   {"hp206c"},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, hp206c_id);
>  
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id hp206c_acpi_match[] = {
> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
> index e68052c118e6..8fb6f7ab97e4 100644
> --- a/drivers/iio/pressure/ms5637.c
> +++ b/drivers/iio/pressure/ms5637.c
> @@ -173,6 +173,7 @@ static const struct i2c_device_id ms5637_id[] = {
>   {"ms8607-temppressure", 1},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
>  
>  static struct i2c_driver ms5637_driver = {
>   .probe = ms5637_probe,
> diff --git a/drivers/iio/temperature/tsys02d.c 
> b/drivers/iio/temperature/tsys02d.c
> index ab6fe8f6f2d1..c0a19a000387 100644
> --- a/drivers/iio/temperature/tsys02d.c
> +++ b/drivers/iio/temperature/tsys02d.c
> @@ -174,6 +174,7 @@ static const struct i2c_device_id tsys02d_id[] = {
>   {"tsys02d", 0},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, tsys02d_id);
>  
>  static struct i2c_driver tsys02d_driver = {
>   .probe = tsys02d_probe,
> 

Re: [PATCH] iio: Export I2C module alias information

[Jonathan Cameron]

On 17/05/16 17:25, Javier Martinez Canillas wrote:
> The I2C drivers have an i2c_device_id array but that information isn't
> exported to the modules using the MODULE_DEVICE_TABLE() macro. So the
> modules autoloading won't work if the I2C device is registered using
> OF or legacy board files due missing alias information in the modules.
> 
> The issue was found using Kieran Bingham's coccinelle semantic patch:
> https://lkml.org/lkml/2016/5/10/520
> 
> Signed-off-by: Javier Martinez Canillas 
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

thanks

Jonathan
> 
> ---
> 
>  drivers/iio/humidity/am2315.c | 1 +
>  drivers/iio/humidity/htu21.c  | 1 +
>  drivers/iio/pressure/hp206c.c | 1 +
>  drivers/iio/pressure/ms5637.c | 1 +
>  drivers/iio/temperature/tsys02d.c | 1 +
>  5 files changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/humidity/am2315.c b/drivers/iio/humidity/am2315.c
> index 3be6d209a159..8de39bd349f9 100644
> --- a/drivers/iio/humidity/am2315.c
> +++ b/drivers/iio/humidity/am2315.c
> @@ -278,6 +278,7 @@ static const struct i2c_device_id am2315_i2c_id[] = {
>   {"am2315", 0},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, am2315_i2c_id);
>  
>  static const struct acpi_device_id am2315_acpi_id[] = {
>   {"AOS2315", 0},
> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
> index 11cbc38b450f..0fbbd8c40894 100644
> --- a/drivers/iio/humidity/htu21.c
> +++ b/drivers/iio/humidity/htu21.c
> @@ -236,6 +236,7 @@ static const struct i2c_device_id htu21_id[] = {
>   {"ms8607-humidity", MS8607},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
>  
>  static struct i2c_driver htu21_driver = {
>   .probe = htu21_probe,
> diff --git a/drivers/iio/pressure/hp206c.c b/drivers/iio/pressure/hp206c.c
> index 90f2b6e4a920..12f769e86355 100644
> --- a/drivers/iio/pressure/hp206c.c
> +++ b/drivers/iio/pressure/hp206c.c
> @@ -401,6 +401,7 @@ static const struct i2c_device_id hp206c_id[] = {
>   {"hp206c"},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, hp206c_id);
>  
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id hp206c_acpi_match[] = {
> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
> index e68052c118e6..8fb6f7ab97e4 100644
> --- a/drivers/iio/pressure/ms5637.c
> +++ b/drivers/iio/pressure/ms5637.c
> @@ -173,6 +173,7 @@ static const struct i2c_device_id ms5637_id[] = {
>   {"ms8607-temppressure", 1},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
>  
>  static struct i2c_driver ms5637_driver = {
>   .probe = ms5637_probe,
> diff --git a/drivers/iio/temperature/tsys02d.c 
> b/drivers/iio/temperature/tsys02d.c
> index ab6fe8f6f2d1..c0a19a000387 100644
> --- a/drivers/iio/temperature/tsys02d.c
> +++ b/drivers/iio/temperature/tsys02d.c
> @@ -174,6 +174,7 @@ static const struct i2c_device_id tsys02d_id[] = {
>   {"tsys02d", 0},
>   {}
>  };
> +MODULE_DEVICE_TABLE(i2c, tsys02d_id);
>  
>  static struct i2c_driver tsys02d_driver = {
>   .probe = tsys02d_probe,
> 

Re: [PATCH 2/3] sched,fair: Fix local starvation

[Mike Galbraith]

On Sat, 2016-05-21 at 16:04 +0200, Mike Galbraith wrote:

> Wakees that were not migrated/normalized eat an unwanted min_vruntime,
> and likely take a size XXL latency hit.  Big box running master bled
> profusely under heavy load until I turned TTWU_QUEUE off.

The below made big box a happy camper again.

sched/fair: Move se->vruntime normalization state into struct sched_entity

Make ->vruntime normalization state explicit.

Signed-off-by: Mike Galbraith 
---
 include/linux/sched.h |1 +
 kernel/sched/core.c   |1 +
 kernel/sched/fair.c   |   42 ++
 3 files changed, 16 insertions(+), 28 deletions(-)

--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1348,6 +1348,7 @@ struct sched_entity {
struct rb_node  run_node;
struct list_headgroup_node;
unsigned inton_rq;
+   boolnormalized;
 
u64 exec_start;
u64 sum_exec_runtime;
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2259,6 +2259,7 @@ static void __sched_fork(unsigned long c
p->se.prev_sum_exec_runtime = 0;
p->se.nr_migrations = 0;
p->se.vruntime  = 0;
+   p->se.normalized= true;
INIT_LIST_HEAD(>se.group_node);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3305,14 +3305,13 @@ static inline void check_schedstat_requi
 static void
 enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
-   bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATED);
bool curr = cfs_rq->curr == se;
 
/*
 * If we're the current task, we must renormalise before calling
 * update_curr().
 */
-   if (renorm && curr)
+   if (se->normalized && curr)
se->vruntime += cfs_rq->min_vruntime;
 
update_curr(cfs_rq);
@@ -3323,9 +3322,11 @@ enqueue_entity(struct cfs_rq *cfs_rq, st
 * placed in the past could significantly boost this task to the
 * fairness detriment of existing tasks.
 */
-   if (renorm && !curr)
+   if (se->normalized && !curr)
se->vruntime += cfs_rq->min_vruntime;
 
+   se->normalized = false;
+
enqueue_entity_load_avg(cfs_rq, se);
account_entity_enqueue(cfs_rq, se);
update_cfs_shares(cfs_rq);
@@ -3422,8 +3423,10 @@ dequeue_entity(struct cfs_rq *cfs_rq, st
 * update can refer to the ->curr item and we need to reflect this
 * movement in our normalized position.
 */
-   if (!(flags & DEQUEUE_SLEEP))
+   if (!(flags & DEQUEUE_SLEEP)) {
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
+   }
 
/* return excess runtime on last dequeue */
return_cfs_rq_runtime(cfs_rq);
@@ -5681,6 +5684,7 @@ static void migrate_task_rq_fair(struct
 #endif
 
se->vruntime -= min_vruntime;
+   se->normalized = true;
}
 
/*
@@ -8591,6 +8595,7 @@ static void task_fork_fair(struct task_s
}
 
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
 
raw_spin_unlock_irqrestore(>lock, flags);
 }
@@ -8619,29 +8624,7 @@ prio_changed_fair(struct rq *rq, struct
 
 static inline bool vruntime_normalized(struct task_struct *p)
 {
-   struct sched_entity *se = >se;
-
-   /*
-* In both the TASK_ON_RQ_QUEUED and TASK_ON_RQ_MIGRATING cases,
-* the dequeue_entity(.flags=0) will already have normalized the
-* vruntime.
-*/
-   if (p->on_rq)
-   return true;
-
-   /*
-* When !on_rq, vruntime of the task has usually NOT been normalized.
-* But there are some cases where it has already been normalized:
-*
-* - A forked child which is waiting for being woken up by
-*   wake_up_new_task().
-* - A task which has been woken up by try_to_wake_up() and
-*   waiting for actually being woken up by sched_ttwu_pending().
-*/
-   if (!se->sum_exec_runtime || p->state == TASK_WAKING)
-   return true;
-
-   return false;
+   return p->se.normalized;
 }
 
 static void detach_task_cfs_rq(struct task_struct *p)
@@ -8656,6 +8639,7 @@ static void detach_task_cfs_rq(struct ta
 */
place_entity(cfs_rq, se, 0);
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
}
 
/* Catch up with the cfs_rq and remove our load when we leave */
@@ -8678,8 +8662,10 @@ static void attach_task_cfs_rq(struct ta
/* Synchronize task with its cfs_rq */
attach_entity_load_avg(cfs_rq, se);
 
-   if (!vruntime_normalized(p))
+   if (vruntime_normalized(p)) {
se->vruntime += 

Re: [PATCH 2/3] sched,fair: Fix local starvation

[Mike Galbraith]

On Sat, 2016-05-21 at 16:04 +0200, Mike Galbraith wrote:

> Wakees that were not migrated/normalized eat an unwanted min_vruntime,
> and likely take a size XXL latency hit.  Big box running master bled
> profusely under heavy load until I turned TTWU_QUEUE off.

The below made big box a happy camper again.

sched/fair: Move se->vruntime normalization state into struct sched_entity

Make ->vruntime normalization state explicit.

Signed-off-by: Mike Galbraith 
---
 include/linux/sched.h |1 +
 kernel/sched/core.c   |1 +
 kernel/sched/fair.c   |   42 ++
 3 files changed, 16 insertions(+), 28 deletions(-)

--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1348,6 +1348,7 @@ struct sched_entity {
struct rb_node  run_node;
struct list_headgroup_node;
unsigned inton_rq;
+   boolnormalized;
 
u64 exec_start;
u64 sum_exec_runtime;
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2259,6 +2259,7 @@ static void __sched_fork(unsigned long c
p->se.prev_sum_exec_runtime = 0;
p->se.nr_migrations = 0;
p->se.vruntime  = 0;
+   p->se.normalized= true;
INIT_LIST_HEAD(>se.group_node);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3305,14 +3305,13 @@ static inline void check_schedstat_requi
 static void
 enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
-   bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATED);
bool curr = cfs_rq->curr == se;
 
/*
 * If we're the current task, we must renormalise before calling
 * update_curr().
 */
-   if (renorm && curr)
+   if (se->normalized && curr)
se->vruntime += cfs_rq->min_vruntime;
 
update_curr(cfs_rq);
@@ -3323,9 +3322,11 @@ enqueue_entity(struct cfs_rq *cfs_rq, st
 * placed in the past could significantly boost this task to the
 * fairness detriment of existing tasks.
 */
-   if (renorm && !curr)
+   if (se->normalized && !curr)
se->vruntime += cfs_rq->min_vruntime;
 
+   se->normalized = false;
+
enqueue_entity_load_avg(cfs_rq, se);
account_entity_enqueue(cfs_rq, se);
update_cfs_shares(cfs_rq);
@@ -3422,8 +3423,10 @@ dequeue_entity(struct cfs_rq *cfs_rq, st
 * update can refer to the ->curr item and we need to reflect this
 * movement in our normalized position.
 */
-   if (!(flags & DEQUEUE_SLEEP))
+   if (!(flags & DEQUEUE_SLEEP)) {
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
+   }
 
/* return excess runtime on last dequeue */
return_cfs_rq_runtime(cfs_rq);
@@ -5681,6 +5684,7 @@ static void migrate_task_rq_fair(struct
 #endif
 
se->vruntime -= min_vruntime;
+   se->normalized = true;
}
 
/*
@@ -8591,6 +8595,7 @@ static void task_fork_fair(struct task_s
}
 
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
 
raw_spin_unlock_irqrestore(>lock, flags);
 }
@@ -8619,29 +8624,7 @@ prio_changed_fair(struct rq *rq, struct
 
 static inline bool vruntime_normalized(struct task_struct *p)
 {
-   struct sched_entity *se = >se;
-
-   /*
-* In both the TASK_ON_RQ_QUEUED and TASK_ON_RQ_MIGRATING cases,
-* the dequeue_entity(.flags=0) will already have normalized the
-* vruntime.
-*/
-   if (p->on_rq)
-   return true;
-
-   /*
-* When !on_rq, vruntime of the task has usually NOT been normalized.
-* But there are some cases where it has already been normalized:
-*
-* - A forked child which is waiting for being woken up by
-*   wake_up_new_task().
-* - A task which has been woken up by try_to_wake_up() and
-*   waiting for actually being woken up by sched_ttwu_pending().
-*/
-   if (!se->sum_exec_runtime || p->state == TASK_WAKING)
-   return true;
-
-   return false;
+   return p->se.normalized;
 }
 
 static void detach_task_cfs_rq(struct task_struct *p)
@@ -8656,6 +8639,7 @@ static void detach_task_cfs_rq(struct ta
 */
place_entity(cfs_rq, se, 0);
se->vruntime -= cfs_rq->min_vruntime;
+   se->normalized = true;
}
 
/* Catch up with the cfs_rq and remove our load when we leave */
@@ -8678,8 +8662,10 @@ static void attach_task_cfs_rq(struct ta
/* Synchronize task with its cfs_rq */
attach_entity_load_avg(cfs_rq, se);
 
-   if (!vruntime_normalized(p))
+   if (vruntime_normalized(p)) {
se->vruntime += cfs_rq->min_vruntime;
+   

[PATCH 0/2] f_fs: better handle excess data on read

[Michal Nazarewicz]

Between this set and Du, Changbin’s patch, we now have implementation
for the following three possibilities to choose from:
1. Buffer excess data (this whole patch set).
2. Fail the transfer (Du, Changbin’s patch).
3. Drop excess data (the first patch from this set).

As per earlier comments, I think 3. is the correct, i.e. the second
patch should not be ocmmited because:
* it complicates the code,
* doesn’t handle AIO anyway,
* introduces weird behaviours when partial read happens just before
  endpoint is disabled (we may end up silently dropping excess data
  anyway),
* goes beyond what UDC does and
* breaks one read -> one request model which has been true so far.

Michal Nazarewicz (2):
  usb: gadget: f_fs: printk error when excess data is dropped on read
  usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests

 drivers/usb/gadget/function/f_fs.c | 179 ++---
 1 file changed, 145 insertions(+), 34 deletions(-)

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

[PATCH 0/2] f_fs: better handle excess data on read

[Michal Nazarewicz]

Between this set and Du, Changbin’s patch, we now have implementation
for the following three possibilities to choose from:
1. Buffer excess data (this whole patch set).
2. Fail the transfer (Du, Changbin’s patch).
3. Drop excess data (the first patch from this set).

As per earlier comments, I think 3. is the correct, i.e. the second
patch should not be ocmmited because:
* it complicates the code,
* doesn’t handle AIO anyway,
* introduces weird behaviours when partial read happens just before
  endpoint is disabled (we may end up silently dropping excess data
  anyway),
* goes beyond what UDC does and
* breaks one read -> one request model which has been true so far.

Michal Nazarewicz (2):
  usb: gadget: f_fs: printk error when excess data is dropped on read
  usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests

 drivers/usb/gadget/function/f_fs.c | 179 ++---
 1 file changed, 145 insertions(+), 34 deletions(-)

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

[PATCH 2/2] usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests

[Michal Nazarewicz]

f_fs rounds up read(2) requests to a multiple of a max packet size
which means that host may provide more data than user has space for.
So far, the excess data has been silently ignored.

This introduces a buffer for a tail of such requests so that they are
returned on next read instead of being ignored.

Signed-off-by: Michal Nazarewicz 
---
 drivers/usb/gadget/function/f_fs.c | 130 +++--
 1 file changed, 109 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index e26a6b4..08a1ac2 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -130,6 +130,12 @@ struct ffs_epfile {
 
struct dentry   *dentry;
 
+   /*
+* Buffer for holding data from partial reads which may happen since
+* we’re rounding user read requests to a multiple of a max packet size.
+*/
+   struct ffs_buffer   *read_buffer;   /* P: epfile->mutex */
+
charname[5];
 
unsigned char   in; /* P: ffs->eps_lock */
@@ -138,6 +144,12 @@ struct ffs_epfile {
unsigned char   _pad;
 };
 
+struct ffs_buffer {
+   size_t length;
+   char *data;
+   char storage[];
+};
+
 /*  ffs_io_data structure ***/
 
 struct ffs_io_data {
@@ -667,6 +679,11 @@ static ssize_t ffs_copy_to_iter(void *data, int data_len, 
struct iov_iter *iter)
 * Was the buffer aligned in the first place, no such problem would
 * happen.
 *
+* Data may be dropped only in AIO reads.  Synchronous reads are handled
+* by splitting a request into multiple parts.  This splitting may still
+* be a problem though so it’s likely best to align the buffer
+* regardless of it being AIO or not..
+*
 * This only affects OUT endpoints, i.e. reading data with a read(2),
 * aio_read(2) etc. system calls.  Writing data to an IN endpoint is not
 * affected.
@@ -717,6 +734,56 @@ static void ffs_epfile_async_io_complete(struct usb_ep 
*_ep,
schedule_work(_data->work);
 }
 
+/* Assumes epfile->mutex is held. */
+static ssize_t __ffs_epfile_read_buffered(struct ffs_epfile *epfile,
+ struct iov_iter *iter)
+{
+   struct ffs_buffer *buf = epfile->read_buffer;
+   ssize_t ret;
+   if (!buf)
+   return 0;
+
+   ret = copy_to_iter(buf->data, buf->length, iter);
+   if (buf->length == ret) {
+   kfree(buf);
+   epfile->read_buffer = NULL;
+   } else if (unlikely(iov_iter_count(iter))) {
+   ret = -EFAULT;
+   } else {
+   buf->length -= ret;
+   buf->data += ret;
+   }
+   return ret;
+}
+
+/* Assumes epfile->mutex is held. */
+static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
+ void *data, int data_len,
+ struct iov_iter *iter)
+{
+   struct ffs_buffer *buf;
+
+   ssize_t ret = copy_to_iter(data, data_len, iter);
+   if (likely(data_len == ret))
+   return ret;
+
+   if (unlikely(iov_iter_count(iter)))
+   return -EFAULT;
+
+   /* See ffs_copy_to_iter for more context. */
+   pr_warn("functionfs read size %d > requested size %zd, splitting 
request into multiple reads.",
+   data_len, ret);
+
+   data_len -= ret;
+   buf = kmalloc(sizeof(*buf) + data_len, GFP_KERNEL);
+   buf->length = data_len;
+   buf->data = buf->storage;
+   memcpy(buf->storage, data + ret, data_len);
+   epfile->read_buffer = buf;
+
+   return ret;
+}
+
 static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 {
struct ffs_epfile *epfile = file->private_data;
@@ -746,21 +813,40 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
if (halt && epfile->isoc)
return -EINVAL;
 
+   /* We will be using request and read_buffer */
+   ret = ffs_mutex_lock(>mutex, file->f_flags & O_NONBLOCK);
+   if (unlikely(ret))
+   goto error;
+
/* Allocate & copy */
if (!halt) {
+   struct usb_gadget *gadget;
+
+   /*
+* Do we have buffered data from previous partial read?  Check
+* that for synchronous case only because we do not have
+* facility to ‘wake up’ a pending asynchronous read and push
+* buffered data to it which we would need to make things behave
+* consistently.
+*/
+   if (!io_data->aio && io_data->read) {
+   ret = __ffs_epfile_read_buffered(epfile, 
_data->data);
+   if (ret)
+

[PATCH 2/2] usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests

[Michal Nazarewicz]

f_fs rounds up read(2) requests to a multiple of a max packet size
which means that host may provide more data than user has space for.
So far, the excess data has been silently ignored.

This introduces a buffer for a tail of such requests so that they are
returned on next read instead of being ignored.

Signed-off-by: Michal Nazarewicz 
---
 drivers/usb/gadget/function/f_fs.c | 130 +++--
 1 file changed, 109 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index e26a6b4..08a1ac2 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -130,6 +130,12 @@ struct ffs_epfile {
 
struct dentry   *dentry;
 
+   /*
+* Buffer for holding data from partial reads which may happen since
+* we’re rounding user read requests to a multiple of a max packet size.
+*/
+   struct ffs_buffer   *read_buffer;   /* P: epfile->mutex */
+
charname[5];
 
unsigned char   in; /* P: ffs->eps_lock */
@@ -138,6 +144,12 @@ struct ffs_epfile {
unsigned char   _pad;
 };
 
+struct ffs_buffer {
+   size_t length;
+   char *data;
+   char storage[];
+};
+
 /*  ffs_io_data structure ***/
 
 struct ffs_io_data {
@@ -667,6 +679,11 @@ static ssize_t ffs_copy_to_iter(void *data, int data_len, 
struct iov_iter *iter)
 * Was the buffer aligned in the first place, no such problem would
 * happen.
 *
+* Data may be dropped only in AIO reads.  Synchronous reads are handled
+* by splitting a request into multiple parts.  This splitting may still
+* be a problem though so it’s likely best to align the buffer
+* regardless of it being AIO or not..
+*
 * This only affects OUT endpoints, i.e. reading data with a read(2),
 * aio_read(2) etc. system calls.  Writing data to an IN endpoint is not
 * affected.
@@ -717,6 +734,56 @@ static void ffs_epfile_async_io_complete(struct usb_ep 
*_ep,
schedule_work(_data->work);
 }
 
+/* Assumes epfile->mutex is held. */
+static ssize_t __ffs_epfile_read_buffered(struct ffs_epfile *epfile,
+ struct iov_iter *iter)
+{
+   struct ffs_buffer *buf = epfile->read_buffer;
+   ssize_t ret;
+   if (!buf)
+   return 0;
+
+   ret = copy_to_iter(buf->data, buf->length, iter);
+   if (buf->length == ret) {
+   kfree(buf);
+   epfile->read_buffer = NULL;
+   } else if (unlikely(iov_iter_count(iter))) {
+   ret = -EFAULT;
+   } else {
+   buf->length -= ret;
+   buf->data += ret;
+   }
+   return ret;
+}
+
+/* Assumes epfile->mutex is held. */
+static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
+ void *data, int data_len,
+ struct iov_iter *iter)
+{
+   struct ffs_buffer *buf;
+
+   ssize_t ret = copy_to_iter(data, data_len, iter);
+   if (likely(data_len == ret))
+   return ret;
+
+   if (unlikely(iov_iter_count(iter)))
+   return -EFAULT;
+
+   /* See ffs_copy_to_iter for more context. */
+   pr_warn("functionfs read size %d > requested size %zd, splitting 
request into multiple reads.",
+   data_len, ret);
+
+   data_len -= ret;
+   buf = kmalloc(sizeof(*buf) + data_len, GFP_KERNEL);
+   buf->length = data_len;
+   buf->data = buf->storage;
+   memcpy(buf->storage, data + ret, data_len);
+   epfile->read_buffer = buf;
+
+   return ret;
+}
+
 static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 {
struct ffs_epfile *epfile = file->private_data;
@@ -746,21 +813,40 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
if (halt && epfile->isoc)
return -EINVAL;
 
+   /* We will be using request and read_buffer */
+   ret = ffs_mutex_lock(>mutex, file->f_flags & O_NONBLOCK);
+   if (unlikely(ret))
+   goto error;
+
/* Allocate & copy */
if (!halt) {
+   struct usb_gadget *gadget;
+
+   /*
+* Do we have buffered data from previous partial read?  Check
+* that for synchronous case only because we do not have
+* facility to ‘wake up’ a pending asynchronous read and push
+* buffered data to it which we would need to make things behave
+* consistently.
+*/
+   if (!io_data->aio && io_data->read) {
+   ret = __ffs_epfile_read_buffered(epfile, 
_data->data);
+   if (ret)
+   

[PATCH 1/2] usb: gadget: f_fs: printk error when excess data is dropped on read

[Michal Nazarewicz]

Add a pr_err when host sent more data then the size of the buffer user
space gave us.  This may happen on UDCs which require OUT requests to
be aligned to max packet size.  The patch includes a description of the
situation.

Signed-off-by: Michal Nazarewicz 
---
 drivers/usb/gadget/function/f_fs.c | 61 --
 1 file changed, 46 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 2c314c1..e26a6b4 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -640,6 +640,44 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, 
struct usb_request *req)
}
 }
 
+static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter 
*iter)
+{
+   ssize_t ret = copy_to_iter(data, data_len, iter);
+   if (likely(ret == data_len))
+   return ret;
+
+   if (unlikely(iov_iter_count(iter)))
+   return -EFAULT;
+
+   /*
+* Dear user space developer!
+*
+* TL;DR: To stop getting below error message in your kernel log, change
+* user space code using functionfs to align read buffers to a max
+* packet size.
+*
+* Some UDCs (e.g. dwc3) require request sizes to be a multiple of a max
+* packet size.  When unaligned buffer is passed to functionfs, it
+* internally uses a larger, aligned buffer so that such UDCs are happy.
+*
+* Unfortunately, this means that host may send more data than was
+* requested in read(2) system call.  f_fs doesn’t know what to do with
+* that excess data so it simply drops it.
+*
+* Was the buffer aligned in the first place, no such problem would
+* happen.
+*
+* This only affects OUT endpoints, i.e. reading data with a read(2),
+* aio_read(2) etc. system calls.  Writing data to an IN endpoint is not
+* affected.
+*/
+   pr_err("functionfs read size %d > requested size %zd, dropping excess 
data. "
+  "Align read buffer size to max packet size to avoid the 
problem.\n",
+  data_len, ret);
+
+   return ret;
+}
+
 static void ffs_user_copy_worker(struct work_struct *work)
 {
struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
@@ -649,9 +687,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
if (io_data->read && ret > 0) {
use_mm(io_data->mm);
-   ret = copy_to_iter(io_data->buf, ret, _data->data);
-   if (ret != io_data->req->actual && 
iov_iter_count(_data->data))
-   ret = -EFAULT;
+   ret = ffs_copy_to_iter(io_data->buf, ret, _data->data);
unuse_mm(io_data->mm);
}
 
@@ -804,18 +840,13 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
interrupted = ep->status < 0;
}
 
-   /*
-* XXX We may end up silently droping data here.  Since data_len
-* (i.e. req->length) may be bigger than len (after being
-* rounded up to maxpacketsize), we may end up with more data
-* then user space has space for.
-*/
-   ret = interrupted ? -EINTR : ep->status;
-   if (io_data->read && ret > 0) {
-   ret = copy_to_iter(data, ret, _data->data);
-   if (!ret)
-   ret = -EFAULT;
-   }
+   if (interrupted)
+   ret = -EINTR;
+   else if (io_data->read && ep->status > 0)
+   ret = ffs_copy_to_iter(data, ep->status,
+  _data->data);
+   else
+   ret = ep->status;
goto error_mutex;
} else if (!(req = usb_ep_alloc_request(ep->ep, GFP_KERNEL))) {
ret = -ENOMEM;
-- 
2.8.0.rc3.226.g39d4020

[PATCH 1/2] usb: gadget: f_fs: printk error when excess data is dropped on read

[Michal Nazarewicz]

Add a pr_err when host sent more data then the size of the buffer user
space gave us.  This may happen on UDCs which require OUT requests to
be aligned to max packet size.  The patch includes a description of the
situation.

Signed-off-by: Michal Nazarewicz 
---
 drivers/usb/gadget/function/f_fs.c | 61 --
 1 file changed, 46 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 2c314c1..e26a6b4 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -640,6 +640,44 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, 
struct usb_request *req)
}
 }
 
+static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter 
*iter)
+{
+   ssize_t ret = copy_to_iter(data, data_len, iter);
+   if (likely(ret == data_len))
+   return ret;
+
+   if (unlikely(iov_iter_count(iter)))
+   return -EFAULT;
+
+   /*
+* Dear user space developer!
+*
+* TL;DR: To stop getting below error message in your kernel log, change
+* user space code using functionfs to align read buffers to a max
+* packet size.
+*
+* Some UDCs (e.g. dwc3) require request sizes to be a multiple of a max
+* packet size.  When unaligned buffer is passed to functionfs, it
+* internally uses a larger, aligned buffer so that such UDCs are happy.
+*
+* Unfortunately, this means that host may send more data than was
+* requested in read(2) system call.  f_fs doesn’t know what to do with
+* that excess data so it simply drops it.
+*
+* Was the buffer aligned in the first place, no such problem would
+* happen.
+*
+* This only affects OUT endpoints, i.e. reading data with a read(2),
+* aio_read(2) etc. system calls.  Writing data to an IN endpoint is not
+* affected.
+*/
+   pr_err("functionfs read size %d > requested size %zd, dropping excess 
data. "
+  "Align read buffer size to max packet size to avoid the 
problem.\n",
+  data_len, ret);
+
+   return ret;
+}
+
 static void ffs_user_copy_worker(struct work_struct *work)
 {
struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
@@ -649,9 +687,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
if (io_data->read && ret > 0) {
use_mm(io_data->mm);
-   ret = copy_to_iter(io_data->buf, ret, _data->data);
-   if (ret != io_data->req->actual && 
iov_iter_count(_data->data))
-   ret = -EFAULT;
+   ret = ffs_copy_to_iter(io_data->buf, ret, _data->data);
unuse_mm(io_data->mm);
}
 
@@ -804,18 +840,13 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
interrupted = ep->status < 0;
}
 
-   /*
-* XXX We may end up silently droping data here.  Since data_len
-* (i.e. req->length) may be bigger than len (after being
-* rounded up to maxpacketsize), we may end up with more data
-* then user space has space for.
-*/
-   ret = interrupted ? -EINTR : ep->status;
-   if (io_data->read && ret > 0) {
-   ret = copy_to_iter(data, ret, _data->data);
-   if (!ret)
-   ret = -EFAULT;
-   }
+   if (interrupted)
+   ret = -EINTR;
+   else if (io_data->read && ep->status > 0)
+   ret = ffs_copy_to_iter(data, ep->status,
+  _data->data);
+   else
+   ret = ep->status;
goto error_mutex;
} else if (!(req = usb_ep_alloc_request(ep->ep, GFP_KERNEL))) {
ret = -ENOMEM;
-- 
2.8.0.rc3.226.g39d4020

RE: [PATCH] ftrace: As disabling interrupts is costly and write_lock variant of tasklist_lock is not held from interrupt context it is not necessary to disable interrupts.

[N, Soumya P]

On Thu, 19 May 2016 13:49:16 +
"N, Soumya P"  wrote:

> Hi Steve,
> 
> Thanks for the explanation.
> I will take care of your comments and send v2 of the same patch.
> 

>>No need, I just pulled in your patch and made the updates to the change log 
>>and subject myself. I'm starting my tests on it now.

> Thanks Steve.

RE: [PATCH] ftrace: As disabling interrupts is costly and write_lock variant of tasklist_lock is not held from interrupt context it is not necessary to disable interrupts.

[N, Soumya P]

On Thu, 19 May 2016 13:49:16 +
"N, Soumya P"  wrote:

> Hi Steve,
> 
> Thanks for the explanation.
> I will take care of your comments and send v2 of the same patch.
> 

>>No need, I just pulled in your patch and made the updates to the change log 
>>and subject myself. I'm starting my tests on it now.

> Thanks Steve.

Re: [PATCH v8 2/4] power: reset: add reboot mode driver

[Bjorn Andersson]

On Sun, Apr 24, 2016 at 11:55 PM, Andy Yan  wrote:
[..]
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig

[..]

> +config SYSCON_REBOOT_MODE
> +   bool "Generic SYSCON regmap reboot mode driver"
> +   depends on OF

As this isn't really useful without syscon (but still compiles), I
would suggest adding:

depends on SYSCON || COMPILE_TEST

> +   select REBOOT_MODE
> +   help
> + Say y here will enable reboot mode driver. This will
> + get reboot mode arguments and store it in SYSCON mapped
> + register, then the bootloader can read it to take different
> + action according to the mode.
> +
>  endif
>

[..]

> diff --git a/drivers/power/reset/reboot-mode.c 
> b/drivers/power/reset/reboot-mode.c

[..]

> +
> +#define PREFIX "mode-"
> +
> +struct mode_info {
> +   const char *mode;
> +   unsigned int magic;

If you're using of_property_read_u32() to populate this directly you
should have this as u32.

> +   struct list_head list;
> +};
> +
> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> +const char *cmd)

Magic comes from an unsigned property and is passed as unsigned to the
regmap api. Please make it unsigned int throughout the implementation.

> +{
> +   const char *normal = "normal";
> +   int magic = 0;
> +   struct mode_info *info;
> +
> +   if (!cmd)
> +   cmd = normal;
> +
> +   list_for_each_entry(info, >head, list) {
> +   if (!strcmp(info->mode, cmd)) {
> +   magic = info->magic;
> +   break;
> +   }
> +   }
> +

[..]

> +int reboot_mode_register(struct reboot_mode_driver *reboot)
> +{
> +   struct mode_info *info;
> +   struct property *prop;
> +   struct device_node *np = reboot->dev->of_node;
> +   size_t len = strlen(PREFIX);
> +   int ret;
> +
> +   INIT_LIST_HEAD(>head);
> +
> +   for_each_property_of_node(np, prop) {
> +   if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, 
> len))

There's no reason to iterate over the string twice here, strncmp will
exit with -1 if you hit a '\0' before strlen(PREFIX). So you can drop
the first check without any difference in functionality.

What passes this checkout though is if the property equals PREFIX, but
this is probably better to handle as an error, rather than skip over.
So do that below

> +   continue;
> +
> +   info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
> +   if (!info) {
> +   ret = -ENOMEM;
> +   goto error;
> +   }
> +
> +   info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);

You need something like this here:

if (!info->mode) {
ret = -ENOMEM;
goto error;
} else if (info->mode[0] == '\0')
dev_err("mode too short");
ret = -EINVAL;
goto error;
}


If you do the kernel a favor and submit a patch to
drivers/base/devres.c adding devm_kstrdup_const() you don't have to do
the goto dance at all.

> +   if (of_property_read_u32(np, prop->name, >magic)) {
> +   dev_err(reboot->dev, "reboot mode %s without magic 
> number\n",
> +   info->mode);
> +   devm_kfree(reboot->dev, info);
> +   continue;
> +   }
> +   list_add_tail(>list, >head);
> +   }
> +
> +   reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> +   ret = register_reboot_notifier(>reboot_notifier);
> +   if (ret)
> +   dev_err(reboot->dev, "can't register reboot notifier\n");

You're returning an error but haven't freed your info->modes.

> +
> +   return ret;
> +
> +error:
> +   list_for_each_entry(info, >head, list)
> +   kfree_const(info->mode);
> +
> +   return ret;
> +}

Regards,
Bjorn

Re: [PATCH v8 2/4] power: reset: add reboot mode driver

[Bjorn Andersson]

On Sun, Apr 24, 2016 at 11:55 PM, Andy Yan  wrote:
[..]
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig

[..]

> +config SYSCON_REBOOT_MODE
> +   bool "Generic SYSCON regmap reboot mode driver"
> +   depends on OF

As this isn't really useful without syscon (but still compiles), I
would suggest adding:

depends on SYSCON || COMPILE_TEST

> +   select REBOOT_MODE
> +   help
> + Say y here will enable reboot mode driver. This will
> + get reboot mode arguments and store it in SYSCON mapped
> + register, then the bootloader can read it to take different
> + action according to the mode.
> +
>  endif
>

[..]

> diff --git a/drivers/power/reset/reboot-mode.c 
> b/drivers/power/reset/reboot-mode.c

[..]

> +
> +#define PREFIX "mode-"
> +
> +struct mode_info {
> +   const char *mode;
> +   unsigned int magic;

If you're using of_property_read_u32() to populate this directly you
should have this as u32.

> +   struct list_head list;
> +};
> +
> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> +const char *cmd)

Magic comes from an unsigned property and is passed as unsigned to the
regmap api. Please make it unsigned int throughout the implementation.

> +{
> +   const char *normal = "normal";
> +   int magic = 0;
> +   struct mode_info *info;
> +
> +   if (!cmd)
> +   cmd = normal;
> +
> +   list_for_each_entry(info, >head, list) {
> +   if (!strcmp(info->mode, cmd)) {
> +   magic = info->magic;
> +   break;
> +   }
> +   }
> +

[..]

> +int reboot_mode_register(struct reboot_mode_driver *reboot)
> +{
> +   struct mode_info *info;
> +   struct property *prop;
> +   struct device_node *np = reboot->dev->of_node;
> +   size_t len = strlen(PREFIX);
> +   int ret;
> +
> +   INIT_LIST_HEAD(>head);
> +
> +   for_each_property_of_node(np, prop) {
> +   if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, 
> len))

There's no reason to iterate over the string twice here, strncmp will
exit with -1 if you hit a '\0' before strlen(PREFIX). So you can drop
the first check without any difference in functionality.

What passes this checkout though is if the property equals PREFIX, but
this is probably better to handle as an error, rather than skip over.
So do that below

> +   continue;
> +
> +   info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL);
> +   if (!info) {
> +   ret = -ENOMEM;
> +   goto error;
> +   }
> +
> +   info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);

You need something like this here:

if (!info->mode) {
ret = -ENOMEM;
goto error;
} else if (info->mode[0] == '\0')
dev_err("mode too short");
ret = -EINVAL;
goto error;
}


If you do the kernel a favor and submit a patch to
drivers/base/devres.c adding devm_kstrdup_const() you don't have to do
the goto dance at all.

> +   if (of_property_read_u32(np, prop->name, >magic)) {
> +   dev_err(reboot->dev, "reboot mode %s without magic 
> number\n",
> +   info->mode);
> +   devm_kfree(reboot->dev, info);
> +   continue;
> +   }
> +   list_add_tail(>list, >head);
> +   }
> +
> +   reboot->reboot_notifier.notifier_call = reboot_mode_notify;
> +   ret = register_reboot_notifier(>reboot_notifier);
> +   if (ret)
> +   dev_err(reboot->dev, "can't register reboot notifier\n");

You're returning an error but haven't freed your info->modes.

> +
> +   return ret;
> +
> +error:
> +   list_for_each_entry(info, >head, list)
> +   kfree_const(info->mode);
> +
> +   return ret;
> +}

Regards,
Bjorn

Re: [GIT PULL] RTC for 4.7

[Linus Torvalds]

On Sat, May 21, 2016 at 8:15 AM, Alexandre Belloni
 wrote:
>
> Here is the pull-request for the RTC subsystem for 4.7.

Grr. I noticed this too late, but this has all been rebased very recently.

Don't f*cking do this! Why do I have to tell people every single merge window?

 Linus

Re: [GIT PULL] RTC for 4.7

[Linus Torvalds]

On Sat, May 21, 2016 at 8:15 AM, Alexandre Belloni
 wrote:
>
> Here is the pull-request for the RTC subsystem for 4.7.

Grr. I noticed this too late, but this has all been rebased very recently.

Don't f*cking do this! Why do I have to tell people every single merge window?

 Linus

Re: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver

[Dmitry Torokhov]

On Wed, May 18, 2016 at 12:07:02AM +0800, jeffrey.lin wrote:
> Hi Dmitry:
> > >static int raydium_i2c_read_message(struct i2c_client *client,
> > >   u32 addr, void *data, size_t len)
> > >{
> > >   __be32 be_addr;
> > >   size_t xfer_len;
> > >   int error;
> > >   while (len) {
> > >   xfer_len = min_t(size_t, len, RM_MAX_READ_SIZE);
> > >
> > >   be_addr = cpu_to_be32(addr);
> > >
> > >   error = raydium_i2c_send(client, RM_CMD_BANK_SWITCH,
> > >_addr, sizeof(be_addr));
> > >   if (!error)
> > >   error = raydium_i2c_read(client, addr & 0xff,
> > >data, xfer_len);
> > Change as:
> > if (!error)
> > error = raydium_i2c_read(client, (be_addr >> 24) & 0xff,
> >  data, xfer_len);
> 
> >I think it is the same on LE, and I suspect it will not work correctly
> >on BE... You want to have the 8 least significant bits of the bank to be
> >used as the address, right?

> This function work correctly with the kernel 3.18 of chromebook in my
> hand. Raydium touch direct access mode can recieve the BE address

That is because it is a little-endian device.

>
> after bank switch command 0xaa. For example, if we'll read 10 bytes
> data begin on 0x12345678. We need send the command sequences as 0xaa->
> 0x12->0x34->0x56-> 0x78 and then recive 10 bytes from 0x78.
> 

Right. So the thing is - on any architecture, be it little- or
big-endian, expression "value & 0xff" will extract the 8 least
significant bits from the value, while "(value >> 24) & 0xff" will
extract the most significant bits (assuming that the value is 32 bits). So with 
your example if you do
cpu_to_be32() on LE architecture it will actually reshuffle the bytes so
that former LSB will become MSB and then you will extract that MSB and
use it. On BE arches cpu_to_be32() is a noop, so addr and be_addr will
have the same value, and your expression will produce 0x12 and not 0x78
as you expect. On the other hand, doing "addr & 0xff" will produce 0x78
regardless of endianness.

> > static int raydium_i2c_fw_write_page(struct i2c_client *client,
> >  u16 page_idx, const void *data, size_t len)
> > {
> > u8 buf[RM_BL_WRT_LEN];
> > u8 pkg_idx = 1;
> > u8 div_cnt;
> > size_t xfer_len;
> > int error;
> > int i;
> > 
> > div_cnt = len % RM_BL_WRT_PKG_SIZE ?
> > len / RM_BL_WRT_PKG_SIZE + 1:len / RM_BL_WRT_PKG_SIZE;
> >
> >Drop this. BTW, if you ever need it we have DIV_ROUND_UP macro.
> 
> > 
> > for (i = 0; i < div_cnt; i++) {
> >
> > while (len) {
> >
> > xfer_len = min_t(size_t, len, RM_BL_WRT_PKG_SIZE);
> > buf[BL_HEADER] = RM_CMD_BOOT_PAGE_WRT;
> > /*FIXME,Touch MCU need zero index as start page*/
> > buf[BL_PAGE_STR] = page_idx ? 0xff : 0;
> > buf[BL_PKG_IDX] = pkg_idx++;
> > 
> > memcpy([BL_DATA_STR], data, xfer_len);
> >
> > /* we need to pad to full page size */
> > if (len < RM_BL_WRT_PKG_SIZE)
> > memset([BL_DATA_STR] + len, 0xff,
> > RM_BL_WRT_PKG_SIZE - len);
> > 
> > if (len == 0)
> > memset(buf + BL_DATA_STR, 0xff, RM_BL_WRT_PKG_SIZE);
> > else if (len < RM_BL_WRT_PKG_SIZE)
> > memset(buf + BL_DATA_STR + xfer_len, 0xff,
> > RM_BL_WRT_PKG_SIZE - xfer_len);
> > 
> > error = raydium_i2c_write_object(client, buf, RM_BL_WRT_LEN,
> >  RAYDIUM_WAIT_READY);
> > if (error) {
> > dev_err(>dev,
> > "page write command failed for page %d, chunk 
> > %d: %d\n",
> > page_idx, pkg_idx, error);
> > return error;
> > }
> > data += RM_BL_WRT_PKG_SIZE;
> > len -= RM_BL_WRT_PKG_SIZE;
> > }
> > 
> > return error;
> > }
> Modify as below.
> 
> static int raydium_i2c_fw_write_page(struct i2c_client *client,
>u16 page_idx, const void *data, size_t len)
> {
>   u8 buf[RM_BL_WRT_LEN];
>   u8 pkg_idx = 1;
>   size_t xfer_len;
>   int error;
> 
>   while (len) {
>   xfer_len = min_t(size_t, len, RM_BL_WRT_PKG_SIZE);
>   buf[BL_HEADER] = RM_CMD_BOOT_PAGE_WRT;
>   /*FIXME,Touch MCU need zero index as start page*/
>   buf[BL_PAGE_STR] = page_idx ? 0xff : 0;
>   buf[BL_PKG_IDX] = pkg_idx++;
> 
>   memcpy([BL_DATA_STR], data, xfer_len);
> 
>   if (len < RM_BL_WRT_PKG_SIZE) {
>   buf[BL_PKG_IDX] = 4;

Why 4???

>   memset(buf + BL_DATA_STR + xfer_len, 0xff,
>   

Re: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver

[Dmitry Torokhov]

On Wed, May 18, 2016 at 12:07:02AM +0800, jeffrey.lin wrote:
> Hi Dmitry:
> > >static int raydium_i2c_read_message(struct i2c_client *client,
> > >   u32 addr, void *data, size_t len)
> > >{
> > >   __be32 be_addr;
> > >   size_t xfer_len;
> > >   int error;
> > >   while (len) {
> > >   xfer_len = min_t(size_t, len, RM_MAX_READ_SIZE);
> > >
> > >   be_addr = cpu_to_be32(addr);
> > >
> > >   error = raydium_i2c_send(client, RM_CMD_BANK_SWITCH,
> > >_addr, sizeof(be_addr));
> > >   if (!error)
> > >   error = raydium_i2c_read(client, addr & 0xff,
> > >data, xfer_len);
> > Change as:
> > if (!error)
> > error = raydium_i2c_read(client, (be_addr >> 24) & 0xff,
> >  data, xfer_len);
> 
> >I think it is the same on LE, and I suspect it will not work correctly
> >on BE... You want to have the 8 least significant bits of the bank to be
> >used as the address, right?

> This function work correctly with the kernel 3.18 of chromebook in my
> hand. Raydium touch direct access mode can recieve the BE address

That is because it is a little-endian device.

>
> after bank switch command 0xaa. For example, if we'll read 10 bytes
> data begin on 0x12345678. We need send the command sequences as 0xaa->
> 0x12->0x34->0x56-> 0x78 and then recive 10 bytes from 0x78.
> 

Right. So the thing is - on any architecture, be it little- or
big-endian, expression "value & 0xff" will extract the 8 least
significant bits from the value, while "(value >> 24) & 0xff" will
extract the most significant bits (assuming that the value is 32 bits). So with 
your example if you do
cpu_to_be32() on LE architecture it will actually reshuffle the bytes so
that former LSB will become MSB and then you will extract that MSB and
use it. On BE arches cpu_to_be32() is a noop, so addr and be_addr will
have the same value, and your expression will produce 0x12 and not 0x78
as you expect. On the other hand, doing "addr & 0xff" will produce 0x78
regardless of endianness.

> > static int raydium_i2c_fw_write_page(struct i2c_client *client,
> >  u16 page_idx, const void *data, size_t len)
> > {
> > u8 buf[RM_BL_WRT_LEN];
> > u8 pkg_idx = 1;
> > u8 div_cnt;
> > size_t xfer_len;
> > int error;
> > int i;
> > 
> > div_cnt = len % RM_BL_WRT_PKG_SIZE ?
> > len / RM_BL_WRT_PKG_SIZE + 1:len / RM_BL_WRT_PKG_SIZE;
> >
> >Drop this. BTW, if you ever need it we have DIV_ROUND_UP macro.
> 
> > 
> > for (i = 0; i < div_cnt; i++) {
> >
> > while (len) {
> >
> > xfer_len = min_t(size_t, len, RM_BL_WRT_PKG_SIZE);
> > buf[BL_HEADER] = RM_CMD_BOOT_PAGE_WRT;
> > /*FIXME,Touch MCU need zero index as start page*/
> > buf[BL_PAGE_STR] = page_idx ? 0xff : 0;
> > buf[BL_PKG_IDX] = pkg_idx++;
> > 
> > memcpy([BL_DATA_STR], data, xfer_len);
> >
> > /* we need to pad to full page size */
> > if (len < RM_BL_WRT_PKG_SIZE)
> > memset([BL_DATA_STR] + len, 0xff,
> > RM_BL_WRT_PKG_SIZE - len);
> > 
> > if (len == 0)
> > memset(buf + BL_DATA_STR, 0xff, RM_BL_WRT_PKG_SIZE);
> > else if (len < RM_BL_WRT_PKG_SIZE)
> > memset(buf + BL_DATA_STR + xfer_len, 0xff,
> > RM_BL_WRT_PKG_SIZE - xfer_len);
> > 
> > error = raydium_i2c_write_object(client, buf, RM_BL_WRT_LEN,
> >  RAYDIUM_WAIT_READY);
> > if (error) {
> > dev_err(>dev,
> > "page write command failed for page %d, chunk 
> > %d: %d\n",
> > page_idx, pkg_idx, error);
> > return error;
> > }
> > data += RM_BL_WRT_PKG_SIZE;
> > len -= RM_BL_WRT_PKG_SIZE;
> > }
> > 
> > return error;
> > }
> Modify as below.
> 
> static int raydium_i2c_fw_write_page(struct i2c_client *client,
>u16 page_idx, const void *data, size_t len)
> {
>   u8 buf[RM_BL_WRT_LEN];
>   u8 pkg_idx = 1;
>   size_t xfer_len;
>   int error;
> 
>   while (len) {
>   xfer_len = min_t(size_t, len, RM_BL_WRT_PKG_SIZE);
>   buf[BL_HEADER] = RM_CMD_BOOT_PAGE_WRT;
>   /*FIXME,Touch MCU need zero index as start page*/
>   buf[BL_PAGE_STR] = page_idx ? 0xff : 0;
>   buf[BL_PKG_IDX] = pkg_idx++;
> 
>   memcpy([BL_DATA_STR], data, xfer_len);
> 
>   if (len < RM_BL_WRT_PKG_SIZE) {
>   buf[BL_PKG_IDX] = 4;

Why 4???

>   memset(buf + BL_DATA_STR + xfer_len, 0xff,
>   

[PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Jan Vesely]

From: Jan Vesely 

Signed-off-by: Jan Vesely 
---
 drivers/iommu/amd_iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 17c76f2..4ff5e40 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3016,6 +3016,7 @@ static void protection_domain_free(struct 
protection_domain *domain)
 
del_domain_from_list(domain);
 
+   mutex_destroy(>api_lock);
if (domain->id)
domain_id_free(domain->id);
 
-- 
2.5.5

[PATCH 1/2] iommu/amd: Remove entry from the list before freeing it

[Jan Vesely]

From: Jan Vesely 

Signed-off-by: Jan Vesely 
---
 drivers/iommu/amd_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 634f636..17c76f2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3288,8 +3288,10 @@ static void amd_iommu_put_dm_regions(struct device *dev,
 {
struct iommu_dm_region *entry, *next;
 
-   list_for_each_entry_safe(entry, next, head, list)
+   list_for_each_entry_safe(entry, next, head, list) {
+   list_del(>list);
kfree(entry);
+   }
 }
 
 static const struct iommu_ops amd_iommu_ops = {
-- 
2.5.5

[PATCH 1/2] iommu/amd: Remove entry from the list before freeing it

[Jan Vesely]

From: Jan Vesely 

Signed-off-by: Jan Vesely 
---
 drivers/iommu/amd_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 634f636..17c76f2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3288,8 +3288,10 @@ static void amd_iommu_put_dm_regions(struct device *dev,
 {
struct iommu_dm_region *entry, *next;
 
-   list_for_each_entry_safe(entry, next, head, list)
+   list_for_each_entry_safe(entry, next, head, list) {
+   list_del(>list);
kfree(entry);
+   }
 }
 
 static const struct iommu_ops amd_iommu_ops = {
-- 
2.5.5

[PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Jan Vesely]

From: Jan Vesely 

Signed-off-by: Jan Vesely 
---
 drivers/iommu/amd_iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 17c76f2..4ff5e40 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3016,6 +3016,7 @@ static void protection_domain_free(struct 
protection_domain *domain)
 
del_domain_from_list(domain);
 
+   mutex_destroy(>api_lock);
if (domain->id)
domain_id_free(domain->id);
 
-- 
2.5.5

Re: [PATCH v2 03/12] of: add J-Core interrupt controller bindings

[Geert Uytterhoeven]

On Sat, May 21, 2016 at 12:34 AM, Rich Felker  wrote:
> On Fri, May 20, 2016 at 10:04:26AM +0200, Geert Uytterhoeven wrote:
>> On Fri, May 20, 2016 at 4:53 AM, Rich Felker  wrote:
>> > +Additional properties required for aic1:
>> > +
>> > +- reg : Memory region for configuration.
>> > +
>> > +- cpu-offset : For SMP, the offset to the per-cpu memory region for
>> > +  configuration, to be scaled by the cpu number.
>>
>> Does cpu-offset apply to aic1 only?
>
> The current kernel driver doesn't have any reason to _need_ cpu-offset
> for aic2, but since there are registers there that a driver (even a
> non-Linux one) may want to use, I think it makes sense that it should
> be present in the bindings.

Hence the "for aic1" should be dropped?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 03/12] of: add J-Core interrupt controller bindings

[Geert Uytterhoeven]

On Sat, May 21, 2016 at 12:34 AM, Rich Felker  wrote:
> On Fri, May 20, 2016 at 10:04:26AM +0200, Geert Uytterhoeven wrote:
>> On Fri, May 20, 2016 at 4:53 AM, Rich Felker  wrote:
>> > +Additional properties required for aic1:
>> > +
>> > +- reg : Memory region for configuration.
>> > +
>> > +- cpu-offset : For SMP, the offset to the per-cpu memory region for
>> > +  configuration, to be scaled by the cpu number.
>>
>> Does cpu-offset apply to aic1 only?
>
> The current kernel driver doesn't have any reason to _need_ cpu-offset
> for aic2, but since there are registers there that a driver (even a
> non-Linux one) may want to use, I think it makes sense that it should
> be present in the bindings.

Hence the "for aic1" should be dropped?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v6 2/8] debugfs: prevent access to removed files' private data

[Nicolai Stange]

Sasha Levin  writes:

> On 05/18/2016 12:05 PM, Greg Kroah-Hartman wrote:
>> On Wed, May 18, 2016 at 11:18:16AM -0400, Sasha Levin wrote:
>>> On 05/18/2016 11:01 AM, Nicolai Stange wrote:
 Thanks a million for reporting!

 1.) Do you have lockdep enabled?
>>>
>>> Yup, nothing there.
>>>
 2.) Does this happen before or after userspace init has been spawned,
 i.e. does the lockup happen at debugfs file creation time or
 possibly at usage time?
>>>
>>> So I looked closer, and it seems to happen after starting syzkaller, which
>>> as far as I know tries to open many different debugfs files.
>>>
>>> Is there debug code I can add it that'll help us figure out what's up?
>> 
>> Trying to figure out _which_ debugfs file is causing this would be
>> great, if at all possible.  strace?
>
> What seems to be failing is syzkaller's attempt to mmap the coverage
> debugfs file. So this isn't actually a kernel deadlock but syzkaller
> misbehaves when that scenario happens.
>
> Either way, it only fails to mmap with that commit that I've pointed
> out.

That info is really helpful here: the proxy file_operations introduced by
this commit doesn't have a ->mmap() defined, i.e. it is NULL from the
VFS layer's point of view.

The simple reason is that at the time I submitted this series, my
Coccinelle script didn't find any debugfs user with a ->mmap()
defined. Thus either that script was broken or things have changed in
the meanwhile.

I'll look into this tomorrow.

Thank you very much for the effort you put into this!

>
>   th->cover_fd = open("/sys/kernel/debug/kcov", O_RDWR);
>   if (th->cover_fd == -1)
>   fail("open of /sys/kernel/debug/kcov failed");
>   if (ioctl(th->cover_fd, KCOV_INIT_TRACE, kCoverSize))
>   fail("cover enable write failed");
>   th->cover_data = (uintptr_t*)mmap(NULL, kCoverSize * 
> sizeof(th->cover_data[0]), PROT_READ | PROT_WRITE, MAP_SHARED, th->cover_fd, 
> 0);
>   if ((void*)th->cover_data == MAP_FAILED)
>   fail("cover mmap failed");
>
> And it's the mmap() that fails with -ENODEV.

Re: [PATCH v6 2/8] debugfs: prevent access to removed files' private data

[Nicolai Stange]

Sasha Levin  writes:

> On 05/18/2016 12:05 PM, Greg Kroah-Hartman wrote:
>> On Wed, May 18, 2016 at 11:18:16AM -0400, Sasha Levin wrote:
>>> On 05/18/2016 11:01 AM, Nicolai Stange wrote:
 Thanks a million for reporting!

 1.) Do you have lockdep enabled?
>>>
>>> Yup, nothing there.
>>>
 2.) Does this happen before or after userspace init has been spawned,
 i.e. does the lockup happen at debugfs file creation time or
 possibly at usage time?
>>>
>>> So I looked closer, and it seems to happen after starting syzkaller, which
>>> as far as I know tries to open many different debugfs files.
>>>
>>> Is there debug code I can add it that'll help us figure out what's up?
>> 
>> Trying to figure out _which_ debugfs file is causing this would be
>> great, if at all possible.  strace?
>
> What seems to be failing is syzkaller's attempt to mmap the coverage
> debugfs file. So this isn't actually a kernel deadlock but syzkaller
> misbehaves when that scenario happens.
>
> Either way, it only fails to mmap with that commit that I've pointed
> out.

That info is really helpful here: the proxy file_operations introduced by
this commit doesn't have a ->mmap() defined, i.e. it is NULL from the
VFS layer's point of view.

The simple reason is that at the time I submitted this series, my
Coccinelle script didn't find any debugfs user with a ->mmap()
defined. Thus either that script was broken or things have changed in
the meanwhile.

I'll look into this tomorrow.

Thank you very much for the effort you put into this!

>
>   th->cover_fd = open("/sys/kernel/debug/kcov", O_RDWR);
>   if (th->cover_fd == -1)
>   fail("open of /sys/kernel/debug/kcov failed");
>   if (ioctl(th->cover_fd, KCOV_INIT_TRACE, kCoverSize))
>   fail("cover enable write failed");
>   th->cover_data = (uintptr_t*)mmap(NULL, kCoverSize * 
> sizeof(th->cover_data[0]), PROT_READ | PROT_WRITE, MAP_SHARED, th->cover_fd, 
> 0);
>   if ((void*)th->cover_data == MAP_FAILED)
>   fail("cover mmap failed");
>
> And it's the mmap() that fails with -ENODEV.

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 10:16 AM, William Breathitt Gray
 wrote:
>
> Would you like me to submit a patchset after your commit to introduce
> the ISA_BUS/ISA_BUS_API Kconfig options, as well as adjust the relevant
> drivers' Kconfig options to depend on the ISA_BUS_API?

Yes please.

Linus

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 10:16 AM, William Breathitt Gray
 wrote:
>
> Would you like me to submit a patchset after your commit to introduce
> the ISA_BUS/ISA_BUS_API Kconfig options, as well as adjust the relevant
> drivers' Kconfig options to depend on the ISA_BUS_API?

Yes please.

Linus

Re: [GIT PULL] Driver core update for 4.7-rc1

[William Breathitt Gray]

On Sat, May 21, 2016 at 09:59:09AM -0700, Linus Torvalds wrote:
>Author: Linus Torvalds 
>Date:   Sat May 21 09:13:41 2016 -0700
>
>x86 isa: add back X86_32 dependency on CONFIG_ISA
>
>Commit b3c1be1b789c ("base: isa: Remove X86_32 dependency") made ISA
>support available on x86-64 too.  That's not right - while there are
>some LPC-style devices that might be useful still and be based on
>ISA-like IP blocks, that is *not* an excuse to try to enable any random
>legacy drivers.
>
>Such drivers should be individually enabled and made to perhaps depend
>on ISA_DMA_API instead (which we have continued to support on x86-64).
>Or we could add another "ISA_XYZ_API" that we support that doesn't
>enable random old drivers that aren't even 64-bit clean nor do we have
>any test coverage for.
>
>Turning off ISA will now also turn off some drivers that have been
>marked as depending on it as part of this series, and that used to work
>on modern platforms.
>
>See for example commits ad7afc38eab3..cc736607c86d, which may also need
>to be reverted.
>
>Cc: William Breathitt Gray 
>Cc: Linus Walleij 
>Cc: Guenter Roeck 
>Cc: Greg Kroah-Hartman 
>Signed-off-by: Linus Torvalds 
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index 48ac29034e1e..0a7b885964ba 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -2447,6 +2447,8 @@ config ISA_DMA_API
> Enables ISA-style DMA support for devices requiring such controllers.
> If unsure, say Y.
> 
>+if X86_32
>+
> config ISA
>   bool "ISA support"
>   ---help---
>@@ -2456,8 +2458,6 @@ config ISA
> (MCA) or VESA.  ISA is an older system, now being displaced by PCI;
> newer boards don't support it.  If you have ISA, say Y, otherwise N.
> 
>-if X86_32
>-
> config EISA
>   bool "EISA support"
>   depends on ISA

Acked-by: William Breathitt Gray 

That makes sense to me. The drivers which switched to use the ISA bus
driver would simply need their respective Kconfig option adjusted to
depend on a "ISA_BUS_API" option, rather than ISA, to allow them to
compile on X86_64.

Would you like me to submit a patchset after your commit to introduce
the ISA_BUS/ISA_BUS_API Kconfig options, as well as adjust the relevant
drivers' Kconfig options to depend on the ISA_BUS_API?

William Breathitt Gray

Re: [GIT PULL] Driver core update for 4.7-rc1

[William Breathitt Gray]

On Sat, May 21, 2016 at 09:59:09AM -0700, Linus Torvalds wrote:
>Author: Linus Torvalds 
>Date:   Sat May 21 09:13:41 2016 -0700
>
>x86 isa: add back X86_32 dependency on CONFIG_ISA
>
>Commit b3c1be1b789c ("base: isa: Remove X86_32 dependency") made ISA
>support available on x86-64 too.  That's not right - while there are
>some LPC-style devices that might be useful still and be based on
>ISA-like IP blocks, that is *not* an excuse to try to enable any random
>legacy drivers.
>
>Such drivers should be individually enabled and made to perhaps depend
>on ISA_DMA_API instead (which we have continued to support on x86-64).
>Or we could add another "ISA_XYZ_API" that we support that doesn't
>enable random old drivers that aren't even 64-bit clean nor do we have
>any test coverage for.
>
>Turning off ISA will now also turn off some drivers that have been
>marked as depending on it as part of this series, and that used to work
>on modern platforms.
>
>See for example commits ad7afc38eab3..cc736607c86d, which may also need
>to be reverted.
>
>Cc: William Breathitt Gray 
>Cc: Linus Walleij 
>Cc: Guenter Roeck 
>Cc: Greg Kroah-Hartman 
>Signed-off-by: Linus Torvalds 
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index 48ac29034e1e..0a7b885964ba 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -2447,6 +2447,8 @@ config ISA_DMA_API
> Enables ISA-style DMA support for devices requiring such controllers.
> If unsure, say Y.
> 
>+if X86_32
>+
> config ISA
>   bool "ISA support"
>   ---help---
>@@ -2456,8 +2458,6 @@ config ISA
> (MCA) or VESA.  ISA is an older system, now being displaced by PCI;
> newer boards don't support it.  If you have ISA, say Y, otherwise N.
> 
>-if X86_32
>-
> config EISA
>   bool "EISA support"
>   depends on ISA

Acked-by: William Breathitt Gray 

That makes sense to me. The drivers which switched to use the ISA bus
driver would simply need their respective Kconfig option adjusted to
depend on a "ISA_BUS_API" option, rather than ISA, to allow them to
compile on X86_64.

Would you like me to submit a patchset after your commit to introduce
the ISA_BUS/ISA_BUS_API Kconfig options, as well as adjust the relevant
drivers' Kconfig options to depend on the ISA_BUS_API?

William Breathitt Gray

Re: sem_lock() vs qspinlocks

[Davidlohr Bueso]

On Sat, 21 May 2016, Peter Zijlstra wrote:


On Fri, May 20, 2016 at 05:48:39PM -0700, Davidlohr Bueso wrote:

On Fri, 20 May 2016, Linus Torvalds wrote:


>Oh, I definitely agree on the stable part, and yes, the "splt things
>up" model should come later if people agree that it's a good thing.

The backporting part is quite nice, yes, but ultimately I think I prefer
Linus' suggestion making things explicit, as opposed to consulting the spinlock
implying barriers. I also hate to have an smp_mb() (particularly for 
spin_is_locked)
given that we are not optimizing for the common case (regular mutual excl).


I'm confused; we _are_ optimizing for the common case. spin_is_locked()
is very unlikely to be used. And arguably should be used less in favour
of lockdep_assert_held().


Indeed we are.

But by 'common case' I was really thinking about spin_is_locked() vs 
spin_wait_unlock().
The former being the more common of the two, and the one which mostly will 
_not_ be used
for lock correctness purposes, hence it doesn't need that new smp_mb. Hence 
allowing users
to explicitly set the ordering needs (ie spin_lock_synchronize()) seems like 
the better
long term alternative. otoh, with your approach all such bugs are automatically 
fixed :)

Thanks,
Davidlohr

Re: sem_lock() vs qspinlocks

[Davidlohr Bueso]

On Sat, 21 May 2016, Peter Zijlstra wrote:


On Fri, May 20, 2016 at 05:48:39PM -0700, Davidlohr Bueso wrote:

On Fri, 20 May 2016, Linus Torvalds wrote:


>Oh, I definitely agree on the stable part, and yes, the "splt things
>up" model should come later if people agree that it's a good thing.

The backporting part is quite nice, yes, but ultimately I think I prefer
Linus' suggestion making things explicit, as opposed to consulting the spinlock
implying barriers. I also hate to have an smp_mb() (particularly for 
spin_is_locked)
given that we are not optimizing for the common case (regular mutual excl).


I'm confused; we _are_ optimizing for the common case. spin_is_locked()
is very unlikely to be used. And arguably should be used less in favour
of lockdep_assert_held().


Indeed we are.

But by 'common case' I was really thinking about spin_is_locked() vs 
spin_wait_unlock().
The former being the more common of the two, and the one which mostly will 
_not_ be used
for lock correctness purposes, hence it doesn't need that new smp_mb. Hence 
allowing users
to explicitly set the ordering needs (ie spin_lock_synchronize()) seems like 
the better
long term alternative. otoh, with your approach all such bugs are automatically 
fixed :)

Thanks,
Davidlohr

Re: [PATCH v2 1/6] power: Introduce Broadcom kona reset driver

[Florian Fainelli]

Le 11/05/2016 14:36, Chris Brand a écrit :
> This driver supports reset on both BCM21664 and BCM23550.
> Code is being moved from arch/arm/mach-bcm/board_bcm21664.c
> 
> Signed-off-by: Chris Brand 

Sebastian, Dmitry, I know we are in the merge window, let me know if you
would want me to take this patch and the 5 others in the next arm-soc
pull request, or if you prefer to take this one in your own tree.

Thanks!
-- 
Florian

Re: [PATCH v2 1/6] power: Introduce Broadcom kona reset driver

[Florian Fainelli]

Le 11/05/2016 14:36, Chris Brand a écrit :
> This driver supports reset on both BCM21664 and BCM23550.
> Code is being moved from arch/arm/mach-bcm/board_bcm21664.c
> 
> Signed-off-by: Chris Brand 

Sebastian, Dmitry, I know we are in the merge window, let me know if you
would want me to take this patch and the 5 others in the next arm-soc
pull request, or if you prefer to take this one in your own tree.

Thanks!
-- 
Florian

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 9:31 AM, Linus Torvalds
 wrote:
>
> We're not suddenly enabling ISA on x86-64 after having successfully
> gotten rid of that insane crap long long ago.
>
> If you have *specific* dribvers that are actually relevant for some
> reason, make those ones available based on other options. For example,
> we've had the ISA_DMA_API config option to say "we support a subset of
> ISA, even when we don't actually want to ever see actual plug-in ISA
> cards".

I am planning on committing something like the attached.

Note that anything that added "depends on ISA" because of the other
patches will now be disabled on x86-64, even if that driver wasn't
disabled before.

Added Linus Walleij and Guenther Roech to the cc because of the
drivers that I'm aware that did this.

To repeat: I'm not at all interested in enabling random old drivers on
x86-64. Any driver enablement needs to be done on a one-by-one basis,
after having actually gotten TESTING, and after having had people make
sure that we don't get tons of new warnings like we did with the
"let's just enable ISA randomly" approach.

I'm ok with enabling some "ISA_BUS_API" support (like we have
ISA_DMA_API) that allows people to enable drivers one-by-one as they
are converted to a convenience function. The patch series seems to
have had that kind of approach initially.

I'd suggest doing that ISA_BUS_API config option as a *general* thing
(not arch-specific), and make it start out like

  config ISA_BUS_API
  def_bool ISA

which means that everybody gets it, and if ISA was enabled, it will be
enabled automatically. Then, architectures that do *not* enable ISA
itself (like x86-64), could choose to have a config option like

config ISA_BUS
bool "support ISA-style drivers on modern systems" if (x86 && EXPERT)
default y
select ISA_BUS_API

or something, which means that ISA_BUS_API would then get enabled (but
not ISA itself).

That's similar to what we do today with ISA_DMA_API, except we made
the mistake of making all the different architectures define the
ISA_DMA_API option.

It's the wholesale "enable random crap" that I will not accept. Not
even if there is then "hide the crap again when it causes warnings".
Even a driver that doesn't warn isn't necessarily useful, and I don't
want people to suddenly start seeing a lot of options for random ISA
drivers that simply aren't relevant, and never will be. Not even if
they compile cleanly.

   Linus
Author: Linus Torvalds 
Date:   Sat May 21 09:13:41 2016 -0700

x86 isa: add back X86_32 dependency on CONFIG_ISA

Commit b3c1be1b789c ("base: isa: Remove X86_32 dependency") made ISA
support available on x86-64 too.  That's not right - while there are
some LPC-style devices that might be useful still and be based on
ISA-like IP blocks, that is *not* an excuse to try to enable any random
legacy drivers.

Such drivers should be individually enabled and made to perhaps depend
on ISA_DMA_API instead (which we have continued to support on x86-64).
Or we could add another "ISA_XYZ_API" that we support that doesn't
enable random old drivers that aren't even 64-bit clean nor do we have
any test coverage for.

Turning off ISA will now also turn off some drivers that have been
marked as depending on it as part of this series, and that used to work
on modern platforms.

See for example commits ad7afc38eab3..cc736607c86d, which may also need
to be reverted.

Cc: William Breathitt Gray 
Cc: Linus Walleij 
Cc: Guenter Roeck 
Cc: Greg Kroah-Hartman 
Signed-off-by: Linus Torvalds 

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 48ac29034e1e..0a7b885964ba 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2447,6 +2447,8 @@ config ISA_DMA_API
  Enables ISA-style DMA support for devices requiring such controllers.
  If unsure, say Y.
 
+if X86_32
+
 config ISA
bool "ISA support"
---help---
@@ -2456,8 +2458,6 @@ config ISA
  (MCA) or VESA.  ISA is an older system, now being displaced by PCI;
  newer boards don't support it.  If you have ISA, say Y, otherwise N.
 
-if X86_32
-
 config EISA
bool "EISA support"
depends on ISA

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 9:31 AM, Linus Torvalds
 wrote:
>
> We're not suddenly enabling ISA on x86-64 after having successfully
> gotten rid of that insane crap long long ago.
>
> If you have *specific* dribvers that are actually relevant for some
> reason, make those ones available based on other options. For example,
> we've had the ISA_DMA_API config option to say "we support a subset of
> ISA, even when we don't actually want to ever see actual plug-in ISA
> cards".

I am planning on committing something like the attached.

Note that anything that added "depends on ISA" because of the other
patches will now be disabled on x86-64, even if that driver wasn't
disabled before.

Added Linus Walleij and Guenther Roech to the cc because of the
drivers that I'm aware that did this.

To repeat: I'm not at all interested in enabling random old drivers on
x86-64. Any driver enablement needs to be done on a one-by-one basis,
after having actually gotten TESTING, and after having had people make
sure that we don't get tons of new warnings like we did with the
"let's just enable ISA randomly" approach.

I'm ok with enabling some "ISA_BUS_API" support (like we have
ISA_DMA_API) that allows people to enable drivers one-by-one as they
are converted to a convenience function. The patch series seems to
have had that kind of approach initially.

I'd suggest doing that ISA_BUS_API config option as a *general* thing
(not arch-specific), and make it start out like

  config ISA_BUS_API
  def_bool ISA

which means that everybody gets it, and if ISA was enabled, it will be
enabled automatically. Then, architectures that do *not* enable ISA
itself (like x86-64), could choose to have a config option like

config ISA_BUS
bool "support ISA-style drivers on modern systems" if (x86 && EXPERT)
default y
select ISA_BUS_API

or something, which means that ISA_BUS_API would then get enabled (but
not ISA itself).

That's similar to what we do today with ISA_DMA_API, except we made
the mistake of making all the different architectures define the
ISA_DMA_API option.

It's the wholesale "enable random crap" that I will not accept. Not
even if there is then "hide the crap again when it causes warnings".
Even a driver that doesn't warn isn't necessarily useful, and I don't
want people to suddenly start seeing a lot of options for random ISA
drivers that simply aren't relevant, and never will be. Not even if
they compile cleanly.

   Linus
Author: Linus Torvalds 
Date:   Sat May 21 09:13:41 2016 -0700

x86 isa: add back X86_32 dependency on CONFIG_ISA

Commit b3c1be1b789c ("base: isa: Remove X86_32 dependency") made ISA
support available on x86-64 too.  That's not right - while there are
some LPC-style devices that might be useful still and be based on
ISA-like IP blocks, that is *not* an excuse to try to enable any random
legacy drivers.

Such drivers should be individually enabled and made to perhaps depend
on ISA_DMA_API instead (which we have continued to support on x86-64).
Or we could add another "ISA_XYZ_API" that we support that doesn't
enable random old drivers that aren't even 64-bit clean nor do we have
any test coverage for.

Turning off ISA will now also turn off some drivers that have been
marked as depending on it as part of this series, and that used to work
on modern platforms.

See for example commits ad7afc38eab3..cc736607c86d, which may also need
to be reverted.

Cc: William Breathitt Gray 
Cc: Linus Walleij 
Cc: Guenter Roeck 
Cc: Greg Kroah-Hartman 
Signed-off-by: Linus Torvalds 

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 48ac29034e1e..0a7b885964ba 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2447,6 +2447,8 @@ config ISA_DMA_API
  Enables ISA-style DMA support for devices requiring such controllers.
  If unsure, say Y.
 
+if X86_32
+
 config ISA
bool "ISA support"
---help---
@@ -2456,8 +2458,6 @@ config ISA
  (MCA) or VESA.  ISA is an older system, now being displaced by PCI;
  newer boards don't support it.  If you have ISA, say Y, otherwise N.
 
-if X86_32
-
 config EISA
bool "EISA support"
depends on ISA

Re: [PATCH] spi: spidev: fix possible arithmetic overflow for multi-transfer message

[Dmitry Torokhov]

On Mon, Mar 23, 2015 at 10:50 AM, Ian Abbott  wrote:
> `spidev_message()` sums the lengths of the individual SPI transfers to
> determine the overall SPI message length.  It restricts the total
> length, returning an error if too long, but it does not check for
> arithmetic overflow.  For example, if the SPI message consisted of two
> transfers and the first has a length of 10 and the second has a length
> of (__u32)(-1), the total length would be seen as 9, even though the
> second transfer is actually very long.  If the second transfer specifies
> a null `rx_buf` and a non-null `tx_buf`, the `copy_from_user()` could
> overrun the spidev's pre-allocated tx buffer before it reaches an
> invalid user memory address.  Fix it by checking that neither the total
> nor the individual transfer lengths exceed the maximum allowed value.
>
> Thanks to Dan Carpenter for reporting the potential integer overflow.
>
> Signed-off-by: Ian Abbott 
> Cc:  # 4.0+
> ---
> This could be backported to kernels prior to 4.0, but the total and
> individual lengths would need to be checked against `bufsiz` instead of
> `INT_MAX`.
> ---
>  drivers/spi/spidev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index bb6b3ab..23ad978 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -249,9 +249,10 @@ static int spidev_message(struct spidev_data *spidev,
> total += k_tmp->len;
> /* Since the function returns the total length of transfers
>  * on success, restrict the total to positive int values to
> -* avoid the return value looking like an error.
> +* avoid the return value looking like an error.  Also check
> +* each transfer length to avoid arithmetic overflow.
>  */
> -   if (total > INT_MAX) {
> +   if (total > INT_MAX || k_tmp->len > INT_MAX) {

What if total is INT_MAX - 2 and k_tmp->len is 3? What about total is
INT_MAX and k_tmp->len is INT_MAX as well? I think the proper check
should be:

if (total < k_tmp->len || total > INT_MAX) {
...
}

> status = -EMSGSIZE;
> goto done;
> }

Thanks.

-- 
Dmitry

Re: [PATCH] spi: spidev: fix possible arithmetic overflow for multi-transfer message

[Dmitry Torokhov]

On Mon, Mar 23, 2015 at 10:50 AM, Ian Abbott  wrote:
> `spidev_message()` sums the lengths of the individual SPI transfers to
> determine the overall SPI message length.  It restricts the total
> length, returning an error if too long, but it does not check for
> arithmetic overflow.  For example, if the SPI message consisted of two
> transfers and the first has a length of 10 and the second has a length
> of (__u32)(-1), the total length would be seen as 9, even though the
> second transfer is actually very long.  If the second transfer specifies
> a null `rx_buf` and a non-null `tx_buf`, the `copy_from_user()` could
> overrun the spidev's pre-allocated tx buffer before it reaches an
> invalid user memory address.  Fix it by checking that neither the total
> nor the individual transfer lengths exceed the maximum allowed value.
>
> Thanks to Dan Carpenter for reporting the potential integer overflow.
>
> Signed-off-by: Ian Abbott 
> Cc:  # 4.0+
> ---
> This could be backported to kernels prior to 4.0, but the total and
> individual lengths would need to be checked against `bufsiz` instead of
> `INT_MAX`.
> ---
>  drivers/spi/spidev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index bb6b3ab..23ad978 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -249,9 +249,10 @@ static int spidev_message(struct spidev_data *spidev,
> total += k_tmp->len;
> /* Since the function returns the total length of transfers
>  * on success, restrict the total to positive int values to
> -* avoid the return value looking like an error.
> +* avoid the return value looking like an error.  Also check
> +* each transfer length to avoid arithmetic overflow.
>  */
> -   if (total > INT_MAX) {
> +   if (total > INT_MAX || k_tmp->len > INT_MAX) {

What if total is INT_MAX - 2 and k_tmp->len is 3? What about total is
INT_MAX and k_tmp->len is INT_MAX as well? I think the proper check
should be:

if (total < k_tmp->len || total > INT_MAX) {
...
}

> status = -EMSGSIZE;
> goto done;
> }

Thanks.

-- 
Dmitry

Re: PROBLEM: Resume form hibernate broken by setting NX on gap

[Kees Cook]

On Fri, May 20, 2016 at 6:57 PM, Logan Gunthorpe  wrote:
> On 20/05/16 04:16 PM, Kees Cook wrote:
>>
>> On Fri, May 20, 2016 at 2:59 PM, Kees Cook  wrote:
>>>
>>> On Fri, May 20, 2016 at 2:46 PM, Rafael J. Wysocki 
>>> wrote:

 On Fri, May 20, 2016 at 3:56 PM, Stephen Smalley 
 wrote:
>
> On 05/20/2016 07:34 AM, Rafael J. Wysocki wrote:
>>
>> On Fri, May 20, 2016 at 9:15 AM, Ingo Molnar  wrote:
>>>
>>>
>>> * Logan Gunthorpe  wrote:
>>>
 Hi,

 I have been working on a bug that causes my laptop to freeze during
 resume from hibernation. I did a bisect to find the offending
 commit:

 [ab76f7b4ab] x86/mm: Set NX on gap between __ex_table and rodata

 There is more information in the bugzilla report [1] that
 I've been working on but I will summarize things below.

 I've experienced intermittent but reproducible freezes when resuming
 from hibernation since about kernel version 3.19. The freeze was
 significantly more reproducible when a few applications were loaded
 before hibernation and would largely not happen if hibernated
 immediately after booting to a desktop. I did some tracing work to
 find
 that the kernel gets as far as the resume_image call in
 swsusp_arch_resume and I could not find any response from the image
 kernel when I hit the bug. I also did testing that seemed to rule
 out
 this being caused by a problematic driver.

 I did a successful bisect between 3.18 and 3.19 which found a bug in
 commit f5b2831d6 that was then later fixed by commit 55696b1f66 in
 4.4.
 Then, I did a second bisect with a ported version of the fix to the
 first bug and found commit ab76f7b4ab in 4.3 to also break
 hibernation
 with what appears to be the exact same symptoms. Reverting that
 commit
 in recent kernels up to and including 4.6 fixes the issue and
 restores
 reliable hibernation. However, it's not at all clear to me why that
 commit would cause this issue or how to fix the issue without
 reverting.
>>>
>>>
>>> I've attached that commit below and also Cc:-ed a few more people who
>>> might have
>>> an idea about why this regressed. Worst-case we'll have to revert it.
>>
>>
>> Without looking deep into mm, my theory would be that after this patch
>> the final jump from the boot kernel to the image kernel's trampoline
>> code during resume may crash the kernel if the trampoline page turns
>> out to be NX in the boot kernel (it has to be executable in both the
>> boot and the image kernels).
>
>
> So, pardon my ignorance, but where is this trampoline page placed in
> kernel memory?


 On 32-bit its location has to be the same in both the boot and the
 image kernels and that's within kernel text in both cases, so that
 shouldn't be a problem.

 On 64-bit its location depends on the image kernel and specifically on
 the location of the restore_registers routine in it.  The (virtual)
 address of that routine is stored in the restore_jump_address
 variable, so the page containing it (the trampoline page) can be found
 with the help of that.

 swsusp_arch_resume() sets up a temporary kernel mapping to finalize
 the image restoration and that page must not be NX in that mapping for
 things to work.
>>>
>>>
>>> It looks like nothing in the swsusp_arch_resume() -> get_safe_page()
>>> -> get_image_page() path sets the page executable...
>>>
>>> Untested, but I wonder if this work work in swsusp_arch_resume()
>>> before the memcpy?
>>
>>
>> I can't type today, it seems. It should read "... if this would work ..."
>>
>> If you can test this and it works for you, I'll send a proper patch... :P
>>
>> -Kees
>>
>
> Hi Kees,
>
> Thanks. I tried the patch but it only resulted in a kernel warning and
> freeze. I've attached a photo showing as much of the messages as I could
> get.
>
> Logan

Ah, dang, ok, thanks for trying it. I'll let Rafael try to figure this one out.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: PROBLEM: Resume form hibernate broken by setting NX on gap

[Kees Cook]

On Fri, May 20, 2016 at 6:57 PM, Logan Gunthorpe  wrote:
> On 20/05/16 04:16 PM, Kees Cook wrote:
>>
>> On Fri, May 20, 2016 at 2:59 PM, Kees Cook  wrote:
>>>
>>> On Fri, May 20, 2016 at 2:46 PM, Rafael J. Wysocki 
>>> wrote:

 On Fri, May 20, 2016 at 3:56 PM, Stephen Smalley 
 wrote:
>
> On 05/20/2016 07:34 AM, Rafael J. Wysocki wrote:
>>
>> On Fri, May 20, 2016 at 9:15 AM, Ingo Molnar  wrote:
>>>
>>>
>>> * Logan Gunthorpe  wrote:
>>>
 Hi,

 I have been working on a bug that causes my laptop to freeze during
 resume from hibernation. I did a bisect to find the offending
 commit:

 [ab76f7b4ab] x86/mm: Set NX on gap between __ex_table and rodata

 There is more information in the bugzilla report [1] that
 I've been working on but I will summarize things below.

 I've experienced intermittent but reproducible freezes when resuming
 from hibernation since about kernel version 3.19. The freeze was
 significantly more reproducible when a few applications were loaded
 before hibernation and would largely not happen if hibernated
 immediately after booting to a desktop. I did some tracing work to
 find
 that the kernel gets as far as the resume_image call in
 swsusp_arch_resume and I could not find any response from the image
 kernel when I hit the bug. I also did testing that seemed to rule
 out
 this being caused by a problematic driver.

 I did a successful bisect between 3.18 and 3.19 which found a bug in
 commit f5b2831d6 that was then later fixed by commit 55696b1f66 in
 4.4.
 Then, I did a second bisect with a ported version of the fix to the
 first bug and found commit ab76f7b4ab in 4.3 to also break
 hibernation
 with what appears to be the exact same symptoms. Reverting that
 commit
 in recent kernels up to and including 4.6 fixes the issue and
 restores
 reliable hibernation. However, it's not at all clear to me why that
 commit would cause this issue or how to fix the issue without
 reverting.
>>>
>>>
>>> I've attached that commit below and also Cc:-ed a few more people who
>>> might have
>>> an idea about why this regressed. Worst-case we'll have to revert it.
>>
>>
>> Without looking deep into mm, my theory would be that after this patch
>> the final jump from the boot kernel to the image kernel's trampoline
>> code during resume may crash the kernel if the trampoline page turns
>> out to be NX in the boot kernel (it has to be executable in both the
>> boot and the image kernels).
>
>
> So, pardon my ignorance, but where is this trampoline page placed in
> kernel memory?


 On 32-bit its location has to be the same in both the boot and the
 image kernels and that's within kernel text in both cases, so that
 shouldn't be a problem.

 On 64-bit its location depends on the image kernel and specifically on
 the location of the restore_registers routine in it.  The (virtual)
 address of that routine is stored in the restore_jump_address
 variable, so the page containing it (the trampoline page) can be found
 with the help of that.

 swsusp_arch_resume() sets up a temporary kernel mapping to finalize
 the image restoration and that page must not be NX in that mapping for
 things to work.
>>>
>>>
>>> It looks like nothing in the swsusp_arch_resume() -> get_safe_page()
>>> -> get_image_page() path sets the page executable...
>>>
>>> Untested, but I wonder if this work work in swsusp_arch_resume()
>>> before the memcpy?
>>
>>
>> I can't type today, it seems. It should read "... if this would work ..."
>>
>> If you can test this and it works for you, I'll send a proper patch... :P
>>
>> -Kees
>>
>
> Hi Kees,
>
> Thanks. I tried the patch but it only resulted in a kernel warning and
> freeze. I've attached a photo showing as much of the messages as I could
> get.
>
> Logan

Ah, dang, ok, thanks for trying it. I'll let Rafael try to figure this one out.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [RFT PATCH] iio: magn: Add support for BMM150 magnetometer

[Jonathan Cameron]

On 18/05/16 15:01, Daniel Baluta wrote:
> On Wed, May 4, 2016 at 1:26 PM, Jonathan Cameron  wrote:
>> On 27/04/16 15:55, Lucas De Marchi wrote:
>>> On Tue, Apr 26, 2016 at 9:39 AM, Daniel Baluta  
>>> wrote:
 BMM150 is register compatible with magnetometer part of
 BMC156.

 Datasheet is at:
 http://www.mouser.com/ds/2/783/BST-BMM150-DS001-01-786480.pdf

 Signed-off-by: Daniel Baluta 
 ---
 Lucas let me know if it works for you. I don't have yet the BMM150 
 evaluation
 board. It should be available in few weeks.

 I'll just leave this patch here in case anyone steps in to test it.
>>>
>>> It will take a while for my test board to arrive here, so expect delay
>>> on testing this.
>> I'm dropping this from my list of outstanding patches for now.
>> It'll resurface when you guys restart the thread.
> 
> Got the shuttle board and basic tests with generic_buffer seem to work fine.
> 
> Jonathan can you pick this up if no other comments?
Done.  It might be worth a follow up to list the supported parts in the Kconfig
description.  Given they aren't listed already, I'm not going to add this one
on it's own and it would make little sense to add them all in this patch.
> 
> thanks,
> Daniel.
> 

Re: [RFT PATCH] iio: magn: Add support for BMM150 magnetometer

[Jonathan Cameron]

On 18/05/16 15:01, Daniel Baluta wrote:
> On Wed, May 4, 2016 at 1:26 PM, Jonathan Cameron  wrote:
>> On 27/04/16 15:55, Lucas De Marchi wrote:
>>> On Tue, Apr 26, 2016 at 9:39 AM, Daniel Baluta  
>>> wrote:
 BMM150 is register compatible with magnetometer part of
 BMC156.

 Datasheet is at:
 http://www.mouser.com/ds/2/783/BST-BMM150-DS001-01-786480.pdf

 Signed-off-by: Daniel Baluta 
 ---
 Lucas let me know if it works for you. I don't have yet the BMM150 
 evaluation
 board. It should be available in few weeks.

 I'll just leave this patch here in case anyone steps in to test it.
>>>
>>> It will take a while for my test board to arrive here, so expect delay
>>> on testing this.
>> I'm dropping this from my list of outstanding patches for now.
>> It'll resurface when you guys restart the thread.
> 
> Got the shuttle board and basic tests with generic_buffer seem to work fine.
> 
> Jonathan can you pick this up if no other comments?
Done.  It might be worth a follow up to list the supported parts in the Kconfig
description.  Given they aren't listed already, I'm not going to add this one
on it's own and it would make little sense to add them all in this patch.
> 
> thanks,
> Daniel.
> 

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 9:20 AM, William Breathitt Gray
 wrote:
>
> I'll submit patches to resolve these warnings and errors.

No.

I will disable that ISA config option. We're not randomly making old
drivers available on modern platforms. Really.

We're not suddenly enabling ISA on x86-64 after having successfully
gotten rid of that insane crap long long ago.

If you have *specific* dribvers that are actually relevant for some
reason, make those ones available based on other options. For example,
we've had the ISA_DMA_API config option to say "we support a subset of
ISA, even when we don't actually want to ever see actual plug-in ISA
cards".

Your patch already resulted in having to add several cases of

- depends on ISA
+ depends on X86 && ISA

because you tried to randomly widen what "ISA" meant. No. No no no.

No more "let's randomly change what ISA is". Do this enabling one
driver at a time, or not at all. I'm not at all interested in seeing
some kind of generic "we will support random shit on modern platfoms"
crap. 99% of all drivers that depend on ISA have no maintainership,
and will never get any maintainership.

 Linus

Re: [GIT PULL] Driver core update for 4.7-rc1

[Linus Torvalds]

On Sat, May 21, 2016 at 9:20 AM, William Breathitt Gray
 wrote:
>
> I'll submit patches to resolve these warnings and errors.

No.

I will disable that ISA config option. We're not randomly making old
drivers available on modern platforms. Really.

We're not suddenly enabling ISA on x86-64 after having successfully
gotten rid of that insane crap long long ago.

If you have *specific* dribvers that are actually relevant for some
reason, make those ones available based on other options. For example,
we've had the ISA_DMA_API config option to say "we support a subset of
ISA, even when we don't actually want to ever see actual plug-in ISA
cards".

Your patch already resulted in having to add several cases of

- depends on ISA
+ depends on X86 && ISA

because you tried to randomly widen what "ISA" meant. No. No no no.

No more "let's randomly change what ISA is". Do this enabling one
driver at a time, or not at all. I'm not at all interested in seeing
some kind of generic "we will support random shit on modern platfoms"
crap. 99% of all drivers that depend on ISA have no maintainership,
and will never get any maintainership.

 Linus

Re: [PATCH 2/2] iio: generic_buffer: Add --device-num option

[Jonathan Cameron]

On 20/05/16 16:45, Crestez Dan Leonard wrote:
> This makes it possible to distinguish between iio devices with the same
> name.
> 
> Signed-off-by: Crestez Dan Leonard 
happy with this and will pickup once the first patch is sorted.

Jonathan
> ---
>  tools/iio/generic_buffer.c | 53 
> ++
>  1 file changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
> index e7bf477..f805ef9 100644
> --- a/tools/iio/generic_buffer.c
> +++ b/tools/iio/generic_buffer.c
> @@ -251,7 +251,9 @@ void print_usage(void)
>   "  -e Disable wait for event (new data)\n"
>   "  -g Use trigger-less mode\n"
>   "  -l  Set buffer length to n samples\n"
> - "  -n   Set device name (mandatory)\n"
> + "  --device-name -n \n"
> + "  --device-num -N \n"
> + "Set device by name or number"
>   "  -t   Set trigger name\n"
>   "  -w  Set delay between reads in us (event-less 
> mode)\n");
>  }
> @@ -319,6 +321,12 @@ void register_cleanup(void)
>   }
>  }
>  
> +static const struct option longopts[] = {
> + { "device-name",1, 0, 'n' },
> + { "device-num", 1, 0, 'N' },
> + { },
> +};
> +
>  int main(int argc, char **argv)
>  {
>   unsigned long num_loops = 2;
> @@ -333,7 +341,7 @@ int main(int argc, char **argv)
>  
>   char *data;
>   ssize_t read_size;
> - int dev_num, trig_num;
> + int dev_num = -1, trig_num;
>   char *buffer_access;
>   int scan_size;
>   int noevents = 0;
> @@ -344,7 +352,7 @@ int main(int argc, char **argv)
>  
>   register_cleanup();
>  
> - while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
> + while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
> != -1) {
>   switch (c) {
>   case 'a':
>   autochannels = AUTOCHANNELS_ENABLED;
> @@ -372,6 +380,12 @@ int main(int argc, char **argv)
>   case 'n':
>   device_name = optarg;
>   break;
> + case 'N':
> + errno = 0;
> + dev_num = strtoul(optarg, , 10);
> + if (errno)
> + return -errno;
> + break;
>   case 't':
>   trigger_name = optarg;
>   break;
> @@ -387,24 +401,37 @@ int main(int argc, char **argv)
>   }
>   }
>  
> - if (!device_name) {
> - fprintf(stderr, "Device name not set\n");
> + /* Find the device requested */
> + if (dev_num < 0 && !device_name) {
> + fprintf(stderr, "Device not set\n");
>   print_usage();
>   return -1;
> + } else if (dev_num >= 0 && device_name) {
> + fprintf(stderr, "Only one of --device-num or --device-name 
> needs to be set\n");
> + print_usage();
> + return -1;
> + } else if (dev_num < 0) {
> + dev_num = find_type_by_name(device_name, "iio:device");
> + if (dev_num < 0) {
> + fprintf(stderr, "Failed to find the %s\n", device_name);
> + return dev_num;
> + }
>   }
> -
> - /* Find the device requested */
> - dev_num = find_type_by_name(device_name, "iio:device");
> - if (dev_num < 0) {
> - fprintf(stderr, "Failed to find the %s\n", device_name);
> - return dev_num;
> - }
> -
>   printf("iio device number being used is %d\n", dev_num);
>  
>   ret = asprintf(_dir_name, "%siio:device%d", iio_dir, dev_num);
>   if (ret < 0)
>   return -ENOMEM;
> + if (!device_name) {
> + device_name = malloc(IIO_MAX_NAME_LENGTH);
> + if (!device_name)
> + return -ENOMEM;
> + ret = read_sysfs_string("name", dev_dir_name, device_name);
> + if (ret < 0) {
> + fprintf(stderr, "Failed to read name of device %d\n", 
> dev_num);
> + return ret;
> + }
> + }
>  
>   if (!notrigger) {
>   if (!trigger_name) {
> 

Re: [PATCH 2/2] iio: generic_buffer: Add --device-num option

[Jonathan Cameron]

On 20/05/16 16:45, Crestez Dan Leonard wrote:
> This makes it possible to distinguish between iio devices with the same
> name.
> 
> Signed-off-by: Crestez Dan Leonard 
happy with this and will pickup once the first patch is sorted.

Jonathan
> ---
>  tools/iio/generic_buffer.c | 53 
> ++
>  1 file changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
> index e7bf477..f805ef9 100644
> --- a/tools/iio/generic_buffer.c
> +++ b/tools/iio/generic_buffer.c
> @@ -251,7 +251,9 @@ void print_usage(void)
>   "  -e Disable wait for event (new data)\n"
>   "  -g Use trigger-less mode\n"
>   "  -l  Set buffer length to n samples\n"
> - "  -n   Set device name (mandatory)\n"
> + "  --device-name -n \n"
> + "  --device-num -N \n"
> + "Set device by name or number"
>   "  -t   Set trigger name\n"
>   "  -w  Set delay between reads in us (event-less 
> mode)\n");
>  }
> @@ -319,6 +321,12 @@ void register_cleanup(void)
>   }
>  }
>  
> +static const struct option longopts[] = {
> + { "device-name",1, 0, 'n' },
> + { "device-num", 1, 0, 'N' },
> + { },
> +};
> +
>  int main(int argc, char **argv)
>  {
>   unsigned long num_loops = 2;
> @@ -333,7 +341,7 @@ int main(int argc, char **argv)
>  
>   char *data;
>   ssize_t read_size;
> - int dev_num, trig_num;
> + int dev_num = -1, trig_num;
>   char *buffer_access;
>   int scan_size;
>   int noevents = 0;
> @@ -344,7 +352,7 @@ int main(int argc, char **argv)
>  
>   register_cleanup();
>  
> - while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
> + while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
> != -1) {
>   switch (c) {
>   case 'a':
>   autochannels = AUTOCHANNELS_ENABLED;
> @@ -372,6 +380,12 @@ int main(int argc, char **argv)
>   case 'n':
>   device_name = optarg;
>   break;
> + case 'N':
> + errno = 0;
> + dev_num = strtoul(optarg, , 10);
> + if (errno)
> + return -errno;
> + break;
>   case 't':
>   trigger_name = optarg;
>   break;
> @@ -387,24 +401,37 @@ int main(int argc, char **argv)
>   }
>   }
>  
> - if (!device_name) {
> - fprintf(stderr, "Device name not set\n");
> + /* Find the device requested */
> + if (dev_num < 0 && !device_name) {
> + fprintf(stderr, "Device not set\n");
>   print_usage();
>   return -1;
> + } else if (dev_num >= 0 && device_name) {
> + fprintf(stderr, "Only one of --device-num or --device-name 
> needs to be set\n");
> + print_usage();
> + return -1;
> + } else if (dev_num < 0) {
> + dev_num = find_type_by_name(device_name, "iio:device");
> + if (dev_num < 0) {
> + fprintf(stderr, "Failed to find the %s\n", device_name);
> + return dev_num;
> + }
>   }
> -
> - /* Find the device requested */
> - dev_num = find_type_by_name(device_name, "iio:device");
> - if (dev_num < 0) {
> - fprintf(stderr, "Failed to find the %s\n", device_name);
> - return dev_num;
> - }
> -
>   printf("iio device number being used is %d\n", dev_num);
>  
>   ret = asprintf(_dir_name, "%siio:device%d", iio_dir, dev_num);
>   if (ret < 0)
>   return -ENOMEM;
> + if (!device_name) {
> + device_name = malloc(IIO_MAX_NAME_LENGTH);
> + if (!device_name)
> + return -ENOMEM;
> + ret = read_sysfs_string("name", dev_dir_name, device_name);
> + if (ret < 0) {
> + fprintf(stderr, "Failed to read name of device %d\n", 
> dev_num);
> + return ret;
> + }
> + }
>  
>   if (!notrigger) {
>   if (!trigger_name) {
> 

Re: [PATCH 1/2] iio: generic_buffer: Cleanup when receiving signals

[Jonathan Cameron]

On 20/05/16 16:55, Peter Meerwald-Stadler wrote:
> 
>> This also drops all the code freeing string buffers at the end of main.
>> Memory is freed when the process exits anyway so there's no point in
>> cluttering the code with all those gotos.
> 
> well, it helps to see that all memory has been released when looking for 
> leaks :)
> e.g. valgrind becomes much less useful when the program exits with tons of 
> memory still allocated
Beyond that we are looking at code here that will get cut and paste into other
peoples applications - they might not pick up that it doesn't clean up properly
after itself.

I'd much prefer to keep these explicit frees in place.

Jonathan
> 
> functions and global variables could be static
> 
>> ---
>>  tools/iio/generic_buffer.c | 162 
>> -
>>  1 file changed, 88 insertions(+), 74 deletions(-)
>>
>> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
>> index 2429c78..e7bf477 100644
>> --- a/tools/iio/generic_buffer.c
>> +++ b/tools/iio/generic_buffer.c
>> @@ -32,6 +32,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include "iio_utils.h"
>>  
>>  /**
>> @@ -254,6 +256,69 @@ void print_usage(void)
>>  "  -w  Set delay between reads in us (event-less 
>> mode)\n");
>>  }
>>  
>> +enum autochan autochannels = AUTOCHANNELS_DISABLED;
>> +char *dev_dir_name = NULL;
>> +char *buf_dir_name = NULL;
>> +bool current_trigger_set = false;
>> +
>> +void cleanup(void)
>> +{
>> +int ret;
>> +
>> +fprintf(stderr, "Cleanup\n");
>> +
>> +/* Disable trigger */
>> +if (dev_dir_name && current_trigger_set) {
>> +/* Disconnect the trigger - just write a dummy name. */
>> +ret = write_sysfs_string("trigger/current_trigger",
>> + dev_dir_name, "NULL");
>> +if (ret < 0)
>> +fprintf(stderr, "Failed to disable trigger: %s\n",
>> +strerror(-ret));
>> +}
>> +
>> +/* Disable buffer */
>> +if (buf_dir_name) {
>> +ret = write_sysfs_int("enable", buf_dir_name, 0);
>> +if (ret < 0)
>> +fprintf(stderr, "Failed to disable buffer: %s\n",
>> +strerror(-ret));
>> +}
>> +
>> +/* Disable channels if auto-enabled */
>> +if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
>> +ret = enable_disable_all_channels(dev_dir_name, 0);
>> +if (ret)
>> +fprintf(stderr, "Failed to disable all channels\n");
>> +}
>> +}
>> +
>> +void sig_handler(int signum)
>> +{
>> +fprintf(stderr, "Caught signal %d\n", signum);
>> +exit(-signum);
>> +}
>> +
>> +void register_cleanup(void)
>> +{
>> +struct sigaction sa = { .sa_handler = sig_handler };
>> +const int signums[] = { SIGINT, SIGTERM, SIGABRT };
>> +int ret, i;
>> +
>> +ret = atexit(cleanup);
>> +if (ret) {
>> +perror("Failed to register atexit");
>> +exit(-1);
>> +}
>> +for (i = 0; i < ARRAY_SIZE(signums); ++i) {
>> +ret = sigaction(signums[i], , NULL);
>> +if (ret) {
>> +perror("Failed to register signal handler");
>> +exit(-1);
>> +}
>> +}
>> +}
>> +
>>  int main(int argc, char **argv)
>>  {
>>  unsigned long num_loops = 2;
>> @@ -265,9 +330,7 @@ int main(int argc, char **argv)
>>  
>>  int num_channels;
>>  char *trigger_name = NULL, *device_name = NULL;
>> -char *dev_dir_name, *buf_dir_name;
>>  
>> -int datardytrigger = 1;
>>  char *data;
>>  ssize_t read_size;
>>  int dev_num, trig_num;
>> @@ -275,11 +338,12 @@ int main(int argc, char **argv)
>>  int scan_size;
>>  int noevents = 0;
>>  int notrigger = 0;
>> -enum autochan autochannels = AUTOCHANNELS_DISABLED;
>>  char *dummy;
>>  
>>  struct iio_channel_info *channels;
>>  
>> +register_cleanup();
>> +
>>  while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
>>  switch (c) {
>>  case 'a':
>> @@ -310,7 +374,6 @@ int main(int argc, char **argv)
>>  break;
>>  case 't':
>>  trigger_name = optarg;
>> -datardytrigger = 0;
>>  break;
>>  case 'w':
>>  errno = 0;
>> @@ -352,10 +415,8 @@ int main(int argc, char **argv)
>>   */
>>  ret = asprintf(_name,
>> "%s-dev%d", device_name, dev_num);
>> -if (ret < 0) {
>> -ret = -ENOMEM;
>> -goto error_free_dev_dir_name;
>> -}
>> +if (ret < 0)
>> +return -ENOMEM;
>>  }
>>  
>>  /* Look for this "-devN" trigger */
>> @@ 

Re: [PATCH 1/2] iio: generic_buffer: Cleanup when receiving signals

[Jonathan Cameron]

On 20/05/16 16:55, Peter Meerwald-Stadler wrote:
> 
>> This also drops all the code freeing string buffers at the end of main.
>> Memory is freed when the process exits anyway so there's no point in
>> cluttering the code with all those gotos.
> 
> well, it helps to see that all memory has been released when looking for 
> leaks :)
> e.g. valgrind becomes much less useful when the program exits with tons of 
> memory still allocated
Beyond that we are looking at code here that will get cut and paste into other
peoples applications - they might not pick up that it doesn't clean up properly
after itself.

I'd much prefer to keep these explicit frees in place.

Jonathan
> 
> functions and global variables could be static
> 
>> ---
>>  tools/iio/generic_buffer.c | 162 
>> -
>>  1 file changed, 88 insertions(+), 74 deletions(-)
>>
>> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
>> index 2429c78..e7bf477 100644
>> --- a/tools/iio/generic_buffer.c
>> +++ b/tools/iio/generic_buffer.c
>> @@ -32,6 +32,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include "iio_utils.h"
>>  
>>  /**
>> @@ -254,6 +256,69 @@ void print_usage(void)
>>  "  -w  Set delay between reads in us (event-less 
>> mode)\n");
>>  }
>>  
>> +enum autochan autochannels = AUTOCHANNELS_DISABLED;
>> +char *dev_dir_name = NULL;
>> +char *buf_dir_name = NULL;
>> +bool current_trigger_set = false;
>> +
>> +void cleanup(void)
>> +{
>> +int ret;
>> +
>> +fprintf(stderr, "Cleanup\n");
>> +
>> +/* Disable trigger */
>> +if (dev_dir_name && current_trigger_set) {
>> +/* Disconnect the trigger - just write a dummy name. */
>> +ret = write_sysfs_string("trigger/current_trigger",
>> + dev_dir_name, "NULL");
>> +if (ret < 0)
>> +fprintf(stderr, "Failed to disable trigger: %s\n",
>> +strerror(-ret));
>> +}
>> +
>> +/* Disable buffer */
>> +if (buf_dir_name) {
>> +ret = write_sysfs_int("enable", buf_dir_name, 0);
>> +if (ret < 0)
>> +fprintf(stderr, "Failed to disable buffer: %s\n",
>> +strerror(-ret));
>> +}
>> +
>> +/* Disable channels if auto-enabled */
>> +if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
>> +ret = enable_disable_all_channels(dev_dir_name, 0);
>> +if (ret)
>> +fprintf(stderr, "Failed to disable all channels\n");
>> +}
>> +}
>> +
>> +void sig_handler(int signum)
>> +{
>> +fprintf(stderr, "Caught signal %d\n", signum);
>> +exit(-signum);
>> +}
>> +
>> +void register_cleanup(void)
>> +{
>> +struct sigaction sa = { .sa_handler = sig_handler };
>> +const int signums[] = { SIGINT, SIGTERM, SIGABRT };
>> +int ret, i;
>> +
>> +ret = atexit(cleanup);
>> +if (ret) {
>> +perror("Failed to register atexit");
>> +exit(-1);
>> +}
>> +for (i = 0; i < ARRAY_SIZE(signums); ++i) {
>> +ret = sigaction(signums[i], , NULL);
>> +if (ret) {
>> +perror("Failed to register signal handler");
>> +exit(-1);
>> +}
>> +}
>> +}
>> +
>>  int main(int argc, char **argv)
>>  {
>>  unsigned long num_loops = 2;
>> @@ -265,9 +330,7 @@ int main(int argc, char **argv)
>>  
>>  int num_channels;
>>  char *trigger_name = NULL, *device_name = NULL;
>> -char *dev_dir_name, *buf_dir_name;
>>  
>> -int datardytrigger = 1;
>>  char *data;
>>  ssize_t read_size;
>>  int dev_num, trig_num;
>> @@ -275,11 +338,12 @@ int main(int argc, char **argv)
>>  int scan_size;
>>  int noevents = 0;
>>  int notrigger = 0;
>> -enum autochan autochannels = AUTOCHANNELS_DISABLED;
>>  char *dummy;
>>  
>>  struct iio_channel_info *channels;
>>  
>> +register_cleanup();
>> +
>>  while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
>>  switch (c) {
>>  case 'a':
>> @@ -310,7 +374,6 @@ int main(int argc, char **argv)
>>  break;
>>  case 't':
>>  trigger_name = optarg;
>> -datardytrigger = 0;
>>  break;
>>  case 'w':
>>  errno = 0;
>> @@ -352,10 +415,8 @@ int main(int argc, char **argv)
>>   */
>>  ret = asprintf(_name,
>> "%s-dev%d", device_name, dev_num);
>> -if (ret < 0) {
>> -ret = -ENOMEM;
>> -goto error_free_dev_dir_name;
>> -}
>> +if (ret < 0)
>> +return -ENOMEM;
>>  }
>>  
>>  /* Look for this "-devN" trigger */
>> @@ 

Re: [PATCH] iio: humidity: hdc100x: add HDC1000 and HDC1008 product names

[Jonathan Cameron]

On 20/05/16 18:34, Matt Ranostay wrote:
> On Fri, May 20, 2016 at 10:05 AM, Alison Schofield  
> wrote:
>> hdc100x supports Texas Instruments HDC1000 and HDC1008 relative
>> humidity and temperature sensors. Add these product names to
>> Kconfig and to the drivers device id structure to enable finding
>> the product by name and using it to add a device.
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
>> ---
>>  drivers/iio/humidity/Kconfig   | 8 
>>  drivers/iio/humidity/hdc100x.c | 2 ++
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index 738a86d..f155386 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -26,11 +26,11 @@ config HDC100X
>> tristate "TI HDC100x relative humidity and temperature sensor"
>> depends on I2C
>> help
>> -Say yes here to build support for the TI HDC100x series of
>> -relative humidity and temperature sensors.
>> + Say yes here to build support for the Texas Instruments
>> + HDC1000 and HDC1008 relative humidity and temperature sensors.
>>
>> -To compile this driver as a module, choose M here: the module
>> -will be called hdc100x.
>> + To compile this driver as a module, choose M here: the module
>> + will be called hdc100x.
>>
>>  config HTU21
>> tristate "Measurement Specialties HTU21 humidity & temperature 
>> sensor"
>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> index fa47676..0deb874 100644
>> --- a/drivers/iio/humidity/hdc100x.c
>> +++ b/drivers/iio/humidity/hdc100x.c
>> @@ -302,6 +302,8 @@ static int hdc100x_probe(struct i2c_client *client,
>>
>>  static const struct i2c_device_id hdc100x_id[] = {
>> { "hdc100x", 0 },
>> +   { "hdc1000", 0 },
>> +   { "hdc1008", 0 },
> 
> Personally I think adding more device ids that don't add any per chip
> configuration is just adding clutter.
> There is a reason I used "hdc100x" when I wrote the driver :)
Hmm. I should have picked up on this in the first place. It's much preferred to
go with complete part names.  Avoids any possible issues with devicetrees / ACPI
bindings where it's kind of assumed a whole part name will be used.

I'd prefer to have them explicitly listed.  We will have to keep the wild card
form as well as by now there will be boards using that out there.
If it was just internal to the kernel I'd agree with the clutter argument, but
it isn't so let us be as explicit in the naming as possible.

Jonathan
> 
> 
>> { }
>>  };
>>  MODULE_DEVICE_TABLE(i2c, hdc100x_id);
>> --
>> 2.1.4
>>

Re: [PATCH] iio: humidity: hdc100x: add HDC1000 and HDC1008 product names

[Jonathan Cameron]

On 20/05/16 18:34, Matt Ranostay wrote:
> On Fri, May 20, 2016 at 10:05 AM, Alison Schofield  
> wrote:
>> hdc100x supports Texas Instruments HDC1000 and HDC1008 relative
>> humidity and temperature sensors. Add these product names to
>> Kconfig and to the drivers device id structure to enable finding
>> the product by name and using it to add a device.
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
>> ---
>>  drivers/iio/humidity/Kconfig   | 8 
>>  drivers/iio/humidity/hdc100x.c | 2 ++
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index 738a86d..f155386 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -26,11 +26,11 @@ config HDC100X
>> tristate "TI HDC100x relative humidity and temperature sensor"
>> depends on I2C
>> help
>> -Say yes here to build support for the TI HDC100x series of
>> -relative humidity and temperature sensors.
>> + Say yes here to build support for the Texas Instruments
>> + HDC1000 and HDC1008 relative humidity and temperature sensors.
>>
>> -To compile this driver as a module, choose M here: the module
>> -will be called hdc100x.
>> + To compile this driver as a module, choose M here: the module
>> + will be called hdc100x.
>>
>>  config HTU21
>> tristate "Measurement Specialties HTU21 humidity & temperature 
>> sensor"
>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> index fa47676..0deb874 100644
>> --- a/drivers/iio/humidity/hdc100x.c
>> +++ b/drivers/iio/humidity/hdc100x.c
>> @@ -302,6 +302,8 @@ static int hdc100x_probe(struct i2c_client *client,
>>
>>  static const struct i2c_device_id hdc100x_id[] = {
>> { "hdc100x", 0 },
>> +   { "hdc1000", 0 },
>> +   { "hdc1008", 0 },
> 
> Personally I think adding more device ids that don't add any per chip
> configuration is just adding clutter.
> There is a reason I used "hdc100x" when I wrote the driver :)
Hmm. I should have picked up on this in the first place. It's much preferred to
go with complete part names.  Avoids any possible issues with devicetrees / ACPI
bindings where it's kind of assumed a whole part name will be used.

I'd prefer to have them explicitly listed.  We will have to keep the wild card
form as well as by now there will be boards using that out there.
If it was just internal to the kernel I'd agree with the clutter argument, but
it isn't so let us be as explicit in the naming as possible.

Jonathan
> 
> 
>> { }
>>  };
>>  MODULE_DEVICE_TABLE(i2c, hdc100x_id);
>> --
>> 2.1.4
>>

Re: [PATCH] iio: humidity: hdc100x: correct humidity integration time mask

[Jonathan Cameron]

On 20/05/16 18:44, Matt Ranostay wrote:
> Reviewed-by: Matt Ranostay 
> 
> On Fri, May 20, 2016 at 10:06 AM, Alison Schofield  
> wrote:
>> Apply the correct mask to enable all available humidity integration
>> times.  Currently, the driver defaults to 6500 and all is okay with that.
>> However, if 3850 is selected we get a stuck bit and can't change back
>> to 6500 or select 2500.  (Verified with HDC1008)
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
Applied to the fixes-togreg-post-rc1 branch of iio.git

Thanks,

Jonathan
>> ---
>>  drivers/iio/humidity/hdc100x.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> index fa47676..59aa1cb 100644
>> --- a/drivers/iio/humidity/hdc100x.c
>> +++ b/drivers/iio/humidity/hdc100x.c
>> @@ -55,7 +55,7 @@ static const struct {
>> },
>> { /* IIO_HUMIDITYRELATIVE channel */
>> .shift = 8,
>> -   .mask = 2,
>> +   .mask = 3,
> 
> Yikes that is embarrassing on my part! I guess our validation was only
> in the high resolution mode... good catch!
> 
>> },
>>  };
>>
>> --
>> 2.1.4
>>

Re: [PATCH] iio: humidity: hdc100x: correct humidity integration time mask

[Jonathan Cameron]

On 20/05/16 18:44, Matt Ranostay wrote:
> Reviewed-by: Matt Ranostay 
> 
> On Fri, May 20, 2016 at 10:06 AM, Alison Schofield  
> wrote:
>> Apply the correct mask to enable all available humidity integration
>> times.  Currently, the driver defaults to 6500 and all is okay with that.
>> However, if 3850 is selected we get a stuck bit and can't change back
>> to 6500 or select 2500.  (Verified with HDC1008)
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
Applied to the fixes-togreg-post-rc1 branch of iio.git

Thanks,

Jonathan
>> ---
>>  drivers/iio/humidity/hdc100x.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> index fa47676..59aa1cb 100644
>> --- a/drivers/iio/humidity/hdc100x.c
>> +++ b/drivers/iio/humidity/hdc100x.c
>> @@ -55,7 +55,7 @@ static const struct {
>> },
>> { /* IIO_HUMIDITYRELATIVE channel */
>> .shift = 8,
>> -   .mask = 2,
>> +   .mask = 3,
> 
> Yikes that is embarrassing on my part! I guess our validation was only
> in the high resolution mode... good catch!
> 
>> },
>>  };
>>
>> --
>> 2.1.4
>>