Re: [PATCH] ANDROID: binder: change down_write to down_read

[Greg Kroah-Hartman]

On Wed, Mar 28, 2018 at 11:42:31AM +0900, Minchan Kim wrote:
> binder_update_page_range needs down_write of mmap_sem because
> vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless
> it is set. However, when I profile binder working, it seems
> every binder buffers should be mapped in advance by binder_mmap.
> It means we could set VM_MIXEDMAP in bider_mmap time which is
> already hold a mmap_sem as down_write so binder_update_page_range
> doesn't need to hold a mmap_sem as down_write.
> 
> Android suffers from mmap_sem contention so let's reduce mmap_sem
> down_write.
> 
> Cc: Martijn Coenen 
> Cc: Todd Kjos 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Minchan Kim 
> ---
>  drivers/android/binder.c   | 2 +-
>  drivers/android/binder_alloc.c | 8 +---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 764b63a5aade..9a14c6dd60c4 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -4722,7 +4722,7 @@ static int binder_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   failure_string = "bad vm_flags";
>   goto err_bad_arg;
>   }
> - vma->vm_flags = (vma->vm_flags | VM_DONTCOPY) & ~VM_MAYWRITE;
> + vma->vm_flags |= (VM_DONTCOPY | VM_MIXEDMAP) & ~VM_MAYWRITE;
>   vma->vm_ops = _vm_ops;
>   vma->vm_private_data = proc;
>  
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 5a426c877dfb..a184bf12eb15 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -219,7 +219,7 @@ static int binder_update_page_range(struct binder_alloc 
> *alloc, int allocate,
>   mm = alloc->vma_vm_mm;
>  
>   if (mm) {
> - down_write(>mmap_sem);
> + down_read(>mmap_sem);
>   vma = alloc->vma;
>   }
>  
> @@ -229,6 +229,8 @@ static int binder_update_page_range(struct binder_alloc 
> *alloc, int allocate,
>   goto err_no_vma;
>   }
>  
> + WARN_ON_ONCE(vma && !(vma->vm_flags & VM_MIXEDMAP));

What can we do with this warning?  What about systems that run in 'panic
on warn' mode?  Is this something we should handle directly instead?

thanks,

greg k-h

Re: [PATCH] ANDROID: binder: change down_write to down_read

[Greg Kroah-Hartman]

On Wed, Mar 28, 2018 at 11:42:31AM +0900, Minchan Kim wrote:
> binder_update_page_range needs down_write of mmap_sem because
> vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless
> it is set. However, when I profile binder working, it seems
> every binder buffers should be mapped in advance by binder_mmap.
> It means we could set VM_MIXEDMAP in bider_mmap time which is
> already hold a mmap_sem as down_write so binder_update_page_range
> doesn't need to hold a mmap_sem as down_write.
> 
> Android suffers from mmap_sem contention so let's reduce mmap_sem
> down_write.
> 
> Cc: Martijn Coenen 
> Cc: Todd Kjos 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Minchan Kim 
> ---
>  drivers/android/binder.c   | 2 +-
>  drivers/android/binder_alloc.c | 8 +---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 764b63a5aade..9a14c6dd60c4 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -4722,7 +4722,7 @@ static int binder_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   failure_string = "bad vm_flags";
>   goto err_bad_arg;
>   }
> - vma->vm_flags = (vma->vm_flags | VM_DONTCOPY) & ~VM_MAYWRITE;
> + vma->vm_flags |= (VM_DONTCOPY | VM_MIXEDMAP) & ~VM_MAYWRITE;
>   vma->vm_ops = _vm_ops;
>   vma->vm_private_data = proc;
>  
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 5a426c877dfb..a184bf12eb15 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -219,7 +219,7 @@ static int binder_update_page_range(struct binder_alloc 
> *alloc, int allocate,
>   mm = alloc->vma_vm_mm;
>  
>   if (mm) {
> - down_write(>mmap_sem);
> + down_read(>mmap_sem);
>   vma = alloc->vma;
>   }
>  
> @@ -229,6 +229,8 @@ static int binder_update_page_range(struct binder_alloc 
> *alloc, int allocate,
>   goto err_no_vma;
>   }
>  
> + WARN_ON_ONCE(vma && !(vma->vm_flags & VM_MIXEDMAP));

What can we do with this warning?  What about systems that run in 'panic
on warn' mode?  Is this something we should handle directly instead?

thanks,

greg k-h

Re: [PATCH] slab, slub: skip unnecessary kasan_cache_shutdown()

[Shakeel Butt]

On Tue, Mar 27, 2018 at 5:16 PM, David Rientjes  wrote:
> On Tue, 27 Mar 2018, Shakeel Butt wrote:
>
>> diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
>> index 49fffb0ca83b..135ce2838c89 100644
>> --- a/mm/kasan/kasan.c
>> +++ b/mm/kasan/kasan.c
>> @@ -382,7 +382,8 @@ void kasan_cache_shrink(struct kmem_cache *cache)
>>
>>  void kasan_cache_shutdown(struct kmem_cache *cache)
>>  {
>> - quarantine_remove_cache(cache);
>> + if (!__kmem_cache_empty(cache))
>> + quarantine_remove_cache(cache);
>>  }
>>
>>  size_t kasan_metadata_size(struct kmem_cache *cache)
>> diff --git a/mm/slab.c b/mm/slab.c
>> index 9212c64bb705..b59f2cdf28d1 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -2291,6 +2291,18 @@ static int drain_freelist(struct kmem_cache *cache,
>>   return nr_freed;
>>  }
>>
>> +bool __kmem_cache_empty(struct kmem_cache *s)
>> +{
>> + int node;
>> + struct kmem_cache_node *n;
>> +
>> + for_each_kmem_cache_node(s, node, n)
>> + if (!list_empty(>slabs_full) ||
>> + !list_empty(>slabs_partial))
>> + return false;
>> + return true;
>> +}
>> +
>>  int __kmem_cache_shrink(struct kmem_cache *cachep)
>>  {
>>   int ret = 0;
>> diff --git a/mm/slab.h b/mm/slab.h
>> index e8981e811c45..68bdf498da3b 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -166,6 +166,7 @@ static inline slab_flags_t kmem_cache_flags(unsigned int 
>> object_size,
>> SLAB_TEMPORARY | \
>> SLAB_ACCOUNT)
>>
>> +bool __kmem_cache_empty(struct kmem_cache *);
>>  int __kmem_cache_shutdown(struct kmem_cache *);
>>  void __kmem_cache_release(struct kmem_cache *);
>>  int __kmem_cache_shrink(struct kmem_cache *);
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 1edc8d97c862..44aa7847324a 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -3707,6 +3707,17 @@ static void free_partial(struct kmem_cache *s, struct 
>> kmem_cache_node *n)
>>   discard_slab(s, page);
>>  }
>>
>> +bool __kmem_cache_empty(struct kmem_cache *s)
>> +{
>> + int node;
>> + struct kmem_cache_node *n;
>> +
>> + for_each_kmem_cache_node(s, node, n)
>> + if (n->nr_partial || slabs_node(s, node))
>> + return false;
>> + return true;
>> +}
>> +
>>  /*
>>   * Release all resources used by a slab cache.
>>   */
>
> Any reason not to just make quarantine_remove_cache() part of
> __kmem_cache_shutdown() instead of duplicating its logic?

Can you please explain what you mean by making
quarantine_remove_cache() part of __kmem_cache_shutdown()? Do you mean
calling quarantine_remove_cache() inside __kmem_cache_shutdown()? The
__kmem_cache_shutdown() of both SLAB & SLUB does per-cpu
draining/flushing and we want the free the quarantined objects before
that. So, I am not sure how to incorporate  quarantine_remove_cache()
into __kmem_cache_shutdown() without duplicating the for-loop &
if-condition.

Re: [PATCH] slab, slub: skip unnecessary kasan_cache_shutdown()

[Shakeel Butt]

On Tue, Mar 27, 2018 at 5:16 PM, David Rientjes  wrote:
> On Tue, 27 Mar 2018, Shakeel Butt wrote:
>
>> diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
>> index 49fffb0ca83b..135ce2838c89 100644
>> --- a/mm/kasan/kasan.c
>> +++ b/mm/kasan/kasan.c
>> @@ -382,7 +382,8 @@ void kasan_cache_shrink(struct kmem_cache *cache)
>>
>>  void kasan_cache_shutdown(struct kmem_cache *cache)
>>  {
>> - quarantine_remove_cache(cache);
>> + if (!__kmem_cache_empty(cache))
>> + quarantine_remove_cache(cache);
>>  }
>>
>>  size_t kasan_metadata_size(struct kmem_cache *cache)
>> diff --git a/mm/slab.c b/mm/slab.c
>> index 9212c64bb705..b59f2cdf28d1 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -2291,6 +2291,18 @@ static int drain_freelist(struct kmem_cache *cache,
>>   return nr_freed;
>>  }
>>
>> +bool __kmem_cache_empty(struct kmem_cache *s)
>> +{
>> + int node;
>> + struct kmem_cache_node *n;
>> +
>> + for_each_kmem_cache_node(s, node, n)
>> + if (!list_empty(>slabs_full) ||
>> + !list_empty(>slabs_partial))
>> + return false;
>> + return true;
>> +}
>> +
>>  int __kmem_cache_shrink(struct kmem_cache *cachep)
>>  {
>>   int ret = 0;
>> diff --git a/mm/slab.h b/mm/slab.h
>> index e8981e811c45..68bdf498da3b 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -166,6 +166,7 @@ static inline slab_flags_t kmem_cache_flags(unsigned int 
>> object_size,
>> SLAB_TEMPORARY | \
>> SLAB_ACCOUNT)
>>
>> +bool __kmem_cache_empty(struct kmem_cache *);
>>  int __kmem_cache_shutdown(struct kmem_cache *);
>>  void __kmem_cache_release(struct kmem_cache *);
>>  int __kmem_cache_shrink(struct kmem_cache *);
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 1edc8d97c862..44aa7847324a 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -3707,6 +3707,17 @@ static void free_partial(struct kmem_cache *s, struct 
>> kmem_cache_node *n)
>>   discard_slab(s, page);
>>  }
>>
>> +bool __kmem_cache_empty(struct kmem_cache *s)
>> +{
>> + int node;
>> + struct kmem_cache_node *n;
>> +
>> + for_each_kmem_cache_node(s, node, n)
>> + if (n->nr_partial || slabs_node(s, node))
>> + return false;
>> + return true;
>> +}
>> +
>>  /*
>>   * Release all resources used by a slab cache.
>>   */
>
> Any reason not to just make quarantine_remove_cache() part of
> __kmem_cache_shutdown() instead of duplicating its logic?

Can you please explain what you mean by making
quarantine_remove_cache() part of __kmem_cache_shutdown()? Do you mean
calling quarantine_remove_cache() inside __kmem_cache_shutdown()? The
__kmem_cache_shutdown() of both SLAB & SLUB does per-cpu
draining/flushing and we want the free the quarantined objects before
that. So, I am not sure how to incorporate  quarantine_remove_cache()
into __kmem_cache_shutdown() without duplicating the for-loop &
if-condition.

WARNING in refcount_dec

[Byoungyoung Lee]

We report the crash: WARNING in refcount_dec

This crash has been found in v4.16-rc3 using RaceFuzzer (a modified
version of Syzkaller), which we describe more at the end of this
report. Our analysis shows that the race occurs when invoking two
syscalls concurrently, (setsockopt$packet_int) and
(setsockopt$packet_rx_ring).

C repro code : 
https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-refcount_dec.c
kernel config: 
https://kiwi.cs.purdue.edu/static/race-fuzzer/kernel-config-v4.16-rc3

---
[  305.838560] refcount_t: decrement hit 0; leaking memory.
[  305.839669] WARNING: CPU: 0 PID: 3867 at
/home/blee/project/race-fuzzer/kernels/kernel_v4.16-rc3/lib/refcount.c:228
refcount_dec+0x62/0x70
[  305.841441] Modules linked in:
[  305.841883] CPU: 0 PID: 3867 Comm: repro.exe Not tainted 4.16.0-rc3 #1
[  305.842803] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[  305.844345] RIP: 0010:refcount_dec+0x62/0x70
[  305.845005] RSP: 0018:880224d374f8 EFLAGS: 00010286
[  305.845802] RAX: 002c RBX:  RCX: 81538991
[  305.846768] RDX:  RSI: 813cd761 RDI: 0005
[  305.847748] RBP: 880224d37500 R08: 88023169a440 R09: 
[  305.848748] R10:  R11:  R12: 88023473ad40
[  305.849738] R13: 88023473b368 R14: 0001 R15: 
[  305.850733] FS:  00c6e940() GS:8800b8a0()
knlGS:
[  305.851837] CS:  0010 DS:  ES:  CR0: 80050033
[  305.852652] CR2: 7fb120571db8 CR3: 05422000 CR4: 06f0
[  305.853619] Call Trace:
[  305.854086]  __unregister_prot_hook+0x15f/0x1d0
[  305.854722]  packet_release+0x77a/0x7a0
[  305.855335]  ? mark_held_locks+0x25/0xb0
[  305.855883]  ? packet_lookup_frame+0x110/0x110
[  305.856576]  ? __lock_is_held+0x39/0xc0
[  305.857109]  ? note_gp_changes+0x300/0x300
[  305.857745]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
[  305.858548]  ? locks_remove_file+0x31b/0x420
[  305.859138]  ? fcntl_setlk+0xad0/0xad0
[  305.859743]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.860534]  ? fsnotify_first_mark+0x2c0/0x2c0
[  305.861234]  sock_release+0x53/0xe0
[  305.861711]  ? sock_alloc_file+0x2c0/0x2c0
[  305.862334]  sock_close+0x16/0x20
[  305.862801]  __fput+0x246/0x4e0
[  305.863310]  ? fput+0x130/0x130
[  305.863743]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.864604]  fput+0x15/0x20
[  305.865046]  task_work_run+0x1a5/0x200
[  305.865636]  ? kmem_cache_free+0x25c/0x2d0
[  305.866194]  ? task_work_cancel+0x1a0/0x1a0
[  305.866829]  ? switch_task_namespaces+0x9e/0xb0
[  305.867458]  do_exit+0xacf/0x10d0
[  305.868023]  ? mm_update_next_owner+0x650/0x650
[  305.868642]  ? __pv_queued_spin_lock_slowpath+0xbf0/0xbf0
[  305.869427]  ? trace_hardirqs_on_caller+0x136/0x2a0
[  305.870102]  ? trace_hardirqs_on+0xd/0x10
[  305.870701]  ? wake_up_new_task+0x41c/0x650
[  305.871324]  ? to_ratio+0x20/0x20
[  305.871816]  ? lock_release+0x530/0x530
[  305.872341]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.873161]  ? match_held_lock+0x7e/0x360
[  305.873777]  ? trace_hardirqs_off+0x10/0x10
[  305.874359]  ? put_pid+0x111/0x140
[  305.874897]  ? task_active_pid_ns+0x70/0x70
[  305.875500]  ? find_held_lock+0xca/0xf0
[  305.876118]  ? do_group_exit+0x1f9/0x260
[  305.876650]  ? lock_downgrade+0x380/0x380
[  305.877297]  ? task_clear_jobctl_pending+0xb5/0xd0
[  305.877951]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  305.878725]  ? do_raw_spin_unlock+0x112/0x1a0
[  305.879309]  ? do_raw_spin_trylock+0x100/0x100
[  305.879969]  ? mark_held_locks+0x25/0xb0
[  305.880505]  ? force_sig+0x30/0x30
[  305.881054]  ? _raw_spin_unlock_irq+0x27/0x50
[  305.881671]  ? trace_hardirqs_on_caller+0x136/0x2a0
[  305.882412]  do_group_exit+0xfb/0x260
[  305.882945]  ? SyS_exit+0x30/0x30
[  305.883442]  ? find_mergeable_anon_vma+0x90/0x90
[  305.884103]  ? SyS_read+0x1c0/0x1c0
[  305.884785]  ? mark_held_locks+0x25/0xb0
[  305.885503]  ? do_syscall_64+0xb2/0x5d0
[  305.886093]  ? do_group_exit+0x260/0x260
[  305.886741]  SyS_exit_group+0x1d/0x20
[  305.887255]  do_syscall_64+0x209/0x5d0
[  305.887888]  ? syscall_return_slowpath+0x3e0/0x3e0
[  305.888611]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  305.889420]  ? syscall_return_slowpath+0x260/0x3e0
[  305.890188]  ? mark_held_locks+0x25/0xb0
[  305.890724]  ? entry_SYSCALL_64_after_hwframe+0x52/0xb7
[  305.891556]  ? trace_hardirqs_off_caller+0xb5/0x120
[  305.892265]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  305.892939]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  305.893676] RIP: 0033:0x44d989
[  305.894100] RSP: 002b:007fff38 EFLAGS: 0202 ORIG_RAX:
00e7
[  305.895158] RAX: ffda RBX: 004002c8 RCX: 0044d989
[  305.896174] RDX: 7fb120d739c0 RSI: 

WARNING in refcount_dec

[Byoungyoung Lee]

We report the crash: WARNING in refcount_dec

This crash has been found in v4.16-rc3 using RaceFuzzer (a modified
version of Syzkaller), which we describe more at the end of this
report. Our analysis shows that the race occurs when invoking two
syscalls concurrently, (setsockopt$packet_int) and
(setsockopt$packet_rx_ring).

C repro code : 
https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-refcount_dec.c
kernel config: 
https://kiwi.cs.purdue.edu/static/race-fuzzer/kernel-config-v4.16-rc3

---
[  305.838560] refcount_t: decrement hit 0; leaking memory.
[  305.839669] WARNING: CPU: 0 PID: 3867 at
/home/blee/project/race-fuzzer/kernels/kernel_v4.16-rc3/lib/refcount.c:228
refcount_dec+0x62/0x70
[  305.841441] Modules linked in:
[  305.841883] CPU: 0 PID: 3867 Comm: repro.exe Not tainted 4.16.0-rc3 #1
[  305.842803] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[  305.844345] RIP: 0010:refcount_dec+0x62/0x70
[  305.845005] RSP: 0018:880224d374f8 EFLAGS: 00010286
[  305.845802] RAX: 002c RBX:  RCX: 81538991
[  305.846768] RDX:  RSI: 813cd761 RDI: 0005
[  305.847748] RBP: 880224d37500 R08: 88023169a440 R09: 
[  305.848748] R10:  R11:  R12: 88023473ad40
[  305.849738] R13: 88023473b368 R14: 0001 R15: 
[  305.850733] FS:  00c6e940() GS:8800b8a0()
knlGS:
[  305.851837] CS:  0010 DS:  ES:  CR0: 80050033
[  305.852652] CR2: 7fb120571db8 CR3: 05422000 CR4: 06f0
[  305.853619] Call Trace:
[  305.854086]  __unregister_prot_hook+0x15f/0x1d0
[  305.854722]  packet_release+0x77a/0x7a0
[  305.855335]  ? mark_held_locks+0x25/0xb0
[  305.855883]  ? packet_lookup_frame+0x110/0x110
[  305.856576]  ? __lock_is_held+0x39/0xc0
[  305.857109]  ? note_gp_changes+0x300/0x300
[  305.857745]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
[  305.858548]  ? locks_remove_file+0x31b/0x420
[  305.859138]  ? fcntl_setlk+0xad0/0xad0
[  305.859743]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.860534]  ? fsnotify_first_mark+0x2c0/0x2c0
[  305.861234]  sock_release+0x53/0xe0
[  305.861711]  ? sock_alloc_file+0x2c0/0x2c0
[  305.862334]  sock_close+0x16/0x20
[  305.862801]  __fput+0x246/0x4e0
[  305.863310]  ? fput+0x130/0x130
[  305.863743]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.864604]  fput+0x15/0x20
[  305.865046]  task_work_run+0x1a5/0x200
[  305.865636]  ? kmem_cache_free+0x25c/0x2d0
[  305.866194]  ? task_work_cancel+0x1a0/0x1a0
[  305.866829]  ? switch_task_namespaces+0x9e/0xb0
[  305.867458]  do_exit+0xacf/0x10d0
[  305.868023]  ? mm_update_next_owner+0x650/0x650
[  305.868642]  ? __pv_queued_spin_lock_slowpath+0xbf0/0xbf0
[  305.869427]  ? trace_hardirqs_on_caller+0x136/0x2a0
[  305.870102]  ? trace_hardirqs_on+0xd/0x10
[  305.870701]  ? wake_up_new_task+0x41c/0x650
[  305.871324]  ? to_ratio+0x20/0x20
[  305.871816]  ? lock_release+0x530/0x530
[  305.872341]  ? trace_event_raw_event_sched_switch+0x320/0x320
[  305.873161]  ? match_held_lock+0x7e/0x360
[  305.873777]  ? trace_hardirqs_off+0x10/0x10
[  305.874359]  ? put_pid+0x111/0x140
[  305.874897]  ? task_active_pid_ns+0x70/0x70
[  305.875500]  ? find_held_lock+0xca/0xf0
[  305.876118]  ? do_group_exit+0x1f9/0x260
[  305.876650]  ? lock_downgrade+0x380/0x380
[  305.877297]  ? task_clear_jobctl_pending+0xb5/0xd0
[  305.877951]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  305.878725]  ? do_raw_spin_unlock+0x112/0x1a0
[  305.879309]  ? do_raw_spin_trylock+0x100/0x100
[  305.879969]  ? mark_held_locks+0x25/0xb0
[  305.880505]  ? force_sig+0x30/0x30
[  305.881054]  ? _raw_spin_unlock_irq+0x27/0x50
[  305.881671]  ? trace_hardirqs_on_caller+0x136/0x2a0
[  305.882412]  do_group_exit+0xfb/0x260
[  305.882945]  ? SyS_exit+0x30/0x30
[  305.883442]  ? find_mergeable_anon_vma+0x90/0x90
[  305.884103]  ? SyS_read+0x1c0/0x1c0
[  305.884785]  ? mark_held_locks+0x25/0xb0
[  305.885503]  ? do_syscall_64+0xb2/0x5d0
[  305.886093]  ? do_group_exit+0x260/0x260
[  305.886741]  SyS_exit_group+0x1d/0x20
[  305.887255]  do_syscall_64+0x209/0x5d0
[  305.887888]  ? syscall_return_slowpath+0x3e0/0x3e0
[  305.888611]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[  305.889420]  ? syscall_return_slowpath+0x260/0x3e0
[  305.890188]  ? mark_held_locks+0x25/0xb0
[  305.890724]  ? entry_SYSCALL_64_after_hwframe+0x52/0xb7
[  305.891556]  ? trace_hardirqs_off_caller+0xb5/0x120
[  305.892265]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  305.892939]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  305.893676] RIP: 0033:0x44d989
[  305.894100] RSP: 002b:007fff38 EFLAGS: 0202 ORIG_RAX:
00e7
[  305.895158] RAX: ffda RBX: 004002c8 RCX: 0044d989
[  305.896174] RDX: 7fb120d739c0 RSI: 

