* RE: [PATCH] net: mvpp2: avoid bouncing buffers
From: Yan Markman @ 2018-08-20 7:02 UTC (permalink / raw)
To: Christoph Hellwig, David Miller
Cc: brian.brooks@linaro.org, antoine.tenart@bootlin.com,
maxime.chevallier@bootlin.com, Stefan Chulski,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
bjorn.topel@intel.com, brian.brooks@arm.com
In-Reply-To: <20180820062326.GA22222@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]
Hi everybody
The MVPP2 code already has DMA's patch taken from OLD Backport and placed by Antoine Tenart.
Please refer the attached
Best regards
Yan Markman
-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org]
Sent: Monday, August 20, 2018 9:23 AM
To: David Miller <davem@davemloft.net>
Cc: brian.brooks@linaro.org; antoine.tenart@bootlin.com; maxime.chevallier@bootlin.com; Yan Markman <ymarkman@marvell.com>; Stefan Chulski <stefanc@marvell.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; bjorn.topel@intel.com; brian.brooks@arm.com
Subject: Re: [PATCH] net: mvpp2: avoid bouncing buffers
On Sun, Aug 19, 2018 at 07:55:05PM -0700, David Miller wrote:
> From: Brian Brooks <brian.brooks@linaro.org>
> Date: Sun, 19 Aug 2018 21:47:30 -0500
>
> > @@ -5126,6 +5126,12 @@ static int mvpp2_probe(struct platform_device *pdev)
> > }
> >
> > if (priv->hw_version == MVPP22) {
> > + /* Platform code may have set dev->dma_mask to point
> > + * to dev->coherent_dma_mask, but we want to ensure
> > + * they take different values due to comment below.
> > + */
> > + pdev->dev.dma_mask = &priv->dma_mask;
>
> The platform code might be doing this exactly because it cannot
> support different coherent and streaming DMA masks.
>
> Well, in any case, the platform code is doing it for a reason and
> overriding this in a "driver" of all places seems totally
> inappropriate and at best a layering violation.
>
> I would rather you fix this in a way that involves well defined APIs
> that set the DMA masks or whatever to the values that you need, rather
> than going behind the platform code's back and changing the DMA mask
> pointer like this.
Agreed. What platform do you see this issue on?
[-- Attachment #2: 0009-net-mvpp2-fix-the-dma_mask-and-coherent_dma_mask-set.patch --]
[-- Type: application/octet-stream, Size: 3572 bytes --]
From 810faf4533d343fe1df84853b7e22fc2733a8ea9 Mon Sep 17 00:00:00 2001
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Wed, 8 Aug 2018 14:30:58 +0300
Subject: [PATCH 09/40] net: mvpp2: fix the dma_mask and coherent_dma_mask
settings
The dev->dma_mask usually points to dev->coherent_dma_mask. This is an
issue as setting both of them will override the other. This is
problematic here as the PPv2 driver uses a 32-bit-mask for coherent
accesses (txq, rxq, bm) and a 40-bit mask for all other accesses due to
an hardware limitation.
This can lead to a memory remap for all dma_map_single() calls when
dealing with memory above 4GB.
Change-Id: Ib8a04a06a1f30bfa43b62250f21cfe09464d91ed
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Signed-off-by: Alex Zemtzov <azemtzov@marvell.com>
Signed-off-by: Yan Markman <ymarkman@marvell.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 ++
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 33 +++++++++++++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index f2d90e3..52a8e1d 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -708,6 +708,8 @@ struct mvpp2 {
/* Workqueue to gather hardware statistics */
char queue_name[30];
struct workqueue_struct *stats_queue;
+
+ bool custom_dma_mask;
};
struct mvpp2_pcpu_stats {
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index f107c08..a80727e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5130,10 +5130,29 @@ static int mvpp2_probe(struct platform_device *pdev)
return -EINVAL;
}
+ priv->custom_dma_mask = false;
if (priv->hw_version != MVPP21) {
+ /* If dma_mask points to coherent_dma_mask, setting both will
+ * override the value of the other. This is problematic as the
+ * PPv2 driver uses a 32-bit-mask for coherent accesses (txq,
+ * rxq, bm) and a 40-bit mask for all other accesses.
+ */
+ if (pdev->dev.dma_mask == &pdev->dev.coherent_dma_mask) {
+ pdev->dev.dma_mask =
+ kzalloc(sizeof(*pdev->dev.dma_mask),
+ GFP_KERNEL);
+ if (!pdev->dev.dma_mask) {
+ err = -ENOMEM;
+ goto err_mg_clk;
+ }
+
+ priv->custom_dma_mask = true;
+ }
+
err = dma_set_mask(&pdev->dev, MVPP2_DESC_DMA_MASK);
if (err)
- goto err_axi_clk;
+ goto err_dma_mask;
+
/* Sadly, the BM pools all share the same register to
* store the high 32 bits of their address. So they
* must all have the same high 32 bits, which forces
@@ -5141,7 +5160,7 @@ static int mvpp2_probe(struct platform_device *pdev)
*/
err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
if (err)
- goto err_axi_clk;
+ goto err_dma_mask;
}
/* Initialize network controller */
@@ -5189,6 +5208,11 @@ static int mvpp2_probe(struct platform_device *pdev)
mvpp2_port_remove(priv->port_list[i]);
i++;
}
+err_dma_mask:
+ if (priv->custom_dma_mask) {
+ kfree(pdev->dev.dma_mask);
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ }
err_axi_clk:
clk_disable_unprepare(priv->axi_clk);
@@ -5238,6 +5262,11 @@ static int mvpp2_remove(struct platform_device *pdev)
aggr_txq->descs_dma);
}
+ if (priv->custom_dma_mask) {
+ kfree(pdev->dev.dma_mask);
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ }
+
if (is_acpi_node(port_fwnode))
return 0;
--
2.0.4
^ permalink raw reply related
* WARNING in xfrm_policy_fini
From: syzbot @ 2018-08-20 4:14 UTC (permalink / raw)
To: davem, herbert, linux-kernel, netdev, steffen.klassert,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 3eb2ce825ea1 Linux 4.16-rc7
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1132989b800000
kernel config: https://syzkaller.appspot.com/x/.config?x=8addcf4530d93e53
dashboard link: https://syzkaller.appspot.com/bug?extid=9bce6db6c82f06b85d8b
compiler: gcc (GCC) 7.1.1 20170620
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+9bce6db6c82f06b85d8b@syzkaller.appspotmail.com
bond0 (unregistering): Released all slaves
WARNING: CPU: 1 PID: 7312 at net/xfrm/xfrm_policy.c:2925
xfrm_policy_fini+0x414/0x560 net/xfrm/xfrm_policy.c:2925
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 7312 Comm: kworker/u4:7 Not tainted 4.16.0-rc7+ #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: netns cleanup_net
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x24d lib/dump_stack.c:53
panic+0x1e4/0x41c kernel/panic.c:183
__warn+0x1dc/0x200 kernel/panic.c:547
report_bug+0x1f4/0x2b0 lib/bug.c:186
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+0x1b/0x40 arch/x86/entry/entry_64.S:986
RIP: 0010:xfrm_policy_fini+0x414/0x560 net/xfrm/xfrm_policy.c:2925
RSP: 0018:ffff8801b25d70d8 EFLAGS: 00010293
RAX: ffff8801b129c540 RBX: ffff8801b5608200 RCX: ffffffff8587ff74
RDX: 0000000000000000 RSI: 1ffff100362539c5 RDI: ffffffff87acabf8
RBP: ffff8801b25d7260 R08: 1ffff100364badd1 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff100364bae27
R13: ffff8801b25d7238 R14: ffff8801b56096f0 R15: ffff8801d71c5820
xfrm_net_exit+0x1d/0x70 net/xfrm/xfrm_policy.c:2980
ops_exit_list.isra.6+0xae/0x150 net/core/net_namespace.c:142
cleanup_net+0x6a1/0xcb0 net/core/net_namespace.c:517
process_one_work+0xc47/0x1bb0 kernel/workqueue.c:2113
worker_thread+0x223/0x1990 kernel/workqueue.c:2247
kthread+0x33c/0x400 kernel/kthread.c:238
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:406
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.
^ permalink raw reply
* Re: [PATCH bpf] xsk: fix return value of xdp_umem_assign_dev()
From: Song Liu @ 2018-08-20 5:11 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Björn Töpel,
Magnus Karlsson, David S . Miller, Networking
In-Reply-To: <20180820005425.2604-1-bhole_prashant_q7@lab.ntt.co.jp>
On Sun, Aug 19, 2018 at 5:54 PM, Prashant Bhole
<bhole_prashant_q7@lab.ntt.co.jp> wrote:
> s/ENOTSUPP/EOPNOTSUPP/ in function umem_assign_dev().
> This function's return value is directly returned by xsk_bind().
> EOPNOTSUPP is bind()'s possible return value.
>
> Fixes: f734607e819b ("xsk: refactor xdp_umem_assign_dev()")
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> net/xdp/xdp_umem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 911ca6d3cb5a..bfe2dbea480b 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -74,14 +74,14 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> return 0;
>
> if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_async_xmit)
> - return force_zc ? -ENOTSUPP : 0; /* fail or fallback */
> + return force_zc ? -EOPNOTSUPP : 0; /* fail or fallback */
>
> bpf.command = XDP_QUERY_XSK_UMEM;
>
> rtnl_lock();
> err = xdp_umem_query(dev, queue_id);
> if (err) {
> - err = err < 0 ? -ENOTSUPP : -EBUSY;
> + err = err < 0 ? -EOPNOTSUPP : -EBUSY;
> goto err_rtnl_unlock;
> }
>
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [LKP] ab9ee8e38b [ 22.890412] WARNING: CPU: 0 PID: 632 at mm/usercopy.c:81 usercopy_warn
From: Kees Cook @ 2018-08-20 5:31 UTC (permalink / raw)
To: kernel test robot
Cc: David Windsor, linux-sctp, Network Development, LKML, LKP
In-Reply-To: <20180820005204.GB31827@shao2-debian>
On Sun, Aug 19, 2018 at 5:52 PM, kernel test robot
<rong.a.chen@intel.com> wrote:
> [ 22.855033] Bad or missing usercopy whitelist? Kernel memory overwrite attempt detected to SLUB object 'SCTP' (offset 1332, size 4)!
For this .config, pahole reports 1332 as the offset of the whitelist,
so I'm not sure what's happening here. It's as if no whitelist was
used when building the cache?
I'm still trying to build a reproducers...
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [bpf-next RFC 1/3] flow_dissector: implements flow dissector BPF hook
From: Song Liu @ 2018-08-20 5:44 UTC (permalink / raw)
To: Petar Penkov
Cc: Petar Penkov, Networking, David S . Miller, Alexei Starovoitov,
Daniel Borkmann, simon.horman, Willem de Bruijn
In-Reply-To: <CAG4SDVVS1Akdg7hkFwkencBk_rZL5zGRYREHtAiK3+Cg=oL3pw@mail.gmail.com>
On Thu, Aug 16, 2018 at 4:14 PM, Petar Penkov <ppenkov@google.com> wrote:
> On Thu, Aug 16, 2018 at 3:40 PM, Song Liu <liu.song.a23@gmail.com> wrote:
>>
>> On Thu, Aug 16, 2018 at 9:44 AM, Petar Penkov <peterpenkov96@gmail.com> wrote:
>> > From: Petar Penkov <ppenkov@google.com>
>> >
>> > Adds a hook for programs of type BPF_PROG_TYPE_FLOW_DISSECTOR and
>> > attach type BPF_FLOW_DISSECTOR that is executed in the flow dissector
>> > path. The BPF program is kept as a global variable so it is
>> > accessible to all flow dissectors.
>> >
>> > Signed-off-by: Petar Penkov <ppenkov@google.com>
>> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>> > ---
>> > include/linux/bpf_types.h | 1 +
>> > include/linux/skbuff.h | 7 +
>> > include/net/flow_dissector.h | 16 +++
>> > include/uapi/linux/bpf.h | 14 +-
>> > kernel/bpf/syscall.c | 8 ++
>> > kernel/bpf/verifier.c | 2 +
>> > net/core/filter.c | 157 ++++++++++++++++++++++
>> > net/core/flow_dissector.c | 76 +++++++++++
>> > tools/bpf/bpftool/prog.c | 1 +
>> > tools/include/uapi/linux/bpf.h | 5 +-
>> > tools/lib/bpf/libbpf.c | 2 +
>> > tools/testing/selftests/bpf/bpf_helpers.h | 3 +
>> > 12 files changed, 290 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
>> > index cd26c090e7c0..22083712dd18 100644
>> > --- a/include/linux/bpf_types.h
>> > +++ b/include/linux/bpf_types.h
>> > @@ -32,6 +32,7 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
>> > #ifdef CONFIG_INET
>> > BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
>> > #endif
>> > +BPF_PROG_TYPE(BPF_PROG_TYPE_FLOW_DISSECTOR, flow_dissector)
>> >
>> > BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
>> > BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
>> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> > index 17a13e4785fc..ce0e863f02a2 100644
>> > --- a/include/linux/skbuff.h
>> > +++ b/include/linux/skbuff.h
>> > @@ -243,6 +243,8 @@ struct scatterlist;
>> > struct pipe_inode_info;
>> > struct iov_iter;
>> > struct napi_struct;
>> > +struct bpf_prog;
>> > +union bpf_attr;
>> >
>> > #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
>> > struct nf_conntrack {
>> > @@ -1192,6 +1194,11 @@ void skb_flow_dissector_init(struct flow_dissector *flow_dissector,
>> > const struct flow_dissector_key *key,
>> > unsigned int key_count);
>> >
>> > +int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
>> > + struct bpf_prog *prog);
>> > +
>> > +int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr);
>> > +
>> > bool __skb_flow_dissect(const struct sk_buff *skb,
>> > struct flow_dissector *flow_dissector,
>> > void *target_container,
>> > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>> > index 6a4586dcdede..edb919d320c1 100644
>> > --- a/include/net/flow_dissector.h
>> > +++ b/include/net/flow_dissector.h
>> > @@ -270,6 +270,22 @@ __be32 flow_get_u32_dst(const struct flow_keys *flow);
>> > extern struct flow_dissector flow_keys_dissector;
>> > extern struct flow_dissector flow_keys_basic_dissector;
>> >
>> > +/* struct bpf_flow_dissect_cb:
>> > + *
>> > + * This struct is used to pass parameters to BPF programs of type
>> > + * BPF_PROG_TYPE_FLOW_DISSECTOR. Before such a program is run, the caller sets
>> > + * the control block of the skb to be a struct of this type. The first field is
>> > + * used to communicate the next header offset between the BPF programs and the
>> > + * first value of it is passed from the kernel. The last two fields are used for
>> > + * writing out flow keys.
>> > + */
>> > +struct bpf_flow_dissect_cb {
>> > + u16 nhoff;
>> > + u16 unused;
>> > + void *target_container;
>> > + struct flow_dissector *flow_dissector;
>> > +};
>> > +
>> > /* struct flow_keys_digest:
>> > *
>> > * This structure is used to hold a digest of the full flow keys. This is a
>> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> > index 66917a4eba27..8bc0fdab685d 100644
>> > --- a/include/uapi/linux/bpf.h
>> > +++ b/include/uapi/linux/bpf.h
>> > @@ -152,6 +152,7 @@ enum bpf_prog_type {
>> > BPF_PROG_TYPE_LWT_SEG6LOCAL,
>> > BPF_PROG_TYPE_LIRC_MODE2,
>> > BPF_PROG_TYPE_SK_REUSEPORT,
>> > + BPF_PROG_TYPE_FLOW_DISSECTOR,
>> > };
>> >
>> > enum bpf_attach_type {
>> > @@ -172,6 +173,7 @@ enum bpf_attach_type {
>> > BPF_CGROUP_UDP4_SENDMSG,
>> > BPF_CGROUP_UDP6_SENDMSG,
>> > BPF_LIRC_MODE2,
>> > + BPF_FLOW_DISSECTOR,
>> > __MAX_BPF_ATTACH_TYPE
>> > };
>> >
>> > @@ -2141,6 +2143,15 @@ union bpf_attr {
>> > * request in the skb.
>> > * Return
>> > * 0 on success, or a negative error in case of failure.
>> > + *
>> > + * int bpf_flow_dissector_write_keys(const struct sk_buff *skb, const void *from, u32 len, enum flow_dissector_key_id key_id)
>> > + * Description
>> > + * Try to write *len* bytes from the source pointer into the offset
>> > + * of the key with id *key_id*. If *len* is different from the
>> > + * size of the key, an error is returned. If the key is not used,
>> > + * this function exits with no effect and code 0.
>> > + * Return
>> > + * 0 on success, negative error in case of failure.
>> > */
>> > #define __BPF_FUNC_MAPPER(FN) \
>> > FN(unspec), \
>> > @@ -2226,7 +2237,8 @@ union bpf_attr {
>> > FN(get_current_cgroup_id), \
>> > FN(get_local_storage), \
>> > FN(sk_select_reuseport), \
>> > - FN(skb_ancestor_cgroup_id),
>> > + FN(skb_ancestor_cgroup_id), \
>> > + FN(flow_dissector_write_keys),
>> >
>> > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>> > * function eBPF program intends to call
>> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> > index 43727ed0d94a..a06568841a92 100644
>> > --- a/kernel/bpf/syscall.c
>> > +++ b/kernel/bpf/syscall.c
>> > @@ -1616,6 +1616,9 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>> > case BPF_LIRC_MODE2:
>> > ptype = BPF_PROG_TYPE_LIRC_MODE2;
>> > break;
>> > + case BPF_FLOW_DISSECTOR:
>> > + ptype = BPF_PROG_TYPE_FLOW_DISSECTOR;
>> > + break;
>> > default:
>> > return -EINVAL;
>> > }
>> > @@ -1637,6 +1640,9 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>> > case BPF_PROG_TYPE_LIRC_MODE2:
>> > ret = lirc_prog_attach(attr, prog);
>> > break;
>> > + case BPF_PROG_TYPE_FLOW_DISSECTOR:
>> > + ret = skb_flow_dissector_bpf_prog_attach(attr, prog);
>> > + break;
>> > default:
>> > ret = cgroup_bpf_prog_attach(attr, ptype, prog);
>> > }
>> > @@ -1689,6 +1695,8 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>> > return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, NULL);
>> > case BPF_LIRC_MODE2:
>> > return lirc_prog_detach(attr);
>> > + case BPF_FLOW_DISSECTOR:
>> > + return skb_flow_dissector_bpf_prog_detach(attr);
>> > default:
>> > return -EINVAL;
>> > }
>> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> > index ca90679a7fe5..6d3f268fa8e0 100644
>> > --- a/kernel/bpf/verifier.c
>> > +++ b/kernel/bpf/verifier.c
>> > @@ -1321,6 +1321,7 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
>> > case BPF_PROG_TYPE_LWT_XMIT:
>> > case BPF_PROG_TYPE_SK_SKB:
>> > case BPF_PROG_TYPE_SK_MSG:
>> > + case BPF_PROG_TYPE_FLOW_DISSECTOR:
>> > if (meta)
>> > return meta->pkt_access;
>> >
>> > @@ -3976,6 +3977,7 @@ static bool may_access_skb(enum bpf_prog_type type)
>> > case BPF_PROG_TYPE_SOCKET_FILTER:
>> > case BPF_PROG_TYPE_SCHED_CLS:
>> > case BPF_PROG_TYPE_SCHED_ACT:
>> > + case BPF_PROG_TYPE_FLOW_DISSECTOR:
>> > return true;
>> > default:
>> > return false;
>> > diff --git a/net/core/filter.c b/net/core/filter.c
>> > index fd423ce3da34..03d3037e6508 100644
>> > --- a/net/core/filter.c
>> > +++ b/net/core/filter.c
>> > @@ -4820,6 +4820,111 @@ bool bpf_helper_changes_pkt_data(void *func)
>> > return false;
>> > }
>> >
>> > +BPF_CALL_4(bpf_flow_dissector_write_keys, const struct sk_buff *, skb,
>> > + const void *, from, u32, len, enum flow_dissector_key_id, key_id)
>> > +{
>> > + struct bpf_flow_dissect_cb *cb;
>> > + void *dest;
>> > +
>> > + cb = (struct bpf_flow_dissect_cb *)bpf_skb_cb(skb);
>> > +
>> > + /* Make sure the dissector actually uses the key. It is not an error if
>> > + * it does not, but we should not continue past this point in that case
>> > + */
>> > + if (!dissector_uses_key(cb->flow_dissector, key_id))
>> > + return 0;
>> > +
>> > + /* Make sure the length is correct */
>> > + switch (key_id) {
>> > + case FLOW_DISSECTOR_KEY_CONTROL:
>> > + case FLOW_DISSECTOR_KEY_ENC_CONTROL:
>> > + if (len != sizeof(struct flow_dissector_key_control))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_BASIC:
>> > + if (len != sizeof(struct flow_dissector_key_basic))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
>> > + case FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS:
>> > + if (len != sizeof(struct flow_dissector_key_ipv4_addrs))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
>> > + case FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS:
>> > + if (len != sizeof(struct flow_dissector_key_ipv6_addrs))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_ICMP:
>> > + if (len != sizeof(struct flow_dissector_key_icmp))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_PORTS:
>> > + case FLOW_DISSECTOR_KEY_ENC_PORTS:
>> > + if (len != sizeof(struct flow_dissector_key_ports))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_ETH_ADDRS:
>> > + if (len != sizeof(struct flow_dissector_key_eth_addrs))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_TIPC:
>> > + if (len != sizeof(struct flow_dissector_key_tipc))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_ARP:
>> > + if (len != sizeof(struct flow_dissector_key_arp))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_VLAN:
>> > + case FLOW_DISSECTOR_KEY_CVLAN:
>> > + if (len != sizeof(struct flow_dissector_key_vlan))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_FLOW_LABEL:
>> > + if (len != sizeof(struct flow_dissector_key_tags))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_GRE_KEYID:
>> > + case FLOW_DISSECTOR_KEY_ENC_KEYID:
>> > + case FLOW_DISSECTOR_KEY_MPLS_ENTROPY:
>> > + if (len != sizeof(struct flow_dissector_key_keyid))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_MPLS:
>> > + if (len != sizeof(struct flow_dissector_key_mpls))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_TCP:
>> > + if (len != sizeof(struct flow_dissector_key_tcp))
>> > + return -EINVAL;
>> > + break;
>> > + case FLOW_DISSECTOR_KEY_IP:
>> > + case FLOW_DISSECTOR_KEY_ENC_IP:
>> > + if (len != sizeof(struct flow_dissector_key_ip))
>> > + return -EINVAL;
>> > + break;
>> > + default:
>> > + return -EINVAL;
>> > + }
>> > +
>> > + dest = skb_flow_dissector_target(cb->flow_dissector, key_id,
>> > + cb->target_container);
>> > +
>> > + memcpy(dest, from, len);
>> > + return 0;
>> > +}
>> > +
>> > +static const struct bpf_func_proto bpf_flow_dissector_write_keys_proto = {
>> > + .func = bpf_flow_dissector_write_keys,
>> > + .gpl_only = false,
>> > + .ret_type = RET_INTEGER,
>> > + .arg1_type = ARG_PTR_TO_CTX,
>> > + .arg2_type = ARG_PTR_TO_MEM,
>> > + .arg3_type = ARG_CONST_SIZE,
>> > + .arg4_type = ARG_ANYTHING,
>> > +};
>> > +
>> > static const struct bpf_func_proto *
>> > bpf_base_func_proto(enum bpf_func_id func_id)
>> > {
>> > @@ -5100,6 +5205,19 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>> > }
>> > }
>> >
>> > +static const struct bpf_func_proto *
>> > +flow_dissector_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>> > +{
>> > + switch (func_id) {
>> > + case BPF_FUNC_skb_load_bytes:
>> > + return &bpf_skb_load_bytes_proto;
>> > + case BPF_FUNC_flow_dissector_write_keys:
>> > + return &bpf_flow_dissector_write_keys_proto;
>> > + default:
>> > + return bpf_base_func_proto(func_id);
>> > + }
>> > +}
>> > +
>> > static const struct bpf_func_proto *
>> > lwt_out_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>> > {
>> > @@ -5738,6 +5856,35 @@ static bool sk_msg_is_valid_access(int off, int size,
>> > return true;
>> > }
>> >
>> > +static bool flow_dissector_is_valid_access(int off, int size,
>> > + enum bpf_access_type type,
>> > + const struct bpf_prog *prog,
>> > + struct bpf_insn_access_aux *info)
>> > +{
>> > + if (type == BPF_WRITE) {
>> > + switch (off) {
>> > + case bpf_ctx_range(struct __sk_buff, cb[0]):
>> > + break;
>> > + default:
>> > + return false;
>> > + }
>> > + }
>> > +
>> > + switch (off) {
>> > + case bpf_ctx_range(struct __sk_buff, data):
>> > + info->reg_type = PTR_TO_PACKET;
>> > + break;
>> > + case bpf_ctx_range(struct __sk_buff, data_end):
>> > + info->reg_type = PTR_TO_PACKET_END;
>> > + break;
>> > + case bpf_ctx_range_till(struct __sk_buff, family, local_port):
>> > + case bpf_ctx_range_till(struct __sk_buff, cb[1], cb[4]):
>> > + return false;
>> > + }
>> > +
>> > + return bpf_skb_is_valid_access(off, size, type, prog, info);
>> > +}
>> > +
>> > static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>> > const struct bpf_insn *si,
>> > struct bpf_insn *insn_buf,
>> > @@ -6995,6 +7142,16 @@ const struct bpf_verifier_ops sk_msg_verifier_ops = {
>> > const struct bpf_prog_ops sk_msg_prog_ops = {
>> > };
>> >
>> > +const struct bpf_verifier_ops flow_dissector_verifier_ops = {
>> > + .get_func_proto = flow_dissector_func_proto,
>> > + .is_valid_access = flow_dissector_is_valid_access,
>> > + .convert_ctx_access = bpf_convert_ctx_access,
>> > + .gen_ld_abs = bpf_gen_ld_abs,
>> > +};
>> > +
>> > +const struct bpf_prog_ops flow_dissector_prog_ops = {
>> > +};
>> > +
>> > int sk_detach_filter(struct sock *sk)
>> > {
>> > int ret = -ENOENT;
>> > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> > index ce9eeeb7c024..767daa231f04 100644
>> > --- a/net/core/flow_dissector.c
>> > +++ b/net/core/flow_dissector.c
>> > @@ -25,6 +25,11 @@
>> > #include <net/flow_dissector.h>
>> > #include <scsi/fc/fc_fcoe.h>
>> > #include <uapi/linux/batadv_packet.h>
>> > +#include <linux/bpf.h>
>> > +
>> > +/* BPF program accessible by all flow dissectors */
>> > +static struct bpf_prog __rcu *flow_dissector_prog;
>> > +static DEFINE_MUTEX(flow_dissector_mutex);
>> >
>> > static void dissector_set_key(struct flow_dissector *flow_dissector,
>> > enum flow_dissector_key_id key_id)
>> > @@ -62,6 +67,40 @@ void skb_flow_dissector_init(struct flow_dissector *flow_dissector,
>> > }
>> > EXPORT_SYMBOL(skb_flow_dissector_init);
>> >
>> > +int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
>> > + struct bpf_prog *prog)
>> > +{
>> > + struct bpf_prog *attached;
>> > +
>> > + mutex_lock(&flow_dissector_mutex);
>> > + attached = rcu_dereference_protected(flow_dissector_prog,
>> > + lockdep_is_held(&flow_dissector_mutex));
>> > + if (attached) {
>> > + /* Only one BPF program can be attached at a time */
>> > + mutex_unlock(&flow_dissector_mutex);
>> > + return -EEXIST;
>> > + }
>> > + rcu_assign_pointer(flow_dissector_prog, prog);
>> > + mutex_unlock(&flow_dissector_mutex);
>> > + return 0;
>> > +}
>> > +
>> > +int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
>> > +{
>> > + struct bpf_prog *attached;
>> > +
>> > + mutex_lock(&flow_dissector_mutex);
>> > + attached = rcu_dereference_protected(flow_dissector_prog,
>> > + lockdep_is_held(&flow_dissector_mutex));
>> > + if (!flow_dissector_prog) {
>> > + mutex_unlock(&flow_dissector_mutex);
>> > + return -EINVAL;
>> > + }
>> > + bpf_prog_put(attached);
>> > + RCU_INIT_POINTER(flow_dissector_prog, NULL);
>> > + mutex_unlock(&flow_dissector_mutex);
>> > + return 0;
>> > +}
>> > /**
>> > * skb_flow_get_be16 - extract be16 entity
>> > * @skb: sk_buff to extract from
>> > @@ -619,6 +658,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>> > struct flow_dissector_key_vlan *key_vlan;
>> > enum flow_dissect_ret fdret;
>> > enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX;
>> > + struct bpf_prog *attached;
>> > int num_hdrs = 0;
>> > u8 ip_proto = 0;
>> > bool ret;
>> > @@ -658,6 +698,42 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>> > FLOW_DISSECTOR_KEY_BASIC,
>> > target_container);
>> >
>> > + rcu_read_lock();
>> > + attached = rcu_dereference(flow_dissector_prog);
>> > + if (attached) {
>> > + /* Note that even though the const qualifier is discarded
>> > + * throughout the execution of the BPF program, all changes(the
>> > + * control block) are reverted after the BPF program returns.
>> > + * Therefore, __skb_flow_dissect does not alter the skb.
>> > + */
>> > + struct bpf_flow_dissect_cb *cb;
>> > + u8 cb_saved[BPF_SKB_CB_LEN];
>> > + u32 result;
>> > +
>> > + cb = (struct bpf_flow_dissect_cb *)(bpf_skb_cb((struct sk_buff *)skb));
>> > +
>> > + /* Save Control Block */
>> > + memcpy(cb_saved, cb, sizeof(cb_saved));
>> > + memset(cb, 0, sizeof(cb_saved));
>> > +
>> > + /* Pass parameters to the BPF program */
>> > + cb->nhoff = nhoff;
>> > + cb->target_container = target_container;
>> > + cb->flow_dissector = flow_dissector;
>> > +
>> > + bpf_compute_data_pointers((struct sk_buff *)skb);
>> > + result = BPF_PROG_RUN(attached, skb);
>> > +
>> > + /* Restore state */
>> > + memcpy(cb, cb_saved, sizeof(cb_saved));
>> > +
>> > + key_control->thoff = min_t(u16, key_control->thoff,
>> > + skb ? skb->len : hlen);
>> > + rcu_read_unlock();
>> > + return result == BPF_OK;
>> > + }
>>
>> If the BPF program cannot handle certain protocol, shall we fall back
>> to the built-in logic? Otherwise, all BPF programs need to have some
>> code for all protocols.
>>
>> Song
>
> I believe that if we fall back to the built-in logic we lose all security
> guarantees from BPF and this is why the code does not support
> fall back.
>
> Petar
I am not really sure we are on the same page. I am proposing 3
different return values from BPF_PROG_RUN(), and they should be
handled as
1. result == BPF_OK => return true;
2. result == BPF_DROP => return false;
3. result == something else => fall back.
Does this proposal make any sense?
Thanks,
Song
>>
>>
>> > + rcu_read_unlock();
>> > +
>> > if (dissector_uses_key(flow_dissector,
>> > FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
>> > struct ethhdr *eth = eth_hdr(skb);
>> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> > index dce960d22106..b1cd3bc8db70 100644
>> > --- a/tools/bpf/bpftool/prog.c
>> > +++ b/tools/bpf/bpftool/prog.c
>> > @@ -74,6 +74,7 @@ static const char * const prog_type_name[] = {
>> > [BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
>> > [BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
>> > [BPF_PROG_TYPE_LIRC_MODE2] = "lirc_mode2",
>> > + [BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
>> > };
>> >
>> > static void print_boot_time(__u64 nsecs, char *buf, unsigned int size)
>> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> > index 66917a4eba27..acd74a0dd063 100644
>> > --- a/tools/include/uapi/linux/bpf.h
>> > +++ b/tools/include/uapi/linux/bpf.h
>> > @@ -152,6 +152,7 @@ enum bpf_prog_type {
>> > BPF_PROG_TYPE_LWT_SEG6LOCAL,
>> > BPF_PROG_TYPE_LIRC_MODE2,
>> > BPF_PROG_TYPE_SK_REUSEPORT,
>> > + BPF_PROG_TYPE_FLOW_DISSECTOR,
>> > };
>> >
>> > enum bpf_attach_type {
>> > @@ -172,6 +173,7 @@ enum bpf_attach_type {
>> > BPF_CGROUP_UDP4_SENDMSG,
>> > BPF_CGROUP_UDP6_SENDMSG,
>> > BPF_LIRC_MODE2,
>> > + BPF_FLOW_DISSECTOR,
>> > __MAX_BPF_ATTACH_TYPE
>> > };
>> >
>> > @@ -2226,7 +2228,8 @@ union bpf_attr {
>> > FN(get_current_cgroup_id), \
>> > FN(get_local_storage), \
>> > FN(sk_select_reuseport), \
>> > - FN(skb_ancestor_cgroup_id),
>> > + FN(skb_ancestor_cgroup_id), \
>> > + FN(flow_dissector_write_keys),
>> >
>> > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>> > * function eBPF program intends to call
>> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> > index 2abd0f112627..0c749ce1b717 100644
>> > --- a/tools/lib/bpf/libbpf.c
>> > +++ b/tools/lib/bpf/libbpf.c
>> > @@ -1502,6 +1502,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
>> > case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
>> > case BPF_PROG_TYPE_LIRC_MODE2:
>> > case BPF_PROG_TYPE_SK_REUSEPORT:
>> > + case BPF_PROG_TYPE_FLOW_DISSECTOR:
>> > return false;
>> > case BPF_PROG_TYPE_UNSPEC:
>> > case BPF_PROG_TYPE_KPROBE:
>> > @@ -2121,6 +2122,7 @@ static const struct {
>> > BPF_PROG_SEC("sk_skb", BPF_PROG_TYPE_SK_SKB),
>> > BPF_PROG_SEC("sk_msg", BPF_PROG_TYPE_SK_MSG),
>> > BPF_PROG_SEC("lirc_mode2", BPF_PROG_TYPE_LIRC_MODE2),
>> > + BPF_PROG_SEC("flow_dissector", BPF_PROG_TYPE_FLOW_DISSECTOR),
>> > BPF_SA_PROG_SEC("cgroup/bind4", BPF_CGROUP_INET4_BIND),
>> > BPF_SA_PROG_SEC("cgroup/bind6", BPF_CGROUP_INET6_BIND),
>> > BPF_SA_PROG_SEC("cgroup/connect4", BPF_CGROUP_INET4_CONNECT),
>> > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>> > index e4be7730222d..4204c496a04f 100644
>> > --- a/tools/testing/selftests/bpf/bpf_helpers.h
>> > +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>> > @@ -143,6 +143,9 @@ static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
>> > (void *) BPF_FUNC_skb_cgroup_id;
>> > static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
>> > (void *) BPF_FUNC_skb_ancestor_cgroup_id;
>> > +static int (*bpf_flow_dissector_write_keys)(void *ctx, void *src, int len,
>> > + int key) =
>> > + (void *) BPF_FUNC_flow_dissector_write_keys;
>> >
>> > /* llvm builtin functions that eBPF C program may use to
>> > * emit BPF_LD_ABS and BPF_LD_IND instructions
>> > --
>> > 2.18.0.865.gffc8e1a3cd6-goog
>> >
^ permalink raw reply
* Re: [PATCH] net: mvpp2: avoid bouncing buffers
From: Christoph Hellwig @ 2018-08-20 6:23 UTC (permalink / raw)
To: David Miller
Cc: brian.brooks, antoine.tenart, maxime.chevallier, ymarkman,
stefanc, netdev, linux-kernel, bjorn.topel, brian.brooks
In-Reply-To: <20180819.195505.1988137313680465320.davem@davemloft.net>
On Sun, Aug 19, 2018 at 07:55:05PM -0700, David Miller wrote:
> From: Brian Brooks <brian.brooks@linaro.org>
> Date: Sun, 19 Aug 2018 21:47:30 -0500
>
> > @@ -5126,6 +5126,12 @@ static int mvpp2_probe(struct platform_device *pdev)
> > }
> >
> > if (priv->hw_version == MVPP22) {
> > + /* Platform code may have set dev->dma_mask to point
> > + * to dev->coherent_dma_mask, but we want to ensure
> > + * they take different values due to comment below.
> > + */
> > + pdev->dev.dma_mask = &priv->dma_mask;
>
> The platform code might be doing this exactly because it cannot support
> different coherent and streaming DMA masks.
>
> Well, in any case, the platform code is doing it for a reason and
> overriding this in a "driver" of all places seems totally
> inappropriate and at best a layering violation.
>
> I would rather you fix this in a way that involves well defined APIs
> that set the DMA masks or whatever to the values that you need, rather
> than going behind the platform code's back and changing the DMA mask
> pointer like this.
Agreed. What platform do you see this issue on?
^ permalink raw reply
* [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
From: Haiqing Bai @ 2018-08-20 10:26 UTC (permalink / raw)
To: jon.maloy, ying.xue, davem, zhenbo.gao; +Cc: netdev, linux-kernel
The tipc multicast demo in tipcutils fails to work on big-endian hardware.
The tipc multicast server can not receive the packets sent by the multicast
client for that the dest port is always zero after tipc_dest_pop, then it
is found that the struct tipc_dest fails to take big/little endian into
account.
Signed-off-by: Haiqing Bai <Haiqing.Bai@windriver.com>
Signed-off-by: Zhenbo Gao <zhenbo.gao@windriver.com>
---
net/tipc/name_table.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 0febba4..6e1e0ab 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -135,8 +135,13 @@ struct tipc_dest {
struct list_head list;
union {
struct {
+#ifdef __LITTLE_ENDIAN_BITFIELD
u32 port;
u32 node;
+#else /* __BIG_ENDIAN_BITFIELD */
+ u32 node;
+ u32 port;
+#endif
};
u64 value;
};
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Srinivas Kandagatla @ 2018-08-20 10:43 UTC (permalink / raw)
To: Boris Brezillon, Alban
Cc: Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori, Kevin Hilman,
Russell King, Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse,
Brian Norris, Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan Carpenter, Florian Fainelli, Ivan
In-Reply-To: <20180819184609.6dcdbb9a@bbrezillon>
Thanks Boris, for looking into this in more detail.
On 19/08/18 17:46, Boris Brezillon wrote:
> On Sun, 19 Aug 2018 13:31:06 +0200
> Alban <albeu@free.fr> wrote:
>
>> On Fri, 17 Aug 2018 18:27:20 +0200
>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>
>>> Hi Bartosz,
>>>
>>> On Fri, 10 Aug 2018 10:05:03 +0200
>>> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>
>>>> From: Alban Bedel <albeu@free.fr>
>>>>
>>>> Allow drivers that use the nvmem API to read data stored on MTD devices.
>>>> For this the mtd devices are registered as read-only NVMEM providers.
>>>>
>>>> Signed-off-by: Alban Bedel <albeu@free.fr>
>>>> [Bartosz:
>>>> - use the managed variant of nvmem_register(),
>>>> - set the nvmem name]
>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> What happened to the 2 other patches of Alban's series? I'd really
>>> like the DT case to be handled/agreed on in the same patchset, but
>>> IIRC, Alban and Srinivas disagreed on how this should be represented.
>>> I hope this time we'll come to an agreement, because the MTD <-> NVMEM
>>> glue has been floating around for quite some time...
>>
>> These other patches were to fix what I consider a fundamental flaw in
>> the generic NVMEM bindings, however we couldn't agree on this point.
>> Bartosz later contacted me to take over this series and I suggested to
>> just change the MTD NVMEM binding to use a compatible string on the
>> NVMEM cells as an alternative solution to fix the clash with the old
>> style MTD partition.
>>
>> However all this has no impact on the code needed to add NVMEM support
>> to MTD, so the above patch didn't change at all.
>
> It does have an impact on the supported binding though.
> nvmem->dev.of_node is automatically assigned to mtd->dev.of_node, which
> means people will be able to define their NVMEM cells directly under
> the MTD device and reference them from other nodes (even if it's not
> documented), and as you said, it conflict with the old MTD partition
> bindings. So we'd better agree on this binding before merging this
> patch.
>
Yes, I agree with you!
> I see several options:
>
> 1/ provide a way to tell the NVMEM framework not to use parent->of_node
> even if it's != NULL. This way we really don't support defining
> NVMEM cells in the DT, and also don't support referencing the nvmem
> device using a phandle.
>
Other options look much better than this one!
> 2/ define a new binding where all nvmem-cells are placed in an
> "nvmem" subnode (just like we have this "partitions" subnode for
> partitions), and then add a config->of_node field so that the
> nvmem provider can explicitly specify the DT node representing the
> nvmem device. We'll also need to set this field to ERR_PTR(-ENOENT)
> in case this node does not exist so that the nvmem framework knows
> that it should not assign nvmem->dev.of_node to parent->of_node
>
This one looks promising, One Question though..
Do we expect that there would be nvmem cells in any of the partitions?
or
nvmem cell are only valid for unpartioned area?
Am sure that the nvmem cells would be in multiple partitions, Is it okay
to have some parts of partition to be in a separate subnode?
I would like this case to be considered too.
> 3/ only declare partitions as nvmem providers. This would solve the
> problem we have with partitions defined in the DT since
> defining sub-partitions in the DT is not (yet?) supported and
> partition nodes are supposed to be leaf nodes. Still, I'm not a big
> fan of this solution because it will prevent us from supporting
> sub-partitions if we ever want/need to.
>
This one is going to come back so, its better we
> 4/ Add a ->of_xlate() hook that would be called if present by the
> framework instead of using the default parsing we have right now.
This looks much cleaner! We could hook that up under
__nvmem_device_get() to do that translation.
>
> 5/ Tell the nvmem framework the name of the subnode containing nvmem
> cell definitions (if NULL that means cells are directly defined
> under the nvmem provider node). We would set it to "nvmem-cells" (or
> whatever you like) for the MTD case.
Option 2 looks better than this.
>
> There are probably other options (some were proposed by Alban and
> Srinivas already), but I'd like to get this sorted out before we merge
> this patch.
>
> Alban, Srinivas, any opinion?
Overall am still not able to clear visualize on how MTD bindings with
nvmem cells would look in both partition and un-partition usecases?
An example DT would be nice here!!
Option 4 looks like much generic solution to me, may be we should try
this once bindings on MTD side w.r.t nvmem cells are decided.
Thanks,
Srini
>
^ permalink raw reply
* [PATCH v2] wcn36xx: Use dma_zalloc_coherent instead of dma_alloc_coherent + memset
From: zhong jiang @ 2018-08-20 10:44 UTC (permalink / raw)
To: kvalo; +Cc: davem, wcn36xx, linux-wireless, netdev, linux-kernel
dma_zalloc_coherent has implemented the dma_alloc_coherent() + memset(),
We prefer to dma_zalloc_coherent instead of open-codeing.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
drivers/net/wireless/ath/wcn36xx/dxe.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
index 06cfe8d..e66ddaa 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -174,13 +174,11 @@ static int wcn36xx_dxe_init_descs(struct device *dev, struct wcn36xx_dxe_ch *wcn
int i;
size = wcn_ch->desc_num * sizeof(struct wcn36xx_dxe_desc);
- wcn_ch->cpu_addr = dma_alloc_coherent(dev, size, &wcn_ch->dma_addr,
- GFP_KERNEL);
+ wcn_ch->cpu_addr = dma_zalloc_coherent(dev, size, &wcn_ch->dma_addr,
+ GFP_KERNEL);
if (!wcn_ch->cpu_addr)
return -ENOMEM;
- memset(wcn_ch->cpu_addr, 0, size);
-
cur_dxe = (struct wcn36xx_dxe_desc *)wcn_ch->cpu_addr;
cur_ctl = wcn_ch->head_blk_ctl;
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH] wireless: Use dma_zalloc_coherent instead of dma_alloc_coherent + memset
From: zhong jiang @ 2018-08-20 10:53 UTC (permalink / raw)
To: Kalle Valo; +Cc: davem, linux-kernel, netdev
In-Reply-To: <87lg93ttkv.fsf@kamboji.qca.qualcomm.com>
On 2018/8/19 2:31, Kalle Valo wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
>> zhong jiang <zhongjiang@huawei.com> writes:
>>
>>> dma_zalloc_coherent has implemented the dma_alloc_coherent() + memset (),
>>> We prefer to dma_zalloc_coherent instead of open-codeing.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>> drivers/net/wireless/ath/wcn36xx/dxe.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>> The correct prefix is "wcn36xx: ", not "wireless:". I can fix it this
>> time.
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong
> Actually please resend this patch and CC linux-wireless so that
> patchwork sees this.
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#who_to_address
>
Thanks. I will resend it in v2.
Best wishes,
zhong jiang
^ permalink raw reply
* Re: [PATCH bpf] xsk: fix return value of xdp_umem_assign_dev()
From: Björn Töpel @ 2018-08-20 8:31 UTC (permalink / raw)
To: bhole_prashant_q7
Cc: ast, Daniel Borkmann, Björn Töpel, Karlsson, Magnus,
David Miller, Netdev
In-Reply-To: <20180820005425.2604-1-bhole_prashant_q7@lab.ntt.co.jp>
Den mån 20 aug. 2018 kl 02:58 skrev Prashant Bhole
<bhole_prashant_q7@lab.ntt.co.jp>:
>
> s/ENOTSUPP/EOPNOTSUPP/ in function umem_assign_dev().
> This function's return value is directly returned by xsk_bind().
> EOPNOTSUPP is bind()'s possible return value.
>
> Fixes: f734607e819b ("xsk: refactor xdp_umem_assign_dev()")
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> net/xdp/xdp_umem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 911ca6d3cb5a..bfe2dbea480b 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -74,14 +74,14 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> return 0;
>
> if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_async_xmit)
> - return force_zc ? -ENOTSUPP : 0; /* fail or fallback */
> + return force_zc ? -EOPNOTSUPP : 0; /* fail or fallback */
>
> bpf.command = XDP_QUERY_XSK_UMEM;
>
> rtnl_lock();
> err = xdp_umem_query(dev, queue_id);
> if (err) {
> - err = err < 0 ? -ENOTSUPP : -EBUSY;
> + err = err < 0 ? -EOPNOTSUPP : -EBUSY;
> goto err_rtnl_unlock;
> }
>
> --
> 2.17.1
>
>
Acked-by: Björn Töpel <bjorn.topel@intel.com>
^ permalink raw reply
* Re: [RFC 0/1] Appletalk AARP probe broken by receipt of own broadcasts.
From: Craig McGeachie @ 2018-08-20 8:48 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David S. Miller, netdev, Craig McGeachie
In-Reply-To: <20180819144107.GA23981@lunn.ch>
On 20/08/18 02:41, Andrew Lunn wrote:
>> I run inside Virtualbox with the Realtek PCIe GBE Family Controller.
>>
>> Assuming I'm reading /sys/class/net/enp0s3/driver correctly, it's using the
>> e1000 driver.
>
> Hi Craig
Andrew,
My apologies. I've wasted your time. PEBKAC.
> Ah. And how do you connect to the network? Please run some tcpdumps
> and collect packets at various points. Make sure your network setup is
> not duplicating packets, in particular, any bridges you might have in
> order to connect the segments together.
Just for the record, in case anyone else experiences this later. It's
Fedora Core 29 with kernel 4.17.12-200.fc28.x86_64 running inside
Virtualbox 4.2.16. The host NIC is Realtek PCIe GBE. The virtual NIC is
Intel 82540EM. The Linux kernel driver for the virtual NIC is e1000. The
virtual NIC is connected to the physical NIC in bridged mode.
>> However, it might not be the ethernet driver's fault. I've been a bit loose
>> with terminology. Appletalk AARP probe packets aren't ethernet broadcasts as
>> such; they're multicast packets, via the psnap driver, to hardware address
>> 09:00:07:ff:ff:ff.
>
> Basically, the same question applies for Multicast as for Broadcast.
> I'm pretty sure the interface should not receiver the packet it
> transmitted itself. But if something on the network has duplicated the
> packet, it will receiver the duplicate. So before we add a filter,
> lets understand where the packets are coming from.
Turns out the problem is WinPCAP running on the host system (Windows
10). I can reliably cause the problem by starting up anything using
WinPCAP on the host system. I can make the AARP packet echo go away by
shutting down everything that uses WinPCAP.
I don't have a real Apple IIgs (much as I might like one) so I've been
using GSport 0.31, which uses WinPCAP to generate Ethertalk packets over
the same physical NIC that the Linux virtual machine is bridged to.
I've also been using Wireshark on both the host system and guest system
to diagnose my problem. Ironically, using Wireshark on the host system
is one way to cause the packet echo. Wireshark on the guest system works
just fine.
Maybe running WinPCAP and the bridged virtual NIC on different physical
NICS would fix the problem. I wouldn't know. My solution is run GSport
on a different computer. Using Win10pcap 10.2.5002 instead of WinPCAP
4.1.3 is not a fix. It has the same problem.
I have trouble believing this affects only Appletalk packets, but it's
probably unfounded speculation to consider anything else.
Anyway, thank you very much for your time Andrew.
Cheers,
Craig.
^ permalink raw reply
* [PATCH 1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Ahmad Fatoum @ 2018-08-20 12:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Nicolas Ferre
Cc: kernel, netdev, mdf, Brad Mouring, Florian Fainelli, stable
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the SPI-connected switch PHY, with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Cc: <stable@vger.kernel.org>
Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++---------
1 file changed, 17 insertions(+), 10 deletions(-)
Fixes since v1:
Added one more error path for failing to register fixed-link
Fixed a whitespace issue
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dc09f9a8a49b..ef6ce8691443 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -482,11 +482,6 @@ static int macb_mii_probe(struct net_device *dev)
if (np) {
if (of_phy_is_fixed_link(np)) {
- if (of_phy_register_fixed_link(np) < 0) {
- dev_err(&bp->pdev->dev,
- "broken fixed-link specification\n");
- return -ENODEV;
- }
bp->phy_node = of_node_get(np);
} else {
bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -569,7 +564,7 @@ static int macb_mii_init(struct macb *bp)
{
struct macb_platform_data *pdata;
struct device_node *np;
- int err;
+ int err = -ENXIO;
/* Enable management port */
macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -592,12 +587,23 @@ static int macb_mii_init(struct macb *bp)
dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
np = bp->pdev->dev.of_node;
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
+ if (np && of_phy_is_fixed_link(np)) {
+ if (of_phy_register_fixed_link(np) < 0) {
+ dev_err(&bp->pdev->dev,
+ "broken fixed-link specification\n");
+ goto err_out_free_mdiobus;
+ }
+
+ err = mdiobus_register(bp->mii_bus);
+ } else {
+ if (pdata)
+ bp->mii_bus->phy_mask = pdata->phy_mask;
+
+ err = of_mdiobus_register(bp->mii_bus, np);
+ }
- err = of_mdiobus_register(bp->mii_bus, np);
if (err)
- goto err_out_free_mdiobus;
+ goto err_out_free_fixed_link;
err = macb_mii_probe(bp->dev);
if (err)
@@ -607,6 +613,7 @@ static int macb_mii_init(struct macb *bp)
err_out_unregister_bus:
mdiobus_unregister(bp->mii_bus);
+err_out_free_fixed_link:
if (np && of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
err_out_free_mdiobus:
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Ahmad Fatoum @ 2018-08-20 12:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: Uwe Kleine-König, David S. Miller, Nicolas Ferre, netdev,
mdf, stable, kernel, Brad Mouring, Florian Fainelli
In-Reply-To: <20180816142443.GA5282@lunn.ch>
On 08/16/2018 04:24 PM, Andrew Lunn wrote:
> 1) A regression. We should find a fix for that. Maybe we should
> special case a child node called 'fixed-link' in
> of_mdiobus_register(). I would suggest adding a single warning if
> such node is found.
I just sent out a v2, which warns if fixed-link is encountered
in of_mdiobus_register(). The actual fixed-link handling remains in the
macb driver, because I think it's would be out of scope for a regression
fix to change where fixed-links are handled for all using drivers...
> 2) Missing functionality. Add support for an mdio container node.
>
> node = of_get_child_by_name(np, "mdio");
> if (node)
> err = of_mdiobus_register(bp->mii_bus, node);
> else
> err = of_mdiobus_register(bp->mii_bus, np);
Done.
> 3) Modify the existing dts files to make use of this container.
> Because of backwards compatibility, we cannot force the use of it,
> but we can encourage it.
Done as well.
Cheers
Ahmad
^ permalink raw reply
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Ahmad Fatoum @ 2018-08-20 12:37 UTC (permalink / raw)
To: Lad, Prabhakar, Andrew Lunn
Cc: Uwe Kleine-König, David S. Miller, Nicolas Ferre, netdev,
mdf, stable, Sascha Hauer, brad.mouring, Florian Fainelli
In-Reply-To: <CA+V-a8tjrJGNh6gLALtzR8WB36xFMoA+prsst-mp7jhtoxHhWg@mail.gmail.com>
On 08/15/2018 03:59 PM, Lad, Prabhakar wrote:
> This didn’t happen in v4.9.x something in core has changed ?
In my case, it was the macb driver that changed. I found the offending commit
with git-bisect, you might want to give it a try.
^ permalink raw reply
* Re: unregister_netdevice: waiting for DEV to become free (2)
From: Julian Anastasov @ 2018-08-20 12:55 UTC (permalink / raw)
To: syzbot; +Cc: ddstreet, dvyukov, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <0000000000007d22100573d66078@google.com>
Hello,
On Sun, 19 Aug 2018, syzbot wrote:
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit: d7857ae43dcc Add linux-next specific files for 20180817
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=13c72fce400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=4b10cd1ea76bb092
> dashboard link: https://syzkaller.appspot.com/bug?extid=30209ea299c09d8785c9
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15df679a400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15242741400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+30209ea299c09d8785c9@syzkaller.appspotmail.com
>
> IPVS: stopping master sync thread 4657 ...
> IPVS: stopping master sync thread 4663 ...
> IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> IPVS: = 0
> IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> IPVS: = 0
> IPVS: stopping master sync thread 4664 ...
> unregister_netdevice: waiting for lo to become free. Usage count = 1
Well, only IPVS and tun in the game? But IPVS does not
take any dev references for sync threads. Can it be a problem
in tun? For example, a side effects from dst_cache_reset?
May be dst_release is called too late? Here is what should happen
on unregistration:
- NETDEV_UNREGISTER event: rt_flush_dev changes dst->dev with lo
but dst is not released
- ndo_uninit/ip_tunnel_uninit: dst_cache_reset is called which
does nothing!?! May be dst_release call is needed here.
- no more references are expected here ...
- netdev_run_todo -> netdev_wait_allrefs: loop here due to refcnt!=0
- dev->priv_destructor (ip_tunnel_dev_free) calls dst_cache_destroy
where dst_release is used but it is not reached because we loop in
netdev_wait_allrefs above
- dst_cache_destroy: really call dst_release
In fact, after calling rt_flush_dev and replacing the
dst->dev we should reach dev->priv_destructor (ip_tunnel_dev_free)
for tun device where dst_release for lo should be called. But may be
something prevents it, exit batching?
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH 1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Andrew Lunn @ 2018-08-20 13:55 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: David S. Miller, Nicolas Ferre, kernel, netdev, mdf, Brad Mouring,
Florian Fainelli, stable
In-Reply-To: <20180820121238.7779-1-a.fatoum@pengutronix.de>
On Mon, Aug 20, 2018 at 02:12:35PM +0200, Ahmad Fatoum wrote:
> The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
> There, of_mdiobus_register was called even for the fixed-link representing
> the SPI-connected switch PHY, with the result that the driver attempts to
> enumerate PHYs on a non-existent MDIO bus:
>
> libphy: MACB_mii_bus: probed
So there are two different things here:
> mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
> [snip]
> mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
These are the result of the fixed-link being considered a PHY in
of_mdiobus_register(). Patch 2 fixes that, turns it into a single
warning.
> macb f0028000.ethernet: broken fixed-link specification
Why is of_phy_register_fixed_link(np) failing?
>
> Cc: <stable@vger.kernel.org>
> Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++---------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> Fixes since v1:
> Added one more error path for failing to register fixed-link
> Fixed a whitespace issue
>
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index dc09f9a8a49b..ef6ce8691443 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -482,11 +482,6 @@ static int macb_mii_probe(struct net_device *dev)
>
> if (np) {
> if (of_phy_is_fixed_link(np)) {
> - if (of_phy_register_fixed_link(np) < 0) {
> - dev_err(&bp->pdev->dev,
> - "broken fixed-link specification\n");
> - return -ENODEV;
> - }
As a separate patch, please can you use the error code which
of_phy_register_fixed_link() returns, not -ENODEV. It is possible it
is returning EPROBE_DEFER.
Andrew
^ permalink raw reply
* [PATCH 3/4] net: macb: Support specifying PHYs in a mdio container dts node
From: Ahmad Fatoum @ 2018-08-20 12:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Nicolas Ferre
Cc: kernel, netdev, mdf, Brad Mouring, Florian Fainelli
In-Reply-To: <20180820121238.7779-1-a.fatoum@pengutronix.de>
To align macb DT entries with those of other MACs.
For backwards compatibility, the old way remains supported.
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/net/ethernet/cadence/macb_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ef6ce8691443..2ebc5698db9d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -596,10 +596,10 @@ static int macb_mii_init(struct macb *bp)
err = mdiobus_register(bp->mii_bus);
} else {
+ struct device_node *node = of_get_child_by_name(np, "mdio") ?: np;
if (pdata)
bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = of_mdiobus_register(bp->mii_bus, np);
+ err = of_mdiobus_register(bp->mii_bus, node);
}
if (err)
--
2.18.0
^ permalink raw reply related
* [PATCH 4/4] ARM: dts: macb: wrap macb PHYs in a mdio container
From: Ahmad Fatoum @ 2018-08-20 12:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Nicolas Ferre
Cc: kernel, netdev, mdf, Brad Mouring, Florian Fainelli
In-Reply-To: <20180820121238.7779-1-a.fatoum@pengutronix.de>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
.../devicetree/bindings/net/macb.txt | 13 +++--
arch/arm/boot/dts/at91-sam9_l9260.dts | 6 ++-
arch/arm/boot/dts/at91-sama5d27_som1.dtsi | 14 ++---
arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 10 ++--
arch/arm/boot/dts/at91-sama5d2_xplained.dts | 10 ++--
arch/arm/boot/dts/at91-sama5d3_xplained.dts | 12 +++--
arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts | 6 ++-
arch/arm/boot/dts/at91-sama5d4_xplained.dts | 10 ++--
arch/arm/boot/dts/at91-sama5d4ek.dts | 10 ++--
arch/arm/boot/dts/at91-vinco.dts | 24 +++++----
arch/arm/boot/dts/at91rm9200ek.dts | 8 +--
arch/arm/boot/dts/sama5d2.dtsi | 5 ++
arch/arm/boot/dts/sama5d3_emac.dtsi | 5 ++
arch/arm/boot/dts/sama5d3_gmac.dtsi | 5 ++
arch/arm/boot/dts/sama5d3xcm_cmp.dtsi | 52 ++++++++++---------
arch/arm/boot/dts/sama5d3xmb_gmac.dtsi | 52 ++++++++++---------
arch/arm/boot/dts/sama5d4.dtsi | 10 ++++
17 files changed, 155 insertions(+), 97 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 457d5ae16f23..f39732372538 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -25,7 +25,8 @@ Required properties:
Optional elements: 'tsu_clk'
- clocks: Phandles to input clocks.
-Optional properties for PHY child node:
+PHY child nodes should be grouped in a mdio container,
+they have following optional properties:
- reset-gpios : Should specify the gpio for phy reset
- magic-packet : If present, indicates that the hardware supports waking
up via magic packet.
@@ -41,8 +42,12 @@ Examples:
local-mac-address = [3a 0e 03 04 05 06];
clock-names = "pclk", "hclk", "tx_clk";
clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
- ethernet-phy@1 {
- reg = <0x1>;
- reset-gpios = <&pioE 6 1>;
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ethernet-phy@1 {
+ reg = <0x1>;
+ reset-gpios = <&pioE 6 1>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sam9_l9260.dts b/arch/arm/boot/dts/at91-sam9_l9260.dts
index 70cb36f7a9d7..9d1fbd3afaea 100644
--- a/arch/arm/boot/dts/at91-sam9_l9260.dts
+++ b/arch/arm/boot/dts/at91-sam9_l9260.dts
@@ -67,8 +67,10 @@
#size-cells = <0>;
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d27_som1.dtsi b/arch/arm/boot/dts/at91-sama5d27_som1.dtsi
index cf0087b4c9e1..f729f128e68e 100644
--- a/arch/arm/boot/dts/at91-sama5d27_som1.dtsi
+++ b/arch/arm/boot/dts/at91-sama5d27_som1.dtsi
@@ -67,12 +67,14 @@
pinctrl-0 = <&pinctrl_macb0_default>;
phy-mode = "rmii";
- ethernet-phy@0 {
- reg = <0x0>;
- interrupt-parent = <&pioA>;
- interrupts = <PIN_PD31 IRQ_TYPE_LEVEL_LOW>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_macb0_phy_irq>;
+ mdio {
+ ethernet-phy@0 {
+ reg = <0x0>;
+ interrupt-parent = <&pioA>;
+ interrupts = <PIN_PD31 IRQ_TYPE_LEVEL_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_macb0_phy_irq>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
index b10dccd0958f..1ba4bad8189b 100644
--- a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
@@ -142,10 +142,12 @@
phy-mode = "rmii";
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioA>;
- interrupts = <56 IRQ_TYPE_LEVEL_LOW>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioA>;
+ interrupts = <56 IRQ_TYPE_LEVEL_LOW>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index fcc85d70f36e..83b435744ffd 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -126,10 +126,12 @@
phy-mode = "rmii";
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioA>;
- interrupts = <PIN_PC9 IRQ_TYPE_LEVEL_LOW>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioA>;
+ interrupts = <PIN_PC9 IRQ_TYPE_LEVEL_LOW>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index 02c1d2958d78..a84fec83f0a5 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -134,8 +134,10 @@
#size-cells = <0>;
status = "okay";
- ethernet-phy@7 {
- reg = <0x7>;
+ mdio {
+ ethernet-phy@7 {
+ reg = <0x7>;
+ };
};
};
@@ -201,8 +203,10 @@
#size-cells = <0>;
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts b/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
index fe05aaa7ac87..a361423a3969 100644
--- a/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
+++ b/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
@@ -63,8 +63,10 @@
phy-mode = "rmii";
status = "okay";
- phy0: ethernet-phy@0 {
- reg = <0>;
+ mdio {
+ phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
index 4b7c762d5f22..8b22ff53b40a 100644
--- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
@@ -95,10 +95,12 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_macb0_rmii &pinctrl_macb0_phy_irq>;
- phy0: ethernet-phy@1 {
- interrupt-parent = <&pioE>;
- interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
- reg = <1>;
+ mdio {
+ phy0: ethernet-phy@1 {
+ interrupt-parent = <&pioE>;
+ interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
+ reg = <1>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-sama5d4ek.dts b/arch/arm/boot/dts/at91-sama5d4ek.dts
index 0702a2f2b336..35ccba00b8cb 100644
--- a/arch/arm/boot/dts/at91-sama5d4ek.dts
+++ b/arch/arm/boot/dts/at91-sama5d4ek.dts
@@ -144,10 +144,12 @@
phy-mode = "rmii";
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioE>;
- interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioE>;
+ interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91-vinco.dts b/arch/arm/boot/dts/at91-vinco.dts
index 1be9889a2b3a..b690031ada28 100644
--- a/arch/arm/boot/dts/at91-vinco.dts
+++ b/arch/arm/boot/dts/at91-vinco.dts
@@ -116,11 +116,13 @@
phy-mode = "rmii";
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
- reset-gpios = <&pioE 8 GPIO_ACTIVE_LOW>;
- interrupt-parent = <&pioB>;
- interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ reset-gpios = <&pioE 8 GPIO_ACTIVE_LOW>;
+ interrupt-parent = <&pioB>;
+ interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
+ };
};
};
@@ -170,11 +172,13 @@
#size-cells = <0>;
status = "okay";
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioB>;
- interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
- reset-gpios = <&pioE 6 GPIO_ACTIVE_LOW>;
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioB>;
+ interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&pioE 6 GPIO_ACTIVE_LOW>;
+ };
};
};
diff --git a/arch/arm/boot/dts/at91rm9200ek.dts b/arch/arm/boot/dts/at91rm9200ek.dts
index 81aaf8151c76..1b1cc47b2376 100644
--- a/arch/arm/boot/dts/at91rm9200ek.dts
+++ b/arch/arm/boot/dts/at91rm9200ek.dts
@@ -54,9 +54,11 @@
phy-mode = "rmii";
status = "okay";
- phy0: ethernet-phy {
- interrupt-parent = <&pioC>;
- interrupts = <4 IRQ_TYPE_EDGE_BOTH>;
+ mdio {
+ phy0: ethernet-phy {
+ interrupt-parent = <&pioC>;
+ interrupts = <4 IRQ_TYPE_EDGE_BOTH>;
+ };
};
};
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 61f68e5c48e9..424aabd0db68 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -1091,6 +1091,11 @@
clocks = <&macb0_clk>, <&macb0_clk>;
clock-names = "hclk", "pclk";
status = "disabled";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
tcb0: timer@f800c000 {
diff --git a/arch/arm/boot/dts/sama5d3_emac.dtsi b/arch/arm/boot/dts/sama5d3_emac.dtsi
index 7cb235ef0fb6..d0187d4da1da 100644
--- a/arch/arm/boot/dts/sama5d3_emac.dtsi
+++ b/arch/arm/boot/dts/sama5d3_emac.dtsi
@@ -49,6 +49,11 @@
clocks = <&macb1_clk>, <&macb1_clk>;
clock-names = "hclk", "pclk";
status = "disabled";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3_gmac.dtsi b/arch/arm/boot/dts/sama5d3_gmac.dtsi
index 23f225fbb756..826d5891a2da 100644
--- a/arch/arm/boot/dts/sama5d3_gmac.dtsi
+++ b/arch/arm/boot/dts/sama5d3_gmac.dtsi
@@ -82,6 +82,11 @@
clocks = <&macb0_clk>, <&macb0_clk>;
clock-names = "hclk", "pclk";
status = "disabled";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3xcm_cmp.dtsi b/arch/arm/boot/dts/sama5d3xcm_cmp.dtsi
index a02f59021364..948df9aa40ac 100644
--- a/arch/arm/boot/dts/sama5d3xcm_cmp.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm_cmp.dtsi
@@ -86,32 +86,34 @@
#address-cells = <1>;
#size-cells = <0>;
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioB>;
- interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
- };
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioB>;
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ txen-skew-ps = <800>;
+ txc-skew-ps = <3000>;
+ rxdv-skew-ps = <400>;
+ rxc-skew-ps = <3000>;
+ rxd0-skew-ps = <400>;
+ rxd1-skew-ps = <400>;
+ rxd2-skew-ps = <400>;
+ rxd3-skew-ps = <400>;
+ };
- ethernet-phy@7 {
- reg = <0x7>;
- interrupt-parent = <&pioB>;
- interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
+ ethernet-phy@7 {
+ reg = <0x7>;
+ interrupt-parent = <&pioB>;
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ txen-skew-ps = <800>;
+ txc-skew-ps = <3000>;
+ rxdv-skew-ps = <400>;
+ rxc-skew-ps = <3000>;
+ rxd0-skew-ps = <400>;
+ rxd1-skew-ps = <400>;
+ rxd2-skew-ps = <400>;
+ rxd3-skew-ps = <400>;
+ };
};
};
diff --git a/arch/arm/boot/dts/sama5d3xmb_gmac.dtsi b/arch/arm/boot/dts/sama5d3xmb_gmac.dtsi
index 65aea7a75b1d..af097ff8bee4 100644
--- a/arch/arm/boot/dts/sama5d3xmb_gmac.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb_gmac.dtsi
@@ -15,32 +15,34 @@
#address-cells = <1>;
#size-cells = <0>;
- ethernet-phy@1 {
- reg = <0x1>;
- interrupt-parent = <&pioB>;
- interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
- };
+ mdio {
+ ethernet-phy@1 {
+ reg = <0x1>;
+ interrupt-parent = <&pioB>;
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ txen-skew-ps = <800>;
+ txc-skew-ps = <3000>;
+ rxdv-skew-ps = <400>;
+ rxc-skew-ps = <3000>;
+ rxd0-skew-ps = <400>;
+ rxd1-skew-ps = <400>;
+ rxd2-skew-ps = <400>;
+ rxd3-skew-ps = <400>;
+ };
- ethernet-phy@7 {
- reg = <0x7>;
- interrupt-parent = <&pioB>;
- interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
+ ethernet-phy@7 {
+ reg = <0x7>;
+ interrupt-parent = <&pioB>;
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ txen-skew-ps = <800>;
+ txc-skew-ps = <3000>;
+ rxdv-skew-ps = <400>;
+ rxc-skew-ps = <3000>;
+ rxd0-skew-ps = <400>;
+ rxd1-skew-ps = <400>;
+ rxd2-skew-ps = <400>;
+ rxd3-skew-ps = <400>;
+ };
};
};
};
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 0cf9beddd556..d623a8dc0116 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -980,6 +980,11 @@
clocks = <&macb0_clk>, <&macb0_clk>;
clock-names = "hclk", "pclk";
status = "disabled";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
i2c2: i2c@f8024000 {
@@ -1220,6 +1225,11 @@
clocks = <&macb1_clk>, <&macb1_clk>;
clock-names = "hclk", "pclk";
status = "disabled";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
trng@fc030000 {
--
2.18.0
^ permalink raw reply related
* [PATCH 2/4] of: phy: Warn about unexpected fixed-links in of_mdiobus_register
From: Ahmad Fatoum @ 2018-08-20 12:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Nicolas Ferre
Cc: kernel, netdev, mdf, Brad Mouring, Florian Fainelli
In-Reply-To: <20180820121238.7779-1-a.fatoum@pengutronix.de>
fixed-links are currently not handled by of_mdiobus_register,
skip them with a warning instead of trying pointlessly to find their PHY
address:
libphy: MACB_mii_bus: probed
mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
[snip]
mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
macb f0028000.ethernet: broken fixed-link specification
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/of/of_mdio.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index e92391d6d1bd..9a7ccd299daf 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -229,6 +229,13 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
/* Loop over the child nodes and register a phy_device for each phy */
for_each_available_child_of_node(np, child) {
+ if (of_phy_is_fixed_link(np)) {
+ /* fixed-links are handled in the MAC drivers */
+ dev_warn(&mdio->dev, FW_BUG
+ "Skipping unexpected fixed-link in device tree");
+ continue;
+ }
+
addr = of_mdio_parse_addr(&mdio->dev, child);
if (addr < 0) {
scanphys = true;
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 2/4] of: phy: Warn about unexpected fixed-links in of_mdiobus_register
From: Ahmad Fatoum @ 2018-08-20 12:31 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Nicolas Ferre
Cc: netdev, Florian Fainelli, mdf, kernel, Brad Mouring
In-Reply-To: <20180820121238.7779-2-a.fatoum@pengutronix.de>
On 08/20/2018 02:12 PM, Ahmad Fatoum wrote:
> /* Loop over the child nodes and register a phy_device for each phy */
> for_each_available_child_of_node(np, child) {
> + if (of_phy_is_fixed_link(np)) {
> + /* fixed-links are handled in the MAC drivers */
> + dev_warn(&mdio->dev, FW_BUG
> + "Skipping unexpected fixed-link in device tree");
> + continue;
> + }
> +
This would emit the warning even for normal PHYs,
because of_phy_is_fixed_link looks up a child...
I will correct this in v3 along with any other potential suggestions.
^ permalink raw reply
* Re: [PATCH 1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Ahmad Fatoum @ 2018-08-20 15:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Nicolas Ferre, kernel, netdev, mdf, Brad Mouring,
Florian Fainelli, stable
In-Reply-To: <20180820135536.GD6583@lunn.ch>
On 08/20/2018 03:55 PM, Andrew Lunn wrote:
> Why is of_phy_register_fixed_link(np) failing?
Apparently, the fixed-link's gpio's FLAG_REQUESTED bit remained set
causing gpiod_request_commit to return -EBUSY in (v4.18.0):
[<c0667750>] (gpiod_request_commit) from [<c066890c>] (gpiod_request+0x64/0x88)
[<c066890c>] (gpiod_request) from [<c066a338>] (gpiod_get_from_of_node+0x48/0x13c)
[<c066a338>] (gpiod_get_from_of_node) from [<c094b92c>] (mdiobus_register_device+0x70/0x124)
[<c094b92c>] (mdiobus_register_device) from [<c0949d2c>] (phy_device_register+0xc/0xa0)
[<c0949d2c>] (phy_device_register) from [<c094e824>] (fixed_phy_register+0xe8/0x1f8)
[<c094e824>] (fixed_phy_register) from [<c0b84260>] (of_phy_register_fixed_link+0x150/0x1e4)
[<c0b84260>] (of_phy_register_fixed_link) from [<c0964908>] (macb_probe+0x548/0xa7c)
[<c0964908>] (macb_probe) from [<c086ebec>] (platform_drv_probe+0x48/0x98)
But that's a separate issue, I'll remove this line from the commit message...
^ permalink raw reply
* Re: [RFC 0/1] Appletalk AARP probe broken by receipt of own broadcasts.
From: Andrew Lunn @ 2018-08-20 13:28 UTC (permalink / raw)
To: Craig McGeachie; +Cc: David S. Miller, netdev, Craig McGeachie
In-Reply-To: <43f861ad-1e40-7e6e-b90c-ce410c60df94@gmail.com>
> Turns out the problem is WinPCAP running on the host system (Windows
> 10).
Hi Craig
It would be good to report this to the WinPCAP people. I hate it when
debug tools actually introduce bugs.
Andrew
^ permalink raw reply
* Re: [PATCH 2/4] of: phy: Warn about unexpected fixed-links in of_mdiobus_register
From: Andrew Lunn @ 2018-08-20 13:37 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: David S. Miller, Nicolas Ferre, kernel, netdev, mdf, Brad Mouring,
Florian Fainelli
In-Reply-To: <20180820121238.7779-2-a.fatoum@pengutronix.de>
On Mon, Aug 20, 2018 at 02:12:36PM +0200, Ahmad Fatoum wrote:
> fixed-links are currently not handled by of_mdiobus_register,
> skip them with a warning instead of trying pointlessly to find their PHY
> address:
>
> libphy: MACB_mii_bus: probed
> mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
> [snip]
> mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
> macb f0028000.ethernet: broken fixed-link specification
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/of/of_mdio.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index e92391d6d1bd..9a7ccd299daf 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -229,6 +229,13 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>
> /* Loop over the child nodes and register a phy_device for each phy */
> for_each_available_child_of_node(np, child) {
> + if (of_phy_is_fixed_link(np)) {
> + /* fixed-links are handled in the MAC drivers */
> + dev_warn(&mdio->dev, FW_BUG
> + "Skipping unexpected fixed-link in device tree");
Hi Ahmad
We should be more specific than "device tree". It is actually the "mdio bus subtree".
Is this patch on its own sufficient to fix your regression? Ideally we
want one small patch which we can add to stable to fix the regression,
and then all the new functionality goes into net-next.
Andrew
^ permalink raw reply
* Re: [PATCH 3/4] net: macb: Support specifying PHYs in a mdio container dts node
From: Andrew Lunn @ 2018-08-20 13:42 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: David S. Miller, Nicolas Ferre, kernel, netdev, mdf, Brad Mouring,
Florian Fainelli
In-Reply-To: <20180820121238.7779-3-a.fatoum@pengutronix.de>
On Mon, Aug 20, 2018 at 02:12:37PM +0200, Ahmad Fatoum wrote:
> To align macb DT entries with those of other MACs.
> For backwards compatibility, the old way remains supported.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index ef6ce8691443..2ebc5698db9d 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -596,10 +596,10 @@ static int macb_mii_init(struct macb *bp)
>
> err = mdiobus_register(bp->mii_bus);
> } else {
> + struct device_node *node = of_get_child_by_name(np, "mdio") ?: np;
This is correct. But i would prefer the more readable
struct device_node *node = of_get_child_by_name(np, "mdio");
if (!node)
/* Allow for the deprecated PHYs in the MAC node. */
node = np;
> if (pdata)
> bp->mii_bus->phy_mask = pdata->phy_mask;
> -
> - err = of_mdiobus_register(bp->mii_bus, np);
> + err = of_mdiobus_register(bp->mii_bus, node);
> }
Also, the device tree binding documentation needs updating.
Thanks
Andrew
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox