Netdev List
 help / color / mirror / Atom feed
* Re: INFO: rcu detected stall in kfree_skbmem
From: Marcelo Ricardo Leitner @ 2018-05-13 16:02 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Eric Dumazet, syzbot, Vladislav Yasevich, Neil Horman, linux-sctp,
	Andrei Vagin, David Miller, Kirill Tkhai, LKML, netdev,
	syzkaller-bugs
In-Reply-To: <CACT4Y+ZG6igbhgimA=oFz4k=x_P4tSYGQgFk0EVrjznm8V8fLA@mail.gmail.com>

On Sun, May 13, 2018 at 03:52:01PM +0200, Dmitry Vyukov wrote:
> On Fri, May 11, 2018 at 10:42 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Fri, May 11, 2018 at 12:08:33PM -0700, Eric Dumazet wrote:
> >>
> >>
> >> On 05/11/2018 11:41 AM, Marcelo Ricardo Leitner wrote:
> >>
> >> > But calling ip6_xmit with rcu_read_lock is expected. tcp stack also
> >> > does it.
> >> > Thus I think this is more of an issue with IPv6 stack. If a host has
> >> > an extensive ip6tables ruleset, it probably generates this more
> >> > easily.
> >> >
> >> >>>  sctp_v6_xmit+0x4a5/0x6b0 net/sctp/ipv6.c:225
> >> >>>  sctp_packet_transmit+0x26f6/0x3ba0 net/sctp/output.c:650
> >> >>>  sctp_outq_flush+0x1373/0x4370 net/sctp/outqueue.c:1197
> >> >>>  sctp_outq_uncork+0x6a/0x80 net/sctp/outqueue.c:776
> >> >>>  sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1820 [inline]
> >> >>>  sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> >> >>>  sctp_do_sm+0x596/0x7160 net/sctp/sm_sideeffect.c:1191
> >> >>>  sctp_generate_heartbeat_event+0x218/0x450 net/sctp/sm_sideeffect.c:406
> >> >>>  call_timer_fn+0x230/0x940 kernel/time/timer.c:1326
> >> >>>  expire_timers kernel/time/timer.c:1363 [inline]
> >> >
> >> > Having this call from a timer means it wasn't processing sctp stack
> >> > for too long.
> >> >
> >>
> >> I feel the problem is that this part is looping, in some infinite loop.
> >>
> >> I have seen this stack traces in other reports.
> >
> > Checked mail history now, seems at least two other reports on RCU
> > stalls had sctp_generate_heartbeat_event involved.
> >
> >>
> >> Maybe some kind of list corruption.
> >
> > Could be.
> > Do we know if it generated a flood of packets?
>
> We only know what's in the bug reports. Do the other ones have

Ok.

> reproducers? It can make sense to mark them as duplicates to not have

No.

> a placer of open bugs about the same root cause.

They may have the same root cause, but right now I cannot tell for
sure.

^ permalink raw reply

* Re: [PATCH v2] {net, IB}/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'
From: Eric Dumazet @ 2018-05-13 16:21 UTC (permalink / raw)
  To: Christophe JAILLET, saeedm, matanb, leon, dledford, jgg, davem
  Cc: netdev, linux-rdma, linux-kernel, kernel-janitors
In-Reply-To: <20180513070041.24246-1-christophe.jaillet@wanadoo.fr>



On 05/13/2018 12:00 AM, Christophe JAILLET wrote:
> When 'kvzalloc()' is used to allocate memory, 'kvfree()' must be used to
> free it.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v1 -> v2: More places to update have been added to the patch


Please add relevant Fixes: tag(s)

^ permalink raw reply

* WARNING in iov_iter_revert
From: syzbot @ 2018-05-13 16:28 UTC (permalink / raw)
  To: aviadye, davejwatson, davem, ilyal, linux-kernel, netdev,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    427fbe89261d Merge branch 'next' of git://git.kernel.org/p..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16b33477800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
dashboard link: https://syzkaller.appspot.com/bug?extid=c226690f7b3126c5ee04
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=144f1997800000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=141d5417800000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c226690f7b3126c5ee04@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
WARNING: CPU: 1 PID: 4542 at lib/iov_iter.c:857 iov_iter_revert+0x2ee/0xaa0  
lib/iov_iter.c:857
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 4542 Comm: syz-executor650 Not tainted 4.17.0-rc4+ #44
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  panic+0x22f/0x4de kernel/panic.c:184
  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
  report_bug+0x252/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:iov_iter_revert+0x2ee/0xaa0 lib/iov_iter.c:857
RSP: 0018:ffff8801ad1bf700 EFLAGS: 00010293
RAX: ffff8801ac55e6c0 RBX: 00000000ffffffff RCX: ffffffff835104a1
RDX: 0000000000000000 RSI: ffffffff8351074e RDI: 0000000000000007
RBP: ffff8801ad1bf760 R08: ffff8801ac55e6c0 R09: ffffed003b5e46c2
R10: 0000000000000003 R11: 0000000000000001 R12: 0000000000000001
R13: ffff8801ad1bfd60 R14: 0000000000000011 R15: ffff8801ae9ac040
  tls_sw_sendmsg+0xf1c/0x12d0 net/tls/tls_sw.c:448
  inet_sendmsg+0x19f/0x690 net/ipv4/af_inet.c:798
  sock_sendmsg_nosec net/socket.c:629 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:639
  ___sys_sendmsg+0x805/0x940 net/socket.c:2117
  __sys_sendmsg+0x115/0x270 net/socket.c:2155
  __do_sys_sendmsg net/socket.c:2164 [inline]
  __se_sys_sendmsg net/socket.c:2162 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2162
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4403a9
RSP: 002b:00007ffdcdfbd6c8 EFLAGS: 00000207 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004403a9
RDX: 0000000000000000 RSI: 0000000020001340 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 000000000000001c R09: 000000000000001c
R10: 0000000020000180 R11: 0000000000000207 R12: 0000000000401cd0
R13: 0000000000401d60 R14: 0000000000000000 R15: 0000000000000000
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* KMSAN: uninit-value in tipc_conn_rcv_sub
From: syzbot @ 2018-05-13 16:38 UTC (permalink / raw)
  To: davem, jon.maloy, linux-kernel, netdev, syzkaller-bugs,
	tipc-discussion, ying.xue

Hello,

syzbot found the following crash on:

HEAD commit:    74ee2200b89f kmsan: bump .config.example to v4.17-rc3
git tree:       https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=12ab8637800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=4ca1e57bafa8ab1f
dashboard link: https://syzkaller.appspot.com/bug?extid=8951a3065ee7fd6d6e23
compiler:       clang version 7.0.0 (trunk 329391)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15a497f7800000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=177c1907800000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8951a3065ee7fd6d6e23@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
==================================================================
BUG: KMSAN: uninit-value in tipc_conn_rcv_sub+0x184/0x950  
net/tipc/topsrv.c:373
CPU: 0 PID: 66 Comm: kworker/u4:4 Not tainted 4.17.0-rc3+ #88
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: tipc_rcv tipc_conn_recv_work
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x185/0x1d0 lib/dump_stack.c:113
  kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
  __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
  tipc_conn_rcv_sub+0x184/0x950 net/tipc/topsrv.c:373
  tipc_conn_rcv_from_sock net/tipc/topsrv.c:409 [inline]
  tipc_conn_recv_work+0x3cd/0x560 net/tipc/topsrv.c:424
  process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2145
  worker_thread+0x113c/0x24f0 kernel/workqueue.c:2279
  kthread+0x539/0x720 kernel/kthread.c:239
  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:412