Re: [PATCH for-4.17 2/2] powerpc: Remove smp_mb() from arch_spin_is_locked()

[Michael Ellerman]

Andrea Parri  writes:
> On Tue, Mar 27, 2018 at 11:06:56AM +1100, Benjamin Herrenschmidt wrote:
>> On Mon, 2018-03-26 at 12:37 +0200, Andrea Parri wrote:
>> > Commit 51d7d5205d338 ("powerpc: Add smp_mb() to arch_spin_is_locked()")
>> > added an smp_mb() to arch_spin_is_locked(), in order to ensure that
>> > 
>> >Thread 0Thread 1
>> > 
>> >spin_lock(A);   spin_lock(B);
>> >r0 = spin_is_locked(B)  r1 = spin_is_locked(A);
>> > 
>> > never ends up with r0 = r1 = 0, and reported one example (in ipc/sem.c)
>> > relying on such guarantee.
>> > 
>> > It's however understood (and undocumented) that spin_is_locked() is not
>> > required to ensure such ordering guarantee,
>> 
>> Shouldn't we start by documenting it ?
>
> I do sympathize with your concern about the documentation! ;) The patch in
> [1] was my (re)action to this concern; the sort of the patch is unclear to
> me by this time (and I'm not aware of other proposals in this respect).
>> 
>> >  guarantee that is currently
>> > _not_ provided by all implementations/arch, and that callers relying on
>> > such ordering should instead use suitable memory barriers before acting
>> > on the result of spin_is_locked().
>> > 
>> > Following a recent auditing[1] of the callers of {,raw_}spin_is_locked()
>> > revealing that none of them are relying on this guarantee anymore, this
>> > commit removes the leading smp_mb() from the primitive thus effectively
>> > reverting 51d7d5205d338.
>> 
>> I would rather wait until it is properly documented. Debugging that IPC
>> problem took a *LOT* of time and energy, I wouldn't want these issues
>> to come and bite us again.
>
> I understand. And I'm grateful for this debugging as well as for the (IMO)
> excellent account of it you provided in 51d7d5205d338.

Thanks, you're welcome :)

> Said this ;) I cannot except myself from saying that I would probably have
> resisted that solution (adding an smp_mb() in my arch_spin_is_locked), and
> instead "blamed"/suggested that caller to fix his memory ordering...

That was tempting, but it leaves unfixed all the other potential
callers, both in in-tree and out-of-tree and in code that's yet to be
written.

Looking today nearly all the callers are debug code, where we probably
don't need the barrier but we also don't care about the overhead of the
barrier.

Documenting it would definitely be good, but even then I'd be inclined
to leave the barrier in our implementation. Matching the documented
behaviour is one thing, but the actual real-world behaviour on well
tested platforms (ie. x86) is more important.

cheers

Re: [PATCH for-4.17 2/2] powerpc: Remove smp_mb() from arch_spin_is_locked()

[Michael Ellerman]

Andrea Parri  writes:
> On Tue, Mar 27, 2018 at 11:06:56AM +1100, Benjamin Herrenschmidt wrote:
>> On Mon, 2018-03-26 at 12:37 +0200, Andrea Parri wrote:
>> > Commit 51d7d5205d338 ("powerpc: Add smp_mb() to arch_spin_is_locked()")
>> > added an smp_mb() to arch_spin_is_locked(), in order to ensure that
>> > 
>> >Thread 0Thread 1
>> > 
>> >spin_lock(A);   spin_lock(B);
>> >r0 = spin_is_locked(B)  r1 = spin_is_locked(A);
>> > 
>> > never ends up with r0 = r1 = 0, and reported one example (in ipc/sem.c)
>> > relying on such guarantee.
>> > 
>> > It's however understood (and undocumented) that spin_is_locked() is not
>> > required to ensure such ordering guarantee,
>> 
>> Shouldn't we start by documenting it ?
>
> I do sympathize with your concern about the documentation! ;) The patch in
> [1] was my (re)action to this concern; the sort of the patch is unclear to
> me by this time (and I'm not aware of other proposals in this respect).
>> 
>> >  guarantee that is currently
>> > _not_ provided by all implementations/arch, and that callers relying on
>> > such ordering should instead use suitable memory barriers before acting
>> > on the result of spin_is_locked().
>> > 
>> > Following a recent auditing[1] of the callers of {,raw_}spin_is_locked()
>> > revealing that none of them are relying on this guarantee anymore, this
>> > commit removes the leading smp_mb() from the primitive thus effectively
>> > reverting 51d7d5205d338.
>> 
>> I would rather wait until it is properly documented. Debugging that IPC
>> problem took a *LOT* of time and energy, I wouldn't want these issues
>> to come and bite us again.
>
> I understand. And I'm grateful for this debugging as well as for the (IMO)
> excellent account of it you provided in 51d7d5205d338.

Thanks, you're welcome :)

> Said this ;) I cannot except myself from saying that I would probably have
> resisted that solution (adding an smp_mb() in my arch_spin_is_locked), and
> instead "blamed"/suggested that caller to fix his memory ordering...

That was tempting, but it leaves unfixed all the other potential
callers, both in in-tree and out-of-tree and in code that's yet to be
written.

Looking today nearly all the callers are debug code, where we probably
don't need the barrier but we also don't care about the overhead of the
barrier.

Documenting it would definitely be good, but even then I'd be inclined
to leave the barrier in our implementation. Matching the documented
behaviour is one thing, but the actual real-world behaviour on well
tested platforms (ie. x86) is more important.

cheers

Re: [PATCH] pinctrl/amd: poll InterruptEnable bits in enable_irq

[Shah, Nehal-bakulchandra]

Hi

On 3/26/2018 2:42 PM, Linus Walleij wrote:
> On Mon, Mar 12, 2018 at 5:45 PM, Daniel Kurtz  wrote:
> 
>> In certain cases interrupt enablement will be delayed relative to when
>> the InterruptEnable bits are written.  One example of this is when
>> a GPIO's "debounce" logice is first enabled.  After enabling debounce,
>> there is a 900 us "warm up" period during which InterruptEnable[0]
>> (bit 11) will read as 0 despite being written 1.  During this time
>> InterruptSts will not be updated, nor will interrupts be delivered, even
>> if the GPIO's interrupt configuration has been written to the register.
>>
>> To work around this delay, poll the InterruptEnable bits after setting
>> them to ensure interrupts have truly been enabled in hardware before
>> returning from the irq_enable handler.
>>
>> Signed-off-by: Daniel Kurtz 
> 
> Patch applied.
> 
> I see the AMD people were not on CC so adding them here so they can
> say if there is any problem with the approach.
> 
> Daniel: maybe you should consider listing yourself as comaintainer of this
> driver?
> 
> Yours,
> Linus Walleij
> 

Looks good. 

Regards
Nehal Shah

Re: [PATCH] pinctrl/amd: poll InterruptEnable bits in enable_irq

[Shah, Nehal-bakulchandra]

Hi

On 3/26/2018 2:42 PM, Linus Walleij wrote:
> On Mon, Mar 12, 2018 at 5:45 PM, Daniel Kurtz  wrote:
> 
>> In certain cases interrupt enablement will be delayed relative to when
>> the InterruptEnable bits are written.  One example of this is when
>> a GPIO's "debounce" logice is first enabled.  After enabling debounce,
>> there is a 900 us "warm up" period during which InterruptEnable[0]
>> (bit 11) will read as 0 despite being written 1.  During this time
>> InterruptSts will not be updated, nor will interrupts be delivered, even
>> if the GPIO's interrupt configuration has been written to the register.
>>
>> To work around this delay, poll the InterruptEnable bits after setting
>> them to ensure interrupts have truly been enabled in hardware before
>> returning from the irq_enable handler.
>>
>> Signed-off-by: Daniel Kurtz 
> 
> Patch applied.
> 
> I see the AMD people were not on CC so adding them here so they can
> say if there is any problem with the approach.
> 
> Daniel: maybe you should consider listing yourself as comaintainer of this
> driver?
> 
> Yours,
> Linus Walleij
> 

Looks good. 

Regards
Nehal Shah

[PATCH v2] perf stat: avoid 10ms limit for printing event counts

[Alexey Budankov]

Currently print count interval for performance counters values is 
limited by 10ms so reading the values at frequencies higher than 100Hz 
is restricted by the tool.

This change avoids that limitation and makes perf stat -I possible 
on frequencies up to 1KHz and, to some extent, makes perf stat -I 
to be on-par with perf record sampling profiling.

When running perf stat -I for monitoring e.g. PCIe uncore counters and 
at the same time profiling some I/O workload by perf record e.g. for 
cpu-cycles and context switches, it is then possible to build and 
observe good-enough consolidated CPU/OS/IO(Uncore) performance picture 
for that workload.

Printing the warning doesn't make sense in case you have output to 
the console because you quickly get your screen scrolled down. 
If the interval is small you may even skip the warning regardless 
of -v option.

Signed-off-by: Alexey Budankov 
---

Distinction from v1 is that the warning message regarding possible 
overhead is moved from code to help text generated by per stat -h.

 tools/perf/builtin-stat.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f5c454855908..147a27e8c937 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1943,7 +1943,8 @@ static const struct option stat_options[] = {
OPT_STRING(0, "post", _cmd, "command",
"command to run after to the measured command"),
OPT_UINTEGER('I', "interval-print", _config.interval,
-   "print counts at regular interval in ms (>= 10)"),
+   "print counts at regular interval in ms "
+   "(overhead is possible for values <= 100ms)"),
OPT_INTEGER(0, "interval-count", _config.times,
"print counts for fixed number of times"),
OPT_UINTEGER(0, "timeout", _config.timeout,
@@ -2923,17 +2924,6 @@ int cmd_stat(int argc, const char **argv)
}
}
 
-   if (interval && interval < 100) {
-   if (interval < 10) {
-   pr_err("print interval must be >= 10ms\n");
-   parse_options_usage(stat_usage, stat_options, "I", 1);
-   goto out;
-   } else
-   pr_warning("print interval < 100ms. "
-  "The overhead percentage could be high in 
some cases. "
-  "Please proceed with caution.\n");
-   }
-
if (stat_config.times && interval)
interval_count = true;
else if (stat_config.times && !interval) {

[PATCH v2] perf stat: avoid 10ms limit for printing event counts

[Alexey Budankov]

Currently print count interval for performance counters values is 
limited by 10ms so reading the values at frequencies higher than 100Hz 
is restricted by the tool.

This change avoids that limitation and makes perf stat -I possible 
on frequencies up to 1KHz and, to some extent, makes perf stat -I 
to be on-par with perf record sampling profiling.

When running perf stat -I for monitoring e.g. PCIe uncore counters and 
at the same time profiling some I/O workload by perf record e.g. for 
cpu-cycles and context switches, it is then possible to build and 
observe good-enough consolidated CPU/OS/IO(Uncore) performance picture 
for that workload.

Printing the warning doesn't make sense in case you have output to 
the console because you quickly get your screen scrolled down. 
If the interval is small you may even skip the warning regardless 
of -v option.

Signed-off-by: Alexey Budankov 
---

Distinction from v1 is that the warning message regarding possible 
overhead is moved from code to help text generated by per stat -h.

 tools/perf/builtin-stat.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f5c454855908..147a27e8c937 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1943,7 +1943,8 @@ static const struct option stat_options[] = {
OPT_STRING(0, "post", _cmd, "command",
"command to run after to the measured command"),
OPT_UINTEGER('I', "interval-print", _config.interval,
-   "print counts at regular interval in ms (>= 10)"),
+   "print counts at regular interval in ms "
+   "(overhead is possible for values <= 100ms)"),
OPT_INTEGER(0, "interval-count", _config.times,
"print counts for fixed number of times"),
OPT_UINTEGER(0, "timeout", _config.timeout,
@@ -2923,17 +2924,6 @@ int cmd_stat(int argc, const char **argv)
}
}
 
-   if (interval && interval < 100) {
-   if (interval < 10) {
-   pr_err("print interval must be >= 10ms\n");
-   parse_options_usage(stat_usage, stat_options, "I", 1);
-   goto out;
-   } else
-   pr_warning("print interval < 100ms. "
-  "The overhead percentage could be high in 
some cases. "
-  "Please proceed with caution.\n");
-   }
-
if (stat_config.times && interval)
interval_count = true;
else if (stat_config.times && !interval) {

linux-next: manual merge of the kvm-arm tree with the arm64 tree

[Stephen Rothwell]

Hi all,

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

  arch/arm64/kernel/cpufeature.c

between commits:

  143ba05d867a ("arm64: capabilities: Prepare for fine grained capabilities")
  12eb369125ab ("arm64: cpufeature: Avoid warnings due to unused symbols")

from the arm64 tree and commit:

  a1efdff442ec ("arm64: cpufeatures: Drop the ARM64_HYP_OFFSET_LOW feature 
flag")

from the kvm-arm tree.

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

-- 
Cheers,
Stephen Rothwell

diff --cc arch/arm64/kernel/cpufeature.c
index 96b15d7b10a8,5b25d56bccfd..
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@@ -838,19 -826,11 +838,6 @@@ static bool has_no_hw_prefetch(const st
MIDR_CPU_VAR_REV(1, MIDR_REVISION_MASK));
  }
  
- static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry,
-  int __unused)
 -static bool runs_at_el2(const struct arm64_cpu_capabilities *entry, int 
__unused)
--{
-   phys_addr_t idmap_addr = __pa_symbol(__hyp_idmap_text_start);
- 
-   /*
-* Activate the lower HYP offset only if:
-* - the idmap doesn't clash with it,
-* - the kernel is not running at EL2.
-*/
-   return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode();
 -  return is_kernel_in_hyp_mode();
--}
--
  static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int 
__unused)
  {
u64 pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);


pgpOYu3LqTBn5.pgp
Description: OpenPGP digital signature

linux-next: manual merge of the kvm-arm tree with the arm64 tree

[Stephen Rothwell]

Hi all,

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

  arch/arm64/kernel/cpufeature.c

between commits:

  143ba05d867a ("arm64: capabilities: Prepare for fine grained capabilities")
  12eb369125ab ("arm64: cpufeature: Avoid warnings due to unused symbols")

from the arm64 tree and commit:

  a1efdff442ec ("arm64: cpufeatures: Drop the ARM64_HYP_OFFSET_LOW feature 
flag")

from the kvm-arm tree.

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

-- 
Cheers,
Stephen Rothwell

diff --cc arch/arm64/kernel/cpufeature.c
index 96b15d7b10a8,5b25d56bccfd..
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@@ -838,19 -826,11 +838,6 @@@ static bool has_no_hw_prefetch(const st
MIDR_CPU_VAR_REV(1, MIDR_REVISION_MASK));
  }
  
- static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry,
-  int __unused)
 -static bool runs_at_el2(const struct arm64_cpu_capabilities *entry, int 
__unused)
--{
-   phys_addr_t idmap_addr = __pa_symbol(__hyp_idmap_text_start);
- 
-   /*
-* Activate the lower HYP offset only if:
-* - the idmap doesn't clash with it,
-* - the kernel is not running at EL2.
-*/
-   return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode();
 -  return is_kernel_in_hyp_mode();
--}
--
  static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int 
__unused)
  {
u64 pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);


pgpOYu3LqTBn5.pgp
Description: OpenPGP digital signature

linux-next: manual merge of the kvm-arm tree with the arm64 tree

[Stephen Rothwell]

Hi all,

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

  arch/arm64/kernel/cpu_errata.c

between commit:

  c0cda3b8ee6b ("arm64: capabilities: Update prototype for enable call back")
  followed by a series of patches cleaning up capabilities

from the arm64 tree and commits:

  4b472ffd1513 ("arm64: Enable ARM64_HARDEN_EL2_VECTORS on Cortex-A57 and A72")
  f9f5dc19509b ("arm64: KVM: Use SMCCC_ARCH_WORKAROUND_1 for Falkor BP 
hardening")

from the kvm-arm tree.

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

-- 
Cheers,
Stephen Rothwell

diff --cc arch/arm64/kernel/cpu_errata.c
index 2df792771053,caa73af7d26e..
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@@ -76,8 -57,11 +76,10 @@@ cpu_enable_trap_ctr_access(const struc
  {
/* Clear SCTLR_EL1.UCT */
config_sctlr_el1(SCTLR_EL1_UCT, 0);
 -  return 0;
  }
  
+ atomic_t arm64_el2_vector_last_slot = ATOMIC_INIT(-1);
+ 
  #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
  #include 
  #include 
@@@ -179,18 -156,31 +174,31 @@@ static void call_hvc_arch_workaround_1(
arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
  }
  
+ static void qcom_link_stack_sanitization(void)
+ {
+   u64 tmp;
+ 
+   asm volatile("mov   %0, x30 \n"
+".rept 16  \n"
+"bl. + 4   \n"
+".endr \n"
+"mov   x30, %0 \n"
+: "=" (tmp));
+ }
+ 
 -static int enable_smccc_arch_workaround_1(void *data)
 +static void
 +enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
  {
 -  const struct arm64_cpu_capabilities *entry = data;
bp_hardening_cb_t cb;
void *smccc_start, *smccc_end;
struct arm_smccc_res res;
+   u32 midr = read_cpuid_id();
  
if (!entry->matches(entry, SCOPE_LOCAL_CPU))
 -  return 0;
 +  return;
  
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
 -  return 0;
 +  return;
  
switch (psci_ops.conduit) {
case PSCI_CONDUIT_HVC:
@@@ -214,139 -204,33 +222,124 @@@
break;
  
default:
 -  return 0;
 +  return;
}
  
+   if (((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR) ||
+   ((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR_V1))
+   cb = qcom_link_stack_sanitization;
+ 
install_bp_hardening_cb(entry, cb, smccc_start, smccc_end);
  
 -  return 0;
 +  return;
  }
  
- static void qcom_link_stack_sanitization(void)
- {
-   u64 tmp;
- 
-   asm volatile("mov   %0, x30 \n"
-".rept 16  \n"
-"bl. + 4   \n"
-".endr \n"
-"mov   x30, %0 \n"
-: "=" (tmp));
- }
- 
- static void
- qcom_enable_link_stack_sanitization(const struct arm64_cpu_capabilities 
*entry)
- {
-   install_bp_hardening_cb(entry, qcom_link_stack_sanitization,
-   __qcom_hyp_sanitize_link_stack_start,
-   __qcom_hyp_sanitize_link_stack_end);
- }
  #endif/* CONFIG_HARDEN_BRANCH_PREDICTOR */
  
 -#define MIDR_RANGE(model, min, max) \
 -  .def_scope = SCOPE_LOCAL_CPU, \
 -  .matches = is_affected_midr_range, \
 -  .midr_model = model, \
 -  .midr_range_min = min, \
 -  .midr_range_max = max
 +#define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max) \
 +  .matches = is_affected_midr_range,  \
 +  .midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
 +
 +#define CAP_MIDR_ALL_VERSIONS(model)  \
 +  .matches = is_affected_midr_range,  \
 +  .midr_range = MIDR_ALL_VERSIONS(model)
 +
 +#define MIDR_FIXED(rev, revidr_mask) \
 +  .fixed_revs = (struct arm64_midr_revidr[]){{ (rev), (revidr_mask) }, {}}
 +
 +#define ERRATA_MIDR_RANGE(model, v_min, r_min, v_max, r_max)  \
 +  .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, \
 +  CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)
 +
 +#define CAP_MIDR_RANGE_LIST(list) \
 +  .matches = is_affected_midr_range_list, \
 +  .midr_range_list = list
 +
 +/* Errata affecting a range of revisions of  given model variant */
 +#define ERRATA_MIDR_REV_RANGE(m, var, r_min, r_max)\
 +  ERRATA_MIDR_RANGE(m, var, 

linux-next: manual merge of the kvm-arm tree with the arm64 tree

[Stephen Rothwell]

Hi all,

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

  arch/arm64/kernel/cpu_errata.c

between commit:

  c0cda3b8ee6b ("arm64: capabilities: Update prototype for enable call back")
  followed by a series of patches cleaning up capabilities

from the arm64 tree and commits:

  4b472ffd1513 ("arm64: Enable ARM64_HARDEN_EL2_VECTORS on Cortex-A57 and A72")
  f9f5dc19509b ("arm64: KVM: Use SMCCC_ARCH_WORKAROUND_1 for Falkor BP 
hardening")

from the kvm-arm tree.

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

-- 
Cheers,
Stephen Rothwell

diff --cc arch/arm64/kernel/cpu_errata.c
index 2df792771053,caa73af7d26e..
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@@ -76,8 -57,11 +76,10 @@@ cpu_enable_trap_ctr_access(const struc
  {
/* Clear SCTLR_EL1.UCT */
config_sctlr_el1(SCTLR_EL1_UCT, 0);
 -  return 0;
  }
  
+ atomic_t arm64_el2_vector_last_slot = ATOMIC_INIT(-1);
+ 
  #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
  #include 
  #include 
@@@ -179,18 -156,31 +174,31 @@@ static void call_hvc_arch_workaround_1(
arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
  }
  
+ static void qcom_link_stack_sanitization(void)
+ {
+   u64 tmp;
+ 
+   asm volatile("mov   %0, x30 \n"
+".rept 16  \n"
+"bl. + 4   \n"
+".endr \n"
+"mov   x30, %0 \n"
+: "=" (tmp));
+ }
+ 
 -static int enable_smccc_arch_workaround_1(void *data)
 +static void
 +enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
  {
 -  const struct arm64_cpu_capabilities *entry = data;
bp_hardening_cb_t cb;
void *smccc_start, *smccc_end;
struct arm_smccc_res res;
+   u32 midr = read_cpuid_id();
  
if (!entry->matches(entry, SCOPE_LOCAL_CPU))
 -  return 0;
 +  return;
  
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
 -  return 0;
 +  return;
  
switch (psci_ops.conduit) {
case PSCI_CONDUIT_HVC:
@@@ -214,139 -204,33 +222,124 @@@
break;
  
default:
 -  return 0;
 +  return;
}
  
+   if (((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR) ||
+   ((midr & MIDR_CPU_MODEL_MASK) == MIDR_QCOM_FALKOR_V1))
+   cb = qcom_link_stack_sanitization;
+ 
install_bp_hardening_cb(entry, cb, smccc_start, smccc_end);
  
 -  return 0;
 +  return;
  }
  
- static void qcom_link_stack_sanitization(void)
- {
-   u64 tmp;
- 
-   asm volatile("mov   %0, x30 \n"
-".rept 16  \n"
-"bl. + 4   \n"
-".endr \n"
-"mov   x30, %0 \n"
-: "=" (tmp));
- }
- 
- static void
- qcom_enable_link_stack_sanitization(const struct arm64_cpu_capabilities 
*entry)
- {
-   install_bp_hardening_cb(entry, qcom_link_stack_sanitization,
-   __qcom_hyp_sanitize_link_stack_start,
-   __qcom_hyp_sanitize_link_stack_end);
- }
  #endif/* CONFIG_HARDEN_BRANCH_PREDICTOR */
  
 -#define MIDR_RANGE(model, min, max) \
 -  .def_scope = SCOPE_LOCAL_CPU, \
 -  .matches = is_affected_midr_range, \
 -  .midr_model = model, \
 -  .midr_range_min = min, \
 -  .midr_range_max = max
 +#define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max) \
 +  .matches = is_affected_midr_range,  \
 +  .midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
 +
 +#define CAP_MIDR_ALL_VERSIONS(model)  \
 +  .matches = is_affected_midr_range,  \
 +  .midr_range = MIDR_ALL_VERSIONS(model)
 +
 +#define MIDR_FIXED(rev, revidr_mask) \
 +  .fixed_revs = (struct arm64_midr_revidr[]){{ (rev), (revidr_mask) }, {}}
 +
 +#define ERRATA_MIDR_RANGE(model, v_min, r_min, v_max, r_max)  \
 +  .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, \
 +  CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)
 +
 +#define CAP_MIDR_RANGE_LIST(list) \
 +  .matches = is_affected_midr_range_list, \
 +  .midr_range_list = list
 +
 +/* Errata affecting a range of revisions of  given model variant */
 +#define ERRATA_MIDR_REV_RANGE(m, var, r_min, r_max)\
 +  ERRATA_MIDR_RANGE(m, var, 

Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.

[Rob Landley]

On 03/23/2018 02:06 PM, Matthew Wilcox wrote:
> On Fri, Mar 23, 2018 at 02:00:24PM -0400, Rich Felker wrote:
>> On Fri, Mar 23, 2018 at 05:48:06AM -0700, Matthew Wilcox wrote:
>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
 Current implementation doesn't randomize address returned by mmap.
 All the entropy ends with choosing mmap_base_addr at the process
 creation. After that mmap build very predictable layout of address
 space. It allows to bypass ASLR in many cases. This patch make
 randomization of address on any mmap call.
>>>
>>> Why should this be done in the kernel rather than libc?  libc is perfectly
>>> capable of specifying random numbers in the first argument of mmap.
>>
>> Generally libc does not have a view of the current vm maps, and thus
>> in passing "random numbers", they would have to be uniform across the
>> whole vm space and thus non-uniform once the kernel rounds up to avoid
>> existing mappings.
> 
> I'm aware that you're the musl author, but glibc somehow manages to
> provide etext, edata and end, demonstrating that it does know where at
> least some of the memory map lies.

You can parse /proc/self/maps, but it's really expensive and disgusting.

Rob

Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.

[Rob Landley]

On 03/23/2018 02:06 PM, Matthew Wilcox wrote:
> On Fri, Mar 23, 2018 at 02:00:24PM -0400, Rich Felker wrote:
>> On Fri, Mar 23, 2018 at 05:48:06AM -0700, Matthew Wilcox wrote:
>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
 Current implementation doesn't randomize address returned by mmap.
 All the entropy ends with choosing mmap_base_addr at the process
 creation. After that mmap build very predictable layout of address
 space. It allows to bypass ASLR in many cases. This patch make
 randomization of address on any mmap call.
>>>
>>> Why should this be done in the kernel rather than libc?  libc is perfectly
>>> capable of specifying random numbers in the first argument of mmap.
>>
>> Generally libc does not have a view of the current vm maps, and thus
>> in passing "random numbers", they would have to be uniform across the
>> whole vm space and thus non-uniform once the kernel rounds up to avoid
>> existing mappings.
> 
> I'm aware that you're the musl author, but glibc somehow manages to
> provide etext, edata and end, demonstrating that it does know where at
> least some of the memory map lies.

You can parse /proc/self/maps, but it's really expensive and disgusting.

Rob

Re: [PATCH] proc: register filesystem last

[Al Viro]

On Sat, Mar 10, 2018 at 03:06:34AM +0300, Alexey Dobriyan wrote:
> On Fri, Mar 09, 2018 at 02:49:38PM -0800, Andrew Morton wrote:
> > On Sat, 10 Mar 2018 01:27:09 +0300 Alexey Dobriyan  
> > wrote:
> > 
> > > As soon as register_filesystem() exits, filesystem can be mounted.
> > > It is better to present fully operational /proc.
> > > 
> > > Of course it doesn't matter because /proc is not modular
> > > but do it anyway.
> > > 
> > > Drop error check, it should be handled by panicking.
> > 
> > So... shouldn't we add a call to panic()?
> 
> via FS_PANIC flag, yes. I have a patch somewhere.
> There are 104 filesystems ATM, some internal, some not.
> Some modular, some not.

You do realize that the only case when register_filesystem() fails is
"another driver has already registered filesystem type with the same
name"?  Is there *ever* a case when
* you could expect that to happen and
* panic would be a sane response?

Re: [PATCH] proc: register filesystem last

[Al Viro]

On Sat, Mar 10, 2018 at 03:06:34AM +0300, Alexey Dobriyan wrote:
> On Fri, Mar 09, 2018 at 02:49:38PM -0800, Andrew Morton wrote:
> > On Sat, 10 Mar 2018 01:27:09 +0300 Alexey Dobriyan  
> > wrote:
> > 
> > > As soon as register_filesystem() exits, filesystem can be mounted.
> > > It is better to present fully operational /proc.
> > > 
> > > Of course it doesn't matter because /proc is not modular
> > > but do it anyway.
> > > 
> > > Drop error check, it should be handled by panicking.
> > 
> > So... shouldn't we add a call to panic()?
> 
> via FS_PANIC flag, yes. I have a patch somewhere.
> There are 104 filesystems ATM, some internal, some not.
> Some modular, some not.

You do realize that the only case when register_filesystem() fails is
"another driver has already registered filesystem type with the same
name"?  Is there *ever* a case when
* you could expect that to happen and
* panic would be a sane response?

Re: [PATCH v6 05/21] tracing: probeevent: Cleanup print argument functions

[Masami Hiramatsu]

On Mon, 26 Mar 2018 13:28:32 -0400
Steven Rostedt  wrote:

> On Mon, 26 Mar 2018 13:17:33 +0900
> Masami Hiramatsu  wrote:
> 
> > > Masami Hiramatsu  wrote:
> > >   
> > > > Current print argument functions prints the argument
> > > > name too. It is not good for printing out multiple
> > > > values for one argument. This change it to just print
> > > > out the value.  
> > > 
> 
> 
> > 
> > Cleanup argument-printing functions to decouple it into name-printing and
> > value-printing, so that it can support more flexible argument expression,
> > like array type.
> 
> Ah, so this is a change to make it possible in the next patches to do
> something more? I'll update the change log to your above.

Yes, I'll update the patch description :)

Thanks!

> 
> That makes more sense. Thanks.
> 
> -- Steve
> 


-- 
Masami Hiramatsu 

Re: [PATCH v6 05/21] tracing: probeevent: Cleanup print argument functions

[Masami Hiramatsu]

On Mon, 26 Mar 2018 13:28:32 -0400
Steven Rostedt  wrote:

> On Mon, 26 Mar 2018 13:17:33 +0900
> Masami Hiramatsu  wrote:
> 
> > > Masami Hiramatsu  wrote:
> > >   
> > > > Current print argument functions prints the argument
> > > > name too. It is not good for printing out multiple
> > > > values for one argument. This change it to just print
> > > > out the value.  
> > > 
> 
> 
> > 
> > Cleanup argument-printing functions to decouple it into name-printing and
> > value-printing, so that it can support more flexible argument expression,
> > like array type.
> 
> Ah, so this is a change to make it possible in the next patches to do
> something more? I'll update the change log to your above.

Yes, I'll update the patch description :)

Thanks!

> 
> That makes more sense. Thanks.
> 
> -- Steve
> 


-- 
Masami Hiramatsu 

Re: [PATCH] vhost-net: add time limitation for tx polling(Internet mail)

[张海斌]

On 2018年03月27日 19:26, Jason wrote
On 2018年03月27日 17:12, haibinzhang wrote:
>> handle_tx() will delay rx for a long time when busy tx polling udp packets
>> with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT
>> takes into account only sent-bytes but no time.
>
>Interesting.
>
>Looking at vhost_can_busy_poll() it tries to poke pending vhost work and 
>exit the busy loop if it found one. So I believe something block the 
>work queuing. E.g did reverting 8241a1e466cd56e6c10472cac9c1ad4e54bc65db 
>fix the issue?

"busy tx polling" means using netperf send udp packets with 1 bytes 
payload(total 47bytes frame lenght), 
and handle_tx() will be busy sending packets continuously.

>
>>   It's not fair for handle_rx(),
>> so needs to limit max time of tx polling.
>>
>> ---
>>   drivers/vhost/net.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 8139bc70ad7d..dc9218a3a75b 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -473,6 +473,7 @@ static void handle_tx(struct vhost_net *net)
>>  struct socket *sock;
>>  struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
>>  bool zcopy, zcopy_used;
>> +unsigned long start = jiffies;
>
>Checking jiffies is tricky, need to convert it to ms or whatever others.
>
>>   
>>  mutex_lock(>mutex);
>>  sock = vq->private_data;
>> @@ -580,7 +581,7 @@ static void handle_tx(struct vhost_net *net)
>>  else
>>  vhost_zerocopy_signal_used(net, vq);
>>  vhost_net_tx_packet(net);
>> -if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
>> +if (unlikely(total_len >= VHOST_NET_WEIGHT) || unlikely(jiffies 
>> - start >= 1)) {
>
>How value 1 is determined here? And we need a complete test to make sure 
>this won't affect other use cases.

We just want <1ms ping latency, but actually we are not sure what value is 
reasonable.
We have some test results using netperf before this patch as follow,

Udp payload1byte100bytes1000bytes1400bytes
  Ping avg latency25ms 10ms   2ms 1.5ms

What is other testcases?

>
>Another thought is introduce another limit of #packets, but this need 
>benchmark too.
>
>Thanks
>
>>  vhost_poll_queue(>poll);
>>  break;
>>  }
>
>

Re: [PATCH] vhost-net: add time limitation for tx polling(Internet mail)

[张海斌]

On 2018年03月27日 19:26, Jason wrote
On 2018年03月27日 17:12, haibinzhang wrote:
>> handle_tx() will delay rx for a long time when busy tx polling udp packets
>> with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT
>> takes into account only sent-bytes but no time.
>
>Interesting.
>
>Looking at vhost_can_busy_poll() it tries to poke pending vhost work and 
>exit the busy loop if it found one. So I believe something block the 
>work queuing. E.g did reverting 8241a1e466cd56e6c10472cac9c1ad4e54bc65db 
>fix the issue?

"busy tx polling" means using netperf send udp packets with 1 bytes 
payload(total 47bytes frame lenght), 
and handle_tx() will be busy sending packets continuously.

>
>>   It's not fair for handle_rx(),
>> so needs to limit max time of tx polling.
>>
>> ---
>>   drivers/vhost/net.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 8139bc70ad7d..dc9218a3a75b 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -473,6 +473,7 @@ static void handle_tx(struct vhost_net *net)
>>  struct socket *sock;
>>  struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
>>  bool zcopy, zcopy_used;
>> +unsigned long start = jiffies;
>
>Checking jiffies is tricky, need to convert it to ms or whatever others.
>
>>   
>>  mutex_lock(>mutex);
>>  sock = vq->private_data;
>> @@ -580,7 +581,7 @@ static void handle_tx(struct vhost_net *net)
>>  else
>>  vhost_zerocopy_signal_used(net, vq);
>>  vhost_net_tx_packet(net);
>> -if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
>> +if (unlikely(total_len >= VHOST_NET_WEIGHT) || unlikely(jiffies 
>> - start >= 1)) {
>
>How value 1 is determined here? And we need a complete test to make sure 
>this won't affect other use cases.

We just want <1ms ping latency, but actually we are not sure what value is 
reasonable.
We have some test results using netperf before this patch as follow,

Udp payload1byte100bytes1000bytes1400bytes
  Ping avg latency25ms 10ms   2ms 1.5ms

What is other testcases?

>
>Another thought is introduce another limit of #packets, but this need 
>benchmark too.
>
>Thanks
>
>>  vhost_poll_queue(>poll);
>>  break;
>>  }
>
>

Regression: 4.14.x x86/mm: Encrypt the initrd earlier for BSP microcode update

[Steven Haigh]

Hi all,

After the following patch, we are unable to build the 4.14 tree with EL6 - 
which uses gcc 4.4.7

The failure logs are as follows:
BUILDSTDERR: arch/x86/kernel/head64.c: In function 'copy_bootdata':
BUILDSTDERR: ./arch/x86/include/asm/mem_encrypt.h:59: sorry, unimplemented: 
inlining failed in call to 'sme_map_bootdata': function not inlinable
BUILDSTDERR: arch/x86/kernel/head64.c:294: sorry, unimplemented: called from 
here
BUILDSTDERR: ./arch/x86/include/asm/mem_encrypt.h:60: sorry, unimplemented: 
inlining failed in call to 'sme_unmap_bootdata': function not inlinable
BUILDSTDERR: arch/x86/kernel/head64.c:310: sorry, unimplemented: called from 
here

Patch in question:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/
commit/arch/x86/include/asm/mem_encrypt.h?
h=linux-4.14.y=e0f82cbde41353ddf63935175583ceb5580cce87

I can confirm that this builds fine with EL7.

-- 
Steven Haigh

 net...@crc.id.au    https://www.crc.id.au
 +61 (3) 9001 6090 0412 935 897

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

Regression: 4.14.x x86/mm: Encrypt the initrd earlier for BSP microcode update

[Steven Haigh]

Hi all,

After the following patch, we are unable to build the 4.14 tree with EL6 - 
which uses gcc 4.4.7

The failure logs are as follows:
BUILDSTDERR: arch/x86/kernel/head64.c: In function 'copy_bootdata':
BUILDSTDERR: ./arch/x86/include/asm/mem_encrypt.h:59: sorry, unimplemented: 
inlining failed in call to 'sme_map_bootdata': function not inlinable
BUILDSTDERR: arch/x86/kernel/head64.c:294: sorry, unimplemented: called from 
here
BUILDSTDERR: ./arch/x86/include/asm/mem_encrypt.h:60: sorry, unimplemented: 
inlining failed in call to 'sme_unmap_bootdata': function not inlinable
BUILDSTDERR: arch/x86/kernel/head64.c:310: sorry, unimplemented: called from 
here

Patch in question:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/
commit/arch/x86/include/asm/mem_encrypt.h?
h=linux-4.14.y=e0f82cbde41353ddf63935175583ceb5580cce87

I can confirm that this builds fine with EL7.

-- 
Steven Haigh

 net...@crc.id.au    https://www.crc.id.au
 +61 (3) 9001 6090 0412 935 897

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

Re: [PATCH 17/19] csky: defconfig

[Guo Ren]

On Tue, Mar 27, 2018 at 09:48:23AM +0200, Arnd Bergmann wrote:
> On Tue, Mar 27, 2018 at 4:21 AM, Guo Ren  wrote:
> > On Mon, Mar 26, 2018 at 03:16:31PM +0200, Arnd Bergmann wrote:
> 
> >> > +CONFIG_CFG80211=y
> >> > +CONFIG_CFG80211_DEBUGFS=y
> >> > +CONFIG_CFG80211_WEXT=y
> >>
> >> I would guess you want IPV6 but not WEXT here.
> > Gx6605s devlepment board support usb-wifi.
> > Perhaps WEXT is needed by iwconfig or iwlist? So I just enable it.
> > However, I'll consider to remove it.
> 
> I think it's only for backwards compatibility with old versions of
> those tools. There are some older drivers that use WEXT as the
> native interface, but CFG80211 based drivers have a netlink
> interface.
Thx for remind. I'll check.

> 
> >> > +CONFIG_FB=y
> >> > +CONFIG_FB_TILEBLITTING=y
> >> > +CONFIG_FB_SIMPLE=y
> >> > +CONFIG_BACKLIGHT_LCD_SUPPORT=y
> >> > +# CONFIG_LCD_CLASS_DEVICE is not set
> >> > +# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
> >> > +# CONFIG_VGA_CONSOLE is not set
> >> > +CONFIG_FRAMEBUFFER_CONSOLE=y
> >> > +CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> >> > +CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
> >> > +CONFIG_LOGO=y
> >> > +# CONFIG_LOGO_LINUX_MONO is not set
> >> > +# CONFIG_LOGO_LINUX_VGA16 is not set
> >>
> >> For new platforms, using the DRM subsystem is the recommend
> >> way to do graphics, it mostly replaces the framebuffer subsystem
> >> here.
> > These used by:
> > https://github.com/c-sky/addons-linux/tree/master/addons/drivers/video/fbdev/nationalchip
> >
> > I'll consider the DRM subsystem.
> 
> It used to be rather hard to write a new DRM driver, but that has
> improved a lot recently. You might be able to fit your driver into
> drivers/gpu/drm/tinydrm/, or maybe do something a bit more complex
> like drivers/gpu/drm/pl111/ or drivers/gpu/drm/stm/.
For this patchset, I won't upstream the fbdev driver. I'll remove the
CONFIG_FB in defconfig first.

Thx for remind about the develop-tips, and it's very helpful.

Best Regards
 Guo Ren

Re: [PATCH 17/19] csky: defconfig

[Guo Ren]

On Tue, Mar 27, 2018 at 09:48:23AM +0200, Arnd Bergmann wrote:
> On Tue, Mar 27, 2018 at 4:21 AM, Guo Ren  wrote:
> > On Mon, Mar 26, 2018 at 03:16:31PM +0200, Arnd Bergmann wrote:
> 
> >> > +CONFIG_CFG80211=y
> >> > +CONFIG_CFG80211_DEBUGFS=y
> >> > +CONFIG_CFG80211_WEXT=y
> >>
> >> I would guess you want IPV6 but not WEXT here.
> > Gx6605s devlepment board support usb-wifi.
> > Perhaps WEXT is needed by iwconfig or iwlist? So I just enable it.
> > However, I'll consider to remove it.
> 
> I think it's only for backwards compatibility with old versions of
> those tools. There are some older drivers that use WEXT as the
> native interface, but CFG80211 based drivers have a netlink
> interface.
Thx for remind. I'll check.

> 
> >> > +CONFIG_FB=y
> >> > +CONFIG_FB_TILEBLITTING=y
> >> > +CONFIG_FB_SIMPLE=y
> >> > +CONFIG_BACKLIGHT_LCD_SUPPORT=y
> >> > +# CONFIG_LCD_CLASS_DEVICE is not set
> >> > +# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
> >> > +# CONFIG_VGA_CONSOLE is not set
> >> > +CONFIG_FRAMEBUFFER_CONSOLE=y
> >> > +CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> >> > +CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
> >> > +CONFIG_LOGO=y
> >> > +# CONFIG_LOGO_LINUX_MONO is not set
> >> > +# CONFIG_LOGO_LINUX_VGA16 is not set
> >>
> >> For new platforms, using the DRM subsystem is the recommend
> >> way to do graphics, it mostly replaces the framebuffer subsystem
> >> here.
> > These used by:
> > https://github.com/c-sky/addons-linux/tree/master/addons/drivers/video/fbdev/nationalchip
> >
> > I'll consider the DRM subsystem.
> 
> It used to be rather hard to write a new DRM driver, but that has
> improved a lot recently. You might be able to fit your driver into
> drivers/gpu/drm/tinydrm/, or maybe do something a bit more complex
> like drivers/gpu/drm/pl111/ or drivers/gpu/drm/stm/.
For this patchset, I won't upstream the fbdev driver. I'll remove the
CONFIG_FB in defconfig first.

Thx for remind about the develop-tips, and it's very helpful.

Best Regards
 Guo Ren

Re: [PATCH 1/4] gpio: Remove VLA from gpiolib

[Lukas Wunner]

On Tue, Mar 27, 2018 at 05:37:18PM -0700, Laura Abbott wrote:
> On 03/18/2018 07:23 AM, Lukas Wunner wrote:
> > Actually, scratch that.  If ngpio is usually smallish, we can just
> > allocate reasonably sized space for mask and bits on the stack,
> > and fall back to the kcalloc slowpath only if chip->ngpio exceeds
> > that limit.  Basically the below (likewise compile-tested only),
> > this is on top of Laura's patch, could be squashed together.
> > Let me know what you think, thanks.
> >
> 
> It seems like there's general consensus this is okay so I'm going
> to fold it into the next version. If not, we can discuss again.

Yes, feel free to squash into your original patch with my S-o-b,
keep your authorship.

You may want to raise FASTPATH_NGPIO to something like 384, 448 or 512
to accommodate for the Intel chips Andy mentioned.  It's kind of a
"640k should be enough for everyone" thing but I'd expect the performance
impact of the extra bytes on the stack / memsetting them to zero
to be absolutely negligible.

Thanks!

Lukas

Re: [PATCH 1/4] gpio: Remove VLA from gpiolib

[Lukas Wunner]

On Tue, Mar 27, 2018 at 05:37:18PM -0700, Laura Abbott wrote:
> On 03/18/2018 07:23 AM, Lukas Wunner wrote:
> > Actually, scratch that.  If ngpio is usually smallish, we can just
> > allocate reasonably sized space for mask and bits on the stack,
> > and fall back to the kcalloc slowpath only if chip->ngpio exceeds
> > that limit.  Basically the below (likewise compile-tested only),
> > this is on top of Laura's patch, could be squashed together.
> > Let me know what you think, thanks.
> >
> 
> It seems like there's general consensus this is okay so I'm going
> to fold it into the next version. If not, we can discuss again.

Yes, feel free to squash into your original patch with my S-o-b,
keep your authorship.

You may want to raise FASTPATH_NGPIO to something like 384, 448 or 512
to accommodate for the Intel chips Andy mentioned.  It's kind of a
"640k should be enough for everyone" thing but I'd expect the performance
impact of the extra bytes on the stack / memsetting them to zero
to be absolutely negligible.

Thanks!

Lukas

Re: [PATCH v1 02/16] dt-bindings: rtc: mediatek: add bindings for PMIC RTC

[Sean Wang]

On Tue, 2018-03-27 at 17:18 +0200, Alexandre Belloni wrote:
> On 25/03/2018 at 03:36:28 +0800, Sean Wang wrote:
> > just reply both replies in the same mail
> > 
> > 1.) the power-off device is a part of rtc, use the same registers rtc
> > has and thus it is put as child nodes under the node rtc to reflect the
> > reality of characteristics the rtc has.
> > 
> > Or am I wrong for a certain aspect in these opinions?
> > 
> 
> My point is that it is also part of the PMIC so it may as well be
> registers from the mfd driver which already registers a bunch of devices
> instead of doing unusual stuff from the rtc driver.
> 
> mt6397_rtc->regmap is mt6397_chip->regmap anyway. You have the added
> benefit that if the RTC driver probe fails for some reason, you may
> still be able to probe the reset driver.
> 
> I don't tink there is any benefit having it as a child of the rtc
> device.
> 


really thanks! it's an optional solution I thought it 's fine and worth
doing

but so far I cannot fully make sure of whether mfd can accept two
devices holding overlay IORESOURCE_MEM.

Or do you like Rob's suggestion in [1] ? By which, I tend to embed a
sub-device with platform_device_register_data api in the rtc probe()
instead of treating it as a dt node under rtc node, but which seems
something a bit violates your preferences :(

Just confirm to know which way I should step into before I produce next
version.

[1]
http://lists.infradead.org/pipermail/linux-mediatek/2018-March/012576.html

> > 2) the other sub-functions for the same pmic already created its own
> > dt-binding document belonged to its corresponding subsystem. Don't we
> > really want to follow it them all?
> > 
> 
> Ok, that's fine.
> 

Re: [PATCH v1 02/16] dt-bindings: rtc: mediatek: add bindings for PMIC RTC

[Sean Wang]

On Tue, 2018-03-27 at 17:18 +0200, Alexandre Belloni wrote:
> On 25/03/2018 at 03:36:28 +0800, Sean Wang wrote:
> > just reply both replies in the same mail
> > 
> > 1.) the power-off device is a part of rtc, use the same registers rtc
> > has and thus it is put as child nodes under the node rtc to reflect the
> > reality of characteristics the rtc has.
> > 
> > Or am I wrong for a certain aspect in these opinions?
> > 
> 
> My point is that it is also part of the PMIC so it may as well be
> registers from the mfd driver which already registers a bunch of devices
> instead of doing unusual stuff from the rtc driver.
> 
> mt6397_rtc->regmap is mt6397_chip->regmap anyway. You have the added
> benefit that if the RTC driver probe fails for some reason, you may
> still be able to probe the reset driver.
> 
> I don't tink there is any benefit having it as a child of the rtc
> device.
> 


really thanks! it's an optional solution I thought it 's fine and worth
doing

but so far I cannot fully make sure of whether mfd can accept two
devices holding overlay IORESOURCE_MEM.

Or do you like Rob's suggestion in [1] ? By which, I tend to embed a
sub-device with platform_device_register_data api in the rtc probe()
instead of treating it as a dt node under rtc node, but which seems
something a bit violates your preferences :(

Just confirm to know which way I should step into before I produce next
version.

[1]
http://lists.infradead.org/pipermail/linux-mediatek/2018-March/012576.html

> > 2) the other sub-functions for the same pmic already created its own
> > dt-binding document belonged to its corresponding subsystem. Don't we
> > really want to follow it them all?
> > 
> 
> Ok, that's fine.
> 

Re: [PATCH 15/19] csky: Build infrastructure

[Guo Ren]

Hi Arnd,

On Tue, Mar 27, 2018 at 09:38:56AM +0200, Arnd Bergmann wrote:
> Usually the way gcc handles this, either each CPU is a strict superset
> of another
> one, so you just need to specify the one with the smallest instruction set,
> or you have an option like -mcpu=generic that produces the common subset.
>
ck807 ck810 ck860 are diffrent architecture, so they can not be strict
superset and there is no option like '-mcpu=generic' in our gcc and
binutils.

I know you want one vmlinux which could run on all ck807 ck810 ck860 with
different dts-setting. But now, may I keep current design for abiv1?

In abiv3, we will take your advice seriously.

Best Regards
 Guo Ren

Re: [PATCH 15/19] csky: Build infrastructure

[Guo Ren]

Hi Arnd,

On Tue, Mar 27, 2018 at 09:38:56AM +0200, Arnd Bergmann wrote:
> Usually the way gcc handles this, either each CPU is a strict superset
> of another
> one, so you just need to specify the one with the smallest instruction set,
> or you have an option like -mcpu=generic that produces the common subset.
>
ck807 ck810 ck860 are diffrent architecture, so they can not be strict
superset and there is no option like '-mcpu=generic' in our gcc and
binutils.

I know you want one vmlinux which could run on all ck807 ck810 ck860 with
different dts-setting. But now, may I keep current design for abiv1?

In abiv3, we will take your advice seriously.

Best Regards
 Guo Ren

Re: [PATCH v2 10/21] kconfig: add 'success' and 'cc-option' macros

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> 'cc-option' will be the most frequently used macro.  It evaluates to 'y'
> if the given argument is supported by the compiler, or 'n' otherwise.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

> ---
>
> Changes in v2:
>   - Implement 'success' as a macro
>
>  init/Kconfig | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0d6fe20..c456030 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -8,6 +8,12 @@ config DEFCONFIG_LIST
> default ARCH_DEFCONFIG
> default "arch/$ARCH/defconfig"
>
> +# expanded to y if the command exits with 0, n otherwise
> +macro success $(shell ($(1) && echo y) || echo n)
> +
> +# expanded to y if the given compiler flag is supported, n otherwise
> +macro cc-option $(success $CC -Werror $(1) -c -x c /dev/null -o /dev/null)

This should be fine since most cc-option uses are quite simple. The
specialized per-arch and other-flag-dependency checks can just
continue to live in Makefile if we actually need them.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 10/21] kconfig: add 'success' and 'cc-option' macros

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> 'cc-option' will be the most frequently used macro.  It evaluates to 'y'
> if the given argument is supported by the compiler, or 'n' otherwise.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

> ---
>
> Changes in v2:
>   - Implement 'success' as a macro
>
>  init/Kconfig | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0d6fe20..c456030 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -8,6 +8,12 @@ config DEFCONFIG_LIST
> default ARCH_DEFCONFIG
> default "arch/$ARCH/defconfig"
>
> +# expanded to y if the command exits with 0, n otherwise
> +macro success $(shell ($(1) && echo y) || echo n)
> +
> +# expanded to y if the given compiler flag is supported, n otherwise
> +macro cc-option $(success $CC -Werror $(1) -c -x c /dev/null -o /dev/null)

This should be fine since most cc-option uses are quite simple. The
specialized per-arch and other-flag-dependency checks can just
continue to live in Makefile if we actually need them.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 09/21] kconfig: add 'macro' keyword to support user-defined function

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now, we got a basic ability to test compiler capability in Kconfig.
>
> config CC_HAS_STACKPROTECTOR
> def_bool $(shell (($CC -Werror -fstack-protector -c -x c /dev/null -o 
> /dev/null) && echo y) || echo n)
>
> This works, but it is ugly to repeat this long boilerplate.
>
> We want to describe like this:
>
> config CC_HAS_STACKPROTECTOR
> bool
> default $(cc-option -fstack-protector)
>
> It is straight-forward to add a new function, but I do not like to
> hard-code specialized functions like this.  Hence, here is another
> feature to add functions from Kconfig files.
>
> A user-defined function is defined with a special keyword 'macro'.
> It can be referenced in the same way as built-in functions.  This
> feature was also inspired by Makefile where user-defined functions
> are referenced by $(call func-name, args...), but I omitted the 'call'
> to makes it shorter.
>
> The macro definition can contain $(1), $(2), ... which will be replaced
> with arguments from the caller.  The macro works just as a textual
> shorthand, which is also expanded in the lexer phase.
>
> [Example Code]
>
>   macro success $(shell ($(1) && echo y) || echo n)
>
>   config TRUE
>   bool "true"
>   default $(success true)
>
>   config FALSE
>   bool "false"
>   default $(success false)
>
> [Result]
>
>   $ make -s alldefconfig
>   $ tail -n 2 .config
>   CONFIG_TRUE=y
>   # CONFIG_FALSE is not set
>
> [Example Code]
>
>   macro success $(shell ($(1) && echo y) || echo n)
>
>   macro cc-option $(success $CC -Werror $(1) -c -x c /dev/null -o /dev/null)
>
>   config CC_HAS_STACKPROTECTOR
>   def_bool $(cc-option -fstack-protector)
>
> [Result]
>   $ make -s alldefconfig
>   $ tail -n 1 .config
>   CONFIG_CC_HAS_STACKPROTECTOR=y
>
> Signed-off-by: Masahiro Yamada 

Cool, I like this "macro" idea for keeping this clean.

> ---
>
> Reminder for myself:
> Update Documentation/kbuild/kconfig-language.txt

Yes please. :)

Otherwise, looks good!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 09/21] kconfig: add 'macro' keyword to support user-defined function

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now, we got a basic ability to test compiler capability in Kconfig.
>
> config CC_HAS_STACKPROTECTOR
> def_bool $(shell (($CC -Werror -fstack-protector -c -x c /dev/null -o 
> /dev/null) && echo y) || echo n)
>
> This works, but it is ugly to repeat this long boilerplate.
>
> We want to describe like this:
>
> config CC_HAS_STACKPROTECTOR
> bool
> default $(cc-option -fstack-protector)
>
> It is straight-forward to add a new function, but I do not like to
> hard-code specialized functions like this.  Hence, here is another
> feature to add functions from Kconfig files.
>
> A user-defined function is defined with a special keyword 'macro'.
> It can be referenced in the same way as built-in functions.  This
> feature was also inspired by Makefile where user-defined functions
> are referenced by $(call func-name, args...), but I omitted the 'call'
> to makes it shorter.
>
> The macro definition can contain $(1), $(2), ... which will be replaced
> with arguments from the caller.  The macro works just as a textual
> shorthand, which is also expanded in the lexer phase.
>
> [Example Code]
>
>   macro success $(shell ($(1) && echo y) || echo n)
>
>   config TRUE
>   bool "true"
>   default $(success true)
>
>   config FALSE
>   bool "false"
>   default $(success false)
>
> [Result]
>
>   $ make -s alldefconfig
>   $ tail -n 2 .config
>   CONFIG_TRUE=y
>   # CONFIG_FALSE is not set
>
> [Example Code]
>
>   macro success $(shell ($(1) && echo y) || echo n)
>
>   macro cc-option $(success $CC -Werror $(1) -c -x c /dev/null -o /dev/null)
>
>   config CC_HAS_STACKPROTECTOR
>   def_bool $(cc-option -fstack-protector)
>
> [Result]
>   $ make -s alldefconfig
>   $ tail -n 1 .config
>   CONFIG_CC_HAS_STACKPROTECTOR=y
>
> Signed-off-by: Masahiro Yamada 

Cool, I like this "macro" idea for keeping this clean.

> ---
>
> Reminder for myself:
> Update Documentation/kbuild/kconfig-language.txt

Yes please. :)