Local variable description: ----s.i@tipc_conn_recv_work
Variable was created at:
  tipc_conn_recv_work+0x65/0x560 net/tipc/topsrv.c:419
  process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2145
==================================================================
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 66 Comm: kworker/u4:4 Tainted: G    B             4.17.0-rc3+  
#88
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: tipc_rcv tipc_conn_recv_work
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x185/0x1d0 lib/dump_stack.c:113
  panic+0x39d/0x940 kernel/panic.c:184
  kmsan_report+0x238/0x240 mm/kmsan/kmsan.c:1083
  __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
  tipc_conn_rcv_sub+0x184/0x950 net/tipc/topsrv.c:373
  tipc_conn_rcv_from_sock net/tipc/topsrv.c:409 [inline]
  tipc_conn_recv_work+0x3cd/0x560 net/tipc/topsrv.c:424
  process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2145
  worker_thread+0x113c/0x24f0 kernel/workqueue.c:2279
  kthread+0x539/0x720 kernel/kthread.c:239
  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:412
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH] [RFC] bpf: tracing: new helper bpf_get_current_cgroup_ino
From: Alban Crequy @ 2018-05-13 17:33 UTC (permalink / raw)
  To: netdev, linux-kernel, containers, cgroups; +Cc: Alban Crequy

From: Alban Crequy <alban@kinvolk.io>

bpf_get_current_cgroup_ino() allows BPF trace programs to get the inode
of the cgroup where the current process resides.

My use case is to get statistics about syscalls done by a specific
Kubernetes container. I have a tracepoint on raw_syscalls/sys_enter and
a BPF map containing the cgroup inode that I want to trace. I use
bpf_get_current_cgroup_ino() and I quickly return from the tracepoint if
the inode is not in the BPF hash map.

Without this BPF helper, I would need to keep track of all pids in the
container. The Netlink proc connector can be used to follow process
creation and destruction but it is racy.

This patch only looks at the memory cgroup, which was enough for me
since each Kubernetes container is placed in a different mem cgroup.
For a generic implementation, I'm not sure how to proceed: it seems I
would need to use 'for_each_root(root)' (see example in
proc_cgroup_show() from kernel/cgroup/cgroup.c) but I don't know if
taking the cgroup mutex is possible in the BPF helper function. It might
be ok in the tracepoint raw_syscalls/sys_enter but could the mutex
already be taken in some other tracepoints?