Otherwise, looks good!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 08/21] kconfig: replace $UNAME_RELEASE with function call

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now that 'shell' function is supported, this can be self-contained in
> Kconfig.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 08/21] kconfig: replace $UNAME_RELEASE with function call

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now that 'shell' function is supported, this can be self-contained in
> Kconfig.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 07/21] kconfig: add function support and implement 'shell' function

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> This commit adds a new concept 'function' to do more text processing
> in Kconfig.
>
> A function call looks like this:
>
>   $(function arg1, arg2, arg3, ...)
>
> (Actually, this syntax was inspired by make.)
>
> Real examples will look like this:
>
>   $(shell echo hello world)
>   $(cc-option -fstackprotector)
>
> This commit adds the basic infrastructure to add, delete, evaluate
> functions, and also the first built-in function $(shell ...).  This
> accepts a single command to execute.  It returns the standard output
> from it.
>
> [Example code]
>
>   config HELLO
>   string
>   default "$(shell echo hello world)"
>
>   config Y
>   def_bool $(shell echo y)
>
> [Result]
>
>   $ make -s alldefconfig && tail -n 2 .config
>   CONFIG_HELLO="hello world"
>   CONFIG_Y=y
>
> Caveat:
> Like environments, functions are expanded in the lexer.  You cannot
> pass symbols to function arguments.  This is a limitation to simplify
> the implementation.  I want to avoid the dynamic function evaluation,
> which would introduce much more complexity.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> Reminder for myself:
> Update Documentation/kbuild/kconfig-language.txt

Yeah, this needs to be included here, especially given the "cannot
pass symbols" aspect which might surprise people.

> [...]
> +/* built-in functions */
> +static char *do_shell(struct function *f, int argc, char *argv[])
> +{
> +   static const char *pre = "(";
> +   static const char *post = ") 2>/dev/null";

Right now the search and help screens in menuconfig just show the line
a config is defined and nothing more. I think it would be extremely
handy to include shell output here in some way. Especially when trying
to answer questions like "why aren't GCC plugins available?" it's got
quite a bit harder to debug.

Could we capture the output (especially stderr) for these kinds of hints?

Beyond that, looks good!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 07/21] kconfig: add function support and implement 'shell' function

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> This commit adds a new concept 'function' to do more text processing
> in Kconfig.
>
> A function call looks like this:
>
>   $(function arg1, arg2, arg3, ...)
>
> (Actually, this syntax was inspired by make.)
>
> Real examples will look like this:
>
>   $(shell echo hello world)
>   $(cc-option -fstackprotector)
>
> This commit adds the basic infrastructure to add, delete, evaluate
> functions, and also the first built-in function $(shell ...).  This
> accepts a single command to execute.  It returns the standard output
> from it.
>
> [Example code]
>
>   config HELLO
>   string
>   default "$(shell echo hello world)"
>
>   config Y
>   def_bool $(shell echo y)
>
> [Result]
>
>   $ make -s alldefconfig && tail -n 2 .config
>   CONFIG_HELLO="hello world"
>   CONFIG_Y=y
>
> Caveat:
> Like environments, functions are expanded in the lexer.  You cannot
> pass symbols to function arguments.  This is a limitation to simplify
> the implementation.  I want to avoid the dynamic function evaluation,
> which would introduce much more complexity.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> Reminder for myself:
> Update Documentation/kbuild/kconfig-language.txt

Yeah, this needs to be included here, especially given the "cannot
pass symbols" aspect which might surprise people.

> [...]
> +/* built-in functions */
> +static char *do_shell(struct function *f, int argc, char *argv[])
> +{
> +   static const char *pre = "(";
> +   static const char *post = ") 2>/dev/null";

Right now the search and help screens in menuconfig just show the line
a config is defined and nothing more. I think it would be extremely
handy to include shell output here in some way. Especially when trying
to answer questions like "why aren't GCC plugins available?" it's got
quite a bit harder to debug.

Could we capture the output (especially stderr) for these kinds of hints?

Beyond that, looks good!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 06/21] kconfig: remove string expansion for mainmenu after yyparse()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now that environments are expanded in the lexer, conf_parse() does
> not need to explicitly expand $ARCH and $KERNELVERSION in the mainmenu.
>
> The hack introduced by commit 0724a7c32a54 ("kconfig: Don't leak
> main menus during parsing") can go away.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 06/21] kconfig: remove string expansion for mainmenu after yyparse()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Now that environments are expanded in the lexer, conf_parse() does
> not need to explicitly expand $ARCH and $KERNELVERSION in the mainmenu.
>
> The hack introduced by commit 0724a7c32a54 ("kconfig: Don't leak
> main menus during parsing") can go away.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 04/21] kconfig: reference environments directly and remove 'option env=' syntax

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> To get an environment value, Kconfig needs to define a symbol using
> "option env=" syntax.  It is tedious to add a config entry for each
> environment given that we need more environments such as 'CC', 'AS',
> 'srctree' etc. to evaluate the compiler capability in Kconfig.
>
> Adding '$' to symbols is weird.  Kconfig can reference symbols directly
> like this:
>
>   config FOO
>   string
>   default BAR
>
> So, I want to use the following syntax to get environment 'BAR' from
> the system:
>
>   config FOO
>   string
>   default $BAR
>
> Looking at the code, the symbols prefixed with 'S' are expanded by:
>  - conf_expand_value()
>This is used to expand 'arch/$ARCH/defconfig' and 'defconfig_list'
>  - expand_string_value()
>This is used to expand strings in 'source' and 'mainmenu'
>
> All of them are fixed values independent of user configuration.  So,
> this kind of syntax should be moved to simply take the environment.
>
> This change makes the code much cleaner.  The bounce symbols 'SRCARCH',
> 'ARCH', 'SUBARCH', 'KERNELVERSION' are gone.
>
> sym_init() hard-coding 'UNAME_RELEASE' is also gone.  'UNAME_RELEASE'
> should be be given from the environment.
>
> ARCH_DEFCONFIG is a normal symbol, so it should be simply referenced
> by 'default ARCH_DEFCONFIG'.
>
> The environments are expanding in the lexer; when '$' is encountered,
> it is expanded, and resulted strings are pushed back to the input
> stream.  This makes the implementation simpler.
>
> For example, the following code works.
>
> [Example code]
>
>   config TOOLCHAIN_LIST
>   string
>   default "My tools: CC=$CC, AS=$AS, CPP=$CPP"
>
> [Result]
>
>   $ make -s alldefconfig && tail -n 1 .config
>   CONFIG_TOOLCHAIN_LIST="My tools: CC=gcc, AS=as, CPP=gcc -E"
>
> Signed-off-by: Masahiro Yamada 

For easier review this patch could maybe get split into a couple (i.e.
refactoring of things like env_write_dep()) but it's probably fine
as-is. :)

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 05/21] kconfig: remove string expansion in file_lookup()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> There are two callers of file_lookup().
>
> [1] zconf_initscan()
> This is used to open the first Kconfig.  However, it cannot
> contain environments in the file path because zconf_fopen() is
> called before file_lookup().  By swapping the call order,
> KBUILD_KCONFIG would be able to contain environments, but I do
> not see practical benefits to support it.
>
> [2] zconf_nextfile()
> This is used to open the next file from 'source' statement like
> source "arch/$SRCARCH/Kconfig"
> but this has already been expanded in the lexer phase.
>
> So, file_lookup() does not need to expand the given path.
>
> By the way, file_lookup() was already buggy; it expanded a given path,
> but it used the path before expansion for look-up:
> if (!strcmp(name, file->name)) {
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 04/21] kconfig: reference environments directly and remove 'option env=' syntax

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> To get an environment value, Kconfig needs to define a symbol using
> "option env=" syntax.  It is tedious to add a config entry for each
> environment given that we need more environments such as 'CC', 'AS',
> 'srctree' etc. to evaluate the compiler capability in Kconfig.
>
> Adding '$' to symbols is weird.  Kconfig can reference symbols directly
> like this:
>
>   config FOO
>   string
>   default BAR
>
> So, I want to use the following syntax to get environment 'BAR' from
> the system:
>
>   config FOO
>   string
>   default $BAR
>
> Looking at the code, the symbols prefixed with 'S' are expanded by:
>  - conf_expand_value()
>This is used to expand 'arch/$ARCH/defconfig' and 'defconfig_list'
>  - expand_string_value()
>This is used to expand strings in 'source' and 'mainmenu'
>
> All of them are fixed values independent of user configuration.  So,
> this kind of syntax should be moved to simply take the environment.
>
> This change makes the code much cleaner.  The bounce symbols 'SRCARCH',
> 'ARCH', 'SUBARCH', 'KERNELVERSION' are gone.
>
> sym_init() hard-coding 'UNAME_RELEASE' is also gone.  'UNAME_RELEASE'
> should be be given from the environment.
>
> ARCH_DEFCONFIG is a normal symbol, so it should be simply referenced
> by 'default ARCH_DEFCONFIG'.
>
> The environments are expanding in the lexer; when '$' is encountered,
> it is expanded, and resulted strings are pushed back to the input
> stream.  This makes the implementation simpler.
>
> For example, the following code works.
>
> [Example code]
>
>   config TOOLCHAIN_LIST
>   string
>   default "My tools: CC=$CC, AS=$AS, CPP=$CPP"
>
> [Result]
>
>   $ make -s alldefconfig && tail -n 1 .config
>   CONFIG_TOOLCHAIN_LIST="My tools: CC=gcc, AS=as, CPP=gcc -E"
>
> Signed-off-by: Masahiro Yamada 

For easier review this patch could maybe get split into a couple (i.e.
refactoring of things like env_write_dep()) but it's probably fine
as-is. :)

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 05/21] kconfig: remove string expansion in file_lookup()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> There are two callers of file_lookup().
>
> [1] zconf_initscan()
> This is used to open the first Kconfig.  However, it cannot
> contain environments in the file path because zconf_fopen() is
> called before file_lookup().  By swapping the call order,
> KBUILD_KCONFIG would be able to contain environments, but I do
> not see practical benefits to support it.
>
> [2] zconf_nextfile()
> This is used to open the next file from 'source' statement like
> source "arch/$SRCARCH/Kconfig"
> but this has already been expanded in the lexer phase.
>
> So, file_lookup() does not need to expand the given path.
>
> By the way, file_lookup() was already buggy; it expanded a given path,
> but it used the path before expansion for look-up:
> if (!strcmp(name, file->name)) {
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] xfs: always free inline data before resetting inode fork during ifree

[Dave Chinner]

On Mon, Mar 26, 2018 at 07:54:31PM -0400, Sasha Levin wrote:
> On Sun, Mar 25, 2018 at 6:33 PM, Dave Chinner  wrote:
> > On Fri, Mar 23, 2018 at 06:23:02PM +, Luis R. Rodriguez wrote:
> >> On Fri, Mar 23, 2018 at 10:26:20AM -0700, Darrick J. Wong wrote:
> >> > On Fri, Mar 23, 2018 at 05:08:13PM +, Luis R. Rodriguez wrote:
> >> > > On Thu, Mar 22, 2018 at 08:41:45PM -0700, Darrick J. Wong wrote:
> >> > > > On Fri, Mar 23, 2018 at 01:30:37AM +, Luis R. Rodriguez wrote:
> >> > > > > On Wed, Nov 22, 2017 at 10:01:37PM -0800, Darrick J. Wong wrote:
> >> > > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> >> > > > > > index 61d1cb7..8012741 100644
> >> > > > > > --- a/fs/xfs/xfs_inode.c
> >> > > > > > +++ b/fs/xfs/xfs_inode.c
> >> > > > > > @@ -2401,6 +2401,24 @@ xfs_ifree_cluster(
> >> > > > > >  }
> >> > > > > >
> >> > > > > >  /*
> >> > > > > > + * Free any local-format buffers sitting around before we reset 
> >> > > > > > to
> >> > > > > > + * extents format.
> >> > > > > > + */
> >> > > > > > +static inline void
> >> > > > > > +xfs_ifree_local_data(
> >> > > > > > +   struct xfs_inode*ip,
> >> > > > > > +   int whichfork)
> >> > > > > > +{
> >> > > > > > +   struct xfs_ifork*ifp;
> >> > > > > > +
> >> > > > > > +   if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> >> > > > > > +   return;
> >> > > > >
> >> > > > > I'm new to all this so this was a bit hard to follow. I'm confused 
> >> > > > > with how
> >> > > > > commit 43518812d2 ("xfs: remove support for inlining data/extents 
> >> > > > > into the
> >> > > > > inode fork") exacerbated the leak, isn't that commit about
> >> > > > > XFS_DINODE_FMT_EXTENTS?
> >> > > >
> >> > > > Not specifically _EXTENTS, merely any fork (EXTENTS or LOCAL) whose
> >> > > > incore data was small enough to fit in if_inline_ata.
> >> > >
> >> > > Got it, I thought those were XFS_DINODE_FMT_EXTENTS by definition.
> >> > >
> >> > > > > Did we have cases where the format was XFS_DINODE_FMT_LOCAL and yet
> >> > > > > ifp->if_u1.if_data == ifp->if_u2.if_inline_data ?
> >> > > >
> >> > > > An empty directory is 6 bytes, which is what you get with a fresh 
> >> > > > mkdir
> >> > > > or after deleting everything in the directory.  Prior to the 
> >> > > > 43518812d2
> >> > > > patch we could get away with not even checking if we had to free 
> >> > > > if_data
> >> > > > when deleting a directory because it fit within if_inline_data.
> >> > >
> >> > > Ah got it. So your fix *is* also applicable even prior to commit 
> >> > > 43518812d2.
> >> >
> >> > You'd have to modify the patch so that it doesn't try to kmem_free
> >> > if_data if if_data == if_inline_data but otherwise (in theory) I think
> >> > that the concept applies to pre-4.15 kernels.
> >> >
> >> > (YMMV, please do run this through QA/kmemleak just in case I'm wrong, 
> >> > etc...)
> >>
> >> Well... so we need a resolution and better get testing this already given 
> >> that
> >> *I believe* the new auto-selection algorithm used to cherry pick patches 
> >> onto
> >> stable for linux-4.14.y (covered on a paper [0] and when used, stable 
> >> patches
> >> are prefixed with AUTOSEL, a recent discussion covered this in November 
> >> 2017
> >> [1]) recommended to merge your commit 98c4f78dcdd8 ("xfs: always free 
> >> inline
> >> data before resetting inode fork during ifree") as stable commit 
> >> 1eccdbd4836a41
> >> on v4.14.17 *without* merging commit 43518812d2 ("xfs: remove support for
> >> inlining data/extents into the inode fork").
> >
> > Yikes. That sets off all my "how to break filesysetms for fun and
> > profit" alarm bells. This is like playing russian roulette with all
> > our user's data.  XFS fixes that look like they are simple often
> > have subtle dependencies in them that automated backports won't ever
> > be able to understand, and if we don't get that right, we break
> > stuff.
> 
> On the other hand, XFS has a few commits that fix possible
> corruptions, that have never ended up in a stable tree. Isn't it just
> as bad ("playing roulette") for users?

No, because most corruption problems we fix are rarely seen by
users. Those that are seen or considered a significant risk are
backported as per the usual process. What we don't do is shovel
things that *look like fixes* back in older kernels.

This is the third time in recent weeks where I've had to explain
this. e.g:

https://marc.info/?l=linux-xfs=152103080002315=2

And note Christoph's followup:
https://marc.info/?l=linux-xfs=152103175702634=2

What's important to note is that the discussion in that thread lead
to the patch being backported, validated and then included in Greg's
stable tree.

Validating that backports to all the stable kernels is effectively a
full time job in itself, and we simply don't have enough upstream
developer resources available to do that. So it's a simple: if we
don't have the resources to validate changes properly, 

Re: [PATCH] xfs: always free inline data before resetting inode fork during ifree

[Dave Chinner]

On Mon, Mar 26, 2018 at 07:54:31PM -0400, Sasha Levin wrote:
> On Sun, Mar 25, 2018 at 6:33 PM, Dave Chinner  wrote:
> > On Fri, Mar 23, 2018 at 06:23:02PM +, Luis R. Rodriguez wrote:
> >> On Fri, Mar 23, 2018 at 10:26:20AM -0700, Darrick J. Wong wrote:
> >> > On Fri, Mar 23, 2018 at 05:08:13PM +, Luis R. Rodriguez wrote:
> >> > > On Thu, Mar 22, 2018 at 08:41:45PM -0700, Darrick J. Wong wrote:
> >> > > > On Fri, Mar 23, 2018 at 01:30:37AM +, Luis R. Rodriguez wrote:
> >> > > > > On Wed, Nov 22, 2017 at 10:01:37PM -0800, Darrick J. Wong wrote:
> >> > > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> >> > > > > > index 61d1cb7..8012741 100644
> >> > > > > > --- a/fs/xfs/xfs_inode.c
> >> > > > > > +++ b/fs/xfs/xfs_inode.c
> >> > > > > > @@ -2401,6 +2401,24 @@ xfs_ifree_cluster(
> >> > > > > >  }
> >> > > > > >
> >> > > > > >  /*
> >> > > > > > + * Free any local-format buffers sitting around before we reset 
> >> > > > > > to
> >> > > > > > + * extents format.
> >> > > > > > + */
> >> > > > > > +static inline void
> >> > > > > > +xfs_ifree_local_data(
> >> > > > > > +   struct xfs_inode*ip,
> >> > > > > > +   int whichfork)
> >> > > > > > +{
> >> > > > > > +   struct xfs_ifork*ifp;
> >> > > > > > +
> >> > > > > > +   if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> >> > > > > > +   return;
> >> > > > >
> >> > > > > I'm new to all this so this was a bit hard to follow. I'm confused 
> >> > > > > with how
> >> > > > > commit 43518812d2 ("xfs: remove support for inlining data/extents 
> >> > > > > into the
> >> > > > > inode fork") exacerbated the leak, isn't that commit about
> >> > > > > XFS_DINODE_FMT_EXTENTS?
> >> > > >
> >> > > > Not specifically _EXTENTS, merely any fork (EXTENTS or LOCAL) whose
> >> > > > incore data was small enough to fit in if_inline_ata.
> >> > >
> >> > > Got it, I thought those were XFS_DINODE_FMT_EXTENTS by definition.
> >> > >
> >> > > > > Did we have cases where the format was XFS_DINODE_FMT_LOCAL and yet
> >> > > > > ifp->if_u1.if_data == ifp->if_u2.if_inline_data ?
> >> > > >
> >> > > > An empty directory is 6 bytes, which is what you get with a fresh 
> >> > > > mkdir
> >> > > > or after deleting everything in the directory.  Prior to the 
> >> > > > 43518812d2
> >> > > > patch we could get away with not even checking if we had to free 
> >> > > > if_data
> >> > > > when deleting a directory because it fit within if_inline_data.
> >> > >
> >> > > Ah got it. So your fix *is* also applicable even prior to commit 
> >> > > 43518812d2.
> >> >
> >> > You'd have to modify the patch so that it doesn't try to kmem_free
> >> > if_data if if_data == if_inline_data but otherwise (in theory) I think
> >> > that the concept applies to pre-4.15 kernels.
> >> >
> >> > (YMMV, please do run this through QA/kmemleak just in case I'm wrong, 
> >> > etc...)
> >>
> >> Well... so we need a resolution and better get testing this already given 
> >> that
> >> *I believe* the new auto-selection algorithm used to cherry pick patches 
> >> onto
> >> stable for linux-4.14.y (covered on a paper [0] and when used, stable 
> >> patches
> >> are prefixed with AUTOSEL, a recent discussion covered this in November 
> >> 2017
> >> [1]) recommended to merge your commit 98c4f78dcdd8 ("xfs: always free 
> >> inline
> >> data before resetting inode fork during ifree") as stable commit 
> >> 1eccdbd4836a41
> >> on v4.14.17 *without* merging commit 43518812d2 ("xfs: remove support for
> >> inlining data/extents into the inode fork").
> >
> > Yikes. That sets off all my "how to break filesysetms for fun and
> > profit" alarm bells. This is like playing russian roulette with all
> > our user's data.  XFS fixes that look like they are simple often
> > have subtle dependencies in them that automated backports won't ever
> > be able to understand, and if we don't get that right, we break
> > stuff.
> 
> On the other hand, XFS has a few commits that fix possible
> corruptions, that have never ended up in a stable tree. Isn't it just
> as bad ("playing roulette") for users?

No, because most corruption problems we fix are rarely seen by
users. Those that are seen or considered a significant risk are
backported as per the usual process. What we don't do is shovel
things that *look like fixes* back in older kernels.

This is the third time in recent weeks where I've had to explain
this. e.g:

https://marc.info/?l=linux-xfs=152103080002315=2

And note Christoph's followup:
https://marc.info/?l=linux-xfs=152103175702634=2

What's important to note is that the discussion in that thread lead
to the patch being backported, validated and then included in Greg's
stable tree.

Validating that backports to all the stable kernels is effectively a
full time job in itself, and we simply don't have enough upstream
developer resources available to do that. So it's a simple: if we
don't have the resources to validate changes properly, then we
*don't change 

Re: [PATCH v2 02/21] kbuild: remove CONFIG_CROSS_COMPILE support

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Kbuild provides a couple of ways to specify CROSS_COMPILE:
>
> [1] Command line
> [2] Environment
> [3] arch/*/Makefile (only some architectures)
> [4] CONFIG_CROSS_COMPILE
>
> [4] is problematic for the compiler capability tests in Kconfig.
> CONFIG_CROSS_COMPILE allows users to change the compiler prefix from
> 'make menuconfig', etc.  It means, the compiler options would have
> to be all re-calculated everytime CONFIG_CROSS_COMPILE is changed.
>
> To avoid complexity and performance issues, I'd like to evaluate
> the shell commands statically, i.e. only parsing Kconfig files.
>
> I guess the majority is [1] or [2].  Currently, there are only
> 4 defconfig files that specify CONFIG_CROSS_COMPILE.
>   arch/arm/configs/lpc18xx_defconfig
>   arch/hexagon/configs/comet_defconfig
>   arch/openrisc/configs/or1ksim_defconfig
>   arch/openrisc/configs/simple_smp_defconfig
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 03/21] kconfig: move and rename sym_expand_string_value()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> This helper expands symbols contained in a string.  I am about to
> change it to expand environments instead of symbols.  Also, I will
> add function expansion later.
>
> Rename it to expand_string_value(), and move it to util.c, which is
> a more suitable place.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 02/21] kbuild: remove CONFIG_CROSS_COMPILE support

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> Kbuild provides a couple of ways to specify CROSS_COMPILE:
>
> [1] Command line
> [2] Environment
> [3] arch/*/Makefile (only some architectures)
> [4] CONFIG_CROSS_COMPILE
>
> [4] is problematic for the compiler capability tests in Kconfig.
> CONFIG_CROSS_COMPILE allows users to change the compiler prefix from
> 'make menuconfig', etc.  It means, the compiler options would have
> to be all re-calculated everytime CONFIG_CROSS_COMPILE is changed.
>
> To avoid complexity and performance issues, I'd like to evaluate
> the shell commands statically, i.e. only parsing Kconfig files.
>
> I guess the majority is [1] or [2].  Currently, there are only
> 4 defconfig files that specify CONFIG_CROSS_COMPILE.
>   arch/arm/configs/lpc18xx_defconfig
>   arch/hexagon/configs/comet_defconfig
>   arch/openrisc/configs/or1ksim_defconfig
>   arch/openrisc/configs/simple_smp_defconfig
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 03/21] kconfig: move and rename sym_expand_string_value()

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> This helper expands symbols contained in a string.  I am about to
> change it to expand environments instead of symbols.  Also, I will
> add function expansion later.
>
> Rename it to expand_string_value(), and move it to util.c, which is
> a more suitable place.
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

[PATCH V2 2/4] clocksource/drivers/imx-tpm: fix typo of clock name

[Anson Huang]

The clock name should be ipg instead of igp.

Signed-off-by: Anson Huang 
---
 drivers/clocksource/timer-imx-tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 21bffdc..3f97d49 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -179,7 +179,7 @@ static int __init tpm_timer_init(struct device_node *np)
ipg = of_clk_get_by_name(np, "ipg");
per = of_clk_get_by_name(np, "per");
if (IS_ERR(ipg) || IS_ERR(per)) {
-   pr_err("tpm: failed to get igp or per clk\n");
+   pr_err("tpm: failed to get ipg or per clk\n");
ret = -ENODEV;
goto err_clk_get;
}
-- 
2.7.4

[PATCH V2 2/4] clocksource/drivers/imx-tpm: fix typo of clock name

[Anson Huang]

The clock name should be ipg instead of igp.

Signed-off-by: Anson Huang 
---
 drivers/clocksource/timer-imx-tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 21bffdc..3f97d49 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -179,7 +179,7 @@ static int __init tpm_timer_init(struct device_node *np)
ipg = of_clk_get_by_name(np, "ipg");
per = of_clk_get_by_name(np, "per");
if (IS_ERR(ipg) || IS_ERR(per)) {
-   pr_err("tpm: failed to get igp or per clk\n");
+   pr_err("tpm: failed to get ipg or per clk\n");
ret = -ENODEV;
goto err_clk_get;
}
-- 
2.7.4

Re: [PATCH v2 01/21] kbuild: remove kbuild cache

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> The kbuild cache was introduced to remember the result of shell
> commands, some of which are expensive to compute, such as
> $(call cc-option,...).
>
> However, this turned out not so clever as I had first expected.
> Actually, it is problematic.  For example, "$(CC) -print-file-name"
> is cached.  If the compiler is updated, the stale search path causes
> build error, which is difficult to figure out.  Another problem
> scenario is cache files could be touched while install targets are
> running under the root permission.  We can patch them if desired,
> but the build infrastructure is getting uglier and uglier.
>
> Now, we are going to move compiler flag tests to the configuration
> phase.  If this is completed, the result of compiler tests will be
> naturally cached in the .config file.  We will not have performance
> issues of incremental building since this testing only happens at
> Kconfig time.
>
> To start this work with a cleaner code base, remove the kbuild
> cache first.
>
> Revert the following commits:
> Commit 9a234a2e3843 ("kbuild: create directory for make cache only when 
> necessary")
> Commit e17c400ae194 ("kbuild: shrink .cache.mk when it exceeds 1000 lines")
> Commit 4e56207130ed ("kbuild: Cache a few more calls to the compiler")
> Commit 3298b690b21c ("kbuild: Add a cache for generated variables")
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 01/21] kbuild: remove kbuild cache

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> The kbuild cache was introduced to remember the result of shell
> commands, some of which are expensive to compute, such as
> $(call cc-option,...).
>
> However, this turned out not so clever as I had first expected.
> Actually, it is problematic.  For example, "$(CC) -print-file-name"
> is cached.  If the compiler is updated, the stale search path causes
> build error, which is difficult to figure out.  Another problem
> scenario is cache files could be touched while install targets are
> running under the root permission.  We can patch them if desired,
> but the build infrastructure is getting uglier and uglier.
>
> Now, we are going to move compiler flag tests to the configuration
> phase.  If this is completed, the result of compiler tests will be
> naturally cached in the .config file.  We will not have performance
> issues of incremental building since this testing only happens at
> Kconfig time.
>
> To start this work with a cleaner code base, remove the kbuild
> cache first.
>
> Revert the following commits:
> Commit 9a234a2e3843 ("kbuild: create directory for make cache only when 
> necessary")
> Commit e17c400ae194 ("kbuild: shrink .cache.mk when it exceeds 1000 lines")
> Commit 4e56207130ed ("kbuild: Cache a few more calls to the compiler")
> Commit 3298b690b21c ("kbuild: Add a cache for generated variables")
>
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security

[PATCH V2 3/4] clocksource/drivers/imx-tpm: correct some registers operation flow

[Anson Huang]

According to i.MX7ULP reference manual, TPM_SC_CPWMS can ONLY be
written when counter is disabled, TPM_SC_TOF is write-1-clear,
TPM_C0SC_CHF is also write-1-clear, correct these registers
initialization flow;

Signed-off-by: Anson Huang 
---
changes since V1:
use separate patch to fix the clock name typo.
 drivers/clocksource/timer-imx-tpm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 3f97d49..7403e49 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -20,6 +20,7 @@
 #define TPM_SC 0x10
 #define TPM_SC_CMOD_INC_PER_CNT(0x1 << 3)
 #define TPM_SC_CMOD_DIV_DEFAULT0x3
+#define TPM_SC_TOF_MASK(0x1 << 7)
 #define TPM_CNT0x14
 #define TPM_MOD0x18
 #define TPM_STATUS 0x1c
@@ -29,6 +30,7 @@
 #define TPM_C0SC_MODE_SHIFT2
 #define TPM_C0SC_MODE_MASK 0x3c
 #define TPM_C0SC_MODE_SW_COMPARE   0x4
+#define TPM_C0SC_CHF_MASK  (0x1 << 7)
 #define TPM_C0V0x24
 
 static void __iomem *timer_base;
@@ -205,9 +207,13 @@ static int __init tpm_timer_init(struct device_node *np)
 * 4) Channel0 disabled
 * 5) DMA transfers disabled
 */
+   /* make sure counter is disabled */
writel(0, timer_base + TPM_SC);
+   /* TOF is W1C */
+   writel(TPM_SC_TOF_MASK, timer_base + TPM_SC);
writel(0, timer_base + TPM_CNT);
-   writel(0, timer_base + TPM_C0SC);
+   /* CHF is W1C */
+   writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
 
/* increase per cnt, div 8 by default */
writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
-- 
2.7.4

[PATCH V2 3/4] clocksource/drivers/imx-tpm: correct some registers operation flow

[Anson Huang]

According to i.MX7ULP reference manual, TPM_SC_CPWMS can ONLY be
written when counter is disabled, TPM_SC_TOF is write-1-clear,
TPM_C0SC_CHF is also write-1-clear, correct these registers
initialization flow;

Signed-off-by: Anson Huang 
---
changes since V1:
use separate patch to fix the clock name typo.
 drivers/clocksource/timer-imx-tpm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 3f97d49..7403e49 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -20,6 +20,7 @@
 #define TPM_SC 0x10
 #define TPM_SC_CMOD_INC_PER_CNT(0x1 << 3)
 #define TPM_SC_CMOD_DIV_DEFAULT0x3
+#define TPM_SC_TOF_MASK(0x1 << 7)
 #define TPM_CNT0x14
 #define TPM_MOD0x18
 #define TPM_STATUS 0x1c
@@ -29,6 +30,7 @@
 #define TPM_C0SC_MODE_SHIFT2
 #define TPM_C0SC_MODE_MASK 0x3c
 #define TPM_C0SC_MODE_SW_COMPARE   0x4
+#define TPM_C0SC_CHF_MASK  (0x1 << 7)
 #define TPM_C0V0x24
 
 static void __iomem *timer_base;
@@ -205,9 +207,13 @@ static int __init tpm_timer_init(struct device_node *np)
 * 4) Channel0 disabled
 * 5) DMA transfers disabled
 */
+   /* make sure counter is disabled */
writel(0, timer_base + TPM_SC);
+   /* TOF is W1C */
+   writel(TPM_SC_TOF_MASK, timer_base + TPM_SC);
writel(0, timer_base + TPM_CNT);
-   writel(0, timer_base + TPM_C0SC);
+   /* CHF is W1C */
+   writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
 
/* increase per cnt, div 8 by default */
writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
-- 
2.7.4

Re: [PATCH v2 12/21] kconfig: show compiler version text in the top comment

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> The kernel configuration phase is now tightly coupled with the compiler
> in use.  It will be nice to show the compiler information in Kconfig.
>
> The compiler information will be displayed like this:
>
>   $ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- config
>   scripts/kconfig/conf  --oldaskconfig Kconfig
>   *
>   * Linux/arm64 4.16.0-rc1 Kernel Configuration
>   *
>   *
>   * Compiler: aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11) 7.2.1 20171011
>   *
>   *
>   * General setup
>   *
>   Compile also drivers which will not load (COMPILE_TEST) [N/y/?]
>
> If you use GUI methods such as menuconfig, it will be displayed in the
> top menu.
>
> This is simply implemented by using 'comment'.  So, it will be saved
> into the .config file as well.
>
> This commit has a very important meaning.  If the compiler is upgraded,
> Kconfig must be re-run since different compilers have different sets
> of supported options.
>
> All referenced environments are written to include/config/auto.conf.cmd
> so that any environment change triggers syncconfig, and prompt the user
> to input new values if needed.
>
> With this commit, something like follows will be added to
> include/config/auto.conf.cmd
>
>   ifneq "$(CC_VERSION_TEXT)" "aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11) 
> 7.2.1 20171011"
>   include/config/auto.conf: FORCE
>   endif
>
> Signed-off-by: Masahiro Yamada 

I like seeing this in the config. Good idea!

Reviewed-by: Kees Cook 

-Kees

> ---
>
> Changes in v2: None
>
>  Kconfig  | 2 ++
>  Makefile | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index e6ece5b..99ed4b8 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -5,4 +5,6 @@
>  #
>  mainmenu "Linux/$ARCH $KERNELVERSION Kernel Configuration"
>
> +comment "Compiler: $CC_VERSION_TEXT"
> +
>  source "arch/$SRCARCH/Kconfig"
> diff --git a/Makefile b/Makefile
> index 5cadffa..3dc9eb8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -450,6 +450,8 @@ export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE 
> KBUILD_LDFLAGS_MODULE
>  export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
>  export KBUILD_ARFLAGS
>
> +export CC_VERSION_TEXT := $(shell $(CC) --version | head -n 1)
> +
>  # When compiling out-of-tree modules, put MODVERDIR in the module
>  # tree rather than in the kernel tree. The kernel tree might
>  # even be read-only.
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security

[PATCH V2 4/4] clocksource/drivers/imx-tpm: add different counter width support

[Anson Huang]

Different TPM modules have different width counters which
is 16-bit or 32-bit, the counter width can be read from
TPM_PARAM register bit[23:16], this patch adds dynamic
check for counter width to support both 16-bit and 32-bit
TPM modules.

Signed-off-by: Anson Huang 
---
changes since V1:
use largest prescaler for 16-bit counter;
use lower rating for 16-bit counter than 32-bit;
remove the computing in counter read function.
 drivers/clocksource/timer-imx-tpm.c | 33 +
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 7403e49..05d97a6 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -17,9 +17,13 @@
 #include 
 #include 
 
+#define TPM_PARAM  0x4
+#define TPM_PARAM_WIDTH_SHIFT  16
+#define TPM_PARAM_WIDTH_MASK   (0xff << 16)
 #define TPM_SC 0x10
 #define TPM_SC_CMOD_INC_PER_CNT(0x1 << 3)
 #define TPM_SC_CMOD_DIV_DEFAULT0x3
+#define TPM_SC_CMOD_DIV_MAX0x7
 #define TPM_SC_TOF_MASK(0x1 << 7)
 #define TPM_CNT0x14
 #define TPM_MOD0x18
@@ -33,6 +37,8 @@
 #define TPM_C0SC_CHF_MASK  (0x1 << 7)
 #define TPM_C0V0x24
 
+static int counter_width;
+static int rating;
 static void __iomem *timer_base;
 static struct clock_event_device clockevent_tpm;
 
@@ -85,10 +91,11 @@ static int __init tpm_clocksource_init(unsigned long rate)
tpm_delay_timer.freq = rate;
register_current_timer_delay(_delay_timer);
 
-   sched_clock_register(tpm_read_sched_clock, 32, rate);
+   sched_clock_register(tpm_read_sched_clock, counter_width, rate);
 
return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
-rate, 200, 32, clocksource_mmio_readl_up);
+rate, rating, counter_width,
+clocksource_mmio_readl_up);
 }
 
 static int tpm_set_next_event(unsigned long delta,
@@ -141,7 +148,6 @@ static struct clock_event_device clockevent_tpm = {
.set_state_oneshot  = tpm_set_state_oneshot,
.set_next_event = tpm_set_next_event,
.set_state_shutdown = tpm_set_state_shutdown,
-   .rating = 200,
 };
 
 static int __init tpm_clockevent_init(unsigned long rate, int irq)
@@ -151,10 +157,11 @@ static int __init tpm_clockevent_init(unsigned long rate, 
int irq)
ret = request_irq(irq, tpm_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
  "i.MX7ULP TPM Timer", _tpm);
 
+   clockevent_tpm.rating = rating;
clockevent_tpm.cpumask = cpumask_of(0);
clockevent_tpm.irq = irq;
-   clockevents_config_and_register(_tpm,
-   rate, 300, 0xfffe);
+   clockevents_config_and_register(_tpm, rate, 300,
+   GENMASK(counter_width - 1, 1));
 
return ret;
 }