Signed-off-by: Alban Crequy <alban@kinvolk.io>
---
 include/uapi/linux/bpf.h | 11 ++++++++++-
 kernel/trace/bpf_trace.c | 25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c5ec89732a8d..38ac3959cdf3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -755,6 +755,14 @@ union bpf_attr {
  *     @addr: pointer to struct sockaddr to bind socket to
  *     @addr_len: length of sockaddr structure
  *     Return: 0 on success or negative error code
+ *
+ * u64 bpf_get_current_cgroup_ino(hierarchy, flags)
+ *     Get the cgroup{1,2} inode of current task under the specified hierarchy.
+ *     @hierarchy: cgroup hierarchy
+ *     @flags: reserved for future use
+ *     Return:
+ *       == 0 error
+ *        > 0 inode of the cgroup
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -821,7 +829,8 @@ union bpf_attr {
 	FN(msg_apply_bytes),		\
 	FN(msg_cork_bytes),		\
 	FN(msg_pull_data),		\
-	FN(bind),
+	FN(bind),			\
+	FN(get_current_cgroup_ino),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 56ba0f2a01db..9bf92a786639 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -524,6 +524,29 @@ static const struct bpf_func_proto bpf_probe_read_str_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_2(bpf_get_current_cgroup_ino, u32, hierarchy, u64, flags)
+{
+	// TODO: pick the correct hierarchy instead of the mem controller
+	struct cgroup *cgrp = task_cgroup(current, memory_cgrp_id);
+
+	if (unlikely(!cgrp))
+		return -EINVAL;
+	if (unlikely(hierarchy))
+		return -EINVAL;
+	if (unlikely(flags))
+		return -EINVAL;
+
+	return cgrp->kn->id.ino;
+}
+
+static const struct bpf_func_proto bpf_get_current_cgroup_ino_proto = {
+	.func           = bpf_get_current_cgroup_ino,
+	.gpl_only       = false,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_DONTCARE,
+	.arg2_type      = ARG_DONTCARE,
+};
+
 static const struct bpf_func_proto *
 tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -564,6 +587,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_get_current_cgroup_ino:
+		return &bpf_get_current_cgroup_ino_proto;
 	default:
 		return NULL;
 	}
-- 
2.14.3

^ permalink raw reply related

* Re: [PATCH bpf v3] x86/cpufeature: bpf hack for clang not supporting asm goto
From: Alexei Starovoitov @ 2018-05-13 17:43 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Borislav Petkov, Peter Zijlstra, Yonghong Song, Ingo Molnar,
	Linus Torvalds, Alexei Starovoitov, Daniel Borkmann, LKML, X86 ML,
	Network Development, Kernel Team
In-Reply-To: <alpine.DEB.2.21.1805122110230.1582@nanos.tec.linutronix.de>

On Sat, May 12, 2018 at 10:30:02PM +0200, Thomas Gleixner wrote:
> On Sat, 12 May 2018, Alexei Starovoitov wrote:
> > On Thu, May 10, 2018 at 10:58 AM, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > I see no option, but to fix the kernel.
> > > Regardless whether it's called user space breakage or kernel breakage.
> 
> There is a big difference. If you are abusing a kernel internal header in a
> user space tool, then there is absolutely ZERO excuse for requesting that
> the header in question has to be modified.
> 
> But yes, the situation is slightly different here because tools which
> create trace event magic _HAVE_ to pull in kernel headers. At the same time
> these tools depend on a compiler which failed to implement asm_goto for
> fricking 8 years.

As a maintainer of a piece of llvm codebase I have to say that
this bullying tactic has the opposite effect.
The inline asm is processed by gcc and llvm very differently.
gcc is leaking internal backend implementation details into inline asm
syntax. It makes little sense for llvm to do the same, since compiler
codegen is completely different. gcc doesn't have integrated assembler
whereas llvm not only can parse asm, but can potentially optimize it as well.
Instead of demanding asm-goto that matches gcc one to one it's better
to work with the community to define the syntax that works for both
kernel and llvm.

> So while Boris is right, that nothing has to fiddle with a kernel only
> header, I grumpily agree with you that we need a workaround in the kernel
> for this particular issue.
> 
> > could you please ack the patch or better yet take it into tip tree
> > and send to Linus asap ?
> 
> Nope. The patch is a horrible hack.
> 
> Why the heck do we need that extra fugly define? That has exactly zero
> value simply because we already have a define which denotes availablity of
> ASM GOTO: CC_HAVE_ASM_GOTO.

I agree. That's why the v1 patch that was using CC_HAVE_ASM_GOTO was better:
https://patchwork.kernel.org/patch/10333829/
I'm fine on adding a warning to it though.

> In case of samples/bpf/ and libbcc the compile does not go through the
> arch/x86 Makefile which stops the build anyway when ASM_GOTO is
> missing. Those builds merily pull in the headers and have their own build
> magic, which is broken btw: Changing a kernel header which gets pulled into
> the build does not rebuild anything in samples/bpf. Qualitee..
> 
> So we can just use CC_HAVE_ASM_GOTO and be done with it.
> 
> But we also want the tools which needs this to be aware of this. Peter
> requested -D __BPF__ several times which got ignored. It's not too much of
> a request to add that.

quite the opposite.
It was explained already why -D__BPF__ makes little sense.
It's like saying that -D__arm__ has to be specified in command line.

clang automatically adds -D__arm__ when '-target arm' is used
and adds -D__BPF__ when '-target bpf' is used.
For samples/bpf, libbcc and other cases we have to use -target native.
If we do '-target native -D__BPF__' that's just like trying to compile
kernel headers with '-target x86 -D__arm__'. Absurd.

> Find a patch which deos exactly this for samples/bpf, but also allows other
> tools to build with a warning emitted so they get fixed.

agree

> Thanks,
> 
> 	tglx
> 
> 8<----------------
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -140,6 +140,20 @@ extern void clear_cpu_cap(struct cpuinfo
>  
>  #define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
>  
> +#ifndef CC_HAVE_ASM_GOTO
> +
> +/*
> + * Workaround for the sake of BPF compilation which utilizes kernel
> + * headers, but clang does not support ASM GOTO and fails the build.
> + */
> +#ifndef __BPF__
> +#warning "Compiler lacks ASM_GOTO support. Add -D __BPF__ to your compiler arguments"
> +#endif

Agree.
The warning makes sense to me, but it has to be different macro name.
How about -D__BPF_TRACING__ or -D__BPF_KPROBES__ or something similar ?
Such name will also make it clear that only tracing bpf programs
need this. Networking programs shouldn't be including kernel headers.
There was never a need, but since the tracing progs are often used
as an example people copy paste makefiles too.
We tried to document it as much as possible, but people still use
'clang -target native -I/kernel/includes bpf_prog.c -emit-llvm | llc -march=bpf'
in their builds.
(sometimes as a workaround for setups where clang is older version,
but llc/llvm is new)
Now they will see this warning and it will force them to think whether
they actually need the kernel headers.

> +
> +#define static_cpu_has(bit)		boot_cpu_has(bit)
> +
> +#else
> +
>  /*
>   * Static testing of CPU features.  Used the same as boot_cpu_has().
>   * These will statically patch the target code for additional
> @@ -195,6 +209,7 @@ static __always_inline __pure bool _stat
>  		boot_cpu_has(bit) :				\
>  		_static_cpu_has(bit)				\
>  )
> +#endif
>  
>  #define cpu_has_bug(c, bit)		cpu_has(c, (bit))
>  #define set_cpu_bug(c, bit)		set_cpu_cap(c, (bit))
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -255,7 +255,7 @@ verify_target_bpf: verify_cmds
>  $(obj)/%.o: $(src)/%.c
>  	$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
>  		-I$(srctree)/tools/testing/selftests/bpf/ \
> -		-D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
> +		-D__KERNEL__ -D__BPF__ -Wno-unused-value -Wno-pointer-sign \

Yep. In samples/bpf and libbcc we can selectively add -D__BPF_TRACING__
I think sysdig and other folks can live with that as well.
Agree?

^ permalink raw reply

* [PATCH net] qede: Fix ref-cnt usage count
From: Michal Kalderon @ 2018-05-13 17:54 UTC (permalink / raw)
  To: michal.kalderon, davem
  Cc: netdev, linux-rdma, chad.dupuis, Michal Kalderon, Ariel Elior

Rebooting while qedr is loaded with a VLAN interface present
results in unregister_netdevice waiting for the usage count
to become free.
The fix is that rdma devices should be removed before unregistering
the netdevice, to assure all references to ndev are decreased.

Fixes: cee9fbd8e2e9 ("qede: Add qedr framework")
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index a01e7d6..f6655e2 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1066,13 +1066,12 @@ static void __qede_remove(struct pci_dev *pdev, enum qede_remove_mode mode)
 
 	DP_INFO(edev, "Starting qede_remove\n");
 
+	qede_rdma_dev_remove(edev);
 	unregister_netdev(ndev);
 	cancel_delayed_work_sync(&edev->sp_task);
 
 	qede_ptp_disable(edev);
 
-	qede_rdma_dev_remove(edev);
-
 	edev->ops->common->set_power_state(cdev, PCI_D0);
 
 	pci_set_drvdata(pdev, NULL);
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH bpf v3] x86/cpufeature: bpf hack for clang not supporting asm goto
From: Thomas Gleixner @ 2018-05-13 18:02 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Borislav Petkov, Peter Zijlstra, Yonghong Song, Ingo Molnar,
	Linus Torvalds, Alexei Starovoitov, Daniel Borkmann, LKML, X86 ML,
	Network Development, Kernel Team
In-Reply-To: <20180513174348.eoh2lrhzqbmqb5nc@ast-mbp>

On Sun, 13 May 2018, Alexei Starovoitov wrote:
> On Sat, May 12, 2018 at 10:30:02PM +0200, Thomas Gleixner wrote:
> > But yes, the situation is slightly different here because tools which
> > create trace event magic _HAVE_ to pull in kernel headers. At the same time
> > these tools depend on a compiler which failed to implement asm_goto for
> > fricking 8 years.
> 
> As a maintainer of a piece of llvm codebase I have to say that
> this bullying tactic has the opposite effect.

I'm not bullying at all. Its a fact that the discussion about asm goto is
dragging out 8 years now. We've stayed away from mandating it for quite
some time, but at some point it just doesn't make sense anymore.

> The inline asm is processed by gcc and llvm very differently.  gcc is
> leaking internal backend implementation details into inline asm
> syntax. It makes little sense for llvm to do the same, since compiler
> codegen is completely different. gcc doesn't have integrated assembler
> whereas llvm not only can parse asm, but can potentially optimize it as
> well.  Instead of demanding asm-goto that matches gcc one to one it's
> better to work with the community to define the syntax that works for
> both kernel and llvm.

Come on, we surely are open for discussions, but what I've seen so far is
just 'oh we can't do this because' instead of a sane proposal how it can be
done w/o rewriting the whole ASM GOTO stuff in the kernel or even
duplicating it.

> > + * Workaround for the sake of BPF compilation which utilizes kernel
> > + * headers, but clang does not support ASM GOTO and fails the build.
> > + */
> > +#ifndef __BPF__
> > +#warning "Compiler lacks ASM_GOTO support. Add -D __BPF__ to your compiler arguments"
> > +#endif
> 
> Agree.
> The warning makes sense to me, but it has to be different macro name.
> How about -D__BPF_TRACING__ or -D__BPF_KPROBES__ or something similar ?

Fair enough.

> Such name will also make it clear that only tracing bpf programs
> need this. Networking programs shouldn't be including kernel headers.
> There was never a need, but since the tracing progs are often used
> as an example people copy paste makefiles too.
> We tried to document it as much as possible, but people still use
> 'clang -target native -I/kernel/includes bpf_prog.c -emit-llvm | llc -march=bpf'
> in their builds.
> (sometimes as a workaround for setups where clang is older version,
> but llc/llvm is new)
> Now they will see this warning and it will force them to think whether
> they actually need the kernel headers.

Makes sense.

> > +
> > +#define static_cpu_has(bit)		boot_cpu_has(bit)
> > +
> > +#else
> > +
> >  /*
> >   * Static testing of CPU features.  Used the same as boot_cpu_has().
> >   * These will statically patch the target code for additional
> > @@ -195,6 +209,7 @@ static __always_inline __pure bool _stat
> >  		boot_cpu_has(bit) :				\
> >  		_static_cpu_has(bit)				\
> >  )
> > +#endif
> >  
> >  #define cpu_has_bug(c, bit)		cpu_has(c, (bit))
> >  #define set_cpu_bug(c, bit)		set_cpu_cap(c, (bit))
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -255,7 +255,7 @@ verify_target_bpf: verify_cmds
> >  $(obj)/%.o: $(src)/%.c
> >  	$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> >  		-I$(srctree)/tools/testing/selftests/bpf/ \
> > -		-D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
> > +		-D__KERNEL__ -D__BPF__ -Wno-unused-value -Wno-pointer-sign \
> 
> Yep. In samples/bpf and libbcc we can selectively add -D__BPF_TRACING__
> I think sysdig and other folks can live with that as well.
> Agree?

Sure. Care to send an updated patch?

Thanks,

	tglx

^ permalink raw reply

* [PATCH RESEND net-next v2 2/8] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: devicetree, Maxime Ripard, netdev, Chen-Yu Tsai, Rob Herring,
	Corentin Labbe, linux-arm-kernel, Icenowy Zheng
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The A83T syscon compatible was appended to the syscon compatibles list,
instead of inserted in to preserve the ordering.

Move it to the proper place to keep the list sorted.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index e04ce75e24a3..1b8e33e71651 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -22,10 +22,10 @@ Required properties:
 - #size-cells: shall be 0
 - syscon: A phandle to the syscon of the SoC with one of the following
  compatible string:
+  - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
-  - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 3/8] dt-bindings: net: dwmac-sun8i: simplify description of syscon property
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The syscon property is used to point to the device that holds the glue
layer control register known as the "EMAC (or GMAC) clock register".

We do not need to explicitly list what compatible strings are needed, as
this information is readily available in the user manuals. Also the
"syscon" device type is more of an implementation detail. There are many
ways to access a register not in a device's address range, the syscon
interface being the most generic and unrestricted one.

Simplify the description so that it says what it is supposed to
describe.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1b8e33e71651..1c0906a5c02b 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -20,12 +20,7 @@ Required properties:
 - phy-handle: See ethernet.txt
 - #address-cells: shall be 1
 - #size-cells: shall be 0
-- syscon: A phandle to the syscon of the SoC with one of the following
- compatible string:
-  - allwinner,sun8i-a83t-system-controller
-  - allwinner,sun8i-h3-system-controller
-  - allwinner,sun8i-v3s-system-controller
-  - allwinner,sun50i-a64-system-controller
+- syscon: A phandle to the device containing the EMAC or GMAC clock register
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 4/8] dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds the R40 specific bits to the dwmac-sun8i binding.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1c0906a5c02b..cfe724398a12 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible: must be one of the following string:
 		"allwinner,sun8i-a83t-emac"
 		"allwinner,sun8i-h3-emac"
+		"allwinner,sun8i-r40-gmac"
 		"allwinner,sun8i-v3s-emac"
 		"allwinner,sun50i-a64-emac"
 - reg: address and length of the register for the device.
@@ -25,8 +26,10 @@ Required properties:
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
 			 Range is 0-700. Default is 0.
+			 Unavailable for allwinner,sun8i-r40-gmac
 - allwinner,rx-delay-ps: RX clock delay chain value in ps.
 			 Range is 0-3100. Default is 0.
+			 Range is 0-700 for allwinner,sun8i-r40-gmac
 Both delay properties need to be a multiple of 100. They control the
 clock delay for external RGMII PHY. They do not apply to the internal
 PHY or external non-RGMII PHYs.
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The clock delay chains found in the glue layer for dwmac-sun8i are only
used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
range of possible values for the delay chain.

This patch reformats the delay chain section of the device tree binding
to make it clear that the delay chains only apply to RGMII PHYs, and
make it easier to add the R40-specific bits later.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 3d6d5fa0c4d5..e04ce75e24a3 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -28,10 +28,13 @@ Required properties:
   - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
-- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
-- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
-Both delay properties need to be a multiple of 100. They control the delay for
-external PHY.
+- allwinner,tx-delay-ps: TX clock delay chain value in ps.
+			 Range is 0-700. Default is 0.
+- allwinner,rx-delay-ps: RX clock delay chain value in ps.
+			 Range is 0-3100. Default is 0.
+Both delay properties need to be a multiple of 100. They control the
+clock delay for external RGMII PHY. They do not apply to the internal
+PHY or external non-RGMII PHYs.
 
 Optional properties for the following compatibles:
   - "allwinner,sun8i-h3-emac",
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 0/8] net: stmmac: dwmac-sun8i: Support R40
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring

This is a resend of the patches for net-next split out from my R40
Ethernet support v2 series, as requested by David Miller. The arm-soc
bits will follow, once I rework the A64 system controller compatible.

Patches 1, 2, and 3 clean up the dwmac-sun8i binding.

Patch 4 adds device tree binding for Allwinner R40's Ethernet
controller.

Patch 5 converts regmap access of the syscon region in the dwmac-sun8i
driver to regmap_field, in anticipation of different field widths on
the R40.

Patch 6 introduces custom plumbing in the dwmac-sun8i driver to fetch
a regmap from another device, by looking up said device via a phandle,
then getting the regmap associated with that device.

Patch 7 adds support for different or absent TX/RX delay chain ranges
to the dwmac-sun8i driver.

Patch 8 adds support for the R40's ethernet controller.


Excerpt from original cover letter:

Changes since v1:

  - Default to fetching regmap from device pointed to by syscon phandle,
    and falling back to syscon API if that fails.

  - Dropped .syscon_from_dev field in device data as a result of the
    previous change.

  - Added a large comment block explaining the first change.

  - Simplified description of syscon property in sun8i-dwmac binding.

  - Regmap now only exposes the EMAC/GMAC register, but retains the
    offset within its address space.

  - Added patches for A64, which reuse the same sun8i-dwmac changes.

This series adds support for the DWMAC based Ethernet controller found
on the Allwinner R40 SoC. The controller is either a DWMAC clone or
DWMAC core with its registers rearranged. This is already supported by
the dwmac-sun8i driver. The glue layer control registers, unlike other
sun8i family SoCs, is not in the system controller region, but in the
clock control unit, like with the older A20 and A31 SoCs.

While we reuse the bindings for dwmac-sun8i using a syscon phandle
reference, we need some custom plumbing for the clock driver to export
a regmap that only allows access to the GMAC register to the dwmac-sun8i
driver. An alternative would be to allow drivers to register custom
syscon devices with their own regmap and locking.


Please have a look.

Regards
ChenYu

Chen-Yu Tsai (8):
  dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
  dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical
    order
  dt-bindings: net: dwmac-sun8i: simplify description of syscon property
  dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40
    SoC
  net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access
  net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external
    device
  net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay
    chains
  net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC

 .../devicetree/bindings/net/dwmac-sun8i.txt   |  21 +--
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 139 +++++++++++++++---
 2 files changed, 130 insertions(+), 30 deletions(-)

-- 
2.17.0

^ permalink raw reply

* [PATCH RESEND net-next v2 8/8] net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds support for it using the framework laid out by previous
patches to map the differences.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c   | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4f5612a3c855..2f7f0915f071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -93,6 +93,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
 	.msb = 31,
 };
 
+/* EMAC clock register @ 0x164 in the CCU address range */
+static const struct reg_field sun8i_ccu_reg_field = {
+	.reg = 0x164,
+	.lsb = 0,
+	.msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
 	.default_syscon_value = 0x58000,
 	.syscon_field = &sun8i_syscon_reg_field,
@@ -121,6 +128,14 @@ static const struct emac_variant emac_variant_a83t = {
 	.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_r40 = {
+	.default_syscon_value = 0,
+	.syscon_field = &sun8i_ccu_reg_field,
+	.support_mii = true,
+	.support_rgmii = true,
+	.rx_delay_max = 7,
+};
+
 static const struct emac_variant emac_variant_a64 = {
 	.default_syscon_value = 0,
 	.syscon_field = &sun8i_syscon_reg_field,
@@ -1160,6 +1175,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
 		.data = &emac_variant_v3s },
 	{ .compatible = "allwinner,sun8i-a83t-emac",
 		.data = &emac_variant_a83t },
+	{ .compatible = "allwinner,sun8i-r40-gmac",
+		.data = &emac_variant_r40 },
 	{ .compatible = "allwinner,sun50i-a64-emac",
 		.data = &emac_variant_a64 },
 	{ }
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 7/8] net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the R40 SoC, the RX delay chain only has a range of 0~7 (hundred
picoseconds), instead of 0~31. Also the TX delay chain is completely
absent.

This patch adds support for different ranges by adding per-compatible
maximum values in the variant data. A maximum of 0 indicates that the
delay chain is not supported or absent.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 79e104a20e20..4f5612a3c855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -47,6 +47,12 @@
  * @support_mii:		Does the MAC handle MII
  * @support_rmii:		Does the MAC handle RMII
  * @support_rgmii:		Does the MAC handle RGMII
+ *
+ * @rx_delay_max:		Maximum raw value for RX delay chain
+ * @tx_delay_max:		Maximum raw value for TX delay chain
+ *				These two also indicate the bitmask for
+ *				the RX and TX delay chain registers. A
+ *				value of zero indicates this is not supported.
  */
 struct emac_variant {
 	u32 default_syscon_value;
@@ -55,6 +61,8 @@ struct emac_variant {
 	bool support_mii;
 	bool support_rmii;
 	bool support_rgmii;
+	u8 rx_delay_max;
+	u8 tx_delay_max;
 };
 
 /* struct sunxi_priv_data - hold all sunxi private data
@@ -91,7 +99,9 @@ static const struct emac_variant emac_variant_h3 = {
 	.soc_has_internal_phy = true,
 	.support_mii = true,
 	.support_rmii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_v3s = {
@@ -106,7 +116,9 @@ static const struct emac_variant emac_variant_a83t = {
 	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_a64 = {
@@ -115,7 +127,9 @@ static const struct emac_variant emac_variant_a64 = {
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rmii = true,
-	.support_rgmii = true
+	.support_rgmii = true,
+	.rx_delay_max = 31,
+	.tx_delay_max = 7,
 };
 
 #define EMAC_BASIC_CTL0 0x00
@@ -219,9 +233,7 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
 
 /* Generic system control EMAC_CLK bits */
-#define SYSCON_ETXDC_MASK		GENMASK(2, 0)
 #define SYSCON_ETXDC_SHIFT		10
-#define SYSCON_ERXDC_MASK		GENMASK(4, 0)
 #define SYSCON_ERXDC_SHIFT		5
 /* EMAC PHY Interface Type */
 #define SYSCON_EPIT			BIT(2) /* 1: RGMII, 0: MII */
@@ -847,8 +859,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		}
 		val /= 100;
 		dev_dbg(priv->device, "set tx-delay to %x\n", val);
-		if (val <= SYSCON_ETXDC_MASK) {
-			reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
+		if (val <= gmac->variant->tx_delay_max) {
+			reg &= ~(gmac->variant->tx_delay_max <<
+				 SYSCON_ETXDC_SHIFT);
 			reg |= (val << SYSCON_ETXDC_SHIFT);
 		} else {
 			dev_err(priv->device, "Invalid TX clock delay: %d\n",
@@ -864,8 +877,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		}
 		val /= 100;
 		dev_dbg(priv->device, "set rx-delay to %x\n", val);
-		if (val <= SYSCON_ERXDC_MASK) {
-			reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
+		if (val <= gmac->variant->rx_delay_max) {
+			reg &= ~(gmac->variant->rx_delay_max <<
+				 SYSCON_ERXDC_SHIFT);
 			reg |= (val << SYSCON_ERXDC_SHIFT);
 		} else {
 			dev_err(priv->device, "Invalid RX clock delay: %d\n",
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 6/8] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external device
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
address space. Using a standard syscon to access it provides no
coordination with the CCU driver for register access. Neither does
it prevent this and other drivers from accessing other, maybe critical,
clock control registers. On other SoCs, the register is in the "system
control" address space, which might also contain controls for mapping
SRAM to devices or the CPU. This hardware has the same issues.

Instead, for these types of setups, we let the device containing the
control register create a regmap tied to it. We can then get the device
from the existing syscon phandle, and retrieve the regmap with
dev_get_regmap().

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 ++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index bbc051474806..79e104a20e20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -983,6 +983,34 @@ static struct mac_device_info *sun8i_dwmac_setup(void *ppriv)
 	return mac;
 }
 
+static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node)
+{
+	struct device_node *syscon_node;
+	struct platform_device *syscon_pdev;
+	struct regmap *regmap = NULL;
+
+	syscon_node = of_parse_phandle(node, "syscon", 0);
+	if (!syscon_node)
+		return ERR_PTR(-ENODEV);
+
+	syscon_pdev = of_find_device_by_node(syscon_node);
+	if (!syscon_pdev) {
+		/* platform device might not be probed yet */
+		regmap = ERR_PTR(-EPROBE_DEFER);
+		goto out_put_node;
+	}
+
+	/* If no regmap is found then the other device driver is at fault */
+	regmap = dev_get_regmap(&syscon_pdev->dev, NULL);
+	if (!regmap)
+		regmap = ERR_PTR(-EINVAL);
+
+	platform_device_put(syscon_pdev);
+out_put_node:
+	of_node_put(syscon_node);
+	return regmap;
+}
+
 static int sun8i_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -1027,7 +1055,27 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 		gmac->regulator = NULL;
 	}
 
-	regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+	/* The "GMAC clock control" register might be located in the
+	 * CCU address range (on the R40), or the system control address
+	 * range (on most other sun8i and later SoCs).
+	 *
+	 * The former controls most if not all clocks in the SoC. The
+	 * latter has an SoC identification register, and on some SoCs,
+	 * controls to map device specific SRAM to either the intended
+	 * peripheral, or the CPU address space.
+	 *
+	 * In either case, there should be a coordinated and restricted
+	 * method of accessing the register needed here. This is done by
+	 * having the device export a custom regmap, instead of a generic
+	 * syscon, which grants all access to all registers.
+	 *
+	 * To support old device trees, we fall back to using the syscon
+	 * interface if possible.
+	 */
+	regmap = sun8i_dwmac_get_syscon_from_dev(pdev->dev.of_node);
+	if (IS_ERR(regmap))
+		regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							 "syscon");
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
 		dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
-- 
2.17.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 5/8] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access
From: Chen-Yu Tsai @ 2018-05-13 19:14 UTC (permalink / raw)
  To: Giuseppe Cavallaro
  Cc: Chen-Yu Tsai, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-1-wens@csie.org>

On the Allwinner R40, the "GMAC clock" register is located in the CCU
block, at a different register address than the other SoCs that have
it in the "system control" block.

This patch converts the use of regmap to regmap_field for mapping and
accessing the syscon register, so we can have the register address in
the variants data, and not in the actual register manipulation code.

This patch only converts regmap_read() and regmap_write() calls to
regmap_field_read() and regmap_field_write() calls. There are some
places where it might make sense to switch to regmap_field_update_bits(),
but this is not done here to keep the patch simple.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 42 ++++++++++++++-----
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3fa65b1ca8e..bbc051474806 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -42,6 +42,7 @@
  *				This value is used for disabling properly EMAC
  *				and used as a good starting value in case of the
  *				boot process(uboot) leave some stuff.
+ * @syscon_field		reg_field for the syscon's gmac register
  * @soc_has_internal_phy:	Does the MAC embed an internal PHY
  * @support_mii:		Does the MAC handle MII
  * @support_rmii:		Does the MAC handle RMII
@@ -49,6 +50,7 @@
  */
 struct emac_variant {
 	u32 default_syscon_value;
+	const struct reg_field *syscon_field;
 	bool soc_has_internal_phy;
 	bool support_mii;
 	bool support_rmii;
@@ -71,13 +73,21 @@ struct sunxi_priv_data {
 	struct regulator *regulator;
 	struct reset_control *rst_ephy;
 	const struct emac_variant *variant;
-	struct regmap *regmap;
+	struct regmap_field *regmap_field;
 	bool internal_phy_powered;
 	void *mux_handle;
 };
 
+/* EMAC clock register @ 0x30 in the "system control" address range */
+static const struct reg_field sun8i_syscon_reg_field = {
+	.reg = 0x30,
+	.lsb = 0,
+	.msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
 	.default_syscon_value = 0x58000,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = true,
 	.support_mii = true,
 	.support_rmii = true,
@@ -86,12 +96,14 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
 	.default_syscon_value = 0x38000,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = true,
 	.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
 	.default_syscon_value = 0,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rgmii = true
@@ -99,6 +111,7 @@ static const struct emac_variant emac_variant_a83t = {
 
 static const struct emac_variant emac_variant_a64 = {
 	.default_syscon_value = 0,
+	.syscon_field = &sun8i_syscon_reg_field,
 	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rmii = true,
@@ -216,7 +229,6 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_ETCS_MII		0x0
 #define SYSCON_ETCS_EXT_GMII	0x1
 #define SYSCON_ETCS_INT_GMII	0x2
-#define SYSCON_EMAC_REG		0x30
 
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
@@ -745,7 +757,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
 	bool need_power_ephy = false;
 
 	if (current_child ^ desired_child) {
-		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
+		regmap_field_read(gmac->regmap_field, &reg);
 		switch (desired_child) {
 		case DWMAC_SUN8I_MDIO_MUX_INTERNAL_ID:
 			dev_info(priv->device, "Switch mux to internal PHY");
@@ -763,7 +775,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
 				desired_child);
 			return -EINVAL;
 		}
-		regmap_write(gmac->regmap, SYSCON_EMAC_REG, val);
+		regmap_field_write(gmac->regmap_field, val);
 		if (need_power_ephy) {
 			ret = sun8i_dwmac_power_internal_phy(priv);
 			if (ret)
@@ -801,7 +813,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 	int ret;
 	u32 reg, val;
 
-	regmap_read(gmac->regmap, SYSCON_EMAC_REG, &val);
+	regmap_field_read(gmac->regmap_field, &val);
 	reg = gmac->variant->default_syscon_value;
 	if (reg != val)
 		dev_warn(priv->device,
@@ -883,7 +895,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		return -EINVAL;
 	}
 
-	regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg);
+	regmap_field_write(gmac->regmap_field, reg);
 
 	return 0;
 }
@@ -892,7 +904,7 @@ static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac)
 {
 	u32 reg = gmac->variant->default_syscon_value;
 
-	regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg);
+	regmap_field_write(gmac->regmap_field, reg);
 }
 
 static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv)
@@ -980,6 +992,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	int ret;
 	struct stmmac_priv *priv;
 	struct net_device *ndev;
+	struct regmap *regmap;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)
@@ -1014,14 +1027,21 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 		gmac->regulator = NULL;
 	}
 