@@ -199,6 +206,11 @@ static int __init tpm_timer_init(struct device_node *np)
goto err_per_clk_enable;
}
 
+   counter_width = (readl(timer_base + TPM_PARAM) & TPM_PARAM_WIDTH_MASK)
+   >> TPM_PARAM_WIDTH_SHIFT;
+   /* use rating 200 for 32-bit counter and 150 for 16-bit counter */
+   rating = counter_width == 0x20 ? 200 : 150;
+
/*
 * Initialize tpm module to a known state
 * 1) Counter disabled
@@ -215,12 +227,17 @@ static int __init tpm_timer_init(struct device_node *np)
/* CHF is W1C */
writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
 
-   /* increase per cnt, div 8 by default */
-   writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
+   /*
+* increase per cnt,
+* div 8 for 32-bit counter and div 128 for 16-bit counter
+*/
+   writel(TPM_SC_CMOD_INC_PER_CNT |
+   (counter_width == 0x20 ?
+   TPM_SC_CMOD_DIV_DEFAULT : TPM_SC_CMOD_DIV_MAX),
 timer_base + TPM_SC);
 
/* set MOD register to maximum for free running mode */
-   writel(0x, timer_base + TPM_MOD);
+   writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
 
rate = clk_get_rate(per) >> 3;
ret = tpm_clocksource_init(rate);
-- 
2.7.4

[PATCH V2 1/4] dt-bindings: timer: tpm: fix typo of clock name

[Anson Huang]

The clock name should be ipg instead of igp.

Signed-off-by: Anson Huang 
---
 Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
index b4aa7dd..f82087b 100644
--- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
@@ -15,7 +15,7 @@ Required properties:
 - interrupts : Should be the clock event device interrupt.
 - clocks : The clocks provided by the SoC to drive the timer, must contain
an entry for each entry in clock-names.
-- clock-names : Must include the following entries: "igp" and "per".
+- clock-names : Must include the following entries: "ipg" and "per".
 
 Example:
 tpm5: tpm@4026 {
-- 
2.7.4

Re: [PATCH v2 12/21] kconfig: show compiler version text in the top comment

[Kees Cook]

On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
 wrote:
> The kernel configuration phase is now tightly coupled with the compiler
> in use.  It will be nice to show the compiler information in Kconfig.
>
> The compiler information will be displayed like this:
>
>   $ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- config
>   scripts/kconfig/conf  --oldaskconfig Kconfig
>   *
>   * Linux/arm64 4.16.0-rc1 Kernel Configuration
>   *
>   *
>   * Compiler: aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11) 7.2.1 20171011
>   *
>   *
>   * General setup
>   *
>   Compile also drivers which will not load (COMPILE_TEST) [N/y/?]
>
> If you use GUI methods such as menuconfig, it will be displayed in the
> top menu.
>
> This is simply implemented by using 'comment'.  So, it will be saved
> into the .config file as well.
>
> This commit has a very important meaning.  If the compiler is upgraded,
> Kconfig must be re-run since different compilers have different sets
> of supported options.
>
> All referenced environments are written to include/config/auto.conf.cmd
> so that any environment change triggers syncconfig, and prompt the user
> to input new values if needed.
>
> With this commit, something like follows will be added to
> include/config/auto.conf.cmd
>
>   ifneq "$(CC_VERSION_TEXT)" "aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11) 
> 7.2.1 20171011"
>   include/config/auto.conf: FORCE
>   endif
>
> Signed-off-by: Masahiro Yamada 

I like seeing this in the config. Good idea!

Reviewed-by: Kees Cook 

-Kees

> ---
>
> Changes in v2: None
>
>  Kconfig  | 2 ++
>  Makefile | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index e6ece5b..99ed4b8 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -5,4 +5,6 @@
>  #
>  mainmenu "Linux/$ARCH $KERNELVERSION Kernel Configuration"
>
> +comment "Compiler: $CC_VERSION_TEXT"
> +
>  source "arch/$SRCARCH/Kconfig"
> diff --git a/Makefile b/Makefile
> index 5cadffa..3dc9eb8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -450,6 +450,8 @@ export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE 
> KBUILD_LDFLAGS_MODULE
>  export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
>  export KBUILD_ARFLAGS
>
> +export CC_VERSION_TEXT := $(shell $(CC) --version | head -n 1)
> +
>  # When compiling out-of-tree modules, put MODVERDIR in the module
>  # tree rather than in the kernel tree. The kernel tree might
>  # even be read-only.
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security

[PATCH V2 4/4] clocksource/drivers/imx-tpm: add different counter width support

[Anson Huang]

Different TPM modules have different width counters which
is 16-bit or 32-bit, the counter width can be read from
TPM_PARAM register bit[23:16], this patch adds dynamic
check for counter width to support both 16-bit and 32-bit
TPM modules.

Signed-off-by: Anson Huang 
---
changes since V1:
use largest prescaler for 16-bit counter;
use lower rating for 16-bit counter than 32-bit;
remove the computing in counter read function.
 drivers/clocksource/timer-imx-tpm.c | 33 +
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c 
b/drivers/clocksource/timer-imx-tpm.c
index 7403e49..05d97a6 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -17,9 +17,13 @@
 #include 
 #include 
 
+#define TPM_PARAM  0x4
+#define TPM_PARAM_WIDTH_SHIFT  16
+#define TPM_PARAM_WIDTH_MASK   (0xff << 16)
 #define TPM_SC 0x10
 #define TPM_SC_CMOD_INC_PER_CNT(0x1 << 3)
 #define TPM_SC_CMOD_DIV_DEFAULT0x3
+#define TPM_SC_CMOD_DIV_MAX0x7
 #define TPM_SC_TOF_MASK(0x1 << 7)
 #define TPM_CNT0x14
 #define TPM_MOD0x18
@@ -33,6 +37,8 @@
 #define TPM_C0SC_CHF_MASK  (0x1 << 7)
 #define TPM_C0V0x24
 
+static int counter_width;
+static int rating;
 static void __iomem *timer_base;
 static struct clock_event_device clockevent_tpm;
 
@@ -85,10 +91,11 @@ static int __init tpm_clocksource_init(unsigned long rate)
tpm_delay_timer.freq = rate;
register_current_timer_delay(_delay_timer);
 
-   sched_clock_register(tpm_read_sched_clock, 32, rate);
+   sched_clock_register(tpm_read_sched_clock, counter_width, rate);
 
return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
-rate, 200, 32, clocksource_mmio_readl_up);
+rate, rating, counter_width,
+clocksource_mmio_readl_up);
 }
 
 static int tpm_set_next_event(unsigned long delta,
@@ -141,7 +148,6 @@ static struct clock_event_device clockevent_tpm = {
.set_state_oneshot  = tpm_set_state_oneshot,
.set_next_event = tpm_set_next_event,
.set_state_shutdown = tpm_set_state_shutdown,
-   .rating = 200,
 };
 
 static int __init tpm_clockevent_init(unsigned long rate, int irq)
@@ -151,10 +157,11 @@ static int __init tpm_clockevent_init(unsigned long rate, 
int irq)
ret = request_irq(irq, tpm_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
  "i.MX7ULP TPM Timer", _tpm);
 
+   clockevent_tpm.rating = rating;
clockevent_tpm.cpumask = cpumask_of(0);
clockevent_tpm.irq = irq;
-   clockevents_config_and_register(_tpm,
-   rate, 300, 0xfffe);
+   clockevents_config_and_register(_tpm, rate, 300,
+   GENMASK(counter_width - 1, 1));
 
return ret;
 }
@@ -199,6 +206,11 @@ static int __init tpm_timer_init(struct device_node *np)
goto err_per_clk_enable;
}
 
+   counter_width = (readl(timer_base + TPM_PARAM) & TPM_PARAM_WIDTH_MASK)
+   >> TPM_PARAM_WIDTH_SHIFT;
+   /* use rating 200 for 32-bit counter and 150 for 16-bit counter */
+   rating = counter_width == 0x20 ? 200 : 150;
+
/*
 * Initialize tpm module to a known state
 * 1) Counter disabled
@@ -215,12 +227,17 @@ static int __init tpm_timer_init(struct device_node *np)
/* CHF is W1C */
writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
 
-   /* increase per cnt, div 8 by default */
-   writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
+   /*
+* increase per cnt,
+* div 8 for 32-bit counter and div 128 for 16-bit counter
+*/
+   writel(TPM_SC_CMOD_INC_PER_CNT |
+   (counter_width == 0x20 ?
+   TPM_SC_CMOD_DIV_DEFAULT : TPM_SC_CMOD_DIV_MAX),
 timer_base + TPM_SC);
 
/* set MOD register to maximum for free running mode */
-   writel(0x, timer_base + TPM_MOD);
+   writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
 
rate = clk_get_rate(per) >> 3;
ret = tpm_clocksource_init(rate);
-- 
2.7.4

[PATCH V2 1/4] dt-bindings: timer: tpm: fix typo of clock name

[Anson Huang]

The clock name should be ipg instead of igp.

Signed-off-by: Anson Huang 
---
 Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt 
b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
index b4aa7dd..f82087b 100644
--- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
@@ -15,7 +15,7 @@ Required properties:
 - interrupts : Should be the clock event device interrupt.
 - clocks : The clocks provided by the SoC to drive the timer, must contain