-	gmac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
-						       "syscon");
-	if (IS_ERR(gmac->regmap)) {
-		ret = PTR_ERR(gmac->regmap);
+	regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
 		dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
 		return ret;
 	}
 
+	gmac->regmap_field = devm_regmap_field_alloc(dev, regmap,
+						     *gmac->variant->syscon_field);
+	if (IS_ERR(gmac->regmap_field)) {
+		ret = PTR_ERR(gmac->regmap_field);
+		dev_err(dev, "Unable to map syscon register: %d\n", ret);
+		return ret;
+	}
+
 	plat_dat->interface = of_get_phy_mode(dev->of_node);
 
 	/* platform data specifying hardware features and callbacks.
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node
From: Chen-Yu Tsai @ 2018-05-13 19:37 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, linux-arm-kernel, Mark Rutland, devicetree,
	Stephen Boyd, netdev, Michael Turquette, Rob Herring,
	Corentin Labbe, Mark Brown, Giuseppe Cavallaro, linux-clk
In-Reply-To: <20180502115437.zlu2rafrguufutvp@flea>

On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote:
>>
>>
>> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到:
>> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard
>> ><maxime.ripard@bootlin.com> wrote:
>> >> Hi,
>> >>
>> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai wrote:
>> >>> From: Icenowy Zheng <icenowy@aosc.io>
>> >>>
>> >>> Allwinner A64 has a SRAM controller, and in the device tree
>> >currently
>> >>> we have a syscon node to enable EMAC driver to access the EMAC clock
>> >>> register. As SRAM controller driver can now export regmap for this
>> >>> register, replace the syscon node to the SRAM controller device
>> >node,
>> >>> and let EMAC driver to acquire its EMAC clock regmap.
>> >>>
>> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >>> ---
>> >>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23
>> >+++++++++++++++----
>> >>>  1 file changed, 19 insertions(+), 4 deletions(-)
>> >>>
>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> index 1b2ef28c42bd..1c37659d9d41 100644
>> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> >>> @@ -168,10 +168,25 @@
>> >>>               #size-cells = <1>;
>> >>>               ranges;
>> >>>
>> >>> -             syscon: syscon@1c00000 {
>> >>> -                     compatible =
>> >"allwinner,sun50i-a64-system-controller",
>> >>> -                             "syscon";
>> >>> +             sram_controller: sram-controller@1c00000 {
>> >>> +                     compatible =
>> >"allwinner,sun50i-a64-sram-controller";
>> >>
>> >> I don't think there's anything preventing us from keeping the
>> >> -system-controller compatible. It's what was in the DT before, and
>> >> it's how it's called in the datasheet.
>> >
>> >I actually meant to ask you about this. The -system-controller
>> >compatible matches the datasheet better. Maybe we should just
>> >switch to that one?
>>
>> No, if we do the switch the system-controller compatible,
>> the device will be probed on the same memory region with
>> a syscon on old DTs.
>
> The device hasn't magically changed either. Maybe we just need to add
> a check to make sure we don't have the syscon compatible in the SRAM
> driver probe so that the double driver issue doesn't happen?

The syscon interface (which is not even a full blown device driver)
only looks at the "syscon" compatible. Either way we're removing that
part from the device tree so things should be ok for new device trees.
As Maxime mentioned we can do a check for the syscon compatible and
either give a warning to the user asking them to update their device
tree, or not register our custom regmap, or not probe the SRAM driver.
Personally I prefer the first option. The system controller block is
probed before any syscon users, so we should be fine, given the dwmac
driver goes the custom regmap path first.

BTW, I still might end up changing the compatible. The manual uses
"system control", not "system controller", which I think makes sense,
since it is just a bunch of register files, kind of like the GRF
(General Register Files) block found in Rockchip SoCs [1], and not an
actual "controller".

ChenYu

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/rockchip/grf.txt

^ permalink raw reply

* [PATCH] net: ipv4: ipconfig: fix unused variable
From: Anders Roxell @ 2018-05-13 19:48 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji; +Cc: netdev, linux-kernel, Anders Roxell

When CONFIG_PROC_FS isn't set, variable ipconfig_dir isn't used.
net/ipv4/ipconfig.c:167:31: warning: ‘ipconfig_dir’ defined but not used [-Wunused-variable]
 static struct proc_dir_entry *ipconfig_dir;
                               ^~~~~~~~~~~~
Move the declaration of ipconfig_dir inside the CONFIG_PROC_FS ifdef to
fix the warning.

Fixes: c04d2cb2009f ("ipconfig: Write NTP server IPs to /proc/net/ipconfig/ntp_servers")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 net/ipv4/ipconfig.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index d839d74853fc..86c9f755de3d 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -163,9 +163,6 @@ static u8 ic_domain[64];		/* DNS (not NIS) domain name */
  * Private state.
  */
 
-/* proc_dir_entry for /proc/net/ipconfig */
-static struct proc_dir_entry *ipconfig_dir;
-
 /* Name of user-selected boot device */
 static char user_dev_name[IFNAMSIZ] __initdata = { 0, };
 
@@ -1292,6 +1289,8 @@ static int __init ic_dynamic(void)
 #endif /* IPCONFIG_DYNAMIC */
 
 #ifdef CONFIG_PROC_FS
+/* proc_dir_entry for /proc/net/ipconfig */
+static struct proc_dir_entry *ipconfig_dir;
 
 /* Name servers: */
 static int pnp_seq_show(struct seq_file *seq, void *v)
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 19:49 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513191425.9801-2-wens@csie.org>

On Mon, May 14, 2018 at 03:14:18AM +0800, Chen-Yu Tsai wrote:
> The clock delay chains found in the glue layer for dwmac-sun8i are only
> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
> range of possible values for the delay chain.
> 
> This patch reformats the delay chain section of the device tree binding
> to make it clear that the delay chains only apply to RGMII PHYs, and
> make it easier to add the R40-specific bits later.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 3d6d5fa0c4d5..e04ce75e24a3 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -28,10 +28,13 @@ Required properties:
>    - allwinner,sun8i-a83t-system-controller
>  
>  Optional properties:
> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
> -Both delay properties need to be a multiple of 100. They control the delay for
> -external PHY.
> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
> +			 Range is 0-700. Default is 0.
> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
> +			 Range is 0-3100. Default is 0.
> +Both delay properties need to be a multiple of 100. They control the
> +clock delay for external RGMII PHY. They do not apply to the internal
> +PHY or external non-RGMII PHYs.