an entry for each entry in clock-names.
-- clock-names : Must include the following entries: "igp" and "per".
+- clock-names : Must include the following entries: "ipg" and "per".
 
 Example:
 tpm5: tpm@4026 {
-- 
2.7.4

[PATCH] x86, msr: fix rdmsrl_safe_on_cpu()

[Eric Dumazet]

When changing rdmsr_safe_on_cpu() to schedule, I missed that
__rdmsr_safe_on_cpu() was also used by rdmsrl_safe_on_cpu()

Lets make rdmsrl_safe_on_cpu() a wrapper instead of copy/pasting
the code I added for the completion handling.

Fixes: 07cde313b2d2 ("x86/msr: Allow rdmsr_safe_on_cpu() to schedule")
Signed-off-by: Eric Dumazet 
Reported-by: kbuild test robot 
Cc: "H. Peter Anvin" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
---
 arch/x86/lib/msr-smp.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index 
761ba062afdaf7f7d0603ed94ed6cc3e46b37f76..fee8b9c0520c9954e3cf9093332e1a9b53009b8b
 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -225,16 +225,13 @@ EXPORT_SYMBOL(wrmsrl_safe_on_cpu);
 
 int rdmsrl_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
 {
+   u32 low, high;
int err;
-   struct msr_info rv;
 
-   memset(, 0, sizeof(rv));
+   err = rdmsr_safe_on_cpu(cpu, msr_no, , );
+   *q = (u64)high << 32 | low;
 
-   rv.msr_no = msr_no;
-   err = smp_call_function_single(cpu, __rdmsr_safe_on_cpu, , 1);
-   *q = rv.reg.q;
-
-   return err ? err : rv.err;
+   return err;
 }
 EXPORT_SYMBOL(rdmsrl_safe_on_cpu);
 
-- 
2.17.0.rc1.321.gba9d0f2565-goog

[PATCH] x86, msr: fix rdmsrl_safe_on_cpu()

[Eric Dumazet]

When changing rdmsr_safe_on_cpu() to schedule, I missed that
__rdmsr_safe_on_cpu() was also used by rdmsrl_safe_on_cpu()

Lets make rdmsrl_safe_on_cpu() a wrapper instead of copy/pasting
the code I added for the completion handling.

Fixes: 07cde313b2d2 ("x86/msr: Allow rdmsr_safe_on_cpu() to schedule")
Signed-off-by: Eric Dumazet 
Reported-by: kbuild test robot 
Cc: "H. Peter Anvin" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
---
 arch/x86/lib/msr-smp.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index 
761ba062afdaf7f7d0603ed94ed6cc3e46b37f76..fee8b9c0520c9954e3cf9093332e1a9b53009b8b
 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -225,16 +225,13 @@ EXPORT_SYMBOL(wrmsrl_safe_on_cpu);
 
 int rdmsrl_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
 {
+   u32 low, high;
int err;
-   struct msr_info rv;
 
-   memset(, 0, sizeof(rv));
+   err = rdmsr_safe_on_cpu(cpu, msr_no, , );
+   *q = (u64)high << 32 | low;
 
-   rv.msr_no = msr_no;
-   err = smp_call_function_single(cpu, __rdmsr_safe_on_cpu, , 1);
-   *q = rv.reg.q;
-
-   return err ? err : rv.err;
+   return err;
 }
 EXPORT_SYMBOL(rdmsrl_safe_on_cpu);
 
-- 
2.17.0.rc1.321.gba9d0f2565-goog

Re: [PATCH] kbuild: move out of

[Masahiro Yamada]

2018-03-28 12:10 GMT+09:00 Kees Cook :
> On Tue, Mar 27, 2018 at 7:40 PM, Masahiro Yamada
>  wrote:
>> Since commit 28128c61e08e ("kconfig.h: Include compiler types to avoid
>> missed struct attributes"),  pulls in kernel-space
>> headers to unrelated file.
>>
>> Commit 0f9da844d877 ("MIPS: boot: Define __ASSEMBLY__ for its.S build")
>> suppressed the build error by defining __ASSEMBLY__, but ITS (i.e. DTS)
>> is not assembly, and should not include  in the
>> first place.
>>
>> Looking at arch/s390/tools/Makefile, host programs gen_facilities and
>> gen_opcode_table now pull in  as well.
>>
>> The motivation for that commit was to define necessary attributes
>> before any struct is defined.  Obviously, this happens only in C.
>>
>> It is enough to include  only when compiling
>> C files, and only when compiling kernel space.
>>
>> Move the  inclusion to c_flags.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> I will also send a correct fix-up for MIPS Makefile later.
>
> As long as the result is the same: it's impossible to define a
> structure without defined attributes, I'm fine with it. :)
>
> Question below...
>
>>
>>
>>
>>  include/linux/kconfig.h | 3 ---
>>  scripts/Makefile.lib| 1 +
>>  2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>> index dcde947..cc8fa10 100644
>> --- a/include/linux/kconfig.h
>> +++ b/include/linux/kconfig.h
>> @@ -70,7 +70,4 @@
>>   */
>>  #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>>
>> -/* Make sure we always have all types and struct attributes defined. */
>> -#include 
>> -
>>  #endif /* __LINUX_KCONFIG_H */
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index c114ce5..07d0740 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -152,6 +152,7 @@ __cpp_flags = $(call flags,_cpp_flags)
>>  endif
>>
>>  c_flags= -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
>> +-include $(srctree)/include/linux/compiler_types.h   \
>
> Why here instead of adding it to LINUXINCLUDE ?
>


LINUXINCLUDE contains '-include $(srctree)/include/linux/kconfig.h'

If you add compier_types.h there, nothing would change.


My intention is to include  only for C.
Not assembly, linker scripts, or anything else.






-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: move out of

[Masahiro Yamada]

2018-03-28 12:10 GMT+09:00 Kees Cook :
> On Tue, Mar 27, 2018 at 7:40 PM, Masahiro Yamada
>  wrote:
>> Since commit 28128c61e08e ("kconfig.h: Include compiler types to avoid
>> missed struct attributes"),  pulls in kernel-space
>> headers to unrelated file.
>>
>> Commit 0f9da844d877 ("MIPS: boot: Define __ASSEMBLY__ for its.S build")
>> suppressed the build error by defining __ASSEMBLY__, but ITS (i.e. DTS)
>> is not assembly, and should not include  in the
>> first place.
>>
>> Looking at arch/s390/tools/Makefile, host programs gen_facilities and
>> gen_opcode_table now pull in  as well.
>>
>> The motivation for that commit was to define necessary attributes
>> before any struct is defined.  Obviously, this happens only in C.
>>
>> It is enough to include  only when compiling
>> C files, and only when compiling kernel space.
>>
>> Move the  inclusion to c_flags.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> I will also send a correct fix-up for MIPS Makefile later.
>
> As long as the result is the same: it's impossible to define a
> structure without defined attributes, I'm fine with it. :)
>
> Question below...
>
>>
>>
>>
>>  include/linux/kconfig.h | 3 ---
>>  scripts/Makefile.lib| 1 +
>>  2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>> index dcde947..cc8fa10 100644
>> --- a/include/linux/kconfig.h
>> +++ b/include/linux/kconfig.h
>> @@ -70,7 +70,4 @@
>>   */
>>  #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>>
>> -/* Make sure we always have all types and struct attributes defined. */
>> -#include 
>> -
>>  #endif /* __LINUX_KCONFIG_H */
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index c114ce5..07d0740 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -152,6 +152,7 @@ __cpp_flags = $(call flags,_cpp_flags)
>>  endif
>>
>>  c_flags= -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
>> +-include $(srctree)/include/linux/compiler_types.h   \
>
> Why here instead of adding it to LINUXINCLUDE ?
>


LINUXINCLUDE contains '-include $(srctree)/include/linux/kconfig.h'

If you add compier_types.h there, nothing would change.


My intention is to include  only for C.
Not assembly, linker scripts, or anything else.






-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: move out of

[Kees Cook]

On Tue, Mar 27, 2018 at 7:40 PM, Masahiro Yamada
 wrote:
> Since commit 28128c61e08e ("kconfig.h: Include compiler types to avoid
> missed struct attributes"),  pulls in kernel-space
> headers to unrelated file.
>
> Commit 0f9da844d877 ("MIPS: boot: Define __ASSEMBLY__ for its.S build")
> suppressed the build error by defining __ASSEMBLY__, but ITS (i.e. DTS)
> is not assembly, and should not include  in the
> first place.
>
> Looking at arch/s390/tools/Makefile, host programs gen_facilities and
> gen_opcode_table now pull in  as well.
>
> The motivation for that commit was to define necessary attributes
> before any struct is defined.  Obviously, this happens only in C.
>
> It is enough to include  only when compiling
> C files, and only when compiling kernel space.
>
> Move the  inclusion to c_flags.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> I will also send a correct fix-up for MIPS Makefile later.

As long as the result is the same: it's impossible to define a
structure without defined attributes, I'm fine with it. :)

Question below...

>
>
>
>  include/linux/kconfig.h | 3 ---
>  scripts/Makefile.lib| 1 +
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index dcde947..cc8fa10 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -70,7 +70,4 @@
>   */
>  #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>
> -/* Make sure we always have all types and struct attributes defined. */
> -#include 
> -
>  #endif /* __LINUX_KCONFIG_H */
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index c114ce5..07d0740 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -152,6 +152,7 @@ __cpp_flags = $(call flags,_cpp_flags)
>  endif
>
>  c_flags= -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
> +-include $(srctree)/include/linux/compiler_types.h   \

Why here instead of adding it to LINUXINCLUDE ?

>  $(__c_flags) $(modkern_cflags)   \
>  $(basename_flags) $(modname_flags)
>
> --
> 2.7.4
>

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] kbuild: move out of

[Kees Cook]

On Tue, Mar 27, 2018 at 7:40 PM, Masahiro Yamada
 wrote:
> Since commit 28128c61e08e ("kconfig.h: Include compiler types to avoid
> missed struct attributes"),  pulls in kernel-space
> headers to unrelated file.
>
> Commit 0f9da844d877 ("MIPS: boot: Define __ASSEMBLY__ for its.S build")
> suppressed the build error by defining __ASSEMBLY__, but ITS (i.e. DTS)
> is not assembly, and should not include  in the
> first place.
>
> Looking at arch/s390/tools/Makefile, host programs gen_facilities and
> gen_opcode_table now pull in  as well.
>
> The motivation for that commit was to define necessary attributes
> before any struct is defined.  Obviously, this happens only in C.
>
> It is enough to include  only when compiling
> C files, and only when compiling kernel space.
>
> Move the  inclusion to c_flags.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> I will also send a correct fix-up for MIPS Makefile later.

As long as the result is the same: it's impossible to define a
structure without defined attributes, I'm fine with it. :)

Question below...

>
>
>
>  include/linux/kconfig.h | 3 ---
>  scripts/Makefile.lib| 1 +
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index dcde947..cc8fa10 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -70,7 +70,4 @@
>   */
>  #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>
> -/* Make sure we always have all types and struct attributes defined. */
> -#include 
> -
>  #endif /* __LINUX_KCONFIG_H */
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index c114ce5..07d0740 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -152,6 +152,7 @@ __cpp_flags = $(call flags,_cpp_flags)
>  endif
>
>  c_flags= -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
> +-include $(srctree)/include/linux/compiler_types.h   \

Why here instead of adding it to LINUXINCLUDE ?

>  $(__c_flags) $(modkern_cflags)   \
>  $(basename_flags) $(modname_flags)
>
> --
> 2.7.4
>

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] spi: mediatek: remove redundant MODULE_ALIAS entries

[Sean Wang]

On Wed, 2018-03-28 at 10:14 +0800, Mark Brown wrote:
> On Mon, Mar 26, 2018 at 06:04:18PM +0800, sean.w...@mediatek.com wrote:
> > From: Sean Wang 
> > 
> > MODULE_ALIAS exports information to allow the module to be auto-loaded at
> > boot for the drivers registered using legacy platform registration.
> > 
> > However, currently the driver is always used by DT-only platform,
> > MODULE_ALIAS is redundant and should be removed properly.
> 
> There is no meaningful cost to having the alias there, why remove it?


It's no explicitly improved to the driver, it only takes less bytes
in .secion modinfo from 644 bytes to 620 bytes in the observation with
objdump.

More importantly, it seems the driver should probably refer to another
qualified driver at the initial development, but MODULE_ALIAS is
forgetting to be removed to meet real usage for the module autoload. 

Even I was curious whether people for the driver know the actual purpose
the macro it's for.

Removing MODULE_ALIAS is just to correct the driver to where it should
be.

Re: [PATCH] spi: mediatek: remove redundant MODULE_ALIAS entries

[Sean Wang]

On Wed, 2018-03-28 at 10:14 +0800, Mark Brown wrote:
> On Mon, Mar 26, 2018 at 06:04:18PM +0800, sean.w...@mediatek.com wrote:
> > From: Sean Wang 
> > 
> > MODULE_ALIAS exports information to allow the module to be auto-loaded at
> > boot for the drivers registered using legacy platform registration.
> > 
> > However, currently the driver is always used by DT-only platform,
> > MODULE_ALIAS is redundant and should be removed properly.
> 
> There is no meaningful cost to having the alias there, why remove it?


It's no explicitly improved to the driver, it only takes less bytes
in .secion modinfo from 644 bytes to 620 bytes in the observation with
objdump.

More importantly, it seems the driver should probably refer to another
qualified driver at the initial development, but MODULE_ALIAS is
forgetting to be removed to meet real usage for the module autoload. 

Even I was curious whether people for the driver know the actual purpose
the macro it's for.

Removing MODULE_ALIAS is just to correct the driver to where it should
be.

[PATCH v3 0/3] ARM64: dts: meson-axg: add AO clock driver

[Yixun Lan]

  These patches try to add AO clock driver for the Amlogic's
AXG SoC, while adding this driver, we found the pclk for uart AO
controller was wrong, so fix it here.
  Note, this patch actually depend on AO clock driver[2]
Please take these patches untill AO clock driver is merged.

changes since v2 at [1]: 
 - previous the DTS patches sit in two seperate series, merge them
   into one patch set.
 
changes since v1 at [0]: 
 -

[0] https://lkml.kernel.org/r/20180209070026.193879-1-yixun@amlogic.com
[1] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
  https://lkml.kernel.org/r/20180326081809.49493-4-yixun@amlogic.com
[2] https://lkml.kernel.org/r/20180328025050.221585-1-yixun@amlogic.com


Qiufang Dai (1):
  arm64: dts: meson-axg: add AO clock driver DT info

Yixun Lan (2):
  ARM64: dts: meson-axg: add an 32K alt aoclk
  ARM64: dts: meson: fix clock source of the pclk for UART_AO

 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 23 +--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |  4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  |  4 ++--
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.15.1

[PATCH v3 0/3] ARM64: dts: meson-axg: add AO clock driver

[Yixun Lan]

  These patches try to add AO clock driver for the Amlogic's
AXG SoC, while adding this driver, we found the pclk for uart AO
controller was wrong, so fix it here.
  Note, this patch actually depend on AO clock driver[2]
Please take these patches untill AO clock driver is merged.

changes since v2 at [1]: 
 - previous the DTS patches sit in two seperate series, merge them
   into one patch set.
 
changes since v1 at [0]: 
 -

[0] https://lkml.kernel.org/r/20180209070026.193879-1-yixun@amlogic.com
[1] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
  https://lkml.kernel.org/r/20180326081809.49493-4-yixun@amlogic.com
[2] https://lkml.kernel.org/r/20180328025050.221585-1-yixun@amlogic.com


Qiufang Dai (1):
  arm64: dts: meson-axg: add AO clock driver DT info

Yixun Lan (2):
  ARM64: dts: meson-axg: add an 32K alt aoclk
  ARM64: dts: meson: fix clock source of the pclk for UART_AO

 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 23 +--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |  4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  |  4 ++--
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.15.1

[PATCH v3 3/3] ARM64: dts: meson: fix clock source of the pclk for UART_AO

[Yixun Lan]

>From the hardware perspective, the actual pclk of the AO uarts
is the corresponding clkc_ao uart gate, not the main clock controller clk81.
This was not problem so far, because the uart_gate had
the CLK_IGNORE_UNUSED flag, which kept the gate open.

We plan to remove the CLK_IGNORE_UNUSED flag in another patch,
but before doing that, we need to fix the clock in the DTS file.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 40ca49fb94a6..dc6ca0895f11 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -798,7 +798,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x3000 0x0 0x18>;
interrupts = ;
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, 
<>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
@@ -807,7 +807,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x4000 0x0 0x18>;
interrupts = ;
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, 
<>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 562c26a0ba33..ea6898f62044 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -749,12 +749,12 @@
 };
 
 _AO {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
 _AO_B {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index e1a39cbed8c9..472106ce9a75 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -697,12 +697,12 @@
 };
 
 _AO {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
 _AO_B {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
-- 
2.15.1

[PATCH v3 2/3] ARM64: dts: meson-axg: add an 32K alt aoclk

[Yixun Lan]

The ao_clk81 in AO domain have two clock source,
one from a 32K alt crystal we name it as ao_alt_clk,
another is the clk81 signal from EE domain.

Acked-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index b0eff7d7f771..40ca49fb94a6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -108,6 +108,13 @@
#clock-cells = <0>;
};
 
+   ao_alt_xtal: ao_alt_xtal-clk {
+   compatible = "fixed-clock";
+   clock-frequency = <3200>;
+   clock-output-names = "ao_alt_xtal";
+   #clock-cells = <0>;
+   };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
-- 
2.15.1

[PATCH v3 1/3] arm64: dts: meson-axg: add AO clock driver DT info

[Yixun Lan]

From: Qiufang Dai 

This add the AO (Always-On part) clock DT info for Meson-AXG SoC

Signed-off-by: Qiufang Dai 
Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index b58808eb3cc8..b0eff7d7f771 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 / {
compatible = "amlogic,meson-axg";
@@ -688,6 +689,17 @@
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xff80 0x0 0x10>;
 
+   sysctrl_AO: sys-ctrl@0 {
+   compatible = "amlogic,meson-axg-ao-sysctrl", 
"syscon", "simple-mfd";
+   reg =  <0x0 0x0 0x0 0x100>;
+
+   clkc_AO: clock-controller {
+   compatible = "amlogic,meson-axg-aoclkc";
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+   };
+
pinctrl_aobus: pinctrl@14 {
compatible = "amlogic,meson-axg-aobus-pinctrl";
#address-cells = <2>;
-- 
2.15.1

[PATCH v3 3/3] ARM64: dts: meson: fix clock source of the pclk for UART_AO

[Yixun Lan]

>From the hardware perspective, the actual pclk of the AO uarts
is the corresponding clkc_ao uart gate, not the main clock controller clk81.
This was not problem so far, because the uart_gate had
the CLK_IGNORE_UNUSED flag, which kept the gate open.

We plan to remove the CLK_IGNORE_UNUSED flag in another patch,
but before doing that, we need to fix the clock in the DTS file.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 40ca49fb94a6..dc6ca0895f11 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -798,7 +798,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x3000 0x0 0x18>;
interrupts = ;
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, 
<>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
@@ -807,7 +807,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x4000 0x0 0x18>;
interrupts = ;
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, 
<>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 562c26a0ba33..ea6898f62044 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -749,12 +749,12 @@
 };
 
 _AO {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
 _AO_B {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index e1a39cbed8c9..472106ce9a75 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -697,12 +697,12 @@
 };
 
 _AO {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART1>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
 _AO_B {
-   clocks = <>, < CLKID_CLK81>, <>;
+   clocks = <>, <_AO CLKID_AO_UART2>, <>;
clock-names = "xtal", "pclk", "baud";
 };
 
-- 
2.15.1

[PATCH v3 2/3] ARM64: dts: meson-axg: add an 32K alt aoclk

[Yixun Lan]

The ao_clk81 in AO domain have two clock source,
one from a 32K alt crystal we name it as ao_alt_clk,
another is the clk81 signal from EE domain.

Acked-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index b0eff7d7f771..40ca49fb94a6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -108,6 +108,13 @@
#clock-cells = <0>;
};
 
+   ao_alt_xtal: ao_alt_xtal-clk {
+   compatible = "fixed-clock";
+   clock-frequency = <3200>;
+   clock-output-names = "ao_alt_xtal";
+   #clock-cells = <0>;
+   };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
-- 
2.15.1

[PATCH v3 1/3] arm64: dts: meson-axg: add AO clock driver DT info

[Yixun Lan]

From: Qiufang Dai 

This add the AO (Always-On part) clock DT info for Meson-AXG SoC

Signed-off-by: Qiufang Dai 
Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index b58808eb3cc8..b0eff7d7f771 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 / {
compatible = "amlogic,meson-axg";
@@ -688,6 +689,17 @@
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xff80 0x0 0x10>;
 
+   sysctrl_AO: sys-ctrl@0 {
+   compatible = "amlogic,meson-axg-ao-sysctrl", 
"syscon", "simple-mfd";
+   reg =  <0x0 0x0 0x0 0x100>;
+
+   clkc_AO: clock-controller {
+   compatible = "amlogic,meson-axg-aoclkc";
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+   };
+
pinctrl_aobus: pinctrl@14 {
compatible = "amlogic,meson-axg-aobus-pinctrl";
#address-cells = <2>;
-- 
2.15.1

[PATCH v3 5/6] clk: meson: drop CLK_SET_RATE_PARENT flag

[Yixun Lan]

The clk81 is not expected to be changed, so drop this flag.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/gxbb-aoclk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 5a922639a264..89b048aad239 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,7 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
+   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH v3 5/6] clk: meson: drop CLK_SET_RATE_PARENT flag

[Yixun Lan]

The clk81 is not expected to be changed, so drop this flag.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/gxbb-aoclk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 5a922639a264..89b048aad239 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,7 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
+   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH v3 6/6] clk: meson: drop CLK_IGNORE_UNUSED flag

[Yixun Lan]

Rely on drivers to request the clock explicitly.

Previous the kernel will leave the clock on while
bootloader adready initilized the clock, this wasn't
optimal way, so fix it here.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/axg-aoclk.c  | 1 -
 drivers/clk/meson/gxbb-aoclk.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index b11cc4450570..89f894f41eac 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -27,7 +27,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 89b048aad239..f646d7527c27 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH v3 6/6] clk: meson: drop CLK_IGNORE_UNUSED flag

[Yixun Lan]

Rely on drivers to request the clock explicitly.

Previous the kernel will leave the clock on while
bootloader adready initilized the clock, this wasn't
optimal way, so fix it here.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/axg-aoclk.c  | 1 -
 drivers/clk/meson/gxbb-aoclk.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index b11cc4450570..89f894f41eac 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -27,7 +27,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 89b048aad239..f646d7527c27 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = _regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH v3 3/6] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings

[Yixun Lan]

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 include/dt-bindings/clock/axg-aoclkc.h | 26 ++
 include/dt-bindings/reset/axg-aoclkc.h | 20 
 2 files changed, 46 insertions(+)
 create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
 create mode 100644 include/dt-bindings/reset/axg-aoclkc.h

diff --git a/include/dt-bindings/clock/axg-aoclkc.h 
b/include/dt-bindings/clock/axg-aoclkc.h
new file mode 100644
index ..61955016a55b
--- /dev/null
+++ b/include/dt-bindings/clock/axg-aoclkc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+
+#define CLKID_AO_REMOTE0
+#define CLKID_AO_I2C_MASTER1
+#define CLKID_AO_I2C_SLAVE 2
+#define CLKID_AO_UART1 3
+#define CLKID_AO_UART2 4
+#define CLKID_AO_IR_BLASTER5
+#define CLKID_AO_SAR_ADC   6
+#define CLKID_AO_CLK81 7
+#define CLKID_AO_SAR_ADC_SEL   8
+#define CLKID_AO_SAR_ADC_DIV   9
+#define CLKID_AO_SAR_ADC_CLK   10
+#define CLKID_AO_ALT_XTAL  11
+
+#endif
diff --git a/include/dt-bindings/reset/axg-aoclkc.h 
b/include/dt-bindings/reset/axg-aoclkc.h
new file mode 100644
index ..d342c0b6b2a7
--- /dev/null
+++ b/include/dt-bindings/reset/axg-aoclkc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+
+#define RESET_AO_REMOTE0
+#define RESET_AO_I2C_MASTER1
+#define RESET_AO_I2C_SLAVE 2
+#define RESET_AO_UART1 3
+#define RESET_AO_UART2 4
+#define RESET_AO_IR_BLASTER5
+
+#endif
-- 
2.15.1

[PATCH v3 3/6] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings

[Yixun Lan]

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 include/dt-bindings/clock/axg-aoclkc.h | 26 ++
 include/dt-bindings/reset/axg-aoclkc.h | 20 
 2 files changed, 46 insertions(+)
 create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
 create mode 100644 include/dt-bindings/reset/axg-aoclkc.h

diff --git a/include/dt-bindings/clock/axg-aoclkc.h 
b/include/dt-bindings/clock/axg-aoclkc.h
new file mode 100644
index ..61955016a55b
--- /dev/null
+++ b/include/dt-bindings/clock/axg-aoclkc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+
+#define CLKID_AO_REMOTE0
+#define CLKID_AO_I2C_MASTER1
+#define CLKID_AO_I2C_SLAVE 2
+#define CLKID_AO_UART1 3
+#define CLKID_AO_UART2 4
+#define CLKID_AO_IR_BLASTER5
+#define CLKID_AO_SAR_ADC   6
+#define CLKID_AO_CLK81 7
+#define CLKID_AO_SAR_ADC_SEL   8
+#define CLKID_AO_SAR_ADC_DIV   9
+#define CLKID_AO_SAR_ADC_CLK   10
+#define CLKID_AO_ALT_XTAL  11
+
+#endif
diff --git a/include/dt-bindings/reset/axg-aoclkc.h 
b/include/dt-bindings/reset/axg-aoclkc.h
new file mode 100644
index ..d342c0b6b2a7
--- /dev/null
+++ b/include/dt-bindings/reset/axg-aoclkc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+
+#define RESET_AO_REMOTE0
+#define RESET_AO_I2C_MASTER1
+#define RESET_AO_I2C_SLAVE 2
+#define RESET_AO_UART1 3
+#define RESET_AO_UART2 4
+#define RESET_AO_IR_BLASTER5
+
+#endif
-- 
2.15.1

[PATCH v3 2/6] dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC

[Yixun Lan]

Update the dt-binding documentation to support new compatible string
for the Amlogic's Meson-AXG SoC.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt 
b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
index 786dc39ca904..3a880528030e 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
@@ -9,6 +9,7 @@ Required Properties:
- GXBB (S905) : "amlogic,meson-gxbb-aoclkc"
- GXL (S905X, S905D) : "amlogic,meson-gxl-aoclkc"
- GXM (S912) : "amlogic,meson-gxm-aoclkc"
+   - AXG (A113D, A113X) : "amlogic,meson-axg-aoclkc"
followed by the common "amlogic,meson-gx-aoclkc"
 
 - #clock-cells: should be 1.
-- 
2.15.1

[PATCH v3 0/6] clk: meson-axg: Add AO Cloclk and Reset driver

[Yixun Lan]

  This patch try to add AO clock and Reset driver for Amlogic's
Meson-AXG SoC.
  Please note that patch 6 need to wait for the DTS changes[3] merged
into mainline first, otherwise it will break the serial console.

 patch 1: factor the common code into a dedicated file
 patch 2-4: add the aoclk driver for AXG SoC
 patch 5-6: drop unnecessary clock flags

changes since v2 at [2]: 
 - rework meson_aoclkc_probe() which leverage the of_match_data
 - merge patch 5-6 into this series
 - seperate DTS patch, will send to Kevin Hilman independently
 
changes since v1 at [0]: 
 - rebase to clk-meson's branch 'next/drivers' [1]
 - fix license, update to BSD-3-Clause
 - drop un-used include header file

[0] https://lkml.kernel.org/r/20180209070026.193879-1-yixun@amlogic.com
[1] git://github.com/BayLibre/clk-meson.git branch: next-drivers
[2] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
[3] https://lkml.kernel.org/r/20180326081809.49493-4-yixun@amlogic.com

Qiufang Dai (1):
  clk: meson-axg: Add AO Clock and Reset controller driver

Yixun Lan (5):
  clk: meson: aoclk: refactor common code into dedicated file
  dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC
  dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
  clk: meson: drop CLK_SET_RATE_PARENT flag
  clk: meson: drop CLK_IGNORE_UNUSED flag

 .../bindings/clock/amlogic,gxbb-aoclkc.txt |   1 +
 drivers/clk/meson/Makefile |   4 +-
 drivers/clk/meson/axg-aoclk.c  | 163 +
 drivers/clk/meson/axg-aoclk.h  |  31 
 drivers/clk/meson/gxbb-aoclk.c |  88 ---
 drivers/clk/meson/gxbb-aoclk.h |   7 +
 drivers/clk/meson/meson-aoclk.c|  79 ++
 drivers/clk/meson/meson-aoclk.h|  35 +
 include/dt-bindings/clock/axg-aoclkc.h |  26 
 include/dt-bindings/reset/axg-aoclkc.h |  20 +++
 10 files changed, 393 insertions(+), 61 deletions(-)
 create mode 100644 drivers/clk/meson/axg-aoclk.c
 create mode 100644 drivers/clk/meson/axg-aoclk.h
 create mode 100644 drivers/clk/meson/meson-aoclk.c
 create mode 100644 drivers/clk/meson/meson-aoclk.h
 create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
 create mode 100644 include/dt-bindings/reset/axg-aoclkc.h

-- 
2.15.1

[PATCH v3 2/6] dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC

[Yixun Lan]

Update the dt-binding documentation to support new compatible string
for the Amlogic's Meson-AXG SoC.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt 
b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
index 786dc39ca904..3a880528030e 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
@@ -9,6 +9,7 @@ Required Properties:
- GXBB (S905) : "amlogic,meson-gxbb-aoclkc"
- GXL (S905X, S905D) : "amlogic,meson-gxl-aoclkc"
- GXM (S912) : "amlogic,meson-gxm-aoclkc"
+   - AXG (A113D, A113X) : "amlogic,meson-axg-aoclkc"
followed by the common "amlogic,meson-gx-aoclkc"
 
 - #clock-cells: should be 1.
-- 
2.15.1

[PATCH v3 0/6] clk: meson-axg: Add AO Cloclk and Reset driver

[Yixun Lan]

  This patch try to add AO clock and Reset driver for Amlogic's
Meson-AXG SoC.
  Please note that patch 6 need to wait for the DTS changes[3] merged
into mainline first, otherwise it will break the serial console.

 patch 1: factor the common code into a dedicated file
 patch 2-4: add the aoclk driver for AXG SoC
 patch 5-6: drop unnecessary clock flags

changes since v2 at [2]: 
 - rework meson_aoclkc_probe() which leverage the of_match_data
 - merge patch 5-6 into this series
 - seperate DTS patch, will send to Kevin Hilman independently
 
changes since v1 at [0]: 
 - rebase to clk-meson's branch 'next/drivers' [1]
 - fix license, update to BSD-3-Clause
 - drop un-used include header file

[0] https://lkml.kernel.org/r/20180209070026.193879-1-yixun@amlogic.com
[1] git://github.com/BayLibre/clk-meson.git branch: next-drivers
[2] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
[3] https://lkml.kernel.org/r/20180326081809.49493-4-yixun@amlogic.com

Qiufang Dai (1):
  clk: meson-axg: Add AO Clock and Reset controller driver

Yixun Lan (5):
  clk: meson: aoclk: refactor common code into dedicated file
  dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC
  dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
  clk: meson: drop CLK_SET_RATE_PARENT flag
  clk: meson: drop CLK_IGNORE_UNUSED flag

 .../bindings/clock/amlogic,gxbb-aoclkc.txt |   1 +
 drivers/clk/meson/Makefile |   4 +-
 drivers/clk/meson/axg-aoclk.c  | 163 +
 drivers/clk/meson/axg-aoclk.h  |  31 
 drivers/clk/meson/gxbb-aoclk.c |  88 ---
 drivers/clk/meson/gxbb-aoclk.h |   7 +
 drivers/clk/meson/meson-aoclk.c|  79 ++
 drivers/clk/meson/meson-aoclk.h|  35 +
 include/dt-bindings/clock/axg-aoclkc.h |  26 
 include/dt-bindings/reset/axg-aoclkc.h |  20 +++
 10 files changed, 393 insertions(+), 61 deletions(-)
 create mode 100644 drivers/clk/meson/axg-aoclk.c
 create mode 100644 drivers/clk/meson/axg-aoclk.h
 create mode 100644 drivers/clk/meson/meson-aoclk.c
 create mode 100644 drivers/clk/meson/meson-aoclk.h
 create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
 create mode 100644 include/dt-bindings/reset/axg-aoclkc.h

-- 
2.15.1

[PATCH v3 4/6] clk: meson-axg: Add AO Clock and Reset controller driver

[Yixun Lan]

From: Qiufang Dai 

Adds a Clock and Reset controller driver for the Always-On part
of the Amlogic Meson-AXG SoC.

Signed-off-by: Qiufang Dai 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile|   2 +-
 drivers/clk/meson/axg-aoclk.c | 164 ++
 drivers/clk/meson/axg-aoclk.h |  31 
 3 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/meson/axg-aoclk.c
 create mode 100644 drivers/clk/meson/axg-aoclk.h

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 555ab9c6ab64..fa6d1e36cef6 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -5,5 +5,5 @@
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
 obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o meson-aoclk.o gxbb-aoclk.o 
gxbb-aoclk-32k.o
-obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o
+obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o meson-aoclk.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
new file mode 100644
index ..b11cc4450570
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-AXG Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clkc.h"
+#include "axg-aoclk.h"
+
+#define AXG_AO_GATE(_name, _bit)   \
+static struct clk_regmap _name##_ao = {
\
+   .data = &(struct clk_regmap_gate_data) {\
+   .offset = (AO_RTI_GEN_CNTL_REG0),   \
+   .bit_idx = (_bit),  \
+   },  \
+   .hw.init = &(struct clk_init_data) {\
+   .name = #_name "_ao",   \
+   .ops = _regmap_gate_ops,\
+   .parent_names = (const char *[]){ "clk81" },\
+   .num_parents = 1,   \
+   .flags = CLK_IGNORE_UNUSED, \
+   },  \
+}
+
+AXG_AO_GATE(remote, 0);
+AXG_AO_GATE(i2c_master, 1);
+AXG_AO_GATE(i2c_slave, 2);
+AXG_AO_GATE(uart1, 3);
+AXG_AO_GATE(uart2, 5);
+AXG_AO_GATE(ir_blaster, 6);
+AXG_AO_GATE(saradc, 7);
+
+static struct clk_regmap ao_clk81 = {
+   .data = &(struct clk_regmap_mux_data) {
+   .offset = AO_RTI_PWR_CNTL_REG0,
+   .mask = 0x1,
+   .shift = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "ao_clk81",
+   .ops = _regmap_mux_ro_ops,
+   .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+   .num_parents = 2,
+   },
+};
+
+static struct clk_regmap axg_saradc_mux = {
+   .data = &(struct clk_regmap_mux_data) {
+   .offset = AO_SAR_CLK,
+   .mask = 0x3,
+   .shift = 9,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_mux",
+   .ops = _regmap_mux_ops,
+   .parent_names = (const char *[]){ "xtal", "ao_clk81" },
+   .num_parents = 2,
+   },
+};
+
+static struct clk_regmap axg_saradc_div = {
+   .data = &(struct clk_regmap_div_data) {
+   .offset = AO_SAR_CLK,
+   .shift = 0,
+   .width = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_div",
+   .ops = _regmap_divider_ops,
+   .parent_names = (const char *[]){ "axg_saradc_mux" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap axg_saradc_gate = {
+   .data = &(struct clk_regmap_gate_data) {
+   .offset = AO_SAR_CLK,
+   .bit_idx = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_gate",
+   .ops = _regmap_gate_ops,
+   .parent_names = (const char *[]){ "axg_saradc_div" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static unsigned int axg_aoclk_reset[] = {
+   [RESET_AO_REMOTE] = 16,
+   [RESET_AO_I2C_MASTER] = 18,
+   [RESET_AO_I2C_SLAVE] = 19,
+   [RESET_AO_UART1] = 17,
+   [RESET_AO_UART2] = 22,
+   [RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct 

[PATCH v3 4/6] clk: meson-axg: Add AO Clock and Reset controller driver

[Yixun Lan]

From: Qiufang Dai 

Adds a Clock and Reset controller driver for the Always-On part
of the Amlogic Meson-AXG SoC.

Signed-off-by: Qiufang Dai 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile|   2 +-
 drivers/clk/meson/axg-aoclk.c | 164 ++
 drivers/clk/meson/axg-aoclk.h |  31 
 3 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/meson/axg-aoclk.c
 create mode 100644 drivers/clk/meson/axg-aoclk.h

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 555ab9c6ab64..fa6d1e36cef6 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -5,5 +5,5 @@
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
 obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o meson-aoclk.o gxbb-aoclk.o 
gxbb-aoclk-32k.o
-obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o
+obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o meson-aoclk.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
new file mode 100644
index ..b11cc4450570
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-AXG Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clkc.h"
+#include "axg-aoclk.h"
+
+#define AXG_AO_GATE(_name, _bit)   \
+static struct clk_regmap _name##_ao = {
\
+   .data = &(struct clk_regmap_gate_data) {\
+   .offset = (AO_RTI_GEN_CNTL_REG0),   \
+   .bit_idx = (_bit),  \
+   },  \
+   .hw.init = &(struct clk_init_data) {\
+   .name = #_name "_ao",   \
+   .ops = _regmap_gate_ops,\
+   .parent_names = (const char *[]){ "clk81" },\
+   .num_parents = 1,   \
+   .flags = CLK_IGNORE_UNUSED, \
+   },  \
+}
+
+AXG_AO_GATE(remote, 0);
+AXG_AO_GATE(i2c_master, 1);
+AXG_AO_GATE(i2c_slave, 2);
+AXG_AO_GATE(uart1, 3);
+AXG_AO_GATE(uart2, 5);
+AXG_AO_GATE(ir_blaster, 6);
+AXG_AO_GATE(saradc, 7);
+
+static struct clk_regmap ao_clk81 = {
+   .data = &(struct clk_regmap_mux_data) {
+   .offset = AO_RTI_PWR_CNTL_REG0,
+   .mask = 0x1,
+   .shift = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "ao_clk81",
+   .ops = _regmap_mux_ro_ops,
+   .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+   .num_parents = 2,
+   },
+};
+
+static struct clk_regmap axg_saradc_mux = {
+   .data = &(struct clk_regmap_mux_data) {
+   .offset = AO_SAR_CLK,
+   .mask = 0x3,
+   .shift = 9,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_mux",
+   .ops = _regmap_mux_ops,
+   .parent_names = (const char *[]){ "xtal", "ao_clk81" },
+   .num_parents = 2,
+   },
+};
+
+static struct clk_regmap axg_saradc_div = {
+   .data = &(struct clk_regmap_div_data) {
+   .offset = AO_SAR_CLK,
+   .shift = 0,
+   .width = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_div",
+   .ops = _regmap_divider_ops,
+   .parent_names = (const char *[]){ "axg_saradc_mux" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap axg_saradc_gate = {
+   .data = &(struct clk_regmap_gate_data) {
+   .offset = AO_SAR_CLK,
+   .bit_idx = 8,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "axg_saradc_gate",
+   .ops = _regmap_gate_ops,
+   .parent_names = (const char *[]){ "axg_saradc_div" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static unsigned int axg_aoclk_reset[] = {
+   [RESET_AO_REMOTE] = 16,
+   [RESET_AO_I2C_MASTER] = 18,
+   [RESET_AO_I2C_SLAVE] = 19,
+   [RESET_AO_UART1] = 17,
+   [RESET_AO_UART2] = 22,
+   [RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct clk_regmap *axg_aoclk_regmap[] = {
+   [CLKID_AO_REMOTE]   = _ao,
+   [CLKID_AO_I2C_MASTER]   = _master_ao,
+  

[PATCH v3 1/6] clk: meson: aoclk: refactor common code into dedicated file

[Yixun Lan]

We try to refactor the common code into one dedicated file,
while preparing to add new Meson-AXG aoclk driver, this would
help us to better share the code by all aoclk drivers.

Suggested-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile  |  2 +-
 drivers/clk/meson/gxbb-aoclk.c  | 87 ++---
 drivers/clk/meson/gxbb-aoclk.h  |  7 
 drivers/clk/meson/meson-aoclk.c | 79 +
 drivers/clk/meson/meson-aoclk.h | 35 +
 5 files changed, 151 insertions(+), 59 deletions(-)
 create mode 100644 drivers/clk/meson/meson-aoclk.c
 create mode 100644 drivers/clk/meson/meson-aoclk.h

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index ffee82e60b7a..555ab9c6ab64 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,6 +4,6 @@
 
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
+obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o meson-aoclk.o gxbb-aoclk.o 
gxbb-aoclk-32k.o
 obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 9ec23ae9a219..5a922639a264 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -52,39 +52,13 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-#include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
-#include 
-#include 
 #include "clk-regmap.h"
 #include "gxbb-aoclk.h"
 
-struct gxbb_aoclk_reset_controller {
-   struct reset_controller_dev reset;
-   unsigned int *data;
-   struct regmap *regmap;
-};
-
-static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
-  unsigned long id)
-{
-   struct gxbb_aoclk_reset_controller *reset =
-   container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
-
-   return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
-   BIT(reset->data[id]));
-}
-
-static const struct reset_control_ops gxbb_aoclk_reset_ops = {
-   .reset = gxbb_aoclk_do_reset,
-};
-
 #define GXBB_AO_GATE(_name, _bit)  \
 static struct clk_regmap _name##_ao = {
\
.data = &(struct clk_regmap_gate_data) {\
@@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data 
= {
[CLKID_AO_IR_BLASTER] = _blaster_ao.hw,
[CLKID_AO_CEC_32K] = _32k_ao.hw,
},
-   .num = 7,
+   .num = NR_CLKS,
 };
 
-static int gxbb_aoclkc_probe(struct platform_device *pdev)
+static int gxbb_aoclkc_register_specific_clk(
+   struct platform_device *pdev)
 {
-   struct gxbb_aoclk_reset_controller *rstc;
struct device *dev = >dev;
struct regmap *regmap;
-   int ret, clkid;
-
-   rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
-   if (!rstc)
-   return -ENOMEM;
+   int ret;
 
regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
if (IS_ERR(regmap)) {
@@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
-   /* Reset Controller */
-   rstc->regmap = regmap;
-   rstc->data = gxbb_aoclk_reset;
-   rstc->reset.ops = _aoclk_reset_ops;
-   rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
-   rstc->reset.of_node = dev->of_node;
-   ret = devm_reset_controller_register(dev, >reset);
-
-   /*
-* Populate regmap and register all clks
-*/
-   for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
-   gxbb_aoclk_gate[clkid]->map = regmap;
-
-   ret = devm_clk_hw_register(dev,
-  gxbb_aoclk_onecell_data.hws[clkid]);
-   if (ret)
-   return ret;
-   }
-
/* Specific clocks */
cec_32k_ao.regmap = regmap;
ret = devm_clk_hw_register(dev, _32k_ao.hw);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "clk cec_32k_ao register failed.\n");
return ret;
+   }
+
+   return 0;
+}
+
+static struct meson_aoclk_data gxbb_aoclkc_data = {
+   .reset_reg  = AO_RTI_GEN_CNTL_REG0,
+   .num_reset  = ARRAY_SIZE(gxbb_aoclk_reset),
+   .reset  = gxbb_aoclk_reset,
+   .num_clks   = ARRAY_SIZE(gxbb_aoclk_gate),
+   .clks   = gxbb_aoclk_gate,
+   .hw_data= _aoclk_onecell_data,
+};
+
+static int gxbb_aoclkc_probe(struct 

[PATCH v3 1/6] clk: meson: aoclk: refactor common code into dedicated file

[Yixun Lan]

We try to refactor the common code into one dedicated file,
while preparing to add new Meson-AXG aoclk driver, this would
help us to better share the code by all aoclk drivers.

Suggested-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile  |  2 +-
 drivers/clk/meson/gxbb-aoclk.c  | 87 ++---
 drivers/clk/meson/gxbb-aoclk.h  |  7 
 drivers/clk/meson/meson-aoclk.c | 79 +
 drivers/clk/meson/meson-aoclk.h | 35 +
 5 files changed, 151 insertions(+), 59 deletions(-)
 create mode 100644 drivers/clk/meson/meson-aoclk.c
 create mode 100644 drivers/clk/meson/meson-aoclk.h

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index ffee82e60b7a..555ab9c6ab64 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,6 +4,6 @@
 
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
+obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o meson-aoclk.o gxbb-aoclk.o 
gxbb-aoclk-32k.o
 obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 9ec23ae9a219..5a922639a264 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -52,39 +52,13 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-#include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
-#include 
-#include 
 #include "clk-regmap.h"
 #include "gxbb-aoclk.h"
 
-struct gxbb_aoclk_reset_controller {
-   struct reset_controller_dev reset;
-   unsigned int *data;
-   struct regmap *regmap;
-};
-
-static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
-  unsigned long id)
-{
-   struct gxbb_aoclk_reset_controller *reset =
-   container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
-
-   return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
-   BIT(reset->data[id]));
-}
-
-static const struct reset_control_ops gxbb_aoclk_reset_ops = {
-   .reset = gxbb_aoclk_do_reset,
-};
-
 #define GXBB_AO_GATE(_name, _bit)  \
 static struct clk_regmap _name##_ao = {
\
.data = &(struct clk_regmap_gate_data) {\
@@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data 
= {
[CLKID_AO_IR_BLASTER] = _blaster_ao.hw,
[CLKID_AO_CEC_32K] = _32k_ao.hw,
},
-   .num = 7,
+   .num = NR_CLKS,
 };
 
-static int gxbb_aoclkc_probe(struct platform_device *pdev)
+static int gxbb_aoclkc_register_specific_clk(
+   struct platform_device *pdev)
 {
-   struct gxbb_aoclk_reset_controller *rstc;
struct device *dev = >dev;
struct regmap *regmap;
-   int ret, clkid;
-
-   rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
-   if (!rstc)
-   return -ENOMEM;
+   int ret;
 
regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
if (IS_ERR(regmap)) {
@@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
-   /* Reset Controller */
-   rstc->regmap = regmap;
-   rstc->data = gxbb_aoclk_reset;
-   rstc->reset.ops = _aoclk_reset_ops;
-   rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
-   rstc->reset.of_node = dev->of_node;
-   ret = devm_reset_controller_register(dev, >reset);
-
-   /*
-* Populate regmap and register all clks
-*/
-   for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
-   gxbb_aoclk_gate[clkid]->map = regmap;
-
-   ret = devm_clk_hw_register(dev,
-  gxbb_aoclk_onecell_data.hws[clkid]);
-   if (ret)
-   return ret;
-   }
-
/* Specific clocks */
cec_32k_ao.regmap = regmap;
ret = devm_clk_hw_register(dev, _32k_ao.hw);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "clk cec_32k_ao register failed.\n");
return ret;
+   }
+
+   return 0;
+}
+
+static struct meson_aoclk_data gxbb_aoclkc_data = {
+   .reset_reg  = AO_RTI_GEN_CNTL_REG0,
+   .num_reset  = ARRAY_SIZE(gxbb_aoclk_reset),
+   .reset  = gxbb_aoclk_reset,
+   .num_clks   = ARRAY_SIZE(gxbb_aoclk_gate),
+   .clks   = gxbb_aoclk_gate,
+   .hw_data= _aoclk_onecell_data,
+};
+
+static int gxbb_aoclkc_probe(struct platform_device *pdev)
+{
+   if 

​​Hi

[Mrs Alaniz Maria]

Hi,
How are you doing? I am Mrs Alaniz Maria.Please I have urgent issue to discus 
with you.

​​Hi

[Mrs Alaniz Maria]

Hi,
How are you doing? I am Mrs Alaniz Maria.Please I have urgent issue to discus 
with you.

Re: [alsa-devel][PATCH 3/3 v3] ASoC: TSCS42xx: Add CCF support to get sysclk

[Mark Brown]

On Tue, Mar 27, 2018 at 12:50:49PM -0500, Steven Eckhoff wrote:
> The TSCS42xx relies on set_sysclk to get a unique clock id and rate,
> which
> prevents it from being used with the simple-card.

I'm missing patches 1 and 2 of this resend.


signature.asc
Description: PGP signature

Re: 答复: Re: [PATCH] scsi: Replace sdev_printk with printk_deferred to avoid

[Sergey Senozhatsky]

On (03/28/18 10:29), wen.yan...@zte.com.cn wrote:
>Hello Bart,
>
>We have a detailed discussion of the problem.
>Sergey Senozhatsky, Petr and many people have made a lot of efforts about
>it.
>Please see this link:
>https://bugzilla.kernel.org/show_bug.cgi?id=199003
>
>1, Petr suggests that it should be modified in this way:
>IMHO, printing the same message so many times is useless. Therefore
>some throttling would make sense. If we want to keep sdev_printk(),

The thing with retelimiting is that - yes, we do less printks but we still
do them under queue spin_lock.

So I was thinking about something like below [a quick-n-dirty workaround]

---

 drivers/scsi/scsi_lib.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0dfec0dedd5e..6c930fbdd24c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1825,9 +1825,13 @@ static void scsi_request_fn(struct request_queue *q)
break;
 
if (unlikely(!scsi_device_online(sdev))) {
+   scsi_kill_request(req, q);
+   spin_unlock_irq(q->queue_lock);
+
sdev_printk(KERN_ERR, sdev,
"rejecting I/O to offline device\n");
-   scsi_kill_request(req, q);
+
+   spin_lock_irq(q->queue_lock);
continue;
}