Hi Chen-Yu

Are these delays the MAC applies? Not the PHY. It would be good to
make it clear here these are MAC imposed delays.

     Andrew

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 19:53 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513194919.GE12738@lunn.ch>

On Sun, May 13, 2018 at 12:49 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, May 14, 2018 at 03:14:18AM +0800, Chen-Yu Tsai wrote:
>> The clock delay chains found in the glue layer for dwmac-sun8i are only
>> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
>> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
>> range of possible values for the delay chain.
>>
>> This patch reformats the delay chain section of the device tree binding
>> to make it clear that the delay chains only apply to RGMII PHYs, and
>> make it easier to add the R40-specific bits later.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
>> ---
>>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> index 3d6d5fa0c4d5..e04ce75e24a3 100644
>> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> @@ -28,10 +28,13 @@ Required properties:
>>    - allwinner,sun8i-a83t-system-controller
>>
>>  Optional properties:
>> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
>> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
>> -Both delay properties need to be a multiple of 100. They control the delay for
>> -external PHY.
>> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
>> +                      Range is 0-700. Default is 0.
>> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
>> +                      Range is 0-3100. Default is 0.
>> +Both delay properties need to be a multiple of 100. They control the
>> +clock delay for external RGMII PHY. They do not apply to the internal
>> +PHY or external non-RGMII PHYs.
>
> Hi Chen-Yu
>
> Are these delays the MAC applies? Not the PHY. It would be good to
> make it clear here these are MAC imposed delays.

Yes these are applied on the MAC side. Being described in the device
tree bindings for the MAC, I thought this was implied to be the case?
Are there known exceptions?

Thanks
ChenYu

^ permalink raw reply

* Re: INFO: trying to register non-static key in del_timer_sync
From: Eric Biggers @ 2018-05-13 19:59 UTC (permalink / raw)
  To: syzbot
  Cc: coreteam, davem, fw, kadlec, linux-kernel, netdev,
	netfilter-devel, pablo, syzkaller-bugs
In-Reply-To: <001a114387b66c88ce0563dab5b8@google.com>

On Sun, Jan 28, 2018 at 10:58:01AM -0800, syzbot wrote:
> Hello,
> 
> syzbot hit the following crash on upstream commit
> c4e0ca7fa24137e372d6135fe16e8df8e123f116 (Fri Jan 26 23:10:50 2018 +0000)
> Merge tag 'riscv-for-linus-4.15-maintainers' of
> git://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux
> 
> So far this crash happened 3 times on net-next, upstream.
> C reproducer is attached.
> syzkaller reproducer is attached.
> Raw console output is attached.
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3659f05802671eb8af9c@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.
> 
> audit: type=1400 audit(1517074079.617:7): avc:  denied  { map } for
> pid=3685 comm="syzkaller985951" path="/root/syzkaller985951088" dev="sda1"
> ino=16481 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
> tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 1 PID: 3685 Comm: syzkaller985951 Not tainted 4.15.0-rc9+ #283
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  register_lock_class+0x542/0x2cd0 kernel/locking/lockdep.c:752
>  __lock_acquire+0x1de/0x3e00 kernel/locking/lockdep.c:3314
>  lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3914
>  del_timer_sync+0xba/0x240 kernel/time/timer.c:1275
>  led_tg_destroy+0x2dd/0x3f0 net/netfilter/xt_LED.c:185
>  cleanup_entry+0x218/0x350 net/ipv4/netfilter/ip_tables.c:659
>  __do_replace+0x7d7/0xa90 net/ipv4/netfilter/ip_tables.c:1096
>  do_replace net/ipv4/netfilter/ip_tables.c:1152 [inline]
>  do_ipt_set_ctl+0x40f/0x5f0 net/ipv4/netfilter/ip_tables.c:1682
>  nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
>  nf_setsockopt+0x67/0xc0 net/netfilter/nf_sockopt.c:115
>  ip_setsockopt+0xa1/0xb0 net/ipv4/ip_sockglue.c:1256
>  tcp_setsockopt+0x82/0xd0 net/ipv4/tcp.c:2875
>  sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2968
>  SYSC_setsockopt net/socket.c:1831 [inline]
>  SyS_setsockopt+0x189/0x360 net/socket.c:1810
>  entry_SYSCALL_64_fastpath+0x29/0xa0
> RIP: 0033:0x4449fa
> RSP: 002b:00007ffee653a948 EFLAGS: 00000206 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 00000000006cd0fc RCX: 00000000004449fa
> RDX: 0000000000000040 RSI: 0000000000000000 RDI: 0000000000000003
> RBP: 00000000006cd0fc R08: 00000000000002d8 R09: 000000000117c880
> R10: 00000000006cd528 R11: 0000000000000206 R12: 0000000000000003
> R13: 00000000006d00a4 R14: 00000000006d0050 R15: 00000000004a39ae
> ------------[ cut here ]------------
> ODEBUG: assert_init not available (active state 0) object type: timer_list
> hint:           (null)
> WARNING: CPU: 1 PID: 3685 at lib/debugobjects.c:291
> debug_print_object+0x166/0x220 lib/debugobjects.c:288
> Kernel panic - not syncing: panic_on_warn set ...
> 
> CPU: 1 PID: 3685 Comm: syzkaller985951 Not tainted 4.15.0-rc9+ #283
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  panic+0x1e4/0x41c kernel/panic.c:183
>  __warn+0x1dc/0x200 kernel/panic.c:547
>  report_bug+0x211/0x2d0 lib/bug.c:184
>  fixup_bug.part.11+0x37/0x80 arch/x86/kernel/traps.c:178
>  fixup_bug arch/x86/kernel/traps.c:247 [inline]
>  do_error_trap+0x2d7/0x3e0 arch/x86/kernel/traps.c:296
>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
>  invalid_op+0x22/0x40 arch/x86/entry/entry_64.S:1096
> RIP: 0010:debug_print_object+0x166/0x220 lib/debugobjects.c:288
> RSP: 0018:ffff8801d9adf7d0 EFLAGS: 00010282
> RAX: dffffc0000000008 RBX: 0000000000000005 RCX: ffffffff8159ebae
> RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000293
> RBP: ffff8801d9adf810 R08: 0000000000000000 R09: 1ffff1003b35be97
> R10: ffff8801d9adf6d0 R11: ffffffff86b38678 R12: 0000000000000001
> R13: ffffffff86b49d00 R14: ffffffff86010440 R15: ffffffff815f1530
>  debug_object_assert_init+0x303/0x570 lib/debugobjects.c:654
>  debug_timer_assert_init kernel/time/timer.c:707 [inline]
>  debug_assert_init kernel/time/timer.c:759 [inline]
>  try_to_del_timer_sync+0x74/0x130 kernel/time/timer.c:1215
>  del_timer_sync+0x18a/0x240 kernel/time/timer.c:1285
>  led_tg_destroy+0x2dd/0x3f0 net/netfilter/xt_LED.c:185
>  cleanup_entry+0x218/0x350 net/ipv4/netfilter/ip_tables.c:659
>  __do_replace+0x7d7/0xa90 net/ipv4/netfilter/ip_tables.c:1096
>  do_replace net/ipv4/netfilter/ip_tables.c:1152 [inline]
>  do_ipt_set_ctl+0x40f/0x5f0 net/ipv4/netfilter/ip_tables.c:1682
>  nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
>  nf_setsockopt+0x67/0xc0 net/netfilter/nf_sockopt.c:115
>  ip_setsockopt+0xa1/0xb0 net/ipv4/ip_sockglue.c:1256
>  tcp_setsockopt+0x82/0xd0 net/ipv4/tcp.c:2875
>  sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2968
>  SYSC_setsockopt net/socket.c:1831 [inline]
>  SyS_setsockopt+0x189/0x360 net/socket.c:1810
>  entry_SYSCALL_64_fastpath+0x29/0xa0
> RIP: 0033:0x4449fa
> RSP: 002b:00007ffee653a948 EFLAGS: 00000206 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 00000000006cd0fc RCX: 00000000004449fa
> RDX: 0000000000000040 RSI: 0000000000000000 RDI: 0000000000000003
> RBP: 00000000006cd0fc R08: 00000000000002d8 R09: 000000000117c880
> R10: 00000000006cd528 R11: 0000000000000206 R12: 0000000000000003
> R13: 00000000006d00a4 R14: 00000000006d0050 R15: 00000000004a39ae
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
> 
> 
> ---
> This bug is generated by a dumb bot. It may contain errors.
> See https://goo.gl/tpsmEJ for details.
> Direct all questions to syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is
> merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title

This was fixed by commit 10414014bc085a:

#syz fix: netfilter: x_tables: fix missing timer initialization in xt_LED

- Eric

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Andrew Lunn @ 2018-05-13 20:05 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <CAGb2v66nqWnioS1pwfWFtMZFZSWvf4SwXr_ExnnGvYsK9RU_LA@mail.gmail.com>

> > Hi Chen-Yu
> >
> > Are these delays the MAC applies? Not the PHY. It would be good to
> > make it clear here these are MAC imposed delays.
> 
> Yes these are applied on the MAC side. Being described in the device
> tree bindings for the MAC, I thought this was implied to be the case?
> Are there known exceptions?

There is frequent confusion with this. Most of the time, the PHY does
the delay, not the MAC, based on the phy-mode. So the MAC doing it is
an exception in itself.

Do you actually need these delays for the board you adding support
for? Does the PHY not support adding the needed delays? If you don't
need the delays, i would not even implement them.

    Andrew

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Chen-Yu Tsai @ 2018-05-13 20:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Giuseppe Cavallaro, linux-arm-kernel, devicetree, netdev,
	Corentin Labbe, Icenowy Zheng, Maxime Ripard, Rob Herring
In-Reply-To: <20180513200529.GF12738@lunn.ch>

On Sun, May 13, 2018 at 1:05 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > Hi Chen-Yu
>> >
>> > Are these delays the MAC applies? Not the PHY. It would be good to
>> > make it clear here these are MAC imposed delays.
>>
>> Yes these are applied on the MAC side. Being described in the device
>> tree bindings for the MAC, I thought this was implied to be the case?
>> Are there known exceptions?
>
> There is frequent confusion with this. Most of the time, the PHY does
> the delay, not the MAC, based on the phy-mode. So the MAC doing it is
> an exception in itself.
>
> Do you actually need these delays for the board you adding support
> for? Does the PHY not support adding the needed delays? If you don't
> need the delays, i would not even implement them.

Yes this is already used on the Bananapi M3. This patch merely reformats
the description and adds a note saying this only applies to RGMII mode.

ChenYu

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Icenowy Zheng @ 2018-05-13 20:10 UTC (permalink / raw)
  To: linux-arm-kernel, Andrew Lunn, Chen-Yu Tsai
  Cc: devicetree, Maxime Ripard, netdev, Rob Herring, Corentin Labbe,
	Giuseppe Cavallaro, linux-arm-kernel
In-Reply-To: <20180513200529.GF12738@lunn.ch>



于 2018年5月14日 GMT+08:00 上午4:05:29, Andrew Lunn <andrew@lunn.ch> 写到:
>> > Hi Chen-Yu
>> >
>> > Are these delays the MAC applies? Not the PHY. It would be good to
>> > make it clear here these are MAC imposed delays.
>> 
>> Yes these are applied on the MAC side. Being described in the device
>> tree bindings for the MAC, I thought this was implied to be the case?
>> Are there known exceptions?
>
>There is frequent confusion with this. Most of the time, the PHY does
>the delay, not the MAC, based on the phy-mode. So the MAC doing it is
>an exception in itself.
>
>Do you actually need these delays for the board you adding support
>for? Does the PHY not support adding the needed delays? If you don't
>need the delays, i would not even implement them.

They are needed if the rx/tx have not equal length.

For several boards if the delay is not present, the network
is unusable. (I have at least met this on Banana Pi M3, an
Allwinner A83T SBC w/ RTL8211E)

>
>    Andrew
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox